From 1cc572a071003583f15cdbca2f5b4d910ebd3504 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Tue, 24 Sep 2013 21:12:25 +0200 Subject: [PATCH] Revert "[1.5.x] Ensure that passwords are never long enough for a DoS." This reverts commit 22b74fa09d7ccbc8c52270d648a0da7f3f0fa2bc. This fix is no longer necessary, our pbkdf2 (see next commit) implementation no longer rehashes the password every iteration. --- django/contrib/auth/forms.py | 48 +++++-------------- django/contrib/auth/hashers.py | 29 +---------- django/contrib/auth/tests/hashers.py | 72 +--------------------------- 3 files changed, 15 insertions(+), 134 deletions(-) diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index d191635a9b..cbce8ad6e2 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -12,9 +12,7 @@ from django.utils.translation import ugettext, ugettext_lazy as _ from django.contrib.auth import authenticate, get_user_model from django.contrib.auth.models import User -from django.contrib.auth.hashers import ( - MAXIMUM_PASSWORD_LENGTH, UNUSABLE_PASSWORD, identify_hasher, -) +from django.contrib.auth.hashers import UNUSABLE_PASSWORD, identify_hasher from django.contrib.auth.tokens import default_token_generator from django.contrib.sites.models import get_current_site @@ -77,10 +75,9 @@ class UserCreationForm(forms.ModelForm): 'invalid': _("This value may contain only letters, numbers and " "@/./+/-/_ characters.")}) password1 = forms.CharField(label=_("Password"), - widget=forms.PasswordInput, max_length=MAXIMUM_PASSWORD_LENGTH) + widget=forms.PasswordInput) password2 = forms.CharField(label=_("Password confirmation"), widget=forms.PasswordInput, - max_length=MAXIMUM_PASSWORD_LENGTH, help_text=_("Enter the same password as above, for verification.")) class Meta: @@ -148,11 +145,7 @@ class AuthenticationForm(forms.Form): username/password logins. """ username = forms.CharField(max_length=254) - password = forms.CharField( - label=_("Password"), - widget=forms.PasswordInput, - max_length=MAXIMUM_PASSWORD_LENGTH, - ) + password = forms.CharField(label=_("Password"), widget=forms.PasswordInput) error_messages = { 'invalid_login': _("Please enter a correct %(username)s and password. " @@ -276,16 +269,10 @@ class SetPasswordForm(forms.Form): error_messages = { 'password_mismatch': _("The two password fields didn't match."), } - new_password1 = forms.CharField( - label=_("New password"), - widget=forms.PasswordInput, - max_length=MAXIMUM_PASSWORD_LENGTH, - ) - new_password2 = forms.CharField( - label=_("New password confirmation"), - widget=forms.PasswordInput, - max_length=MAXIMUM_PASSWORD_LENGTH, - ) + new_password1 = forms.CharField(label=_("New password"), + widget=forms.PasswordInput) + new_password2 = forms.CharField(label=_("New password confirmation"), + widget=forms.PasswordInput) def __init__(self, user, *args, **kwargs): self.user = user @@ -316,11 +303,8 @@ class PasswordChangeForm(SetPasswordForm): 'password_incorrect': _("Your old password was entered incorrectly. " "Please enter it again."), }) - old_password = forms.CharField( - label=_("Old password"), - widget=forms.PasswordInput, - max_length=MAXIMUM_PASSWORD_LENGTH, - ) + old_password = forms.CharField(label=_("Old password"), + widget=forms.PasswordInput) def clean_old_password(self): """ @@ -345,16 +329,10 @@ class AdminPasswordChangeForm(forms.Form): error_messages = { 'password_mismatch': _("The two password fields didn't match."), } - password1 = forms.CharField( - label=_("Password"), - widget=forms.PasswordInput, - max_length=MAXIMUM_PASSWORD_LENGTH, - ) - password2 = forms.CharField( - label=_("Password (again)"), - widget=forms.PasswordInput, - max_length=MAXIMUM_PASSWORD_LENGTH, - ) + password1 = forms.CharField(label=_("Password"), + widget=forms.PasswordInput) + password2 = forms.CharField(label=_("Password (again)"), + widget=forms.PasswordInput) def __init__(self, user, *args, **kwargs): self.user = user diff --git a/django/contrib/auth/hashers.py b/django/contrib/auth/hashers.py index a9d5d7b963..b49362f736 100644 --- a/django/contrib/auth/hashers.py +++ b/django/contrib/auth/hashers.py @@ -1,7 +1,6 @@ from __future__ import unicode_literals import base64 -import functools import hashlib from django.dispatch import receiver @@ -17,7 +16,6 @@ from django.utils.translation import ugettext_noop as _ UNUSABLE_PASSWORD = '!' # This will never be a valid encoded hash -MAXIMUM_PASSWORD_LENGTH = 4096 # The maximum length a password can be to prevent DoS HASHERS = None # lazily loaded from PASSWORD_HASHERS PREFERRED_HASHER = None # defaults to first item in PASSWORD_HASHERS @@ -29,18 +27,6 @@ def reset_hashers(**kwargs): PREFERRED_HASHER = None -def password_max_length(max_length): - def inner(fn): - @functools.wraps(fn) - def wrapper(self, password, *args, **kwargs): - if len(password) > max_length: - raise ValueError("Invalid password; Must be less than or equal" - " to %d bytes" % max_length) - return fn(self, password, *args, **kwargs) - return wrapper - return inner - - def is_password_usable(encoded): if encoded is None or encoded == UNUSABLE_PASSWORD: return False @@ -239,7 +225,6 @@ class PBKDF2PasswordHasher(BasePasswordHasher): iterations = 10000 digest = hashlib.sha256 - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def encode(self, password, salt, iterations=None): assert password assert salt and '$' not in salt @@ -249,7 +234,6 @@ class PBKDF2PasswordHasher(BasePasswordHasher): hash = base64.b64encode(hash).decode('ascii').strip() return "%s$%d$%s$%s" % (self.algorithm, iterations, salt, hash) - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def verify(self, password, encoded): algorithm, iterations, salt, hash = encoded.split('$', 3) assert algorithm == self.algorithm @@ -295,7 +279,6 @@ class BCryptPasswordHasher(BasePasswordHasher): bcrypt = self._load_library() return bcrypt.gensalt(self.rounds) - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def encode(self, password, salt): bcrypt = self._load_library() # Need to reevaluate the force_bytes call once bcrypt is supported on @@ -303,7 +286,6 @@ class BCryptPasswordHasher(BasePasswordHasher): data = bcrypt.hashpw(force_bytes(password), salt) return "%s$%s" % (self.algorithm, data) - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def verify(self, password, encoded): algorithm, data = encoded.split('$', 1) assert algorithm == self.algorithm @@ -328,14 +310,12 @@ class SHA1PasswordHasher(BasePasswordHasher): """ algorithm = "sha1" - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def encode(self, password, salt): assert password assert salt and '$' not in salt hash = hashlib.sha1(force_bytes(salt + password)).hexdigest() return "%s$%s$%s" % (self.algorithm, salt, hash) - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def verify(self, password, encoded): algorithm, salt, hash = encoded.split('$', 2) assert algorithm == self.algorithm @@ -358,14 +338,12 @@ class MD5PasswordHasher(BasePasswordHasher): """ algorithm = "md5" - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def encode(self, password, salt): assert password assert salt and '$' not in salt hash = hashlib.md5(force_bytes(salt + password)).hexdigest() return "%s$%s$%s" % (self.algorithm, salt, hash) - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def verify(self, password, encoded): algorithm, salt, hash = encoded.split('$', 2) assert algorithm == self.algorithm @@ -396,13 +374,11 @@ class UnsaltedSHA1PasswordHasher(BasePasswordHasher): def salt(self): return '' - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def encode(self, password, salt): assert salt == '' hash = hashlib.sha1(force_bytes(password)).hexdigest() return 'sha1$$%s' % hash - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def verify(self, password, encoded): encoded_2 = self.encode(password, '') return constant_time_compare(encoded, encoded_2) @@ -432,12 +408,10 @@ class UnsaltedMD5PasswordHasher(BasePasswordHasher): def salt(self): return '' - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def encode(self, password, salt): assert salt == '' return hashlib.md5(force_bytes(password)).hexdigest() - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def verify(self, password, encoded): if len(encoded) == 37 and encoded.startswith('md5$$'): encoded = encoded[5:] @@ -463,7 +437,6 @@ class CryptPasswordHasher(BasePasswordHasher): def salt(self): return get_random_string(2) - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def encode(self, password, salt): crypt = self._load_library() assert len(salt) == 2 @@ -471,7 +444,6 @@ class CryptPasswordHasher(BasePasswordHasher): # we don't need to store the salt, but Django used to do this return "%s$%s$%s" % (self.algorithm, '', data) - @password_max_length(MAXIMUM_PASSWORD_LENGTH) def verify(self, password, encoded): crypt = self._load_library() algorithm, salt, data = encoded.split('$', 2) @@ -486,3 +458,4 @@ class CryptPasswordHasher(BasePasswordHasher): (_('salt'), salt), (_('hash'), mask_hash(data, show=3)), ]) + diff --git a/django/contrib/auth/tests/hashers.py b/django/contrib/auth/tests/hashers.py index 8c35f543cc..2b2243cb0c 100644 --- a/django/contrib/auth/tests/hashers.py +++ b/django/contrib/auth/tests/hashers.py @@ -4,8 +4,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, check_password, make_password, PBKDF2PasswordHasher, load_hashers, - PBKDF2SHA1PasswordHasher, get_hasher, identify_hasher, UNUSABLE_PASSWORD, - MAXIMUM_PASSWORD_LENGTH, password_max_length) + PBKDF2SHA1PasswordHasher, get_hasher, identify_hasher, UNUSABLE_PASSWORD) from django.utils import unittest from django.utils.unittest import skipUnless @@ -32,12 +31,6 @@ class TestUtilsHashPass(unittest.TestCase): self.assertTrue(is_password_usable(encoded)) self.assertTrue(check_password('lètmein', encoded)) self.assertFalse(check_password('lètmeinz', encoded)) - # Long password - self.assertRaises( - ValueError, - make_password, - b"1" * (MAXIMUM_PASSWORD_LENGTH + 1), - ) def test_pkbdf2(self): encoded = make_password('lètmein', 'seasalt', 'pbkdf2_sha256') @@ -47,14 +40,6 @@ 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") - # Long password - self.assertRaises( - ValueError, - make_password, - b"1" * (MAXIMUM_PASSWORD_LENGTH + 1), - "seasalt", - "pbkdf2_sha256", - ) def test_sha1(self): encoded = make_password('lètmein', 'seasalt', 'sha1') @@ -64,14 +49,6 @@ 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") - # Long password - self.assertRaises( - ValueError, - make_password, - b"1" * (MAXIMUM_PASSWORD_LENGTH + 1), - "seasalt", - "sha1", - ) def test_md5(self): encoded = make_password('lètmein', 'seasalt', 'md5') @@ -81,14 +58,6 @@ 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") - # Long password - self.assertRaises( - ValueError, - make_password, - b"1" * (MAXIMUM_PASSWORD_LENGTH + 1), - "seasalt", - "md5", - ) def test_unsalted_md5(self): encoded = make_password('lètmein', '', 'unsalted_md5') @@ -102,14 +71,6 @@ 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)) - # Long password - self.assertRaises( - ValueError, - make_password, - b"1" * (MAXIMUM_PASSWORD_LENGTH + 1), - "", - "unsalted_md5", - ) def test_unsalted_sha1(self): encoded = make_password('lètmein', '', 'unsalted_sha1') @@ -121,14 +82,6 @@ class TestUtilsHashPass(unittest.TestCase): # Raw SHA1 isn't acceptable alt_encoded = encoded[6:] self.assertFalse(check_password('lètmein', alt_encoded)) - # Long password - self.assertRaises( - ValueError, - make_password, - b"1" * (MAXIMUM_PASSWORD_LENGTH + 1), - "", - "unslated_sha1", - ) @skipUnless(crypt, "no crypt module to generate password.") def test_crypt(self): @@ -138,14 +91,6 @@ 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") - # Long password - self.assertRaises( - ValueError, - make_password, - b"1" * (MAXIMUM_PASSWORD_LENGTH + 1), - "seasalt", - "crypt", - ) @skipUnless(bcrypt, "py-bcrypt not installed") def test_bcrypt(self): @@ -155,13 +100,6 @@ 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") - # Long password - self.assertRaises( - ValueError, - make_password, - b"1" * (MAXIMUM_PASSWORD_LENGTH + 1), - hasher="bcrypt", - ) def test_unusable(self): encoded = make_password(None) @@ -183,14 +121,6 @@ class TestUtilsHashPass(unittest.TestCase): self.assertFalse(is_password_usable('lètmein_badencoded')) self.assertFalse(is_password_usable('')) - def test_max_password_length_decorator(self): - @password_max_length(10) - def encode(s, password, salt): - return True - - self.assertTrue(encode(None, b"1234", b"1234")) - self.assertRaises(ValueError, encode, None, b"1234567890A", b"1234") - def test_low_level_pkbdf2(self): hasher = PBKDF2PasswordHasher() encoded = hasher.encode('lètmein', 'seasalt')