Password trunction makes password insecure
The trunc_password function attempts to correct and truncate password. It is not recommended to 'fix' invalid input and continue on processing and logging it. Instead, strict check is introduced to validate password. If a password exceeds the maximum length, an HTTP 403 Forbidden error is thrown. In order to keep compatibility, an option 'strict_password_check' is also introduced to let operator decide which method to use. DocImpact Change-Id: I560daa843b94a05412af59a059de5a98bad2925e Closes-Bug: #1175904
This commit is contained in:
parent
fa7812e3d4
commit
94a2053cd0
|
@ -109,6 +109,14 @@
|
||||||
# policy.v3cloudsample as an example). (boolean value)
|
# policy.v3cloudsample as an example). (boolean value)
|
||||||
#domain_id_immutable=true
|
#domain_id_immutable=true
|
||||||
|
|
||||||
|
# If set to true, strict password length checking is performed
|
||||||
|
# for password manipulation.
|
||||||
|
# If a password exceeds the maximum length, the operation will
|
||||||
|
# fail with 403 Forbidden Error.
|
||||||
|
# If set to false, passwords are automatically truncated to
|
||||||
|
# the maximum length.
|
||||||
|
# strict_password_check=false
|
||||||
|
|
||||||
|
|
||||||
#
|
#
|
||||||
# Options defined in oslo.messaging
|
# Options defined in oslo.messaging
|
||||||
|
|
|
@ -119,7 +119,13 @@ FILE_OPTIONS = {
|
||||||
'recommended if the scope of a domain admin is being '
|
'recommended if the scope of a domain admin is being '
|
||||||
'restricted by use of an appropriate policy file '
|
'restricted by use of an appropriate policy file '
|
||||||
'(see policy.v3cloudsample as an example).'),
|
'(see policy.v3cloudsample as an example).'),
|
||||||
],
|
cfg.BoolOpt('strict_password_check', default=False,
|
||||||
|
help='If set to true, strict password length checking is '
|
||||||
|
'performed for password manipulation.'
|
||||||
|
'If a password exceeds the maximum length, '
|
||||||
|
'the operation will fail with 403 Forbidden Error.'
|
||||||
|
'If set to false, passwords are automatically '
|
||||||
|
'truncated to the maximum length.')],
|
||||||
'identity': [
|
'identity': [
|
||||||
cfg.StrOpt('default_domain_id', default='default',
|
cfg.StrOpt('default_domain_id', default='default',
|
||||||
help='This references the domain to use for all '
|
help='This references the domain to use for all '
|
||||||
|
|
|
@ -86,14 +86,20 @@ class SmarterEncoder(json.JSONEncoder):
|
||||||
return super(SmarterEncoder, self).default(obj)
|
return super(SmarterEncoder, self).default(obj)
|
||||||
|
|
||||||
|
|
||||||
def trunc_password(password):
|
def verify_length_and_trunc_password(password):
|
||||||
"""Truncate passwords to the max_length."""
|
"""Verify and truncate the provided password to the max_password_length."""
|
||||||
max_length = CONF.identity.max_password_length
|
max_length = CONF.identity.max_password_length
|
||||||
try:
|
try:
|
||||||
if len(password) > max_length:
|
if len(password) > max_length:
|
||||||
LOG.warning(
|
if CONF.strict_password_check:
|
||||||
_('Truncating user password to %s characters.'), max_length)
|
raise exception.PasswordVerificationError(size=max_length)
|
||||||
return password[:max_length]
|
else:
|
||||||
|
LOG.warning(
|
||||||
|
_('Truncating user password to '
|
||||||
|
'%d characters.'), max_length)
|
||||||
|
return password[:max_length]
|
||||||
|
else:
|
||||||
|
return password
|
||||||
except TypeError:
|
except TypeError:
|
||||||
raise exception.ValidationError(attribute='string', target='password')
|
raise exception.ValidationError(attribute='string', target='password')
|
||||||
|
|
||||||
|
@ -115,7 +121,7 @@ def hash_user_password(user):
|
||||||
|
|
||||||
def hash_password(password):
|
def hash_password(password):
|
||||||
"""Hash a password. Hard."""
|
"""Hash a password. Hard."""
|
||||||
password_utf8 = trunc_password(password).encode('utf-8')
|
password_utf8 = verify_length_and_trunc_password(password).encode('utf-8')
|
||||||
return passlib.hash.sha512_crypt.encrypt(
|
return passlib.hash.sha512_crypt.encrypt(
|
||||||
password_utf8, rounds=CONF.crypt_strength)
|
password_utf8, rounds=CONF.crypt_strength)
|
||||||
|
|
||||||
|
@ -129,7 +135,7 @@ def check_password(password, hashed):
|
||||||
"""
|
"""
|
||||||
if password is None or hashed is None:
|
if password is None or hashed is None:
|
||||||
return False
|
return False
|
||||||
password_utf8 = trunc_password(password).encode('utf-8')
|
password_utf8 = verify_length_and_trunc_password(password).encode('utf-8')
|
||||||
return passlib.hash.sha512_crypt.verify(password_utf8, hashed)
|
return passlib.hash.sha512_crypt.verify(password_utf8, hashed)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -109,6 +109,14 @@ class ValidationSizeError(Error):
|
||||||
title = 'Bad Request'
|
title = 'Bad Request'
|
||||||
|
|
||||||
|
|
||||||
|
class PasswordVerificationError(Error):
|
||||||
|
message_format = _("The password length must be less than or equal "
|
||||||
|
"to %(size)i. The server could not comply with the "
|
||||||
|
"request because the password is invalid.")
|
||||||
|
code = 403
|
||||||
|
title = 'Forbidden'
|
||||||
|
|
||||||
|
|
||||||
class PKITokenExpected(Error):
|
class PKITokenExpected(Error):
|
||||||
message_format = _('The certificates you requested are not available. '
|
message_format = _('The certificates you requested are not available. '
|
||||||
'It is likely that this server does not use PKI tokens '
|
'It is likely that this server does not use PKI tokens '
|
||||||
|
|
|
@ -31,11 +31,15 @@ import datetime
|
||||||
import functools
|
import functools
|
||||||
import os
|
import os
|
||||||
import time
|
import time
|
||||||
|
import uuid
|
||||||
|
|
||||||
from keystone.common import utils
|
from keystone.common import utils
|
||||||
|
from keystone import config
|
||||||
|
from keystone import exception
|
||||||
from keystone import service
|
from keystone import service
|
||||||
from keystone import tests
|
from keystone import tests
|
||||||
|
|
||||||
|
CONF = config.CONF
|
||||||
|
|
||||||
TZ = None
|
TZ = None
|
||||||
|
|
||||||
|
@ -70,10 +74,40 @@ class UtilsTestCase(tests.TestCase):
|
||||||
self.assertTrue(utils.check_password(password, hashed))
|
self.assertTrue(utils.check_password(password, hashed))
|
||||||
self.assertFalse(utils.check_password(wrong, hashed))
|
self.assertFalse(utils.check_password(wrong, hashed))
|
||||||
|
|
||||||
def test_hash_long_password(self):
|
def test_verify_normal_password_strict(self):
|
||||||
bigboy = '0' * 9999999
|
self.config_fixture.config(strict_password_check=False)
|
||||||
hashed = utils.hash_password(bigboy)
|
normal_password = uuid.uuid4().hex
|
||||||
self.assertTrue(utils.check_password(bigboy, hashed))
|
verified = utils.verify_length_and_trunc_password(normal_password)
|
||||||
|
self.assertEqual(normal_password, verified)
|
||||||
|
|
||||||
|
def test_verify_long_password_strict(self):
|
||||||
|
self.config_fixture.config(strict_password_check=False)
|
||||||
|
self.config_fixture.config(group='identity', max_password_length=5)
|
||||||
|
max_length = CONF.identity.max_password_length
|
||||||
|
invalid_password = 'passw0rd'
|
||||||
|
truncated = utils.verify_length_and_trunc_password(invalid_password)
|
||||||
|
self.assertEqual(invalid_password[:max_length], truncated)
|
||||||
|
|
||||||
|
def test_verify_long_password_strict_raises_exception(self):
|
||||||
|
self.config_fixture.config(strict_password_check=True)
|
||||||
|
self.config_fixture.config(group='identity', max_password_length=5)
|
||||||
|
invalid_password = 'passw0rd'
|
||||||
|
self.assertRaises(exception.PasswordVerificationError,
|
||||||
|
utils.verify_length_and_trunc_password,
|
||||||
|
invalid_password)
|
||||||
|
|
||||||
|
def test_hash_long_password_truncation(self):
|
||||||
|
self.config_fixture.config(strict_password_check=False)
|
||||||
|
invalid_length_password = '0' * 9999999
|
||||||
|
hashed = utils.hash_password(invalid_length_password)
|
||||||
|
self.assertTrue(utils.check_password(invalid_length_password, hashed))
|
||||||
|
|
||||||
|
def test_hash_long_password_strict(self):
|
||||||
|
self.config_fixture.config(strict_password_check=True)
|
||||||
|
invalid_length_password = '0' * 9999999
|
||||||
|
self.assertRaises(exception.PasswordVerificationError,
|
||||||
|
utils.hash_password,
|
||||||
|
invalid_length_password)
|
||||||
|
|
||||||
def _create_test_user(self, password=OPTIONAL):
|
def _create_test_user(self, password=OPTIONAL):
|
||||||
user = {"name": "hthtest"}
|
user = {"name": "hthtest"}
|
||||||
|
|
Loading…
Reference in New Issue