passlib.handlers: cases where crypt() returns malformed hash

now return a single unified InternalBackendError() class,
instead of AssertionError.

This change has a couple of parts:

* assert statements replaced with permanent checks,
  since crypt() is unpredictable enough that we need to have this always on,
  even if production runs code in "-O2" mode.

* added debug_only_repr() helper which allows including sensitive stuff
  like salts & hash digests within error tracebacks -- will only do so
  when global flag is enabled; and that's currently only set by unittest suite.

* added new InternalBackendError() exception class (a RuntimeError subclass);
  which is raised instead of an AssertionError.

10 files changed, 105 insertions, 43 deletions

diff --git a/docs/history/1.7.rst b/docs/history/1.7.rst
index 99c5bec..c5b15d0 100644
--- a/docs/history/1.7.rst
+++ b/docs/history/1.7.rst
@@ -89,6 +89,10 @@ Other Changes
   a load-time error (though they will still receive an error if an attempt is made to actually
   *use* a FIPS-disabled hash).
 
+* Internal errors calling stdlib's :func:`crypt.crypt`, or third party libraries,
+  will now raise the new :exc:`~passlib.exc.InternalBackendError` (a RuntimeError);
+  where previously it would raise an :exc:`AssertionError`.
+
 * Various Python 3.9 compatibility fixes (including ``NotImplemented``-related warning, :issue:`125`)
 
 
diff --git a/docs/lib/passlib.exc.rst b/docs/lib/passlib.exc.rst
index f6e67ee..90565c8 100644
--- a/docs/lib/passlib.exc.rst
+++ b/docs/lib/passlib.exc.rst
@@ -12,6 +12,8 @@ Exceptions
 ==========
 .. autoexception:: MissingBackendError
 
+.. autoexception:: InternalBackendError
+
 .. index::
     pair: environmental variable; PASSLIB_MAX_PASSWORD_SIZE
 
diff --git a/passlib/exc.py b/passlib/exc.py
index 280043d..4539c7d 100644
--- a/passlib/exc.py
+++ b/passlib/exc.py
@@ -15,6 +15,10 @@ class UnknownBackendError(ValueError):
         message = "%s: unknown backend: %r" % (hasher.name, backend)
         ValueError.__init__(self, message)
 
+
+# XXX: add a PasslibRuntimeError as base for Missing/Internal/Security runtime errors?
+
+
 class MissingBackendError(RuntimeError):
     """Error raised if multi-backend handler has no available backends;
     or if specifically requested backend is not available.
@@ -28,6 +32,15 @@ class MissingBackendError(RuntimeError):
     """
 
 
+class InternalBackendError(RuntimeError):
+    """
+    Error raised if something unrecoverable goes wrong with backend call;
+    such as if ``crypt.crypt()`` returning a malformed hash.
+
+    .. versionadded:: 1.7.3
+    """
+
+
 class PasswordValueError(ValueError):
     """
     Error raised if a password can't be hashed / verified for various reasons.
@@ -102,6 +115,7 @@ class PasswordTruncateError(PasswordSizeError):
                    (cls.name, cls.truncate_size))
         PasswordSizeError.__init__(self, cls.truncate_size, msg)
 
+
 class PasslibSecurityError(RuntimeError):
     """
     Error raised if critical security issue is detected
@@ -338,5 +352,40 @@ def ChecksumSizeError(handler, raw=False):
     return MalformedHashError(handler, reason)
 
 #=============================================================================
+# sensitive info helpers
+#=============================================================================
+
+#: global flag, set temporarily by UTs to allow debug_only_repr() to display sensitive values.
+ENABLE_DEBUG_ONLY_REPR = False
+
+
+def debug_only_repr(value, param="hash"):
+    """
+    helper used to display sensitive data (hashes etc) within error messages.
+    currently returns placeholder test UNLESS unittests are running,
+    in which case the real value is displayed.
+
+    mainly useful to prevent hashes / secrets from being exposed in production tracebacks;
+    while still being visible from test failures.
+
+    NOTE: api subject to change, may formalize this more in the future.
+    """
+    if ENABLE_DEBUG_ONLY_REPR or value is None or isinstance(value, bool):
+        return repr(value)
+    return "<%s %s value omitted>" % (param, type(value))
+
+
+def CryptBackendError(handler, config, hash,  # *
+                      source="crypt.crypt()"):
+    """
+    helper to generate standard message when ``crypt.crypt()`` returns invalid result.
+    takes care of automatically masking contents of config & hash outside of UTs.
+    """
+    name = _get_name(handler)
+    msg = "%s returned invalid %s hash: config=%s hash=%s" % \
+          (source, name, debug_only_repr(config), debug_only_repr(hash))
+    raise InternalBackendError(msg)
+
+#=============================================================================
 # eof
 #=============================================================================
diff --git a/passlib/handlers/bcrypt.py b/passlib/handlers/bcrypt.py
index 5a7ef87..c203fdd 100644
--- a/passlib/handlers/bcrypt.py
+++ b/passlib/handlers/bcrypt.py
@@ -304,14 +304,15 @@ 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.
+                #   os_crypt -- internal code below throws
+                #       - PasswordValueError if there's encoding issue w/ password.
+                #       - InternalBackendError if crypt fails for unknown reason
+                #         (trapped below so we can debug it)
                 #   pybcrypt, bcrypt -- raises ValueError
                 #   bcryptor -- raises bcryptor.engine.SaltError
                 return NotImplemented
-            except AssertionError as err:
-                # _calc_checksum() code may also throw AssertionError
+            except uh.exc.InternalBackendError:
+                # _calc_checksum() code may also throw CryptBackendError
                 # if correct hash isn't returned (e.g. 2y hash converted to 2b,
                 # such as happens with bcrypt 3.0.0)
                 log.debug("trapped unexpected response from %r backend: verify(%r, %r):",
@@ -652,9 +653,9 @@ class _BcryptBackend(_BcryptCommon):
         if isinstance(config, unicode):
             config = config.encode("ascii")
         hash = _bcrypt.hashpw(secret, config)
-        assert hash.startswith(config) and len(hash) == len(config)+31, \
-            "config mismatch: %r => %r" % (config, hash)
         assert isinstance(hash, bytes)
+        if not hash.startswith(config) or len(hash) != len(config)+31:
+            raise uh.exc.CryptBackendError(self, config, hash, source="`bcrypt` package")
         return hash[-31:].decode("ascii")
 
 #-----------------------------------------------------------------------
@@ -689,7 +690,8 @@ class _BcryptorBackend(_BcryptCommon):
         secret, ident = self._prepare_digest_args(secret)
         config = self._get_config(ident)
         hash = _bcryptor.engine.Engine(False).hash_key(secret, config)
-        assert hash.startswith(config) and len(hash) == len(config)+31
+        if not hash.startswith(config) or len(hash) != len(config) + 31:
+            raise uh.exc.CryptBackendError(self, config, hash, source="bcryptor library")
         return str_to_uascii(hash[-31:])
 
 #-----------------------------------------------------------------------
@@ -758,7 +760,8 @@ class _PyBcryptBackend(_BcryptCommon):
         secret, ident = self._prepare_digest_args(secret)
         config = self._get_config(ident)
         hash = _pybcrypt.hashpw(secret, config)
-        assert hash.startswith(config) and len(hash) == len(config)+31
+        if not hash.startswith(config) or len(hash) != len(config) + 31:
+            raise uh.exc.CryptBackendError(self, config, hash, source="pybcrypt library")
         return str_to_uascii(hash[-31:])
 
     _calc_checksum = _calc_checksum_raw
@@ -789,8 +792,9 @@ class _OsCryptBackend(_BcryptCommon):
         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
+        if hash is not None:
+            if not hash.startswith(config) or len(hash) != len(config) + 31:
+                raise uh.exc.CryptBackendError(self, config, hash)
             return hash[-31:]
 
         #
@@ -829,11 +833,12 @@ class _OsCryptBackend(_BcryptCommon):
         # 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(
+        debug_only_repr = uh.exc.debug_only_repr
+        raise uh.exc.InternalBackendError(
             "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),
+            "(config=%s, secret=%s)" % (debug_only_repr(config), debug_only_repr(secret)),
             )
 
 #-----------------------------------------------------------------------
diff --git a/passlib/handlers/des_crypt.py b/passlib/handlers/des_crypt.py
index 9561ab4..68a4ca7 100644
--- a/passlib/handlers/des_crypt.py
+++ b/passlib/handlers/des_crypt.py
@@ -217,13 +217,13 @@ class des_crypt(uh.TruncateMixin, uh.HasManyBackends, uh.HasSalt, uh.GenericHand
         # NOTE: we let safe_crypt() encode unicode secret -> utf8;
         #       no official policy since des-crypt predates unicode
         hash = safe_crypt(secret, self.salt)
-        if hash:
-            assert hash.startswith(self.salt) and len(hash) == 13
-            return hash[2:]
-        else:
+        if hash is None:
             # py3's crypt.crypt() can't handle non-utf8 bytes.
             # fallback to builtin alg, which is always available.
             return self._calc_checksum_builtin(secret)
+        if not hash.startswith(self.salt) or len(hash) != 13:
+            raise uh.exc.CryptBackendError(self, self.salt, hash)
+        return hash[2:]
 
     #---------------------------------------------------------------
     # builtin backend
@@ -380,13 +380,13 @@ class bsdi_crypt(uh.HasManyBackends, uh.HasRounds, uh.HasSalt, uh.GenericHandler
     def _calc_checksum_os_crypt(self, secret):
         config = self.to_string()
         hash = safe_crypt(secret, config)
-        if hash:
-            assert hash.startswith(config[:9]) and len(hash) == 20
-            return hash[-11:]
-        else:
+        if hash is None:
             # py3's crypt.crypt() can't handle non-utf8 bytes.
             # fallback to builtin alg, which is always available.
             return self._calc_checksum_builtin(secret)
+        if not hash.startswith(config[:9]) or len(hash) != 20:
+            raise uh.exc.CryptBackendError(self, config, hash)
+        return hash[-11:]
 
     #---------------------------------------------------------------
     # builtin backend
diff --git a/passlib/handlers/md5_crypt.py b/passlib/handlers/md5_crypt.py
index 993db4d..e3a2dfa 100644
--- a/passlib/handlers/md5_crypt.py
+++ b/passlib/handlers/md5_crypt.py
@@ -279,13 +279,13 @@ class md5_crypt(uh.HasManyBackends, _MD5_Common):
     def _calc_checksum_os_crypt(self, secret):
         config = self.ident + self.salt
         hash = safe_crypt(secret, config)
-        if hash:
-            assert hash.startswith(config) and len(hash) == len(config) + 23
-            return hash[-22:]
-        else:
+        if hash is None:
             # py3's crypt.crypt() can't handle non-utf8 bytes.
             # fallback to builtin alg, which is always available.
             return self._calc_checksum_builtin(secret)
+        if not hash.startswith(config) or len(hash) != len(config) + 23:
+            raise uh.exc.CryptBackendError(self, config, hash)
+        return hash[-22:]
 
     #---------------------------------------------------------------
     # builtin backend
diff --git a/passlib/handlers/sha1_crypt.py b/passlib/handlers/sha1_crypt.py
index d3e972c..8f9aa71 100644
--- a/passlib/handlers/sha1_crypt.py
+++ b/passlib/handlers/sha1_crypt.py
@@ -109,13 +109,13 @@ class sha1_crypt(uh.HasManyBackends, uh.HasRounds, uh.HasSalt, uh.GenericHandler
     def _calc_checksum_os_crypt(self, secret):
         config = self.to_string(config=True)
         hash = safe_crypt(secret, config)
-        if hash:
-            assert hash.startswith(config) and len(hash) == len(config) + 29
-            return hash[-28:]
-        else:
+        if hash is None:
             # py3's crypt.crypt() can't handle non-utf8 bytes.
             # fallback to builtin alg, which is always available.
             return self._calc_checksum_builtin(secret)
+        if not hash.startswith(config) or len(hash) != len(config) + 29:
+            raise uh.exc.CryptBackendError(self, config, hash)
+        return hash[-28:]
 
     #---------------------------------------------------------------
     # builtin backend
diff --git a/passlib/handlers/sha2_crypt.py b/passlib/handlers/sha2_crypt.py
index 807de5e..6223616 100644
--- a/passlib/handlers/sha2_crypt.py
+++ b/passlib/handlers/sha2_crypt.py
@@ -367,17 +367,18 @@ class _SHA2_Common(uh.HasManyBackends, uh.HasRounds, uh.HasSalt,
             return False
 
     def _calc_checksum_os_crypt(self, secret):
-        hash = safe_crypt(secret, self.to_string())
-        if hash:
-            # NOTE: avoiding full parsing routine via from_string().checksum,
-            # and just extracting the bit we need.
-            cs = self.checksum_size
-            assert hash.startswith(self.ident) and hash[-cs-1] == _UDOLLAR
-            return hash[-cs:]
-        else:
+        config = self.to_string()
+        hash = safe_crypt(secret, config)
+        if hash is None:
             # py3's crypt.crypt() can't handle non-utf8 bytes.
             # fallback to builtin alg, which is always available.
             return self._calc_checksum_builtin(secret)
+        # NOTE: avoiding full parsing routine via from_string().checksum,
+        # and just extracting the bit we need.
+        cs = self.checksum_size
+        if not hash.startswith(self.ident) or hash[-cs-1] != _UDOLLAR:
+            raise uh.exc.CryptBackendError(self, config, hash)
+        return hash[-cs:]
 
     #---------------------------------------------------------------
     # builtin backend
diff --git a/passlib/tests/utils.py b/passlib/tests/utils.py
index 07b0f5a..a698ac7 100644
--- a/passlib/tests/utils.py
+++ b/passlib/tests/utils.py
@@ -356,6 +356,8 @@ class TestCase(_TestCase):
     def setUp(self):
         super(TestCase, self).setUp()
         self.setUpWarnings()
+        # have uh.debug_only_repr() return real values for duration of test
+        self.patchAttr(exc, "ENABLE_DEBUG_ONLY_REPR", True)
 
     def setUpWarnings(self):
         """helper to init warning filters before subclass setUp()"""
@@ -3230,7 +3232,7 @@ class OsCryptMixin(HandlerCase):
     def test_80_faulty_crypt(self):
         """test with faulty crypt()"""
         hash = self.get_sample_hash()[1]
-        exc_types = (AssertionError,)
+        exc_types = (exc.InternalBackendError,)
         mock_crypt = self._use_mock_crypt()
 
         def test(value):
@@ -3260,11 +3262,11 @@ class OsCryptMixin(HandlerCase):
             self.assertTrue(self.do_verify("stub", h1))
         else:
             # handler should give up
-            from passlib.exc import MissingBackendError
+            from passlib.exc import InternalBackendError as err_type
             hash = self.get_sample_hash()[1]
-            self.assertRaises(MissingBackendError, self.do_encrypt, 'stub')
-            self.assertRaises(MissingBackendError, self.do_genhash, 'stub', hash)
-            self.assertRaises(MissingBackendError, self.do_verify, 'stub', hash)
+            self.assertRaises(err_type, self.do_encrypt, 'stub')
+            self.assertRaises(err_type, self.do_genhash, 'stub', hash)
+            self.assertRaises(err_type, self.do_verify, 'stub', hash)
 
     @doesnt_require_backend
     def test_82_crypt_support(self):
diff --git a/passlib/utils/handlers.py b/passlib/utils/handlers.py
index 7e04114..f8681fa 100644
--- a/passlib/utils/handlers.py
+++ b/passlib/utils/handlers.py
@@ -346,7 +346,6 @@ def mask_value(value, show=4, pct=0.125, char=u"*"):
     show = min(show, int(size * pct))
     return value[:show] + char * (size - show)
 
-
 #=============================================================================
 # parameter helpers
 #=============================================================================