Commit 5d74853e authored by Florian Apolloner's avatar Florian Apolloner
Browse files

Revert "Ensure that passwords are never long enough for a DoS."

This reverts commit aae5a96d.

This fix is no longer necessary, our pbkdf2 (see next commit) implementation
no longer rehashes the password every iteration.
parent 9a041807
Loading
Loading
Loading
Loading
+13 −35
Original line number Diff line number Diff line
@@ -14,9 +14,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_PREFIX, identify_hasher,
)
from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX, identify_hasher
from django.contrib.auth.tokens import default_token_generator
from django.contrib.sites.models import get_current_site

@@ -82,10 +80,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:
@@ -159,11 +156,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. "
@@ -286,16 +279,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
@@ -328,11 +315,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):
        """
@@ -359,16 +343,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
+1 −28
Original line number Diff line number Diff line
@@ -3,7 +3,6 @@ from __future__ import unicode_literals
import base64
import binascii
from collections import OrderedDict
import functools
import hashlib
import importlib

@@ -20,7 +19,6 @@ from django.utils.translation import ugettext_noop as _

UNUSABLE_PASSWORD_PREFIX = '!'  # This will never be a valid encoded hash
UNUSABLE_PASSWORD_SUFFIX_LENGTH = 40  # number of random chars to add after UNUSABLE_PASSWORD_PREFIX
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

@@ -33,18 +31,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.startswith(UNUSABLE_PASSWORD_PREFIX):
        return False
@@ -239,7 +225,6 @@ class PBKDF2PasswordHasher(BasePasswordHasher):
    iterations = 12000
    digest = hashlib.sha256

    @password_max_length(MAXIMUM_PASSWORD_LENGTH)
    def encode(self, password, salt, iterations=None):
        assert password is not None
        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
@@ -296,7 +280,6 @@ class BCryptSHA256PasswordHasher(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
@@ -314,7 +297,6 @@ class BCryptSHA256PasswordHasher(BasePasswordHasher):
        data = bcrypt.hashpw(password, salt)
        return "%s$%s" % (self.algorithm, force_text(data))

    @password_max_length(MAXIMUM_PASSWORD_LENGTH)
    def verify(self, password, encoded):
        algorithm, data = encoded.split('$', 1)
        assert algorithm == self.algorithm
@@ -371,14 +353,12 @@ class SHA1PasswordHasher(BasePasswordHasher):
    """
    algorithm = "sha1"

    @password_max_length(MAXIMUM_PASSWORD_LENGTH)
    def encode(self, password, salt):
        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)

    @password_max_length(MAXIMUM_PASSWORD_LENGTH)
    def verify(self, password, encoded):
        algorithm, salt, hash = encoded.split('$', 2)
        assert algorithm == self.algorithm
@@ -401,14 +381,12 @@ class MD5PasswordHasher(BasePasswordHasher):
    """
    algorithm = "md5"

    @password_max_length(MAXIMUM_PASSWORD_LENGTH)
    def encode(self, password, salt):
        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)

    @password_max_length(MAXIMUM_PASSWORD_LENGTH)
    def verify(self, password, encoded):
        algorithm, salt, hash = encoded.split('$', 2)
        assert algorithm == self.algorithm
@@ -439,13 +417,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)
@@ -475,12 +451,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:]
@@ -506,7 +480,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
@@ -514,7 +487,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)
@@ -529,3 +501,4 @@ class CryptPasswordHasher(BasePasswordHasher):
            (_('salt'), salt),
            (_('hash'), mask_hash(data, show=3)),
        ])
+3 −82
Original line number Diff line number Diff line
@@ -5,12 +5,9 @@ import unittest
from unittest import skipUnless

from django.conf.global_settings import PASSWORD_HASHERS as default_hashers
from django.contrib.auth.hashers import (
    is_password_usable, BasePasswordHasher, check_password, make_password,
    PBKDF2PasswordHasher, load_hashers, PBKDF2SHA1PasswordHasher, get_hasher,
    identify_hasher, UNUSABLE_PASSWORD_PREFIX, UNUSABLE_PASSWORD_SUFFIX_LENGTH,
    MAXIMUM_PASSWORD_LENGTH, password_max_length
)
from django.contrib.auth.hashers import (is_password_usable, BasePasswordHasher,
    check_password, make_password, PBKDF2PasswordHasher, load_hashers, PBKDF2SHA1PasswordHasher,
    get_hasher, identify_hasher, UNUSABLE_PASSWORD_PREFIX, UNUSABLE_PASSWORD_SUFFIX_LENGTH)
from django.utils import six


@@ -42,12 +39,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_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')
@@ -63,14 +54,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_encoded))
        # 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')
@@ -86,14 +69,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_encoded))
        # 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')
@@ -109,14 +84,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_encoded))
        # 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')
@@ -135,14 +102,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_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')
@@ -160,14 +119,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_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):
@@ -183,14 +134,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_encoded))
        # Long password
        self.assertRaises(
            ValueError,
            make_password,
            b"1" * (MAXIMUM_PASSWORD_LENGTH + 1),
            "seasalt",
            "crypt",
        )

    @skipUnless(bcrypt, "bcrypt not installed")
    def test_bcrypt_sha256(self):
@@ -213,13 +156,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_encoded))
        # Long password
        self.assertRaises(
            ValueError,
            make_password,
            b"1" * (MAXIMUM_PASSWORD_LENGTH + 1),
            hasher="bcrypt_sha256",
        )

    @skipUnless(bcrypt, "bcrypt not installed")
    def test_bcrypt(self):
@@ -235,13 +171,6 @@ class TestUtilsHashPass(unittest.TestCase):
        self.assertTrue(is_password_usable(blank_encoded))
        self.assertTrue(check_password('', blank_encoded))
        self.assertFalse(check_password(' ', blank_encoded))
        # Long password
        self.assertRaises(
            ValueError,
            make_password,
            b"1" * (MAXIMUM_PASSWORD_LENGTH + 1),
            hasher="bcrypt",
        )

    def test_unusable(self):
        encoded = make_password(None)
@@ -274,14 +203,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', 'seasalt2')