diff --git a/keystone/common/password_hashing.py b/keystone/common/password_hashing.py index b38d3cba79..7c3c617e4e 100644 --- a/keystone/common/password_hashing.py +++ b/keystone/common/password_hashing.py @@ -68,7 +68,7 @@ def verify_length_and_trunc_password(password): # When using bcrypt, we limit the password length to 54 to ensure all # bytes are fully mixed. See: # https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues - BCRYPT_MAX_LENGTH = 54 + BCRYPT_MAX_LENGTH = 72 if (CONF.identity.password_hash_algorithm == 'bcrypt' and # nosec: B105 CONF.identity.max_password_length > BCRYPT_MAX_LENGTH): msg = "Truncating password to algorithm specific maximum length %d characters." @@ -78,16 +78,17 @@ def verify_length_and_trunc_password(password): max_length = CONF.identity.max_password_length try: - if len(password) > max_length: + password_utf8 = password.encode('utf-8') + if len(password_utf8) > max_length: if CONF.strict_password_check: raise exception.PasswordVerificationError(size=max_length) else: msg = "Truncating user password to %d characters." LOG.warning(msg, max_length) - return password[:max_length] + return password_utf8[:max_length] else: - return password - except TypeError: + return password_utf8 + except AttributeError: raise exception.ValidationError(attribute='string', target='password') @@ -100,7 +101,7 @@ def check_password(password, hashed): """ if password is None or hashed is None: return False - password_utf8 = verify_length_and_trunc_password(password).encode('utf-8') + password_utf8 = verify_length_and_trunc_password(password) hasher = _get_hasher_from_ident(hashed) return hasher.verify(password_utf8, hashed) @@ -117,7 +118,7 @@ def hash_user_password(user): def hash_password(password): """Hash a password. Harder.""" params = {} - password_utf8 = verify_length_and_trunc_password(password).encode('utf-8') + password_utf8 = verify_length_and_trunc_password(password) conf_hasher = CONF.identity.password_hash_algorithm hasher = _HASHER_NAME_MAP.get(conf_hasher) diff --git a/keystone/conf/identity.py b/keystone/conf/identity.py index 5cce78cf91..d2cb34da17 100644 --- a/keystone/conf/identity.py +++ b/keystone/conf/identity.py @@ -103,7 +103,7 @@ performance. Changing this value does not effect existing passwords. This value can also be overridden by certain hashing algorithms maximum allowed length which takes precedence over the configured value. -The bcrypt max_password_length is 54. +The bcrypt max_password_length is 72 bytes. """)) list_limit = cfg.IntOpt( diff --git a/keystone/tests/unit/common/test_utils.py b/keystone/tests/unit/common/test_utils.py index 673175aea8..15ebc273ed 100644 --- a/keystone/tests/unit/common/test_utils.py +++ b/keystone/tests/unit/common/test_utils.py @@ -77,7 +77,7 @@ class UtilsTestCase(unit.BaseTestCase): self.config_fixture.config(strict_password_check=False) password = uuid.uuid4().hex verified = common_utils.verify_length_and_trunc_password(password) - self.assertEqual(password, verified) + self.assertEqual(password.encode('utf-8'), verified) def test_that_a_hash_can_not_be_validated_against_a_hash(self): # NOTE(dstanek): Bug 1279849 reported a problem where passwords @@ -97,7 +97,7 @@ class UtilsTestCase(unit.BaseTestCase): max_length = CONF.identity.max_password_length invalid_password = 'passw0rd' trunc = common_utils.verify_length_and_trunc_password(invalid_password) - self.assertEqual(invalid_password[:max_length], trunc) + self.assertEqual(invalid_password.encode('utf-8')[:max_length], trunc) def test_verify_long_password_strict_raises_exception(self): self.config_fixture.config(strict_password_check=True) @@ -139,8 +139,8 @@ class UtilsTestCase(unit.BaseTestCase): self.config_fixture.config(group='identity', password_hash_algorithm='bcrypt') self.config_fixture.config(group='identity', - max_password_length='64') - invalid_length_password = '0' * 64 + max_password_length='96') + invalid_length_password = '0' * 96 self.assertRaises(exception.PasswordVerificationError, common_utils.hash_password, invalid_length_password) diff --git a/releasenotes/notes/bcrypt_truncation_fix-674dc5d7f1e776f2.yaml b/releasenotes/notes/bcrypt_truncation_fix-674dc5d7f1e776f2.yaml new file mode 100644 index 0000000000..479a7aaafb --- /dev/null +++ b/releasenotes/notes/bcrypt_truncation_fix-674dc5d7f1e776f2.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Passwords that are hashed using bcrypt are now truncated properly to the + maximum allowed length by the algorythm. This solves regression, when + passwords longer then 54 symbols are getting invalidated after the + Keystone upgrade.