Fixed #14445 - Use HMAC and constant-time comparison functions where needed. (45c7f427) · Commits · Dom Sekotill / django

django/contrib/auth/tests/tokens.py

+22 −0

Original line number	Diff line number	Diff line
		@@ -50,3 +50,25 @@ class TokenGeneratorTest(TestCase):

		p2 = Mocked(date.today() + timedelta(settings.PASSWORD_RESET_TIMEOUT_DAYS + 1))
		self.assertFalse(p2.check_token(user, tk1))

		def test_django12_hash(self):
		"""
		Ensure we can use the hashes generated by Django 1.2
		"""
		# Hard code in the Django 1.2 algorithm (not the result, as it is time
		# dependent)
		def _make_token(user):
		from django.utils.hashcompat import sha_constructor
		from django.utils.http import int_to_base36

		timestamp = (date.today() - date(2001,1,1)).days
		ts_b36 = int_to_base36(timestamp)
		hash = sha_constructor(settings.SECRET_KEY + unicode(user.id) +
		user.password + user.last_login.strftime('%Y-%m-%d %H:%M:%S') +
		unicode(timestamp)).hexdigest()[::2]
		return "%s-%s" % (ts_b36, hash)

		user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw')
		p0 = PasswordResetTokenGenerator()
		tk1 = _make_token(user)
		self.assertTrue(p0.check_token(user, tk1))

django/contrib/auth/tokens.py

+19 −3

Original line number	Diff line number	Diff line
		from datetime import date

		from django.conf import settings
		from django.utils.hashcompat import sha_constructor
		from django.utils.http import int_to_base36, base36_to_int
		from django.utils.crypto import constant_time_compare, salted_hmac

		class PasswordResetTokenGenerator(object):
		"""
		@@ -30,7 +33,11 @@ class PasswordResetTokenGenerator(object):
		return False

		# Check that the timestamp/uid has not been tampered with
		if self._make_token_with_timestamp(user, ts) != token:
		if not constant_time_compare(self._make_token_with_timestamp(user, ts), token):
		# Fallback to Django 1.2 method for compatibility.
		# PendingDeprecationWarning <- here to remind us to remove this in
		# Django 1.5
		if not constant_time_compare(self._make_token_with_timestamp_old(user, ts), token):
		return False

		# Check the timestamp is within limit
		@@ -50,7 +57,16 @@ class PasswordResetTokenGenerator(object):
		# last_login will also change), we produce a hash that will be
		# invalid as soon as it is used.
		# We limit the hash to 20 chars to keep URL short
		from django.utils.hashcompat import sha_constructor
		key_salt = "django.contrib.auth.tokens.PasswordResetTokenGenerator"
		value = unicode(user.id) + \
		user.password + user.last_login.strftime('%Y-%m-%d %H:%M:%S') + \
		unicode(timestamp)
		hash = salted_hmac(key_salt, value).hexdigest()[::2]
		return "%s-%s" % (ts_b36, hash)

		def _make_token_with_timestamp_old(self, user, timestamp):
		# The Django 1.2 method
		ts_b36 = int_to_base36(timestamp)
		hash = sha_constructor(settings.SECRET_KEY + unicode(user.id) +
		user.password + user.last_login.strftime('%Y-%m-%d %H:%M:%S') +
		unicode(timestamp)).hexdigest()[::2]

django/contrib/comments/forms.py

+18 −2

Original line number	Diff line number	Diff line
		@@ -6,6 +6,7 @@ from django.forms.util import ErrorDict
		from django.conf import settings
		from django.contrib.contenttypes.models import ContentType
		from models import Comment
		from django.utils.crypto import salted_hmac, constant_time_compare
		from django.utils.encoding import force_unicode
		from django.utils.hashcompat import sha_constructor
		from django.utils.text import get_text_list
		@@ -46,7 +47,12 @@ class CommentSecurityForm(forms.Form):
		}
		expected_hash = self.generate_security_hash(**security_hash_dict)
		actual_hash = self.cleaned_data["security_hash"]
		if expected_hash != actual_hash:
		if not constant_time_compare(expected_hash, actual_hash):
		# Fallback to Django 1.2 method for compatibility
		# PendingDeprecationWarning <- here to remind us to remove this
		# fallback in Django 1.5
		expected_hash_old = self._generate_security_hash_old(**security_hash_dict)
		if not constant_time_compare(expected_hash_old, actual_hash):
		raise forms.ValidationError("Security hash check failed.")
		return actual_hash

		@@ -82,7 +88,17 @@ class CommentSecurityForm(forms.Form):
		return self.generate_security_hash(**initial_security_dict)

		def generate_security_hash(self, content_type, object_pk, timestamp):
		"""
		Generate a HMAC security hash from the provided info.
		"""
		info = (content_type, object_pk, timestamp)
		key_salt = "django.contrib.forms.CommentSecurityForm"
		value = "-".join(info)
		return salted_hmac(key_salt, value).hexdigest()

		def _generate_security_hash_old(self, content_type, object_pk, timestamp):
		"""Generate a (SHA1) security hash from the provided info."""
		# Django 1.2 compatibility
		info = (content_type, object_pk, timestamp, settings.SECRET_KEY)
		return sha_constructor("".join(info)).hexdigest()

django/contrib/formtools/preview.py

+24 −1

Original line number	Diff line number	Diff line
		@@ -9,6 +9,7 @@ from django.http import Http404
		from django.shortcuts import render_to_response
		from django.template.context import RequestContext
		from django.utils.hashcompat import md5_constructor
		from django.utils.crypto import constant_time_compare
		from django.contrib.formtools.utils import security_hash

		AUTO_ID = 'formtools_%s' # Each form here uses this as its auto_id parameter.
		@@ -67,11 +68,33 @@ class FormPreview(object):
		else:
		return render_to_response(self.form_template, context, context_instance=RequestContext(request))

		def _check_security_hash(self, token, request, form):
		expected = self.security_hash(request, form)
		if constant_time_compare(token, expected):
		return True
		else:
		# Fall back to Django 1.2 method, for compatibility with forms that
		# are in the middle of being used when the upgrade occurs. However,
		# we don't want to do this fallback if a subclass has provided their
		# own security_hash method - because they might have implemented a
		# more secure method, and this would punch a hole in that.

		# PendingDeprecationWarning <- left here to remind us that this
		# compatibility fallback should be removed in Django 1.5
		FormPreview_expected = FormPreview.security_hash(self, request, form)
		if expected == FormPreview_expected:
		# They didn't override security_hash, do the fallback:
		old_expected = security_hash(request, form)
		return constant_time_compare(token, old_expected)
		else:
		return False

		def post_post(self, request):
		"Validates the POST data. If valid, calls done(). Else, redisplays form."
		f = self.form(request.POST, auto_id=AUTO_ID)
		if f.is_valid():
		if self.security_hash(request, f) != request.POST.get(self.unused_name('hash')):
		if not self._check_security_hash(request.POST.get(self.unused_name('hash'), ''),
		request, f):
		return self.failed_hash(request) # Security hash failed.
		return self.done(request, f.cleaned_data)
		else:

django/contrib/formtools/tests.py→django/contrib/formtools/tests/init.py

+172 −11

Original line number	Diff line number	Diff line
		import os

		from django import forms
		from django import http
		from django.conf import settings
		from django.contrib.formtools import preview, wizard, utils
		from django.test import TestCase
		from django.utils import unittest

		success_string = "Done was called!"


		class TestFormPreview(preview.FormPreview):

		def done(self, request, cleaned_data):
		return http.HttpResponse(success_string)


		class TestForm(forms.Form):
		field1 = forms.CharField()
		field1_ = forms.CharField()
		bool1 = forms.BooleanField(required=False)


		class UserSecuredFormPreview(TestFormPreview):
		"""
		FormPreview with a custum security_hash method
		"""
		def security_hash(self, request, form):
		return "123"


		class PreviewTests(TestCase):
		urls = 'django.contrib.formtools.test_urls'
		urls = 'django.contrib.formtools.tests.urls'

		def setUp(self):
		# Create a FormPreview instance to share between tests
		@@ -102,6 +116,39 @@ class PreviewTests(TestCase):
		response = self.client.post('/test1/', self.test_data)
		self.assertEqual(response.content, success_string)

		def test_form_submit_django12_hash(self):
		"""
		Test contrib.formtools.preview form submittal, using the hash function
		used in Django 1.2
		"""
		# Pass strings for form submittal and add stage variable to
		# show we previously saw first stage of the form.
		self.test_data.update({'stage':2})
		response = self.client.post('/test1/', self.test_data)
		self.failIfEqual(response.content, success_string)
		hash = utils.security_hash(None, TestForm(self.test_data))
		self.test_data.update({'hash': hash})
		response = self.client.post('/test1/', self.test_data)
		self.assertEqual(response.content, success_string)


		def test_form_submit_django12_hash_custom_hash(self):
		"""
		Test contrib.formtools.preview form submittal, using the hash function
		used in Django 1.2 and a custom security_hash method.
		"""
		# Pass strings for form submittal and add stage variable to
		# show we previously saw first stage of the form.
		self.test_data.update({'stage':2})
		response = self.client.post('/test2/', self.test_data)
		self.assertEqual(response.status_code, 200)
		self.failIfEqual(response.content, success_string)
		hash = utils.security_hash(None, TestForm(self.test_data))
		self.test_data.update({'hash': hash})
		response = self.client.post('/test2/', self.test_data)
		self.failIfEqual(response.content, success_string)


		class SecurityHashTests(unittest.TestCase):

		def test_textfield_hash(self):
		@@ -127,10 +174,41 @@ class SecurityHashTests(unittest.TestCase):
		hash2 = utils.security_hash(None, f2)
		self.assertEqual(hash1, hash2)


		class FormHmacTests(unittest.TestCase):
		"""
		Same as SecurityHashTests, but with form_hmac
		"""

		def test_textfield_hash(self):
		"""
		Regression test for #10034: the hash generation function should ignore
		leading/trailing whitespace so as to be friendly to broken browsers that
		submit it (usually in textareas).
		"""
		f1 = HashTestForm({'name': 'joe', 'bio': 'Nothing notable.'})
		f2 = HashTestForm({'name': ' joe', 'bio': 'Nothing notable. '})
		hash1 = utils.form_hmac(f1)
		hash2 = utils.form_hmac(f2)
		self.assertEqual(hash1, hash2)

		def test_empty_permitted(self):
		"""
		Regression test for #10643: the security hash should allow forms with
		empty_permitted = True, or forms where data has not changed.
		"""
		f1 = HashTestBlankForm({})
		f2 = HashTestForm({}, empty_permitted=True)
		hash1 = utils.form_hmac(f1)
		hash2 = utils.form_hmac(f2)
		self.assertEqual(hash1, hash2)


		class HashTestForm(forms.Form):
		name = forms.CharField()
		bio = forms.CharField()


		class HashTestBlankForm(forms.Form):
		name = forms.CharField(required=False)
		bio = forms.CharField(required=False)
		@@ -139,20 +217,38 @@ class HashTestBlankForm(forms.Form):
		# FormWizard tests
		#


		class WizardPageOneForm(forms.Form):
		field = forms.CharField()


		class WizardPageTwoForm(forms.Form):
		field = forms.CharField()


		class WizardPageThreeForm(forms.Form):
		field = forms.CharField()


		class WizardClass(wizard.FormWizard):
		def render_template(self, args, *kw):
		return http.HttpResponse("")

		def get_template(self, step):
		return 'formwizard/wizard.html'

		def done(self, request, cleaned_data):
		return http.HttpResponse(success_string)


		class UserSecuredWizardClass(WizardClass):
		"""
		Wizard with a custum security_hash method
		"""
		def security_hash(self, request, form):
		return "123"


		class DummyRequest(http.HttpRequest):

		def __init__(self, POST=None):
		super(DummyRequest, self).__init__()
		self.method = POST and "POST" or "GET"
		@@ -160,22 +256,87 @@ class DummyRequest(http.HttpRequest):
		self.POST.update(POST)
		self._dont_enforce_csrf_checks = True


		class WizardTests(TestCase):
		urls = 'django.contrib.formtools.tests.urls'

		def setUp(self):
		self.old_TEMPLATE_DIRS = settings.TEMPLATE_DIRS
		settings.TEMPLATE_DIRS = (
		os.path.join(
		os.path.dirname(__file__),
		'templates'
		),
		)
		# Use a known SECRET_KEY to make security_hash tests deterministic
		self.old_SECRET_KEY = settings.SECRET_KEY
		settings.SECRET_KEY = "123"

		def tearDown(self):
		settings.TEMPLATE_DIRS = self.old_TEMPLATE_DIRS
		settings.SECRET_KEY = self.old_SECRET_KEY

		def test_step_starts_at_zero(self):
		"""
		step should be zero for the first form
		"""
		wizard = WizardClass([WizardPageOneForm, WizardPageTwoForm])
		request = DummyRequest()
		wizard(request)
		self.assertEquals(0, wizard.step)
		response = self.client.get('/wizard/')
		self.assertEquals(0, response.context['step0'])

		def test_step_increments(self):
		"""
		step should be incremented when we go to the next page
		"""
		wizard = WizardClass([WizardPageOneForm, WizardPageTwoForm])
		request = DummyRequest(POST={"0-field":"test", "wizard_step":"0"})
		response = wizard(request)
		self.assertEquals(1, wizard.step)
		response = self.client.post('/wizard/', {"0-field":"test", "wizard_step":"0"})
		self.assertEquals(1, response.context['step0'])

		def test_bad_hash(self):
		"""
		Form should not advance if the hash is missing or bad
		"""
		response = self.client.post('/wizard/',
		{"0-field":"test",
		"1-field":"test2",
		"wizard_step": "1"})
		self.assertEquals(0, response.context['step0'])

		def test_good_hash_django12(self):
		"""
		Form should advance if the hash is present and good, as calculated using
		django 1.2 method.
		"""
		# We are hard-coding a hash value here, but that is OK, since we want to
		# ensure that we don't accidentally change the algorithm.
		data = {"0-field": "test",
		"1-field": "test2",
		"hash_0": "2fdbefd4c0cad51509478fbacddf8b13",
		"wizard_step": "1"}
		response = self.client.post('/wizard/', data)
		self.assertEquals(2, response.context['step0'])

		def test_good_hash_django12_subclass(self):
		"""
		The Django 1.2 method of calulating hashes should not be used as a
		fallback if the FormWizard subclass has provided their own method
		of calculating a hash.
		"""
		# We are hard-coding a hash value here, but that is OK, since we want to
		# ensure that we don't accidentally change the algorithm.
		data = {"0-field": "test",
		"1-field": "test2",
		"hash_0": "2fdbefd4c0cad51509478fbacddf8b13",
		"wizard_step": "1"}
		response = self.client.post('/wizard2/', data)
		self.assertEquals(0, response.context['step0'])

		def test_good_hash_current(self):
		"""
		Form should advance if the hash is present and good, as calculated using
		current method.
		"""
		data = {"0-field": "test",
		"1-field": "test2",
		"hash_0": "7e9cea465f6a10a6fb47fcea65cb9a76350c9a5c",
		"wizard_step": "1"}
		response = self.client.post('/wizard/', data)
		self.assertEquals(2, response.context['step0'])