Author: lukeplant
Date: 2010-10-14 15:54:30 -0500 (Thu, 14 Oct 2010)
New Revision: 14218
Added:
django/trunk/django/contrib/formtools/tests/
django/trunk/django/contrib/formtools/tests/__init__.py
django/trunk/django/contrib/formtools/tests/templates/
django/trunk/django/contrib/formtools/tests/templates/formwizard/
django/trunk/django/contrib/formtools/tests/templates/formwizard/wizard.html
django/trunk/django/contrib/formtools/tests/urls.py
django/trunk/django/utils/crypto.py
Removed:
django/trunk/django/contrib/formtools/test_urls.py
django/trunk/django/contrib/formtools/tests.py
Modified:
django/trunk/django/contrib/auth/tests/tokens.py
django/trunk/django/contrib/auth/tokens.py
django/trunk/django/contrib/comments/forms.py
django/trunk/django/contrib/formtools/preview.py
django/trunk/django/contrib/formtools/utils.py
django/trunk/django/contrib/formtools/wizard.py
django/trunk/django/contrib/messages/storage/cookie.py
django/trunk/django/contrib/sessions/backends/base.py
django/trunk/django/contrib/sessions/tests.py
django/trunk/django/middleware/csrf.py
django/trunk/docs/internals/deprecation.txt
django/trunk/docs/ref/contrib/formtools/form-wizard.txt
django/trunk/tests/regressiontests/comment_tests/tests/comment_form_tests.py
Log:
Fixed #14445 - Use HMAC and constant-time comparison functions where needed.
All adhoc MAC applications have been updated to use HMAC, using SHA1 to
generate unique keys for each application based on the SECRET_KEY, which is
common practice for this situation. In all cases, backwards compatibility
with existing hashes has been maintained, aiming to phase this out as per
the normal deprecation process. In this way, under most normal
circumstances the old hashes will have expired (e.g. by session expiration
etc.) before they become invalid.
In the case of the messages framework and the cookie backend, which was
already using HMAC, there is the possibility of a backwards incompatibility
if the SECRET_KEY is shorter than the default 50 bytes, but the low
likelihood and low impact meant compatibility code was not worth it.
All known instances where tokens/hashes were compared using simple string
equality, which could potentially open timing based attacks, have also been
fixed using a constant-time comparison function.
There are no known practical attacks against the existing implementations,
so these security improvements will not be backported.
Modified: django/trunk/django/contrib/auth/tests/tokens.py
===================================================================
--- django/trunk/django/contrib/auth/tests/tokens.py 2010-10-14 18:38:43 UTC
(rev 14217)
+++ django/trunk/django/contrib/auth/tests/tokens.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -50,3 +50,25 @@
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', '[email protected]',
'testpw')
+ p0 = PasswordResetTokenGenerator()
+ tk1 = _make_token(user)
+ self.assertTrue(p0.check_token(user, tk1))
Modified: django/trunk/django/contrib/auth/tokens.py
===================================================================
--- django/trunk/django/contrib/auth/tokens.py 2010-10-14 18:38:43 UTC (rev
14217)
+++ django/trunk/django/contrib/auth/tokens.py 2010-10-14 20:54:30 UTC (rev
14218)
@@ -1,6 +1,9 @@
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,8 +33,12 @@
return False
# Check that the timestamp/uid has not been tampered with
- if self._make_token_with_timestamp(user, ts) != token:
- return False
+ 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
if (self._num_days(self._today()) - ts) >
settings.PASSWORD_RESET_TIMEOUT_DAYS:
@@ -50,7 +57,16 @@
# 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]
Modified: django/trunk/django/contrib/comments/forms.py
===================================================================
--- django/trunk/django/contrib/comments/forms.py 2010-10-14 18:38:43 UTC
(rev 14217)
+++ django/trunk/django/contrib/comments/forms.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -6,6 +6,7 @@
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,8 +47,13 @@
}
expected_hash = self.generate_security_hash(**security_hash_dict)
actual_hash = self.cleaned_data["security_hash"]
- if expected_hash != actual_hash:
- raise forms.ValidationError("Security hash check failed.")
+ 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
def clean_timestamp(self):
@@ -82,7 +88,17 @@
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()
Modified: django/trunk/django/contrib/formtools/preview.py
===================================================================
--- django/trunk/django/contrib/formtools/preview.py 2010-10-14 18:38:43 UTC
(rev 14217)
+++ django/trunk/django/contrib/formtools/preview.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -9,6 +9,7 @@
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 @@
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:
Deleted: django/trunk/django/contrib/formtools/test_urls.py
===================================================================
--- django/trunk/django/contrib/formtools/test_urls.py 2010-10-14 18:38:43 UTC
(rev 14217)
+++ django/trunk/django/contrib/formtools/test_urls.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -1,10 +0,0 @@
-"""
-This is a URLconf to be loaded by tests.py. Add any URLs needed for tests only.
-"""
-
-from django.conf.urls.defaults import *
-from django.contrib.formtools.tests import *
-
-urlpatterns = patterns('',
- (r'^test1/', TestFormPreview(TestForm)),
- )
Copied: django/trunk/django/contrib/formtools/tests/__init__.py (from rev
14216, django/trunk/django/contrib/formtools/tests.py)
===================================================================
--- django/trunk/django/contrib/formtools/tests/__init__.py
(rev 0)
+++ django/trunk/django/contrib/formtools/tests/__init__.py 2010-10-14
20:54:30 UTC (rev 14218)
@@ -0,0 +1,342 @@
+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.tests.urls'
+
+ def setUp(self):
+ # Create a FormPreview instance to share between tests
+ self.preview = preview.FormPreview(TestForm)
+ input_template = '<input type="hidden" name="%s" value="%s" />'
+ self.input = input_template % (self.preview.unused_name('stage'), "%d")
+ self.test_data = {'field1':u'foo', 'field1_':u'asdf'}
+
+ def test_unused_name(self):
+ """
+ Verifies name mangling to get uniue field name.
+ """
+ self.assertEqual(self.preview.unused_name('field1'), 'field1__')
+
+ def test_form_get(self):
+ """
+ Test contrib.formtools.preview form retrieval.
+
+ Use the client library to see if we can sucessfully retrieve
+ the form (mostly testing the setup ROOT_URLCONF
+ process). Verify that an additional hidden input field
+ is created to manage the stage.
+
+ """
+ response = self.client.get('/test1/')
+ stage = self.input % 1
+ self.assertContains(response, stage, 1)
+
+ def test_form_preview(self):
+ """
+ Test contrib.formtools.preview form preview rendering.
+
+ Use the client library to POST to the form to see if a preview
+ is returned. If we do get a form back check that the hidden
+ value is correctly managing the state of the form.
+
+ """
+ # Pass strings for form submittal and add stage variable to
+ # show we previously saw first stage of the form.
+ self.test_data.update({'stage': 1})
+ response = self.client.post('/test1/', self.test_data)
+ # Check to confirm stage is set to 2 in output form.
+ stage = self.input % 2
+ self.assertContains(response, stage, 1)
+
+ def test_form_submit(self):
+ """
+ Test contrib.formtools.preview form submittal.
+
+ Use the client library to POST to the form with stage set to 3
+ to see if our forms done() method is called. Check first
+ without the security hash, verify failure, retry with security
+ hash and verify sucess.
+
+ """
+ # 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 = self.preview.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_bool_submit(self):
+ """
+ Test contrib.formtools.preview form submittal when form contains:
+ BooleanField(required=False)
+
+ Ticket: #6209 - When an unchecked BooleanField is previewed, the
preview
+ form's hash would be computed with no value for ``bool1``. However,
when
+ the preview form is rendered, the unchecked hidden BooleanField would
be
+ rendered with the string value 'False'. So when the preview form is
+ resubmitted, the hash would be computed with the value 'False' for
+ ``bool1``. We need to make sure the hashes are the same in both cases.
+
+ """
+ self.test_data.update({'stage':2})
+ hash = self.preview.security_hash(None, TestForm(self.test_data))
+ self.test_data.update({'hash':hash, 'bool1':u'False'})
+ 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):
+ """
+ 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.security_hash(None, f1)
+ hash2 = utils.security_hash(None, 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.security_hash(None, f1)
+ 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)
+
+#
+# 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 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"
+ if POST is not None:
+ 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
+ """
+ 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
+ """
+ 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'])
Added:
django/trunk/django/contrib/formtools/tests/templates/formwizard/wizard.html
===================================================================
---
django/trunk/django/contrib/formtools/tests/templates/formwizard/wizard.html
(rev 0)
+++
django/trunk/django/contrib/formtools/tests/templates/formwizard/wizard.html
2010-10-14 20:54:30 UTC (rev 14218)
@@ -0,0 +1,9 @@
+<p>Step {{ step }} of {{ step_count }}</p>
+<form action="." method="post">{% csrf_token %}
+<table>
+{{ form }}
+</table>
+<input type="hidden" name="{{ step_field }}" value="{{ step0 }}" />
+{{ previous_fields|safe }}
+<input type="submit">
+</form>
Copied: django/trunk/django/contrib/formtools/tests/urls.py (from rev 14216,
django/trunk/django/contrib/formtools/test_urls.py)
===================================================================
--- django/trunk/django/contrib/formtools/tests/urls.py
(rev 0)
+++ django/trunk/django/contrib/formtools/tests/urls.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -0,0 +1,17 @@
+"""
+This is a URLconf to be loaded by tests.py. Add any URLs needed for tests only.
+"""
+
+from django.conf.urls.defaults import *
+from django.contrib.formtools.tests import *
+
+urlpatterns = patterns('',
+ (r'^test1/', TestFormPreview(TestForm)),
+ (r'^test2/', UserSecuredFormPreview(TestForm)),
+ (r'^wizard/$', WizardClass([WizardPageOneForm,
+ WizardPageTwoForm,
+ WizardPageThreeForm])),
+ (r'^wizard2/$',
UserSecuredWizardClass([WizardPageOneForm,
+
WizardPageTwoForm,
+
WizardPageThreeForm]))
+ )
Deleted: django/trunk/django/contrib/formtools/tests.py
===================================================================
--- django/trunk/django/contrib/formtools/tests.py 2010-10-14 18:38:43 UTC
(rev 14217)
+++ django/trunk/django/contrib/formtools/tests.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -1,181 +0,0 @@
-from django import forms
-from django import http
-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 PreviewTests(TestCase):
- urls = 'django.contrib.formtools.test_urls'
-
- def setUp(self):
- # Create a FormPreview instance to share between tests
- self.preview = preview.FormPreview(TestForm)
- input_template = '<input type="hidden" name="%s" value="%s" />'
- self.input = input_template % (self.preview.unused_name('stage'), "%d")
- self.test_data = {'field1':u'foo', 'field1_':u'asdf'}
-
- def test_unused_name(self):
- """
- Verifies name mangling to get uniue field name.
- """
- self.assertEqual(self.preview.unused_name('field1'), 'field1__')
-
- def test_form_get(self):
- """
- Test contrib.formtools.preview form retrieval.
-
- Use the client library to see if we can sucessfully retrieve
- the form (mostly testing the setup ROOT_URLCONF
- process). Verify that an additional hidden input field
- is created to manage the stage.
-
- """
- response = self.client.get('/test1/')
- stage = self.input % 1
- self.assertContains(response, stage, 1)
-
- def test_form_preview(self):
- """
- Test contrib.formtools.preview form preview rendering.
-
- Use the client library to POST to the form to see if a preview
- is returned. If we do get a form back check that the hidden
- value is correctly managing the state of the form.
-
- """
- # Pass strings for form submittal and add stage variable to
- # show we previously saw first stage of the form.
- self.test_data.update({'stage': 1})
- response = self.client.post('/test1/', self.test_data)
- # Check to confirm stage is set to 2 in output form.
- stage = self.input % 2
- self.assertContains(response, stage, 1)
-
- def test_form_submit(self):
- """
- Test contrib.formtools.preview form submittal.
-
- Use the client library to POST to the form with stage set to 3
- to see if our forms done() method is called. Check first
- without the security hash, verify failure, retry with security
- hash and verify sucess.
-
- """
- # 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 = self.preview.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_bool_submit(self):
- """
- Test contrib.formtools.preview form submittal when form contains:
- BooleanField(required=False)
-
- Ticket: #6209 - When an unchecked BooleanField is previewed, the
preview
- form's hash would be computed with no value for ``bool1``. However,
when
- the preview form is rendered, the unchecked hidden BooleanField would
be
- rendered with the string value 'False'. So when the preview form is
- resubmitted, the hash would be computed with the value 'False' for
- ``bool1``. We need to make sure the hashes are the same in both cases.
-
- """
- self.test_data.update({'stage':2})
- hash = self.preview.security_hash(None, TestForm(self.test_data))
- self.test_data.update({'hash':hash, 'bool1':u'False'})
- response = self.client.post('/test1/', self.test_data)
- self.assertEqual(response.content, success_string)
-
-class SecurityHashTests(unittest.TestCase):
-
- 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.security_hash(None, f1)
- hash2 = utils.security_hash(None, 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.security_hash(None, f1)
- hash2 = utils.security_hash(None, 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)
-
-#
-# FormWizard tests
-#
-
-class WizardPageOneForm(forms.Form):
- field = forms.CharField()
-
-class WizardPageTwoForm(forms.Form):
- field = forms.CharField()
-
-class WizardClass(wizard.FormWizard):
- def render_template(self, *args, **kw):
- return http.HttpResponse("")
-
- def done(self, request, cleaned_data):
- return http.HttpResponse(success_string)
-
-class DummyRequest(http.HttpRequest):
- def __init__(self, POST=None):
- super(DummyRequest, self).__init__()
- self.method = POST and "POST" or "GET"
- if POST is not None:
- self.POST.update(POST)
- self._dont_enforce_csrf_checks = True
-
-class WizardTests(TestCase):
- 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)
-
- 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)
-
Modified: django/trunk/django/contrib/formtools/utils.py
===================================================================
--- django/trunk/django/contrib/formtools/utils.py 2010-10-14 18:38:43 UTC
(rev 14217)
+++ django/trunk/django/contrib/formtools/utils.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -4,9 +4,11 @@
import pickle
from django.conf import settings
+from django.forms import BooleanField
+from django.utils.crypto import salted_hmac
from django.utils.hashcompat import md5_constructor
-from django.forms import BooleanField
+
def security_hash(request, form, *args):
"""
Calculates a security hash for the given Form instance.
@@ -15,7 +17,9 @@
order, pickles the result with the SECRET_KEY setting, then takes an md5
hash of that.
"""
-
+ import warnings
+ warnings.warn("security_hash is deprecated; use form_hmac instead",
+ PendingDeprecationWarning)
data = []
for bf in form:
# Get the value from the form data. If the form allows empty or hasn't
@@ -37,3 +41,23 @@
return md5_constructor(pickled).hexdigest()
+
+def form_hmac(form):
+ """
+ Calculates a security hash for the given Form instance.
+ """
+ data = []
+ for bf in form:
+ # Get the value from the form data. If the form allows empty or hasn't
+ # changed then don't call clean() to avoid trigger validation errors.
+ if form.empty_permitted and not form.has_changed():
+ value = bf.data or ''
+ else:
+ value = bf.field.clean(bf.data) or ''
+ if isinstance(value, basestring):
+ value = value.strip()
+ data.append((bf.name, value))
+
+ pickled = pickle.dumps(data, pickle.HIGHEST_PROTOCOL)
+ key_salt = 'django.contrib.formtools'
+ return salted_hmac(key_salt, pickled).hexdigest()
Modified: django/trunk/django/contrib/formtools/wizard.py
===================================================================
--- django/trunk/django/contrib/formtools/wizard.py 2010-10-14 18:38:43 UTC
(rev 14217)
+++ django/trunk/django/contrib/formtools/wizard.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -8,12 +8,13 @@
from django import forms
from django.conf import settings
+from django.contrib.formtools.utils import security_hash, form_hmac
from django.http import Http404
from django.shortcuts import render_to_response
from django.template.context import RequestContext
+from django.utils.crypto import constant_time_compare
from django.utils.hashcompat import md5_constructor
from django.utils.translation import ugettext_lazy as _
-from django.contrib.formtools.utils import security_hash
from django.utils.decorators import method_decorator
from django.views.decorators.csrf import csrf_protect
@@ -53,6 +54,27 @@
# hook methods might alter self.form_list.
return len(self.form_list)
+ 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
+ FormWizard_expected = FormWizard.security_hash(self, request, form)
+ if expected == FormWizard_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
+
@method_decorator(csrf_protect)
def __call__(self, request, *args, **kwargs):
"""
@@ -72,7 +94,7 @@
# TODO: Move "hash_%d" to a method to make it configurable.
for i in range(current_step):
form = self.get_form(i, request.POST)
- if request.POST.get("hash_%d" % i, '') !=
self.security_hash(request, form):
+ if not self._check_security_hash(request.POST.get("hash_%d" % i,
''), request, form):
return self.render_hash_failure(request, i)
self.process_step(request, form, i)
@@ -95,6 +117,21 @@
# Validate all the forms. If any of them fail validation, that
# must mean the validator relied on some other input, such as
# an external Web site.
+
+ # It is also possible that validation might fail under certain
+ # attack situations: an attacker might be able to bypass
previous
+ # stages, and generate correct security hashes for all the
+ # skipped stages by virtue of:
+ # 1) having filled out an identical form which doesn't have
the
+ # validation (and does something different at the end),
+ # 2) or having filled out a previous version of the same form
+ # which had some validation missing,
+ # 3) or previously having filled out the form when they had
+ # more privileges than they do now.
+ #
+ # Since the hashes only take into account values, and not other
+ # other validation the form might do, we must re-do validation
+ # now for security reasons.
for i, f in enumerate(final_form_list):
if not f.is_valid():
return self.render_revalidation_failure(request, i, f)
@@ -155,7 +192,7 @@
Subclasses may want to take into account request-specific information,
such as the IP address.
"""
- return security_hash(request, form)
+ return form_hmac(form)
def determine_step(self, request, *args, **kwargs):
"""
Modified: django/trunk/django/contrib/messages/storage/cookie.py
===================================================================
--- django/trunk/django/contrib/messages/storage/cookie.py 2010-10-14
18:38:43 UTC (rev 14217)
+++ django/trunk/django/contrib/messages/storage/cookie.py 2010-10-14
20:54:30 UTC (rev 14218)
@@ -1,11 +1,9 @@
-import hmac
-
from django.conf import settings
from django.contrib.messages import constants
from django.contrib.messages.storage.base import BaseStorage, Message
from django.http import CompatCookie
from django.utils import simplejson as json
-from django.utils.hashcompat import sha_hmac
+from django.utils.crypto import salted_hmac, constant_time_compare
class MessageEncoder(json.JSONEncoder):
@@ -111,8 +109,8 @@
Creates an HMAC/SHA1 hash based on the value and the project setting's
SECRET_KEY, modified to make it unique for the present purpose.
"""
- key = 'django.contrib.messages' + settings.SECRET_KEY
- return hmac.new(key, value, sha_hmac).hexdigest()
+ key_salt = 'django.contrib.messages'
+ return salted_hmac(key_salt, value).hexdigest()
def _encode(self, messages, encode_empty=False):
"""
@@ -139,7 +137,7 @@
bits = data.split('$', 1)
if len(bits) == 2:
hash, value = bits
- if hash == self._hash(value):
+ if constant_time_compare(hash, self._hash(value)):
try:
# If we get here (and the JSON decode works), everything is
# good. In any other case, drop back and return None.
Modified: django/trunk/django/contrib/sessions/backends/base.py
===================================================================
--- django/trunk/django/contrib/sessions/backends/base.py 2010-10-14
18:38:43 UTC (rev 14217)
+++ django/trunk/django/contrib/sessions/backends/base.py 2010-10-14
20:54:30 UTC (rev 14218)
@@ -12,6 +12,7 @@
from django.conf import settings
from django.core.exceptions import SuspiciousOperation
from django.utils.hashcompat import md5_constructor
+from django.utils.crypto import constant_time_compare, salted_hmac
# Use the system (hardware-based) random number generator if it exists.
if hasattr(random, 'SystemRandom'):
@@ -83,23 +84,45 @@
def delete_test_cookie(self):
del self[self.TEST_COOKIE_NAME]
+ def _hash(self, value):
+ key_salt = "django.contrib.sessions" + self.__class__.__name__
+ return salted_hmac(key_salt, value).hexdigest()
+
def encode(self, session_dict):
"Returns the given session dictionary pickled and encoded as a string."
pickled = pickle.dumps(session_dict, pickle.HIGHEST_PROTOCOL)
- pickled_md5 = md5_constructor(pickled +
settings.SECRET_KEY).hexdigest()
- return base64.encodestring(pickled + pickled_md5)
+ hash = self._hash(pickled)
+ return base64.encodestring(hash + ":" + pickled)
def decode(self, session_data):
encoded_data = base64.decodestring(session_data)
+ try:
+ # could produce ValueError if there is no ':'
+ hash, pickled = encoded_data.split(':', 1)
+ expected_hash = self._hash(pickled)
+ if not constant_time_compare(hash, expected_hash):
+ raise SuspiciousOperation("Session data corrupted")
+ else:
+ return pickle.loads(pickled)
+ except Exception:
+ # ValueError, SuspiciousOperation, unpickling exceptions
+ # Fall back to Django 1.2 method
+ # PendingDeprecationWarning <- here to remind us to
+ # remove this fallback in Django 1.5
+ try:
+ return self._decode_old(session_data)
+ except Exception:
+ # Unpickling can cause a variety of exceptions. If something
happens,
+ # just return an empty dictionary (an empty session).
+ return {}
+
+ def _decode_old(self, session_data):
+ encoded_data = base64.decodestring(session_data)
pickled, tamper_check = encoded_data[:-32], encoded_data[-32:]
- if md5_constructor(pickled + settings.SECRET_KEY).hexdigest() !=
tamper_check:
+ if not constant_time_compare(md5_constructor(pickled +
settings.SECRET_KEY).hexdigest(),
+ tamper_check):
raise SuspiciousOperation("User tampered with session cookie.")
- try:
- return pickle.loads(pickled)
- # Unpickling can cause a variety of exceptions. If something happens,
- # just return an empty dictionary (an empty session).
- except:
- return {}
+ return pickle.loads(pickled)
def update(self, dict_):
self._session.update(dict_)
Modified: django/trunk/django/contrib/sessions/tests.py
===================================================================
--- django/trunk/django/contrib/sessions/tests.py 2010-10-14 18:38:43 UTC
(rev 14217)
+++ django/trunk/django/contrib/sessions/tests.py 2010-10-14 20:54:30 UTC
(rev 14218)
@@ -1,4 +1,6 @@
+import base64
from datetime import datetime, timedelta
+import pickle
import shutil
import tempfile
@@ -12,6 +14,7 @@
from django.core.exceptions import ImproperlyConfigured
from django.test import TestCase
from django.utils import unittest
+from django.utils.hashcompat import md5_constructor
class SessionTestsMixin(object):
@@ -237,7 +240,25 @@
finally:
settings.SESSION_EXPIRE_AT_BROWSER_CLOSE =
original_expire_at_browser_close
+ def test_decode(self):
+ # Ensure we can decode what we encode
+ data = {'a test key': 'a test value'}
+ encoded = self.session.encode(data)
+ self.assertEqual(self.session.decode(encoded), data)
+ def test_decode_django12(self):
+ # Ensure we can decode values encoded using Django 1.2
+ # Hard code the Django 1.2 method here:
+ def encode(session_dict):
+ pickled = pickle.dumps(session_dict, pickle.HIGHEST_PROTOCOL)
+ pickled_md5 = md5_constructor(pickled +
settings.SECRET_KEY).hexdigest()
+ return base64.encodestring(pickled + pickled_md5)
+
+ data = {'a test key': 'a test value'}
+ encoded = encode(data)
+ self.assertEqual(self.session.decode(encoded), data)
+
+
class DatabaseSessionTests(SessionTestsMixin, TestCase):
backend = DatabaseSession
Modified: django/trunk/django/middleware/csrf.py
===================================================================
--- django/trunk/django/middleware/csrf.py 2010-10-14 18:38:43 UTC (rev
14217)
+++ django/trunk/django/middleware/csrf.py 2010-10-14 20:54:30 UTC (rev
14218)
@@ -15,6 +15,7 @@
from django.utils.hashcompat import md5_constructor
from django.utils.log import getLogger
from django.utils.safestring import mark_safe
+from django.utils.crypto import constant_time_compare
_POST_FORM_RE = \
re.compile(r'(<form\W[^>]*\bmethod\s*=\s*(\'|"|)POST(\'|"|)\b[^>]*>)',
re.IGNORECASE)
@@ -216,8 +217,8 @@
csrf_token = request.META["CSRF_COOKIE"]
# check incoming token
- request_csrf_token = request.POST.get('csrfmiddlewaretoken', None)
- if request_csrf_token != csrf_token:
+ request_csrf_token = request.POST.get('csrfmiddlewaretoken', '')
+ if not constant_time_compare(request_csrf_token, csrf_token):
if cookie_is_new:
# probably a problem setting the CSRF cookie
logger.warning('Forbidden (%s): %s' %
(REASON_NO_CSRF_COOKIE, request.path),
Added: django/trunk/django/utils/crypto.py
===================================================================
--- django/trunk/django/utils/crypto.py (rev 0)
+++ django/trunk/django/utils/crypto.py 2010-10-14 20:54:30 UTC (rev 14218)
@@ -0,0 +1,45 @@
+"""
+Django's standard crypto functions and utilities.
+"""
+import hmac
+
+from django.conf import settings
+from django.utils.hashcompat import sha_constructor
+
+
+def salted_hmac(key_salt, value, secret=None):
+ """
+ Returns the HMAC-SHA1 of 'value', using a key generated from key_salt and a
+ secret (which defaults to settings.SECRET_KEY).
+
+ A different key_salt should be passed in for every application of HMAC.
+ """
+ if secret is None:
+ secret = settings.SECRET_KEY
+
+ # We need to generate a derived key from our base key. We can do this by
+ # passing the key_salt and our base key through a pseudo-random function
and
+ # SHA1 works nicely.
+
+ key = sha_constructor(key_salt + secret).digest()
+
+ # If len(key_salt + secret) > sha_constructor().block_size, the above
+ # line is redundant and could be replaced by key = key_salt + secret, since
+ # the hmac module does the same thing for keys longer than the block size.
+ # However, we need to ensure that we *always* do this.
+
+ return hmac.new(key, msg=value, digestmod=sha_constructor)
+
+
+def constant_time_compare(val1, val2):
+ """
+ Returns True if the two strings are equal, False otherwise.
+
+ The time taken is independent of the number of characters that match.
+ """
+ if len(val1) != len(val2):
+ return False
+ result = 0
+ for x, y in zip(val1, val2):
+ result |= ord(x) ^ ord(y)
+ return result == 0
Modified: django/trunk/docs/internals/deprecation.txt
===================================================================
--- django/trunk/docs/internals/deprecation.txt 2010-10-14 18:38:43 UTC (rev
14217)
+++ django/trunk/docs/internals/deprecation.txt 2010-10-14 20:54:30 UTC (rev
14218)
@@ -114,6 +114,10 @@
:class:`~django.test.simple.DjangoTestRunner` will be removed in
favor of using the unittest-native class.
+ * The undocumented function
+ :func:`django.contrib.formtools.utils.security_hash`
+ is deprecated, in favour of
:func:`django.contrib.formtools.utils.form_hmac`
+
* 2.0
* ``django.views.defaults.shortcut()``. This function has been moved
to ``django.contrib.contenttypes.views.shortcut()`` as part of the
Modified: django/trunk/docs/ref/contrib/formtools/form-wizard.txt
===================================================================
--- django/trunk/docs/ref/contrib/formtools/form-wizard.txt 2010-10-14
18:38:43 UTC (rev 14217)
+++ django/trunk/docs/ref/contrib/formtools/form-wizard.txt 2010-10-14
20:54:30 UTC (rev 14218)
@@ -240,7 +240,7 @@
Calculates the security hash for the given request object and
:class:`~django.forms.Form` instance.
- By default, this uses an MD5 hash of the form data and your
+ By default, this generates a SHA1 HMAC using your form data and your
:setting:`SECRET_KEY` setting. It's rare that somebody would need to
override this.
Modified:
django/trunk/tests/regressiontests/comment_tests/tests/comment_form_tests.py
===================================================================
---
django/trunk/tests/regressiontests/comment_tests/tests/comment_form_tests.py
2010-10-14 18:38:43 UTC (rev 14217)
+++
django/trunk/tests/regressiontests/comment_tests/tests/comment_form_tests.py
2010-10-14 20:54:30 UTC (rev 14218)
@@ -2,6 +2,7 @@
from django.conf import settings
from django.contrib.comments.models import Comment
from django.contrib.comments.forms import CommentForm
+from django.utils.hashcompat import sha_constructor
from regressiontests.comment_tests.models import Article
from regressiontests.comment_tests.tests import CommentTestCase
@@ -43,6 +44,23 @@
def testObjectPKTampering(self):
self.tamperWithForm(object_pk="3")
+ def testDjango12Hash(self):
+ # Ensure we can use the hashes generated by Django 1.2
+ a = Article.objects.get(pk=1)
+ d = self.getValidData(a)
+
+ content_type = d['content_type']
+ object_pk = d['object_pk']
+ timestamp = d['timestamp']
+
+ # The Django 1.2 method hard-coded here:
+ info = (content_type, object_pk, timestamp, settings.SECRET_KEY)
+ security_hash = sha_constructor("".join(info)).hexdigest()
+
+ d['security_hash'] = security_hash
+ f = CommentForm(a, data=d)
+ self.assertTrue(f.is_valid(), f.errors)
+
def testSecurityErrors(self):
f = self.tamperWithForm(honeypot="I am a robot")
self.assert_("honeypot" in f.security_errors())
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
