bcrypt / os_crypt backend: now throws new PasswordValueError()

when encoding issue is found, to separate this from an unexpected error
when calling crypt.crypt() (these will still raise MissingBackendError).

also tweaked internal safe_verify() helper to catch errors thrown by
os_crypt backend (MissingBackendError would previously slip through,
causing spurious UT failures)

4 files changed, 78 insertions, 18 deletions

diff --git a/docs/history/1.7.rst b/docs/history/1.7.rst
index cb497f2..12eb363 100644
--- a/docs/history/1.7.rst
+++ b/docs/history/1.7.rst
@@ -55,6 +55,13 @@ Bugfixes
 Other Changes
 -------------
 
+* .. py:currentmodule:: passlib.hash
+
+  :class:`bcrypt`: OS native backend now raises the new :exc:`~passlib.exc.PasswordValueError`
+  if password is provided as non-UTF8 bytes under python 3.
+  These can't be passed through, due to limitation in stdlib's :func:`!crypt.crypt`.
+  (Prior to this release, it would it incorrectly raise :exc:`~passlib.exc.MissingBackendError` instead).
+
 * Modified some internals to help run on FIPS systems (:issue:`116`):
 
   In particular, when MD5 hash is not available, :class:`~passlib.hash.hex_md5`
diff --git a/docs/lib/passlib.exc.rst b/docs/lib/passlib.exc.rst
index 10647a3..f6e67ee 100644
--- a/docs/lib/passlib.exc.rst
+++ b/docs/lib/passlib.exc.rst
@@ -15,6 +15,8 @@ Exceptions
 .. index::
     pair: environmental variable; PASSLIB_MAX_PASSWORD_SIZE
 
+.. autoexception:: PasswordValueError
+
 .. autoexception:: PasswordSizeError
 
 .. autoexception:: PasswordTruncateError
diff --git a/passlib/exc.py b/passlib/exc.py
index 335fe91..42d3928 100644
--- a/passlib/exc.py
+++ b/passlib/exc.py
@@ -27,7 +27,21 @@ class MissingBackendError(RuntimeError):
     :class:`~passlib.hash.bcrypt`).
     """
 
-class PasswordSizeError(ValueError):
+
+class PasswordValueError(ValueError):
+    """
+    Error raised if a password can't be hashed / verified for various reasons.
+
+    May be thrown directly when password violates internal invariants of hasher
+    (e.g. some don't support NULL characters);  may also throw more specified subclasses,
+    such as :exc:`!PasswordSizeError`.
+
+    .. versionadded:: 1.7.3
+    """
+    pass
+
+
+class PasswordSizeError(PasswordValueError):
     """
     Error raised if a password exceeds the maximum size allowed
     by Passlib (by default, 4096 characters); or if password exceeds
@@ -59,7 +73,7 @@ class PasswordSizeError(ValueError):
         self.max_size = max_size
         if msg is None:
             msg = "password exceeds maximum allowed size"
-        ValueError.__init__(self, msg)
+        PasswordValueError.__init__(self, msg)
 
     # this also prevents a glibc crypt segfault issue, detailed here ...
     # http://www.openwall.com/lists/oss-security/2011/11/15/1
@@ -288,7 +302,7 @@ def MissingDigestError(handler=None):
 def NullPasswordError(handler=None):
     """raised by OS crypt() supporting hashes, which forbid NULLs in password"""
     name = _get_name(handler)
-    return ValueError("%s does not allow NULL bytes in password" % name)
+    return PasswordValueError("%s does not allow NULL bytes in password" % name)
 
 #------------------------------------------------------------------------
 # errors when parsing hashes
diff --git a/passlib/handlers/bcrypt.py b/passlib/handlers/bcrypt.py
index a2b0d27..0f6ecc2 100644
--- a/passlib/handlers/bcrypt.py
+++ b/passlib/handlers/bcrypt.py
@@ -30,7 +30,7 @@ from passlib.utils import safe_crypt, repeat_string, to_bytes, parse_version, \
                           rng, getrandstr, test_crypt, to_unicode
 from passlib.utils.binary import bcrypt64
 from passlib.utils.compat import get_unbound_method_function
-from passlib.utils.compat import u, uascii_to_str, unicode, str_to_uascii
+from passlib.utils.compat import u, uascii_to_str, unicode, str_to_uascii, PY3, error_from
 import passlib.utils.handlers as uh
 
 # local
@@ -291,7 +291,7 @@ class _BcryptCommon(uh.SubclassBackendMixin, uh.TruncateMixin, uh.HasManyIdents,
 
         verify = mixin_cls.verify
 
-        err_types = (ValueError,)
+        err_types = (ValueError, uh.exc.MissingBackendError)
         if _bcryptor:
             err_types += (_bcryptor.engine.SaltError,)
 
@@ -302,6 +302,9 @@ class _BcryptCommon(uh.SubclassBackendMixin, uh.TruncateMixin, uh.HasManyIdents,
             except err_types:
                 # backends without support for given ident will throw various
                 # errors about unrecognized version:
+                #   os_crypt -- internal code below throws MissingBackendError
+                #               if crypt fails for unknown reason;
+                #               and PasswordValueError if there's encoding issue w/ password.
                 #   pybcrypt, bcrypt -- raises ValueError
                 #   bcryptor -- raises bcryptor.engine.SaltError
                 return NotImplemented
@@ -741,25 +744,59 @@ class _OsCryptBackend(_BcryptCommon):
         return mixin_cls._finalize_backend_mixin(name, dryrun)
 
     def _calc_checksum(self, secret):
+        #
+        # run secret through crypt.crypt().
+        # if everything goes right, we'll get back a properly formed bcrypt hash.
+        #
         secret, ident = self._prepare_digest_args(secret)
         config = self._get_config(ident)
         hash = safe_crypt(secret, config)
         if hash:
             assert hash.startswith(config) and len(hash) == len(config)+31
             return hash[-31:]
-        else:
-            # NOTE: Have to raise this error because python3's crypt.crypt() only accepts unicode.
-            #       This means it can't handle any passwords that aren't either unicode
-            #       or utf-8 encoded bytes.  However, hashing a password with an alternate
-            #       encoding should be a pretty rare edge case; if user needs it, they can just
-            #       install bcrypt backend.
-            # XXX: is this the right error type to raise?
-            #      maybe have safe_crypt() not swallow UnicodeDecodeError, and have handlers
-            #      like sha256_crypt trap it if they have alternate method of handling them?
-            raise uh.exc.MissingBackendError(
-                "non-utf8 encoded passwords can't be handled by crypt.crypt() under python3, "
-                "recommend running `pip install bcrypt`.",
-                )
+
+        #
+        # Check if this failed due to non-UTF8 bytes
+        # In detail: under py3, crypt.crypt() requires unicode inputs, which are then encoded to
+        # utf8 before passing them to os crypt() call.  this is done according to the "s" format
+        # specifier for PyArg_ParseTuple (https://docs.python.org/3/c-api/arg.html).
+        # There appears no way to get around that to pass raw bytes; so we just throw error here
+        # to let user know they need to use another backend if they want raw bytes support.
+        #
+        # XXX: maybe just let safe_crypt() throw UnicodeDecodeError under passlib 2.0,
+        #      and then catch it above? maybe have safe_crypt ALWAYS throw error
+        #      instead of returning None? (would save re-detecting what went wrong)
+        # XXX: isn't secret ALWAYS bytes at this point?
+        #
+        if PY3 and isinstance(secret, bytes):
+            try:
+                secret.decode("utf-8")
+            except UnicodeDecodeError:
+                raise error_from(uh.exc.PasswordValueError(
+                    "python3 crypt.crypt() ony supports bytes passwords using UTF8; "
+                    "passlib recommends running `pip install bcrypt` for general bcrypt support.",
+                    ), None)
+
+        #
+        # else crypt() call failed for unknown reason.
+        #
+        # NOTE: getting here should be considered a bug in passlib --
+        #       if os_crypt backend detection said there's support,
+        #       and we've already checked all known reasons above;
+        #       want them to file bug so we can figure out what happened.
+        #       in the meantime, users can avoid this by installing bcrypt-cffi backend;
+        #       which won't have this (or utf8) edgecases.
+        #
+        # XXX: throw something more specific, like an "InternalBackendError"?
+        # NOTE: if do change this error, need to update test_81_crypt_fallback() expectations
+        #       about what will be thrown; as well as safe_verify() above.
+        #
+        raise uh.exc.MissingBackendError(
+            "crypt.crypt() failed for unknown reason; "
+            "passlib recommends running `pip install bcrypt` for general bcrypt support."
+            # for debugging UTs --
+            "(secret=%r, config=%r)" % (secret, config),
+            )
 
 #-----------------------------------------------------------------------
 # builtin backend