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

reverted due to lack of bcrypt in vendor

apps/users/management/commands/strengthen_user_passwords.py

@@ -1,16 +0,0 @@
-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,15 +1,11 @@
-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
@@ -33,21 +29,6 @@ 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()
 
@@ -57,21 +38,9 @@ def rand_string(length):
 
 
 def create_password(algorithm, raw_password):
-    if algorithm == 'bcrypt':
+    salt = get_hexdigest(algorithm, rand_string(12), rand_string(12))[:64]
-        try:
+    hsh = get_hexdigest(algorithm, salt, raw_password)
-            latest_key_id = max(settings.HMAC_KEYS.keys())
+    return '$'.join([algorithm, salt, hsh])
-        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):
@@ -292,47 +261,21 @@ class UserProfile(amo.models.OnChangeMixin, amo.models.ModelBase):
             delete_user.delete()
 
     def check_password(self, raw_password):
-        def passwords_match(algo_and_hash, key_ver, hmac_input):
+        if '$' not in self.password:
-            shared_key = settings.HMAC_KEYS.get(key_ver, None)
+            valid = (get_hexdigest('md5', '', raw_password) == self.password)
-            if not shared_key:
+            if valid:
-                log.info('Invalid shared key version "{0}"'.format(key_ver))
+                # Upgrade an old password.
-                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 True
+            return valid
-        else:
-            algo, salt, hsh = self.password.split('$')
-            return hsh == get_hexdigest(algo, salt, raw_password)
 
-    def set_password(self, raw_password, algorithm=None):
+        algo, salt, hsh = self.password.split('$')
-        if algorithm is None:
+        return hsh == get_hexdigest(algo, salt, raw_password)
-            algorithm = settings.PWD_ALGORITHM
+
+    def set_password(self, raw_password, algorithm='sha512'):
         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,7 +6,6 @@ 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_
@@ -183,72 +182,28 @@ class TestUserProfile(amo.tests.TestCase):
 class TestPasswords(amo.tests.TestCase):
     utf = u'\u0627\u0644\u062a\u0637\u0628'
 
-    def setUp(self):
+    def test_invalid_old_password(self):
-        settings.HMAC_KEYS = {'2011-12-01': 'secret1'}
+        u = UserProfile(password=self.utf)
+        assert u.check_password(self.utf) is False
 
-    def tearDown(self):
+    def test_invalid_new_password(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.password.startswith('bcrypt')
+        assert u.check_password('wrong') is False
-        assert u.check_password(self.utf)
 
-    def test_invalid_bcrypt_password(self):
+    def test_valid_old_password(self):
-        u = UserProfile()
+        hsh = hashlib.md5(encoding.smart_str(self.utf)).hexdigest()
-        u.set_password(self.utf, algorithm='bcrypt')
+        u = UserProfile(password=hsh)
-        assert not u.check_password('wrong')
+        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_hh_password(self):
+    def test_valid_new_password(self):
         u = UserProfile()
-        u.set_password(self.utf, algorithm='sha512')
+        u.set_password(self.utf)
-        u.upgrade_password_to(algorithm='bcrypt')
+        assert u.check_password(self.utf) is True
-        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,17 +1356,3 @@ 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',
-#             }