Support for bcrypt+hmac for password hashing (bug 639696)

Includes upgrade for existing sha512 passwords via
`./manage.py strengthen_user_passwords`.

Requires an HMAC_KEYS dict to be added to `settings_local.py`.

apps/users/management/commands/strengthen_user_passwords.py

@@ -0,0 +1,16 @@
+from django.conf import settings
+from django.core.management.base import NoArgsCommand
+from users.models import UserProfile
+
+
+class Command(NoArgsCommand):
+    requires_model_validation = False
+    output_transaction = True
+
+    def handle_noargs(self, **options):
+
+        if not settings.PWD_ALGORITHM == 'bcrypt':
+            return
+
+        for user in UserProfile.objects.all():
+            user.upgrade_password_to(algorithm='bcrypt')

apps/users/models.py

@@ -1,11 +1,15 @@
+import base64
 from datetime import datetime
 import hashlib
+import hmac
 import os
 import random
 import re
 import string
 import time
 
+import bcrypt
+
 from django import forms, dispatch
 from django.conf import settings
 from django.contrib.auth.models import User as DjangoUser
@@ -29,6 +33,21 @@ from translations.query import order_by_translation
 log = commonware.log.getLogger('z.users')
 
 
+def _hmac_create(userpwd, shared_key):
+    """Create HMAC value based on pwd and system-local and per-user salt."""
+    hmac_value = base64.b64encode(hmac.new(
+        smart_str(shared_key), smart_str(userpwd), hashlib.sha512).digest())
+    return hmac_value
+
+
+def _bcrypt_create(hmac_value):
+    """Create bcrypt hash."""
+    rounds = getattr(settings, 'BCRYPT_ROUNDS', 12)
+    # No need for us to create a user salt, bcrypt creates its own.
+    bcrypt_value = bcrypt.hashpw(hmac_value, bcrypt.gensalt(int(rounds)))
+    return bcrypt_value
+
+
 def get_hexdigest(algorithm, salt, raw_password):
     return hashlib.new(algorithm, smart_str(salt + raw_password)).hexdigest()
 
@@ -38,9 +57,21 @@ def rand_string(length):
 
 
 def create_password(algorithm, raw_password):
-    salt = get_hexdigest(algorithm, rand_string(12), rand_string(12))[:64]
-    hsh = get_hexdigest(algorithm, salt, raw_password)
-    return '$'.join([algorithm, salt, hsh])
+    if algorithm == 'bcrypt':
+        try:
+            latest_key_id = max(settings.HMAC_KEYS.keys())
+        except (AttributeError, ValueError):
+            raise ValueError('settings.HMAC_KEYS must be set to a dict whose '
+                             'values are secret keys.')
+        shared_key = settings.HMAC_KEYS[latest_key_id]
+        hmac_value = _hmac_create(raw_password, shared_key)
+        return ''.join((
+                'bcrypt', _bcrypt_create(hmac_value),
+                '$', latest_key_id))
+    else:
+        salt = get_hexdigest(algorithm, rand_string(12), rand_string(12))[:64]
+        hsh = get_hexdigest(algorithm, salt, raw_password)
+        return '$'.join([algorithm, salt, hsh])
 
 
 class UserForeignKey(models.ForeignKey):
@@ -261,21 +292,47 @@ class UserProfile(amo.models.OnChangeMixin, amo.models.ModelBase):
             delete_user.delete()
 
     def check_password(self, raw_password):
-        if '$' not in self.password:
-            valid = (get_hexdigest('md5', '', raw_password) == self.password)
-            if valid:
-                # Upgrade an old password.
+        def passwords_match(algo_and_hash, key_ver, hmac_input):
+            shared_key = settings.HMAC_KEYS.get(key_ver, None)
+            if not shared_key:
+                log.info('Invalid shared key version "{0}"'.format(key_ver))
+                return False
+            bcrypt_value = algo_and_hash[6:]
+            hmac_value = _hmac_create(hmac_input, shared_key)
+            return bcrypt.hashpw(hmac_value, bcrypt_value) == bcrypt_value
+
+        if self.password.startswith('bcrypt'):
+            algo_and_hash, key_ver = self.password.rsplit('$', 1)
+            return passwords_match(algo_and_hash, key_ver, raw_password)
+        elif self.password.startswith('hh'):
+            alg, salt, bc_pwd = self.password.split('$', 3)[1:]
+            hsh = get_hexdigest(alg, salt, raw_password)
+            algo_and_hash, key_ver = bc_pwd.rsplit('$', 1)
+            if passwords_match(algo_and_hash, key_ver,
+                               '$'.join((alg, salt, hsh))):
+                # Re-create the hash in regular bcrypt format.
                 self.set_password(raw_password)
                 self.save()
-            return valid
+                return True
+        else:
+            algo, salt, hsh = self.password.split('$')
+            return hsh == get_hexdigest(algo, salt, raw_password)
 
-        algo, salt, hsh = self.password.split('$')
-        return hsh == get_hexdigest(algo, salt, raw_password)
-
-    def set_password(self, raw_password, algorithm='sha512'):
+    def set_password(self, raw_password, algorithm=None):
+        if algorithm is None:
+            algorithm = settings.PWD_ALGORITHM
         self.password = create_password(algorithm, raw_password)
         # Can't do CEF logging here because we don't have a request object.
 
+    def upgrade_password_to(self, algorithm):
+        if (not self.password or
+            self.password.startswith(('hh', 'bcrypt'))):
+            return
+        algo, salt, hsh = self.password.split('$')
+        bc_value = create_password('bcrypt', self.password)
+        self.password = u'$'.join(['hh', algo, salt, bc_value])
+        self.save()
+
     def email_confirmation_code(self):
         from amo.utils import send_mail
         log.debug("Sending account confirmation code for user (%s)", self)

apps/users/tests/test_models.py

@@ -6,6 +6,7 @@ from django import forms
 from django.contrib.auth.models import User
 from django.core import mail
 from django.utils import encoding
+from django.conf import settings
 
 from mock import patch
 from nose.tools import eq_
@@ -182,28 +183,72 @@ class TestUserProfile(amo.tests.TestCase):
 class TestPasswords(amo.tests.TestCase):
     utf = u'\u0627\u0644\u062a\u0637\u0628'
 
-    def test_invalid_old_password(self):
-        u = UserProfile(password=self.utf)
-        assert u.check_password(self.utf) is False
+    def setUp(self):
+        settings.HMAC_KEYS = {'2011-12-01': 'secret1'}
 
-    def test_invalid_new_password(self):
+    def tearDown(self):
+        del settings.HMAC_KEYS
+
+    def test_invalid_sha512_password(self):
+        u = UserProfile()
+        u.set_password(self.utf, algorithm='sha512')
+        assert not u.check_password('wrong')
+
+    def test_valid_sha512_password(self):
+        u = UserProfile()
+        u.set_password(self.utf, algorithm='sha512')
+        assert u.check_password(self.utf)
+
+    def test_valid_bcrypt_password(self):
+        u = UserProfile()
+        u.set_password(self.utf, algorithm='bcrypt')
+        assert u.password.startswith('bcrypt')
+        assert u.check_password(self.utf)
+
+    def test_bcrypt_is_default(self):
         u = UserProfile()
         u.set_password(self.utf)
-        assert u.check_password('wrong') is False
+        assert u.password.startswith('bcrypt')
+        assert u.check_password(self.utf)
 
-    def test_valid_old_password(self):
-        hsh = hashlib.md5(encoding.smart_str(self.utf)).hexdigest()
-        u = UserProfile(password=hsh)
-        assert u.check_password(self.utf) is True
-        # Make sure we updated the old password.
-        algo, salt, hsh = u.password.split('$')
-        eq_(algo, 'sha512')
-        eq_(hsh, get_hexdigest(algo, salt, self.utf))
-
-    def test_valid_new_password(self):
+    def test_invalid_bcrypt_password(self):
         u = UserProfile()
-        u.set_password(self.utf)
-        assert u.check_password(self.utf) is True
+        u.set_password(self.utf, algorithm='bcrypt')
+        assert not u.check_password('wrong')
+
+    def test_valid_hh_password(self):
+        u = UserProfile()
+        u.set_password(self.utf, algorithm='sha512')
+        u.upgrade_password_to(algorithm='bcrypt')
+        assert u.password.startswith('hh$')
+        assert u.check_password(self.utf)
+
+    def test_already_bcrypt(self):
+        u = UserProfile()
+        u.set_password(self.utf, algorithm='bcrypt')
+        old_password = u.password
+        u.upgrade_password_to(algorithm='bcrypt')
+        eq_(old_password, u.password)
+
+    @patch.object(settings, 'HMAC_KEYS', {})
+    def test_no_hmac_key(self):
+        u = UserProfile()
+        with self.assertRaises(ValueError) as a:
+            u.set_password(self.utf, algorithm='bcrypt')
+        assert 'HMAC_KEYS' in a.exception.message
+
+    def test_stale_hmac_key_bcrypt(self):
+        u = UserProfile()
+        u.set_password(self.utf, algorithm='bcrypt')
+        settings.HMAC_KEYS = {'2011-12-02': 'secret2'}
+        assert not u.check_password(self.utf)
+
+    def test_stale_hmac_key_hh(self):
+        u = UserProfile()
+        u.set_password(self.utf, algorithm='sha512')
+        u.upgrade_password_to(algorithm='bcrypt')
+        settings.HMAC_KEYS = {'2011-12-02': 'secret2'}
+        assert not u.check_password(self.utf)
 
 
 class TestBlacklistedUsername(amo.tests.TestCase):

settings.py

@@ -1356,3 +1356,17 @@ IGNORE_NON_CRITICAL_CRONS = False
 # To enable new default to compatible checks in services/update.py set to True.
 # Set to False in case of emergency to switch back to old code.
 DEFAULT_TO_COMPATIBLE = True
+
+
+# Use bcrypt for new passwords, and enable upgrade of old ones.
+PWD_ALGORITHM = 'bcrypt'
+
+# Add an HMAC_KEYS dict in your local settings, containing identifiers
+# as keys and secrets as values. The largest key will be put as
+# plaintext into the hashed password field, and the corresponding
+# value will be used as the secret for HMAC hashing in passwords. For
+# example:
+
+# HMAC_KEYS = {'2011-12-01': 'ChristmasIsMagic',
+#              '2012-01-01': 'PotchHasAUnicornRanch',
+#             }