Skip to content

Loading…

Added the ability to easily change the work factor in configurable hashing algorithms #406

Closed
wants to merge 7 commits into from

3 participants

@jbuckner

This addition allows users to easily change the 'work factor' for bcrypt or 'iterations' for pbkdf2 by setting BCRYPT_ROUNDS and PBKDF2_ITERATIONS, respectively without having to create their own hasher.

There are checks in place to update the password with the new work factor as required.

Ticket: https://code.djangoproject.com/ticket/19043

@apollo13
Django member

Where is the ticket for this?

Jason Buckner and others added some commits
Jason Buckner Added is_current() method and call to contrib.auth.hashers and check_…
…password()

This addition allows users to easily change the work factor for bcrypt or
iterations for pbkdf2 by setting settings.BCRYPT_ROUNDS and settings.PBKDF2_ITERATIONS,
respectively without having to create their own hasher. There are checks in place
to update the password with the new work factor as required
53deeea
Jason Buckner added documentation for new password hasher settings 4841927
@jbuckner jbuckner added a missing skipUnless decorator and grouped the new tests 1c6e3ba
@jbuckner jbuckner adding a test to make sure is_current isn't called when hash has prop…
…er difficulty
999276c
@jbuckner jbuckner changing test case to use PBKDF2 instead of BCrypt so it will not be …
…skipped on systems without bcrypt
f9800ae
@jbuckner jbuckner fixed is_current() method on BCrypt and PBKDF2 PasswordHashers cd654a6
@jbuckner

Oh I hadn't created one. Here it is: https://code.djangoproject.com/ticket/19043

@aaugustin aaugustin closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 30, 2012
  1. @jbuckner

    Added is_current() method and call to contrib.auth.hashers and check_…

    Jason Buckner committed with jbuckner
    …password()
    
    This addition allows users to easily change the work factor for bcrypt or
    iterations for pbkdf2 by setting settings.BCRYPT_ROUNDS and settings.PBKDF2_ITERATIONS,
    respectively without having to create their own hasher. There are checks in place
    to update the password with the new work factor as required
  2. @jbuckner

    added documentation for new password hasher settings

    Jason Buckner committed with jbuckner
  3. @jbuckner
  4. @jbuckner
  5. @jbuckner

    changing test case to use PBKDF2 instead of BCrypt so it will not be …

    jbuckner committed
    …skipped on systems without bcrypt
  6. @jbuckner
  7. @jbuckner
Showing with 79 additions and 4 deletions.
  1. +21 −3 django/contrib/auth/hashers.py
  2. +43 −1 django/contrib/auth/tests/hashers.py
  3. +15 −0 docs/ref/settings.txt
View
24 django/contrib/auth/hashers.py
@@ -51,8 +51,11 @@ def check_password(password, encoded, setter=None, preferred='default'):
preferred = get_hasher(preferred)
hasher = identify_hasher(encoded)
- must_update = hasher.algorithm != preferred.algorithm
+ must_update = (
+ hasher.algorithm != preferred.algorithm or
+ not hasher.is_current(encoded))
is_correct = hasher.verify(password, encoded)
+
if setter and is_correct and must_update:
setter(password)
return is_correct
@@ -176,6 +179,13 @@ def _load_library(self):
raise ValueError("Hasher '%s' doesn't specify a library attribute" %
self.__class__)
+ def is_current(self):
+ """
+ Determines whether or not a password needs to be reencoded based on
+ arbitrary criteria defined by the Hasher
+ """
+ return True
+
def salt(self):
"""
Generates a cryptographically secure nonce salt in ascii
@@ -216,9 +226,13 @@ class PBKDF2PasswordHasher(BasePasswordHasher):
safely but you must rename the algorithm if you change SHA256.
"""
algorithm = "pbkdf2_sha256"
- iterations = 10000
+ iterations = getattr(settings, 'PBKDF2_ITERATIONS', 10000)
digest = hashlib.sha256
+ def is_current(self, encoded):
+ summary = self.safe_summary(encoded)
+ return int(summary['iterations']) == self.iterations
+
def encode(self, password, salt, iterations=None):
assert password
assert salt and '$' not in salt
@@ -267,7 +281,11 @@ class BCryptPasswordHasher(BasePasswordHasher):
"""
algorithm = "bcrypt"
library = ("py-bcrypt", "bcrypt")
- rounds = 12
+ rounds = getattr(settings, 'BCRYPT_ROUNDS', 12)
+
+ def is_current(self, encoded):
+ summary = self.safe_summary(encoded)
+ return int(summary['work factor']) == self.rounds
def salt(self):
bcrypt = self._load_library()
View
44 django/contrib/auth/tests/hashers.py
@@ -3,7 +3,8 @@
from django.conf.global_settings import PASSWORD_HASHERS as default_hashers
from django.contrib.auth.hashers import (is_password_usable,
check_password, make_password, PBKDF2PasswordHasher, load_hashers,
- PBKDF2SHA1PasswordHasher, get_hasher, identify_hasher, UNUSABLE_PASSWORD)
+ PBKDF2SHA1PasswordHasher, BCryptPasswordHasher, get_hasher, identify_hasher,
+ UNUSABLE_PASSWORD)
from django.utils import unittest
from django.utils.unittest import skipUnless
@@ -118,6 +119,47 @@ def test_low_level_pbkdf2_sha1(self):
'pbkdf2_sha1$10000$seasalt$91JiNKgwADC8j2j86Ije/cc4vfQ=')
self.assertTrue(hasher.verify('letmein', encoded))
+ def test_pbkdf2_is_current_returns_false_if_iterations_differs(self):
+ hasher = PBKDF2PasswordHasher()
+ hasher.iterations = 1000
+ encoded = hasher.encode('letmein', 'seasalt')
+ hasher.iterations = 2000
+ self.assertFalse(hasher.is_current(encoded))
+
+ @skipUnless(bcrypt, "py-bcrypt not installed")
+ def test_bcrypt_is_current_returns_false_if_work_factor_differs(self):
+ hasher = BCryptPasswordHasher()
+ hasher.rounds = 2
+ encoded = hasher.encode('letmein', hasher.salt())
+ hasher.rounds = 3
+ self.assertFalse(hasher.is_current(encoded))
+
+ def test_check_password_setter_called_when_is_current_false(self):
+ class ExceptionSetterCalled(Exception):
+ pass
+ def setter(password):
+ raise ExceptionSetterCalled
+ raw_password = 'letmein'
+ hasher = PBKDF2PasswordHasher()
+ # We're going to hash with an iteration of 1. In check_password() below,
+ # it is going to instantiate a new Hasher with the iterations value from
+ # settings or default of 10000, which will make is_current() False.
+ # Is there a better way to guarantee is_current() is false?
+ hasher.iterations = 1
+ encoded = make_password(raw_password, hasher=hasher)
+ self.assertRaises(ExceptionSetterCalled,
+ check_password, *(raw_password, encoded), **{ 'setter': setter })
+
+ def test_check_password_setter_not_called_when_is_current_true(self):
+ class ExceptionSetterCalled(Exception):
+ pass
+ def setter(password):
+ raise ExceptionSetterCalled
+ raw_password = 'letmein'
+ hasher = PBKDF2PasswordHasher()
+ encoded = make_password(raw_password, hasher=hasher)
+ self.assertTrue(check_password(raw_password, encoded, setter))
+
def test_upgrade(self):
self.assertEqual('pbkdf2_sha256', get_hasher('default').algorithm)
for algo in ('sha1', 'md5'):
View
15 docs/ref/settings.txt
@@ -119,6 +119,13 @@ Default: 'auth.User'
The model to use to represent a User. See :ref:`auth-custom-user`.
+BCRYPT_ROUNDS
+---------------
+
+Default: '12'
+
+The work factor used for generating bcrypt hashes when using Django's BCryptPasswordHasher.
+
.. setting:: CACHES
CACHES
@@ -1459,6 +1466,14 @@ The number of days a password reset link is valid for. Used by the
.. setting:: PREPEND_WWW
+PBKDF2_ITERATIONS
+---------------
+
+Default: '10000'
+
+The number of iterations used when generating PBKDF2 hashes when using Django's
+PBKDF2PasswordHasher.
+
PREPEND_WWW
-----------
Something went wrong with that request. Please try again.