Code

Ticket #20760: 20760_fix_hash_once.diff

File 20760_fix_hash_once.diff, 3.1 KB (added by jpaglier, 12 months ago)

set_password() solution and test

Line 
1diff --git a/django/contrib/auth/backends.py b/django/contrib/auth/backends.py
2index 6b31f72..4ee326b 100644
3--- a/django/contrib/auth/backends.py
4+++ b/django/contrib/auth/backends.py
5@@ -10,6 +10,9 @@ class ModelBackend(object):
6 
7     def authenticate(self, username=None, password=None, **kwargs):
8         UserModel = get_user_model()
9+
10+        user = UserModel()
11+
12         if username is None:
13             username = kwargs.get(UserModel.USERNAME_FIELD)
14         try:
15@@ -17,6 +20,11 @@ class ModelBackend(object):
16             if user.check_password(password):
17                 return user
18         except UserModel.DoesNotExist:
19+            #This call to set password is used to force the hasher to run.
20+            #It helps to even out the timing between a successful and
21+            #unsuccessful authentication attempt, helping to mitigate a timing
22+            #vuln.
23+            user.set_password(password)
24             return None
25 
26     def get_group_permissions(self, user_obj, obj=None):
27diff --git a/django/contrib/auth/tests/test_auth_backends.py b/django/contrib/auth/tests/test_auth_backends.py
28index fc5a80e..aa69220 100644
29--- a/django/contrib/auth/tests/test_auth_backends.py
30+++ b/django/contrib/auth/tests/test_auth_backends.py
31@@ -12,7 +12,22 @@ from django.contrib.auth import authenticate, get_user
32 from django.http import HttpRequest
33 from django.test import TestCase
34 from django.test.utils import override_settings
35+from django.contrib.auth.hashers import MD5PasswordHasher
36 
37+"""
38+Mock hasher which counts number of times hashes are computed
39+
40+Used in test_authentication_timing
41+
42+Refs #20760
43+"""
44+class MockHasher(MD5PasswordHasher):
45+    def __init__(self):
46+        MockHasher.hash_count = 0
47+
48+    def encode(self, password, salt, iterations=None):
49+        MockHasher.hash_count = MockHasher.hash_count + 1
50+        return super(MockHasher, self).encode(password, salt, iterations)
51 
52 class BaseModelBackendTest(object):
53     """
54@@ -111,6 +126,26 @@ class BaseModelBackendTest(object):
55         user = self.UserModel._default_manager.get(pk=self.superuser.pk)
56         self.assertEqual(len(user.get_all_permissions()), len(Permission.objects.all()))
57 
58+    @override_settings(PASSWORD_HASHERS=('django.contrib.auth.tests.test_auth_backends.MockHasher',))
59+    def test_authentication_timing(self):
60+        """
61+        Tests that the hasher is run the same number of times whether a user exists or not
62+
63+        Refs #20760
64+        """
65+        #ensures the proper natural key is passed in custom models
66+        kwargs = {self.UserModel.USERNAME_FIELD : self.UserModel._default_manager.get(pk=1).natural_key()[0]}
67+        authenticate(password='test', **kwargs)
68+        exists = MockHasher.hash_count
69+
70+        MockHasher.hash_count = 0
71+
72+        #don't use these creds in a test account
73+        kwargs = {self.UserModel.USERNAME_FIELD : 'paglierani'}
74+        authenticate(password='justin', **kwargs)
75+        dne = MockHasher.hash_count
76+        self.assertEqual(exists, dne) #counts should be the same
77+
78 
79 @skipIfCustomUser
80 class ModelBackendTest(BaseModelBackendTest, TestCase):