diff --git a/keystone/common/password_hashing.py b/keystone/common/password_hashing.py index 4e62d9c386..9c48f4ec4b 100644 --- a/keystone/common/password_hashing.py +++ b/keystone/common/password_hashing.py @@ -57,8 +57,37 @@ def _get_hasher_from_ident(hashed): def verify_length_and_trunc_password(password): - """Verify and truncate the provided password to the max_password_length.""" - max_length = CONF.identity.max_password_length + """Verify and truncate the provided password to the max_password_length. + + We also need to check that the configured password hashing algorithm does + not silently truncate the password. For example, passlib.hash.bcrypt does + this: + https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues + + """ + conf_max_length = CONF.identity.max_password_length + conf_hasher = CONF.identity.password_hash_algorithm + if conf_hasher == 'bcrypt': + # Setting this to 54 to ensure that all bytes are fully mixed per the + # passlib.hash.bcrypt security issue. + # https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues + algorithm_max_length = 54 + elif conf_hasher == 'sha512_crypt': + # Per the security issues for sha512_crypt (the same as sha256_crypt) + # the maximum password size is set to 4k by default. + # https://passlib.readthedocs.io/en/stable/lib/passlib.hash.sha256_crypt.html + algorithm_max_length = 4096 + else: + algorithm_max_length = None + + if algorithm_max_length is not None: + if algorithm_max_length < conf_max_length: + max_length = algorithm_max_length + else: + max_length = conf_max_length + else: + max_length = conf_max_length + try: if len(password) > max_length: if CONF.strict_password_check: diff --git a/keystone/conf/identity.py b/keystone/conf/identity.py index 0dffe58d60..2955ca0a0d 100644 --- a/keystone/conf/identity.py +++ b/keystone/conf/identity.py @@ -99,7 +99,12 @@ max_password_length = cfg.IntOpt( max=passlib.utils.MAX_PASSWORD_SIZE, help=utils.fmt(""" Maximum allowed length for user passwords. Decrease this value to improve -performance. Changing this value does not effect existing passwords. +performance. Changing this value does not effect existing passwords. This value +can also be overridden by certain hashing algorithms maximum allowed length. + +The bcrypt max_password_length is 54. + +The sha512_crypt max_password_length is 4096. """)) list_limit = cfg.IntOpt( diff --git a/releasenotes/notes/max-password-length-truncation-and-warning-bd69090315ec18a7.yaml b/releasenotes/notes/max-password-length-truncation-and-warning-bd69090315ec18a7.yaml new file mode 100644 index 0000000000..a2cee918c5 --- /dev/null +++ b/releasenotes/notes/max-password-length-truncation-and-warning-bd69090315ec18a7.yaml @@ -0,0 +1,10 @@ +--- +security: + - | + Passwords will now be automatically truncated if the max_password_length is + greater than the allowed length for the selected password hashing algorithm. + Currently only bcrypt and sha512_crypt have fixed allowed lengths defined, + 54 for bcrypt and 4096 for sha512_crypt. A warning will be generated in the + log if a password is truncated. This will not affect existing passwords, + however only the first 54 characters of existing bcrypt passwords and the + first 4096 characters of existing sha512_crypt passwords will be validated.