From 2c4fe761a0e2b28e2c5c3b4bc506ee06824a443d Mon Sep 17 00:00:00 2001 From: Erik Romijn Date: Mon, 17 Jun 2013 18:06:26 +0200 Subject: [PATCH] Fixed #20593 -- Allow blank passwords in check_password() and set_password() --- django/contrib/auth/hashers.py | 12 ++--- django/contrib/auth/tests/test_hashers.py | 53 +++++++++++++++++++++++ docs/ref/contrib/auth.txt | 16 +++++++ docs/releases/1.6.txt | 9 ++++ docs/topics/auth/customizing.txt | 16 +++++++ docs/topics/auth/passwords.txt | 6 +++ 6 files changed, 106 insertions(+), 6 deletions(-) diff --git a/django/contrib/auth/hashers.py b/django/contrib/auth/hashers.py index 36d247d71e..d8a04e1473 100644 --- a/django/contrib/auth/hashers.py +++ b/django/contrib/auth/hashers.py @@ -47,7 +47,7 @@ def check_password(password, encoded, setter=None, preferred='default'): If setter is specified, it'll be called when you need to regenerate the password. """ - if not password or not is_password_usable(encoded): + if not is_password_usable(encoded): return False preferred = get_hasher(preferred) @@ -65,10 +65,10 @@ def make_password(password, salt=None, hasher='default'): Turn a plain-text password into a hash for database storage Same as encode() but generates a new random salt. If - password is None or blank then UNUSABLE_PASSWORD will be + password is None then UNUSABLE_PASSWORD will be returned which disallows logins. """ - if not password: + if password is None: return UNUSABLE_PASSWORD hasher = get_hasher(hasher) @@ -222,7 +222,7 @@ class PBKDF2PasswordHasher(BasePasswordHasher): digest = hashlib.sha256 def encode(self, password, salt, iterations=None): - assert password + assert password is not None assert salt and '$' not in salt if not iterations: iterations = self.iterations @@ -350,7 +350,7 @@ class SHA1PasswordHasher(BasePasswordHasher): algorithm = "sha1" def encode(self, password, salt): - assert password + assert password is not None assert salt and '$' not in salt hash = hashlib.sha1(force_bytes(salt + password)).hexdigest() return "%s$%s$%s" % (self.algorithm, salt, hash) @@ -378,7 +378,7 @@ class MD5PasswordHasher(BasePasswordHasher): algorithm = "md5" def encode(self, password, salt): - assert password + assert password is not None assert salt and '$' not in salt hash = hashlib.md5(force_bytes(salt + password)).hexdigest() return "%s$%s$%s" % (self.algorithm, salt, hash) diff --git a/django/contrib/auth/tests/test_hashers.py b/django/contrib/auth/tests/test_hashers.py index 4becc2dffe..9df6bd8592 100644 --- a/django/contrib/auth/tests/test_hashers.py +++ b/django/contrib/auth/tests/test_hashers.py @@ -32,6 +32,12 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(is_password_usable(encoded)) self.assertTrue(check_password('lètmein', encoded)) self.assertFalse(check_password('lètmeinz', encoded)) + # Blank passwords + blank_encoded = make_password('') + self.assertTrue(blank_encoded.startswith('pbkdf2_sha256$')) + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) def test_pkbdf2(self): encoded = make_password('lètmein', 'seasalt', 'pbkdf2_sha256') @@ -41,6 +47,12 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(check_password('lètmein', encoded)) self.assertFalse(check_password('lètmeinz', encoded)) self.assertEqual(identify_hasher(encoded).algorithm, "pbkdf2_sha256") + # Blank passwords + blank_encoded = make_password('', 'seasalt', 'pbkdf2_sha256') + self.assertTrue(blank_encoded.startswith('pbkdf2_sha256$')) + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) def test_sha1(self): encoded = make_password('lètmein', 'seasalt', 'sha1') @@ -50,6 +62,12 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(check_password('lètmein', encoded)) self.assertFalse(check_password('lètmeinz', encoded)) self.assertEqual(identify_hasher(encoded).algorithm, "sha1") + # Blank passwords + blank_encoded = make_password('', 'seasalt', 'sha1') + self.assertTrue(blank_encoded.startswith('sha1$')) + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) def test_md5(self): encoded = make_password('lètmein', 'seasalt', 'md5') @@ -59,6 +77,12 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(check_password('lètmein', encoded)) self.assertFalse(check_password('lètmeinz', encoded)) self.assertEqual(identify_hasher(encoded).algorithm, "md5") + # Blank passwords + blank_encoded = make_password('', 'seasalt', 'md5') + self.assertTrue(blank_encoded.startswith('md5$')) + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) def test_unsalted_md5(self): encoded = make_password('lètmein', '', 'unsalted_md5') @@ -72,6 +96,11 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(is_password_usable(alt_encoded)) self.assertTrue(check_password('lètmein', alt_encoded)) self.assertFalse(check_password('lètmeinz', alt_encoded)) + # Blank passwords + blank_encoded = make_password('', '', 'unsalted_md5') + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) def test_unsalted_sha1(self): encoded = make_password('lètmein', '', 'unsalted_sha1') @@ -83,6 +112,12 @@ class TestUtilsHashPass(unittest.TestCase): # Raw SHA1 isn't acceptable alt_encoded = encoded[6:] self.assertFalse(check_password('lètmein', alt_encoded)) + # Blank passwords + blank_encoded = make_password('', '', 'unsalted_sha1') + self.assertTrue(blank_encoded.startswith('sha1$')) + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) @skipUnless(crypt, "no crypt module to generate password.") def test_crypt(self): @@ -92,6 +127,12 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(check_password('lètmei', encoded)) self.assertFalse(check_password('lètmeiz', encoded)) self.assertEqual(identify_hasher(encoded).algorithm, "crypt") + # Blank passwords + blank_encoded = make_password('', 'ab', 'crypt') + self.assertTrue(blank_encoded.startswith('crypt$')) + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) @skipUnless(bcrypt, "bcrypt not installed") def test_bcrypt_sha256(self): @@ -108,6 +149,12 @@ class TestUtilsHashPass(unittest.TestCase): encoded = make_password(password, hasher='bcrypt_sha256') self.assertTrue(check_password(password, encoded)) self.assertFalse(check_password(password[:72], encoded)) + # Blank passwords + blank_encoded = make_password('', hasher='bcrypt_sha256') + self.assertTrue(blank_encoded.startswith('bcrypt_sha256$')) + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) @skipUnless(bcrypt, "bcrypt not installed") def test_bcrypt(self): @@ -117,6 +164,12 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(check_password('lètmein', encoded)) self.assertFalse(check_password('lètmeinz', encoded)) self.assertEqual(identify_hasher(encoded).algorithm, "bcrypt") + # Blank passwords + blank_encoded = make_password('', hasher='bcrypt') + self.assertTrue(blank_encoded.startswith('bcrypt$')) + self.assertTrue(is_password_usable(blank_encoded)) + self.assertTrue(check_password('', blank_encoded)) + self.assertFalse(check_password(' ', blank_encoded)) def test_unusable(self): encoded = make_password(None) diff --git a/docs/ref/contrib/auth.txt b/docs/ref/contrib/auth.txt index 40b3629f63..afbc6ec048 100644 --- a/docs/ref/contrib/auth.txt +++ b/docs/ref/contrib/auth.txt @@ -132,12 +132,28 @@ Methods password hashing. Doesn't save the :class:`~django.contrib.auth.models.User` object. + When the ``raw_password`` is ``None``, the password will be set to an + unusable password, as if + :meth:`~django.contrib.auth.models.User.set_unusable_password()` + were used. + + .. versionchanged:: 1.6 + + In Django 1.4 and 1.5, a blank string was unintentionally stored + as an unsable password. + .. method:: check_password(raw_password) Returns ``True`` if the given raw string is the correct password for the user. (This takes care of the password hashing in making the comparison.) + .. versionchanged:: 1.6 + + In Django 1.4 and 1.5, a blank string was unintentionally + considered to be an unusable password, resulting in this method + returning ``False`` for such a password. + .. method:: set_unusable_password() Marks the user as having no password set. This isn't the same as diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index f260a1eac0..8dea37d2da 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -701,6 +701,15 @@ Miscellaneous * :class:`~django.views.generic.base.RedirectView` now has a `pattern_name` attribute which allows it to choose the target by reversing the URL. +* In Django 1.4 and 1.5, a blank string was unintentionally not considered to + be a valid password. This meant + :meth:`~django.contrib.auth.models.User.set_password()` would save a blank + password as an unusable password like + :meth:`~django.contrib.auth.models.User.set_unusable_password()` does, and + thus :meth:`~django.contrib.auth.models.User.check_password()` always + returned ``False`` for blank passwords. This has been corrected in this + release: blank passwords are now valid. + Features deprecated in 1.6 ========================== diff --git a/docs/topics/auth/customizing.txt b/docs/topics/auth/customizing.txt index bc021b14ad..ea4ec8686d 100644 --- a/docs/topics/auth/customizing.txt +++ b/docs/topics/auth/customizing.txt @@ -583,12 +583,28 @@ The following methods are available on any subclass of password hashing. Doesn't save the :class:`~django.contrib.auth.models.AbstractBaseUser` object. + When the raw_password is ``None``, the password will be set to an + unusable password, as if + :meth:`~django.contrib.auth.models.AbstractBaseUser.set_unusable_password()` + were used. + + .. versionchanged:: 1.6 + + In Django 1.4 and 1.5, a blank string was unintentionally stored + as an unsable password as well. + .. method:: models.AbstractBaseUser.check_password(raw_password) Returns ``True`` if the given raw string is the correct password for the user. (This takes care of the password hashing in making the comparison.) + .. versionchanged:: 1.6 + + In Django 1.4 and 1.5, a blank string was unintentionally + considered to be an unusable password, resulting in this method + returning ``False`` for such a password. + .. method:: models.AbstractBaseUser.set_unusable_password() Marks the user as having no password set. This isn't the same as diff --git a/docs/topics/auth/passwords.txt b/docs/topics/auth/passwords.txt index 206e7d856c..7e4b59a99c 100644 --- a/docs/topics/auth/passwords.txt +++ b/docs/topics/auth/passwords.txt @@ -206,6 +206,12 @@ from the ``User`` model. database to check against, and returns ``True`` if they match, ``False`` otherwise. + .. versionchanged:: 1.6 + + In Django 1.4 and 1.5, a blank string was unintentionally considered + to be an unusable password, resulting in this method returning + ``False`` for such a password. + .. function:: make_password(password[, salt, hashers]) Creates a hashed password in the format used by this application. It takes