Merge "Password trunction makes password insecure"

etc/keystone.conf.sample

@@ -109,6 +109,14 @@
 # policy.v3cloudsample as an example). (boolean value)
 #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

keystone/common/config.py

@@ -125,7 +125,13 @@ FILE_OPTIONS = {
                          'recommended if the scope of a domain admin is being '
                          'restricted by use of an appropriate policy file '
                          '(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': [
         cfg.StrOpt('default_domain_id', default='default',
                    help='This references the domain to use for all '

keystone/common/utils.py

@@ -86,14 +86,20 @@ class SmarterEncoder(jsonutils.json.JSONEncoder):
         return super(SmarterEncoder, self).default(obj)
 
 
-def trunc_password(password):
-    """Truncate passwords to the max_length."""
+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
     try:
         if len(password) > max_length:
-            LOG.warning(
-                _('Truncating user password to %s characters.'), max_length)
-        return password[:max_length]
+            if CONF.strict_password_check:
+                raise exception.PasswordVerificationError(size=max_length)
+            else:
+                LOG.warning(
+                    _('Truncating user password to '
+                      '%d characters.'), max_length)
+                return password[:max_length]
+        else:
+            return password
     except TypeError:
         raise exception.ValidationError(attribute='string', target='password')
 
@@ -115,7 +121,7 @@ def hash_user_password(user):
 
 def hash_password(password):
     """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(
         password_utf8, rounds=CONF.crypt_strength)
 
@@ -129,7 +135,7 @@ def check_password(password, hashed):
     """
     if password is None or hashed is None:
         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)
 
 

keystone/exception.py

@@ -109,6 +109,14 @@ class ValidationSizeError(Error):
     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):
     message_format = _('The certificates you requested are not available. '
                        'It is likely that this server does not use PKI tokens '

keystone/tests/test_utils.py

@@ -31,11 +31,15 @@ import datetime
 import functools
 import os
 import time
+import uuid
 
 from keystone.common import utils
+from keystone import config
+from keystone import exception
 from keystone import service
 from keystone import tests
 
+CONF = config.CONF
 
 TZ = None
 
@@ -70,10 +74,40 @@ class UtilsTestCase(tests.TestCase):
         self.assertTrue(utils.check_password(password, hashed))
         self.assertFalse(utils.check_password(wrong, hashed))
 
-    def test_hash_long_password(self):
-        bigboy = '0' * 9999999
-        hashed = utils.hash_password(bigboy)
-        self.assertTrue(utils.check_password(bigboy, hashed))
+    def test_verify_normal_password_strict(self):
+        self.config_fixture.config(strict_password_check=False)
+        normal_password = uuid.uuid4().hex
+        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):
         user = {"name": "hthtest"}