Merge pull request #958 from dstufft/prehash-bcrypt

Fixed #20138 -- Added BCryptSHA256PasswordHasher

AUTHORS

@@ -35,6 +35,7 @@ The PRIMARY AUTHORS are (and/or have been):
     * Bryan Veloso
     * Preston Holmes
     * Simon Charette
+    * Donald Stufft
 
 More information on the main contributors to Django can be found in
 docs/internals/committers.txt.

django/conf/global_settings.py

@@ -515,6 +515,7 @@ PASSWORD_RESET_TIMEOUT_DAYS = 3
 PASSWORD_HASHERS = (
     'django.contrib.auth.hashers.PBKDF2PasswordHasher',
     'django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher',
+    'django.contrib.auth.hashers.BCryptSHA256PasswordHasher',
     'django.contrib.auth.hashers.BCryptPasswordHasher',
     'django.contrib.auth.hashers.SHA1PasswordHasher',
     'django.contrib.auth.hashers.MD5PasswordHasher',

django/contrib/auth/hashers.py

@@ -1,6 +1,7 @@
 from __future__ import unicode_literals
 
 import base64
+import binascii
 import hashlib
 
 from django.dispatch import receiver
@@ -257,7 +258,7 @@ class PBKDF2SHA1PasswordHasher(PBKDF2PasswordHasher):
     digest = hashlib.sha1
 
 
-class BCryptPasswordHasher(BasePasswordHasher):
+class BCryptSHA256PasswordHasher(BasePasswordHasher):
     """
     Secure password hashing using the bcrypt algorithm (recommended)
 
@@ -266,7 +267,8 @@ class BCryptPasswordHasher(BasePasswordHasher):
     this library depends on native C code and might cause portability
     issues.
     """
-    algorithm = "bcrypt"
+    algorithm = "bcrypt_sha256"
+    digest = hashlib.sha256
     library = ("py-bcrypt", "bcrypt")
     rounds = 12
 
@@ -278,14 +280,34 @@ class BCryptPasswordHasher(BasePasswordHasher):
         bcrypt = self._load_library()
         # Need to reevaluate the force_bytes call once bcrypt is supported on
         # Python 3
-        data = bcrypt.hashpw(force_bytes(password), salt)
+
+        # Hash the password prior to using bcrypt to prevent password truncation
+        #   See: https://code.djangoproject.com/ticket/20138
+        if self.digest is not None:
+            # We use binascii.hexlify here because Python3 decided that a hex encoded
+            #   bytestring is somehow a unicode.
+            password = binascii.hexlify(self.digest(force_bytes(password)).digest())
+        else:
+            password = force_bytes(password)
+
+        data = bcrypt.hashpw(password, salt)
         return "%s$%s" % (self.algorithm, data)
 
     def verify(self, password, encoded):
         algorithm, data = encoded.split('$', 1)
         assert algorithm == self.algorithm
         bcrypt = self._load_library()
-        return constant_time_compare(data, bcrypt.hashpw(force_bytes(password), data))
+
+        # Hash the password prior to using bcrypt to prevent password truncation
+        #   See: https://code.djangoproject.com/ticket/20138
+        if self.digest is not None:
+            # We use binascii.hexlify here because Python3 decided that a hex encoded
+            #   bytestring is somehow a unicode.
+            password = binascii.hexlify(self.digest(force_bytes(password)).digest())
+        else:
+            password = force_bytes(password)
+
+        return constant_time_compare(data, bcrypt.hashpw(password, data))
 
     def safe_summary(self, encoded):
         algorithm, empty, algostr, work_factor, data = encoded.split('$', 4)
@@ -299,6 +321,25 @@ class BCryptPasswordHasher(BasePasswordHasher):
         ])
 
 
+class BCryptPasswordHasher(BCryptSHA256PasswordHasher):
+    """
+    Secure password hashing using the bcrypt algorithm
+
+    This is considered by many to be the most secure algorithm but you
+    must first install the py-bcrypt library.  Please be warned that
+    this library depends on native C code and might cause portability
+    issues.
+
+    This hasher does not first hash the password which means it is subject to
+    the 72 character bcrypt password truncation, most use cases should prefer
+    the BCryptSha512PasswordHasher.
+
+    See: https://code.djangoproject.com/ticket/20138
+    """
+    algorithm = "bcrypt"
+    digest = None
+
+
 class SHA1PasswordHasher(BasePasswordHasher):
     """
     The SHA1 password hashing algorithm (not recommended)

django/contrib/auth/tests/hashers.py

@@ -92,6 +92,22 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertFalse(check_password('lètmeiz', encoded))
         self.assertEqual(identify_hasher(encoded).algorithm, "crypt")
 
+    @skipUnless(bcrypt, "py-bcrypt not installed")
+    def test_bcrypt_sha256(self):
+        encoded = make_password('lètmein', hasher='bcrypt_sha256')
+        self.assertTrue(is_password_usable(encoded))
+        self.assertTrue(encoded.startswith('bcrypt_sha256$'))
+        self.assertTrue(check_password('lètmein', encoded))
+        self.assertFalse(check_password('lètmeinz', encoded))
+        self.assertEqual(identify_hasher(encoded).algorithm, "bcrypt_sha256")
+
+        # Verify that password truncation no longer works
+        password = ('VSK0UYV6FFQVZ0KG88DYN9WADAADZO1CTSIVDJUNZSUML6IBX7LN7ZS3R5'
+                    'JGB3RGZ7VI7G7DJQ9NI8BQFSRPTG6UWTTVESA5ZPUN')
+        encoded = make_password(password, hasher='bcrypt_sha256')
+        self.assertTrue(check_password(password, encoded))
+        self.assertFalse(check_password(password[:72], encoded))
+
     @skipUnless(bcrypt, "py-bcrypt not installed")
     def test_bcrypt(self):
         encoded = make_password('lètmein', hasher='bcrypt')

docs/releases/1.6.txt

@@ -181,6 +181,9 @@ Minor features
   and the undocumented limit of the higher of 1000 or ``max_num`` forms
   was changed so it is always 1000 more than ``max_num``.
 
+* Added ``BCryptSHA256PasswordHasher`` to resolve the password truncation issue
+  with bcrypt.
+
 Backwards incompatible changes in 1.6
 =====================================
 

docs/topics/auth/passwords.txt

@@ -52,6 +52,7 @@ The default for :setting:`PASSWORD_HASHERS` is::
     PASSWORD_HASHERS = (
         'django.contrib.auth.hashers.PBKDF2PasswordHasher',
         'django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher',
+        'django.contrib.auth.hashers.BCryptSHA256PasswordHasher',
         'django.contrib.auth.hashers.BCryptPasswordHasher',
         'django.contrib.auth.hashers.SHA1PasswordHasher',
         'django.contrib.auth.hashers.MD5PasswordHasher',
@@ -79,10 +80,11 @@ To use Bcrypt as your default storage algorithm, do the following:
    py-bcrypt``, or downloading the library and installing it with ``python
    setup.py install``).
 
-2. Modify :setting:`PASSWORD_HASHERS` to list ``BCryptPasswordHasher``
+2. Modify :setting:`PASSWORD_HASHERS` to list ``BCryptSHA256PasswordHasher``
    first. That is, in your settings file, you'd put::
 
         PASSWORD_HASHERS = (
+            'django.contrib.auth.hashers.BCryptSHA256PasswordHasher',
             'django.contrib.auth.hashers.BCryptPasswordHasher',
             'django.contrib.auth.hashers.PBKDF2PasswordHasher',
             'django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher',
@@ -97,6 +99,22 @@ To use Bcrypt as your default storage algorithm, do the following:
 That's it -- now your Django install will use Bcrypt as the default storage
 algorithm.
 
+.. admonition:: Password truncation with BCryptPasswordHasher
+
+    The designers of bcrypt truncate all passwords at 72 characters which means
+    that ``bcrypt(password_with_100_chars) == bcrypt(password_with_100_chars[:72])``.
+    The original ``BCryptPasswordHasher`` does not have any special handling and
+    thus is also subject to this hidden password length limit.
+    ``BCryptSHA256PasswordHasher`` fixes this by first first hashing the
+    password using sha256. This prevents the password truncation and so should
+    be preferred over the ``BCryptPasswordHasher``. The practical ramification
+    of this truncation is pretty marginal as the average user does not have a
+    password greater than 72 characters in length and even being truncated at 72
+    the compute powered required to brute force bcrypt in any useful amount of
+    time is still astronomical. Nonetheless, we recommend you use
+    ``BCryptSHA256PasswordHasher`` anyway on the principle of "better safe than
+    sorry.
+
 .. admonition:: Other bcrypt implementations
 
    There are several other implementations that allow bcrypt to be
@@ -138,6 +156,7 @@ default PBKDF2 algorithm:
             'myproject.hashers.MyPBKDF2PasswordHasher',
             'django.contrib.auth.hashers.PBKDF2PasswordHasher',
             'django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher',
+            'django.contrib.auth.hashers.BCryptSHA256PasswordHasher',
             'django.contrib.auth.hashers.BCryptPasswordHasher',
             'django.contrib.auth.hashers.SHA1PasswordHasher',
             'django.contrib.auth.hashers.MD5PasswordHasher',
@@ -194,8 +213,8 @@ from the ``User`` model.
     provide a salt and a hashing algorithm to use, if you don't want to use the
     defaults (first entry of ``PASSWORD_HASHERS`` setting).
     Currently supported algorithms are: ``'pbkdf2_sha256'``, ``'pbkdf2_sha1'``,
-    ``'bcrypt'`` (see :ref:`bcrypt_usage`), ``'sha1'``, ``'md5'``,
-    ``'unsalted_md5'`` (only for backward compatibility) and ``'crypt'``
+    ``'bcrypt_sha256'`` (see :ref:`bcrypt_usage`), ``'bcrypt'``, ``'sha1'``,
+    ``'md5'``, ``'unsalted_md5'`` (only for backward compatibility) and ``'crypt'``
     if you have the ``crypt`` library installed. If the password argument is
     ``None``, an unusable password is returned (a one that will be never
     accepted by :func:`check_password`).