From f1255a3c0904a55ef267fa5f8687a1ce78f6894a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 25 Feb 2013 20:01:57 +0100 Subject: [PATCH] Fixed #18144 -- Restored compatibility with SHA1 hashes with empty salt. Thanks dahool for the report and initial version of the patch. --- django/conf/global_settings.py | 1 + django/contrib/auth/hashers.py | 50 +++++++++++++++++++++++++--- django/contrib/auth/tests/hashers.py | 17 ++++++++-- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py index 659f2f42b7..eaa8a9911c 100644 --- a/django/conf/global_settings.py +++ b/django/conf/global_settings.py @@ -510,6 +510,7 @@ PASSWORD_HASHERS = ( 'django.contrib.auth.hashers.BCryptPasswordHasher', 'django.contrib.auth.hashers.SHA1PasswordHasher', 'django.contrib.auth.hashers.MD5PasswordHasher', + 'django.contrib.auth.hashers.UnsaltedSHA1PasswordHasher', 'django.contrib.auth.hashers.UnsaltedMD5PasswordHasher', 'django.contrib.auth.hashers.CryptPasswordHasher', ) diff --git a/django/contrib/auth/hashers.py b/django/contrib/auth/hashers.py index bd760cd3cc..480fde69ce 100644 --- a/django/contrib/auth/hashers.py +++ b/django/contrib/auth/hashers.py @@ -127,9 +127,14 @@ def identify_hasher(encoded): get_hasher() to return hasher. Raises ValueError if algorithm cannot be identified, or if hasher is not loaded. """ + # Ancient versions of Django created plain MD5 passwords and accepted + # MD5 passwords with an empty salt. if ((len(encoded) == 32 and '$' not in encoded) or (len(encoded) == 37 and encoded.startswith('md5$$'))): algorithm = 'unsalted_md5' + # Ancient versions of Django accepted SHA1 passwords with an empty salt. + elif len(encoded) == 46 and encoded.startswith('sha1$$'): + algorithm = 'unsalted_sha1' else: algorithm = encoded.split('$', 1)[0] return get_hasher(algorithm) @@ -350,14 +355,48 @@ class MD5PasswordHasher(BasePasswordHasher): ]) +class UnsaltedSHA1PasswordHasher(BasePasswordHasher): + """ + Very insecure algorithm that you should *never* use; stores SHA1 hashes + with an empty salt. + + This class is implemented because Django used to accept such password + hashes. Some older Django installs still have these values lingering + around so we need to handle and upgrade them properly. + """ + algorithm = "unsalted_sha1" + + def salt(self): + return '' + + def encode(self, password, salt): + assert salt == '' + hash = hashlib.sha1(force_bytes(password)).hexdigest() + return 'sha1$$%s' % hash + + def verify(self, password, encoded): + encoded_2 = self.encode(password, '') + return constant_time_compare(encoded, encoded_2) + + def safe_summary(self, encoded): + assert encoded.startswith('sha1$$') + hash = encoded[6:] + return SortedDict([ + (_('algorithm'), self.algorithm), + (_('hash'), mask_hash(hash)), + ]) + + class UnsaltedMD5PasswordHasher(BasePasswordHasher): """ - I am an incredibly insecure algorithm you should *never* use; - stores unsalted MD5 hashes without the algorithm prefix. + Incredibly insecure algorithm that you should *never* use; stores unsalted + MD5 hashes without the algorithm prefix, also accepts MD5 hashes with an + empty salt. - This class is implemented because Django used to store passwords - this way. Some older Django installs still have these values - lingering around so we need to handle and upgrade them properly. + This class is implemented because Django used to store passwords this way + and to accept such password hashes. Some older Django installs still have + these values lingering around so we need to handle and upgrade them + properly. """ algorithm = "unsalted_md5" @@ -365,6 +404,7 @@ class UnsaltedMD5PasswordHasher(BasePasswordHasher): return '' def encode(self, password, salt): + assert salt == '' return hashlib.md5(force_bytes(password)).hexdigest() def verify(self, password, encoded): diff --git a/django/contrib/auth/tests/hashers.py b/django/contrib/auth/tests/hashers.py index d53ec1c173..2b2243cb0c 100644 --- a/django/contrib/auth/tests/hashers.py +++ b/django/contrib/auth/tests/hashers.py @@ -2,7 +2,7 @@ from __future__ import unicode_literals from django.conf.global_settings import PASSWORD_HASHERS as default_hashers -from django.contrib.auth.hashers import (is_password_usable, +from django.contrib.auth.hashers import (is_password_usable, check_password, make_password, PBKDF2PasswordHasher, load_hashers, PBKDF2SHA1PasswordHasher, get_hasher, identify_hasher, UNUSABLE_PASSWORD) from django.utils import unittest @@ -52,7 +52,7 @@ class TestUtilsHashPass(unittest.TestCase): def test_md5(self): encoded = make_password('lètmein', 'seasalt', 'md5') - self.assertEqual(encoded, + self.assertEqual(encoded, 'md5$seasalt$3f86d0d3d465b7b458c231bf3555c0e3') self.assertTrue(is_password_usable(encoded)) self.assertTrue(check_password('lètmein', encoded)) @@ -60,7 +60,7 @@ class TestUtilsHashPass(unittest.TestCase): self.assertEqual(identify_hasher(encoded).algorithm, "md5") def test_unsalted_md5(self): - encoded = make_password('lètmein', 'seasalt', 'unsalted_md5') + encoded = make_password('lètmein', '', 'unsalted_md5') self.assertEqual(encoded, '88a434c88cca4e900f7874cd98123f43') self.assertTrue(is_password_usable(encoded)) self.assertTrue(check_password('lètmein', encoded)) @@ -72,6 +72,17 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(check_password('lètmein', alt_encoded)) self.assertFalse(check_password('lètmeinz', alt_encoded)) + def test_unsalted_sha1(self): + encoded = make_password('lètmein', '', 'unsalted_sha1') + self.assertEqual(encoded, 'sha1$$6d138ca3ae545631b3abd71a4f076ce759c5700b') + self.assertTrue(is_password_usable(encoded)) + self.assertTrue(check_password('lètmein', encoded)) + self.assertFalse(check_password('lètmeinz', encoded)) + self.assertEqual(identify_hasher(encoded).algorithm, "unsalted_sha1") + # Raw SHA1 isn't acceptable + alt_encoded = encoded[6:] + self.assertFalse(check_password('lètmein', alt_encoded)) + @skipUnless(crypt, "no crypt module to generate password.") def test_crypt(self): encoded = make_password('lètmei', 'ab', 'crypt')