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

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

@@ -119,7 +119,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(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"}