Fixed #18184 -- Moved algorithm identification code to hashers module

Thanks Eli Collins for the report and the patch.

django/contrib/auth/forms.py

@@ -7,7 +7,7 @@ from django.utils.translation import ugettext, ugettext_lazy as _
 
 from django.contrib.auth import authenticate
 from django.contrib.auth.models import User
-from django.contrib.auth.hashers import UNUSABLE_PASSWORD, is_password_usable, get_hasher
+from django.contrib.auth.hashers import UNUSABLE_PASSWORD, is_password_usable, identify_hasher
 from django.contrib.auth.tokens import default_token_generator
 from django.contrib.sites.models import get_current_site
 
@@ -25,13 +25,8 @@ class ReadOnlyPasswordHashWidget(forms.Widget):
 
         final_attrs = self.build_attrs(attrs)
 
-        if len(encoded) == 32 and '$' not in encoded:
-            algorithm = 'unsalted_md5'
-        else:
-            algorithm = encoded.split('$', 1)[0]
-
         try:
-            hasher = get_hasher(algorithm)
+            hasher = identify_hasher(encoded)
         except ValueError:
             summary = "<strong>Invalid password format or unknown hashing algorithm.</strong>"
         else:

django/contrib/auth/hashers.py

@@ -40,12 +40,7 @@ def check_password(password, encoded, setter=None, preferred='default'):
         return False
 
     preferred = get_hasher(preferred)
-
+    hasher = identify_hasher(encoded)
-    if len(encoded) == 32 and '$' not in encoded:
-        hasher = get_hasher('unsalted_md5')
-    else:
-        algorithm = encoded.split('$', 1)[0]
-        hasher = get_hasher(algorithm)
 
     must_update = hasher.algorithm != preferred.algorithm
     is_correct = hasher.verify(password, encoded)
@@ -120,6 +115,21 @@ def get_hasher(algorithm='default'):
         return HASHERS[algorithm]
 
 
+def identify_hasher(encoded):
+    """
+    Returns an instance of a loaded password hasher.
+
+    Identifies hasher algorithm by examining encoded hash, and calls
+    get_hasher() to return hasher. Raises ValueError if
+    algorithm cannot be identified, or if hasher is not loaded.
+    """
+    if len(encoded) == 32 and '$' not in encoded:
+        algorithm = 'unsalted_md5'
+    else:
+        algorithm = encoded.split('$', 1)[0]
+    return get_hasher(algorithm)
+
+
 def mask_hash(hash, show=6, char="*"):
     """
     Returns the given hash, with only the first ``show`` number shown. The

django/contrib/auth/tests/hashers.py

@@ -1,7 +1,7 @@
 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, UNUSABLE_PASSWORD)
+    PBKDF2SHA1PasswordHasher, get_hasher, identify_hasher, UNUSABLE_PASSWORD)
 from django.utils import unittest
 from django.utils.unittest import skipUnless
 
@@ -36,6 +36,7 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(is_password_usable(encoded))
         self.assertTrue(check_password(u'letmein', encoded))
         self.assertFalse(check_password('letmeinz', encoded))
+        self.assertEqual(identify_hasher(encoded).algorithm, "pbkdf2_sha256")
 
     def test_sha1(self):
         encoded = make_password('letmein', 'seasalt', 'sha1')
@@ -44,6 +45,7 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(is_password_usable(encoded))
         self.assertTrue(check_password(u'letmein', encoded))
         self.assertFalse(check_password('letmeinz', encoded))
+        self.assertEqual(identify_hasher(encoded).algorithm, "sha1")
 
     def test_md5(self):
         encoded = make_password('letmein', 'seasalt', 'md5')
@@ -52,6 +54,7 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(is_password_usable(encoded))
         self.assertTrue(check_password(u'letmein', encoded))
         self.assertFalse(check_password('letmeinz', encoded))
+        self.assertEqual(identify_hasher(encoded).algorithm, "md5")
 
     def test_unsalted_md5(self):
         encoded = make_password('letmein', 'seasalt', 'unsalted_md5')
@@ -59,6 +62,7 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(is_password_usable(encoded))
         self.assertTrue(check_password(u'letmein', encoded))
         self.assertFalse(check_password('letmeinz', encoded))
+        self.assertEqual(identify_hasher(encoded).algorithm, "unsalted_md5")
 
     @skipUnless(crypt, "no crypt module to generate password.")
     def test_crypt(self):
@@ -67,6 +71,7 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(is_password_usable(encoded))
         self.assertTrue(check_password(u'letmein', encoded))
         self.assertFalse(check_password('letmeinz', encoded))
+        self.assertEqual(identify_hasher(encoded).algorithm, "crypt")
 
     @skipUnless(bcrypt, "py-bcrypt not installed")
     def test_bcrypt(self):
@@ -75,6 +80,7 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(encoded.startswith('bcrypt$'))
         self.assertTrue(check_password(u'letmein', encoded))
         self.assertFalse(check_password('letmeinz', encoded))
+        self.assertEqual(identify_hasher(encoded).algorithm, "bcrypt")
 
     def test_unusable(self):
         encoded = make_password(None)
@@ -84,11 +90,13 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertFalse(check_password('', encoded))
         self.assertFalse(check_password(u'letmein', encoded))
         self.assertFalse(check_password('letmeinz', encoded))
+        self.assertRaises(ValueError, identify_hasher, encoded)
 
     def test_bad_algorithm(self):
         def doit():
             make_password('letmein', hasher='lolcat')
         self.assertRaises(ValueError, doit)
+        self.assertRaises(ValueError, identify_hasher, "lolcat$salt$hash")
 
     def test_low_level_pkbdf2(self):
         hasher = PBKDF2PasswordHasher()