Skip to content

Commit

Permalink
Fixed #20593 -- Allow blank passwords in check_password() and set_pas…
Browse files Browse the repository at this point in the history
…sword()
  • Loading branch information
mxsasha authored and timgraham committed Jun 18, 2013
1 parent 3128f3d commit 2c4fe76
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 6 deletions.
12 changes: 6 additions & 6 deletions django/contrib/auth/hashers.py
Expand Up @@ -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 If setter is specified, it'll be called when you need to
regenerate the password. regenerate the password.
""" """
if not password or not is_password_usable(encoded): if not is_password_usable(encoded):
return False return False


preferred = get_hasher(preferred) preferred = get_hasher(preferred)
Expand All @@ -65,10 +65,10 @@ def make_password(password, salt=None, hasher='default'):
Turn a plain-text password into a hash for database storage Turn a plain-text password into a hash for database storage
Same as encode() but generates a new random salt. If 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. returned which disallows logins.
""" """
if not password: if password is None:
return UNUSABLE_PASSWORD return UNUSABLE_PASSWORD


hasher = get_hasher(hasher) hasher = get_hasher(hasher)
Expand Down Expand Up @@ -222,7 +222,7 @@ class PBKDF2PasswordHasher(BasePasswordHasher):
digest = hashlib.sha256 digest = hashlib.sha256


def encode(self, password, salt, iterations=None): def encode(self, password, salt, iterations=None):
assert password assert password is not None
assert salt and '$' not in salt assert salt and '$' not in salt
if not iterations: if not iterations:
iterations = self.iterations iterations = self.iterations
Expand Down Expand Up @@ -350,7 +350,7 @@ class SHA1PasswordHasher(BasePasswordHasher):
algorithm = "sha1" algorithm = "sha1"


def encode(self, password, salt): def encode(self, password, salt):
assert password assert password is not None
assert salt and '$' not in salt assert salt and '$' not in salt
hash = hashlib.sha1(force_bytes(salt + password)).hexdigest() hash = hashlib.sha1(force_bytes(salt + password)).hexdigest()
return "%s$%s$%s" % (self.algorithm, salt, hash) return "%s$%s$%s" % (self.algorithm, salt, hash)
Expand Down Expand Up @@ -378,7 +378,7 @@ class MD5PasswordHasher(BasePasswordHasher):
algorithm = "md5" algorithm = "md5"


def encode(self, password, salt): def encode(self, password, salt):
assert password assert password is not None
assert salt and '$' not in salt assert salt and '$' not in salt
hash = hashlib.md5(force_bytes(salt + password)).hexdigest() hash = hashlib.md5(force_bytes(salt + password)).hexdigest()
return "%s$%s$%s" % (self.algorithm, salt, hash) return "%s$%s$%s" % (self.algorithm, salt, hash)
Expand Down
53 changes: 53 additions & 0 deletions django/contrib/auth/tests/test_hashers.py
Expand Up @@ -32,6 +32,12 @@ def test_simple(self):
self.assertTrue(is_password_usable(encoded)) self.assertTrue(is_password_usable(encoded))
self.assertTrue(check_password('lètmein', encoded)) self.assertTrue(check_password('lètmein', encoded))
self.assertFalse(check_password('lètmeinz', 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): def test_pkbdf2(self):
encoded = make_password('lètmein', 'seasalt', 'pbkdf2_sha256') encoded = make_password('lètmein', 'seasalt', 'pbkdf2_sha256')
Expand All @@ -41,6 +47,12 @@ def test_pkbdf2(self):
self.assertTrue(check_password('lètmein', encoded)) self.assertTrue(check_password('lètmein', encoded))
self.assertFalse(check_password('lètmeinz', encoded)) self.assertFalse(check_password('lètmeinz', encoded))
self.assertEqual(identify_hasher(encoded).algorithm, "pbkdf2_sha256") 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): def test_sha1(self):
encoded = make_password('lètmein', 'seasalt', 'sha1') encoded = make_password('lètmein', 'seasalt', 'sha1')
Expand All @@ -50,6 +62,12 @@ def test_sha1(self):
self.assertTrue(check_password('lètmein', encoded)) self.assertTrue(check_password('lètmein', encoded))
self.assertFalse(check_password('lètmeinz', encoded)) self.assertFalse(check_password('lètmeinz', encoded))
self.assertEqual(identify_hasher(encoded).algorithm, "sha1") 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): def test_md5(self):
encoded = make_password('lètmein', 'seasalt', 'md5') encoded = make_password('lètmein', 'seasalt', 'md5')
Expand All @@ -59,6 +77,12 @@ def test_md5(self):
self.assertTrue(check_password('lètmein', encoded)) self.assertTrue(check_password('lètmein', encoded))
self.assertFalse(check_password('lètmeinz', encoded)) self.assertFalse(check_password('lètmeinz', encoded))
self.assertEqual(identify_hasher(encoded).algorithm, "md5") 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): def test_unsalted_md5(self):
encoded = make_password('lètmein', '', 'unsalted_md5') encoded = make_password('lètmein', '', 'unsalted_md5')
Expand All @@ -72,6 +96,11 @@ def test_unsalted_md5(self):
self.assertTrue(is_password_usable(alt_encoded)) self.assertTrue(is_password_usable(alt_encoded))
self.assertTrue(check_password('lètmein', alt_encoded)) self.assertTrue(check_password('lètmein', alt_encoded))
self.assertFalse(check_password('lètmeinz', 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): def test_unsalted_sha1(self):
encoded = make_password('lètmein', '', 'unsalted_sha1') encoded = make_password('lètmein', '', 'unsalted_sha1')
Expand All @@ -83,6 +112,12 @@ def test_unsalted_sha1(self):
# Raw SHA1 isn't acceptable # Raw SHA1 isn't acceptable
alt_encoded = encoded[6:] alt_encoded = encoded[6:]
self.assertFalse(check_password('lètmein', alt_encoded)) 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.") @skipUnless(crypt, "no crypt module to generate password.")
def test_crypt(self): def test_crypt(self):
Expand All @@ -92,6 +127,12 @@ def test_crypt(self):
self.assertTrue(check_password('lètmei', encoded)) self.assertTrue(check_password('lètmei', encoded))
self.assertFalse(check_password('lètmeiz', encoded)) self.assertFalse(check_password('lètmeiz', encoded))
self.assertEqual(identify_hasher(encoded).algorithm, "crypt") 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") @skipUnless(bcrypt, "bcrypt not installed")
def test_bcrypt_sha256(self): def test_bcrypt_sha256(self):
Expand All @@ -108,6 +149,12 @@ def test_bcrypt_sha256(self):
encoded = make_password(password, hasher='bcrypt_sha256') encoded = make_password(password, hasher='bcrypt_sha256')
self.assertTrue(check_password(password, encoded)) self.assertTrue(check_password(password, encoded))
self.assertFalse(check_password(password[:72], 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") @skipUnless(bcrypt, "bcrypt not installed")
def test_bcrypt(self): def test_bcrypt(self):
Expand All @@ -117,6 +164,12 @@ def test_bcrypt(self):
self.assertTrue(check_password('lètmein', encoded)) self.assertTrue(check_password('lètmein', encoded))
self.assertFalse(check_password('lètmeinz', encoded)) self.assertFalse(check_password('lètmeinz', encoded))
self.assertEqual(identify_hasher(encoded).algorithm, "bcrypt") 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): def test_unusable(self):
encoded = make_password(None) encoded = make_password(None)
Expand Down
16 changes: 16 additions & 0 deletions docs/ref/contrib/auth.txt
Expand Up @@ -132,12 +132,28 @@ Methods
password hashing. Doesn't save the password hashing. Doesn't save the
:class:`~django.contrib.auth.models.User` object. :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) .. method:: check_password(raw_password)


Returns ``True`` if the given raw string is the correct password for Returns ``True`` if the given raw string is the correct password for
the user. (This takes care of the password hashing in making the the user. (This takes care of the password hashing in making the
comparison.) 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() .. method:: set_unusable_password()


Marks the user as having no password set. This isn't the same as Marks the user as having no password set. This isn't the same as
Expand Down
9 changes: 9 additions & 0 deletions docs/releases/1.6.txt
Expand Up @@ -701,6 +701,15 @@ Miscellaneous
* :class:`~django.views.generic.base.RedirectView` now has a `pattern_name` * :class:`~django.views.generic.base.RedirectView` now has a `pattern_name`
attribute which allows it to choose the target by reversing the URL. 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 Features deprecated in 1.6
========================== ==========================


Expand Down
16 changes: 16 additions & 0 deletions docs/topics/auth/customizing.txt
Expand Up @@ -583,12 +583,28 @@ The following methods are available on any subclass of
password hashing. Doesn't save the password hashing. Doesn't save the
:class:`~django.contrib.auth.models.AbstractBaseUser` object. :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) .. method:: models.AbstractBaseUser.check_password(raw_password)


Returns ``True`` if the given raw string is the correct password for Returns ``True`` if the given raw string is the correct password for
the user. (This takes care of the password hashing in making the the user. (This takes care of the password hashing in making the
comparison.) 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() .. method:: models.AbstractBaseUser.set_unusable_password()


Marks the user as having no password set. This isn't the same as Marks the user as having no password set. This isn't the same as
Expand Down
6 changes: 6 additions & 0 deletions docs/topics/auth/passwords.txt
Expand Up @@ -206,6 +206,12 @@ from the ``User`` model.
database to check against, and returns ``True`` if they match, ``False`` database to check against, and returns ``True`` if they match, ``False``
otherwise. 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]) .. function:: make_password(password[, salt, hashers])


Creates a hashed password in the format used by this application. It takes Creates a hashed password in the format used by this application. It takes
Expand Down

0 comments on commit 2c4fe76

Please sign in to comment.