From fd5f041dbd807f18cb250ce13c16c0c4b7362cac Mon Sep 17 00:00:00 2001 From: Eli Collins Date: Tue, 12 May 2020 12:05:34 -0400 Subject: 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) --- docs/history/1.7.rst | 7 +++++ docs/lib/passlib.exc.rst | 2 ++ passlib/exc.py | 20 +++++++++++--- passlib/handlers/bcrypt.py | 67 +++++++++++++++++++++++++++++++++++----------- 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 -- cgit v1.2.1