openstack/keystone
Code Issues Proposed changes

Force algo specific maximum length

Browse Source 
Two of the algorithms that we use for password hashing silently
length limit the size of the password that is hashed giving the
user a false sense of security [0], [1].  This patch adds a check
in the verify_length_and_trunc_password function for the hash in
use and updates the max_length accordingly, this will override
the configured value and log a warning if the password is truncated.

Closes-bug: #1901891
Change-Id: I8d0bb2438b23227b5a66b94af6f8e198084fcd8d
This commit is contained in:
Dave Wilde (d34dh0r53) 2022-02-09 11:28:59 -06:00
parent f69a449a16
commit 7859ed2600
3 changed files with 47 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
keystone/common/password_hashing.py
View File

@ -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:

7
keystone/conf/identity.py
View File

@ -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(

10
releasenotes/notes/max-password-length-truncation-and-warning-bd69090315ec18a7.yaml Normal file
View File

@ -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.