diff --git a/django/contrib/auth/tests/tokens.py b/django/contrib/auth/tests/tokens.py index e9e90493be..623e5da66a 100644 --- a/django/contrib/auth/tests/tokens.py +++ b/django/contrib/auth/tests/tokens.py @@ -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)) diff --git a/django/contrib/auth/tokens.py b/django/contrib/auth/tokens.py index f61f52d903..f03ed0c93f 100644 --- a/django/contrib/auth/tokens.py +++ b/django/contrib/auth/tokens.py @@ -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 @@ class PasswordResetTokenGenerator(object): 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 @@ 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] diff --git a/django/contrib/comments/forms.py b/django/contrib/comments/forms.py index 0c4b28528b..4ecc4c4c06 100644 --- a/django/contrib/comments/forms.py +++ b/django/contrib/comments/forms.py @@ -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,8 +47,13 @@ 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: - 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 @@ 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() diff --git a/django/contrib/formtools/preview.py b/django/contrib/formtools/preview.py index f202084da3..b85c6ef7d1 100644 --- a/django/contrib/formtools/preview.py +++ b/django/contrib/formtools/preview.py @@ -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: diff --git a/django/contrib/formtools/test_urls.py b/django/contrib/formtools/test_urls.py deleted file mode 100644 index 44a58d2a7f..0000000000 --- a/django/contrib/formtools/test_urls.py +++ /dev/null @@ -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)), - ) diff --git a/django/contrib/formtools/tests.py b/django/contrib/formtools/tests/__init__.py similarity index 50% rename from django/contrib/formtools/tests.py rename to django/contrib/formtools/tests/__init__.py index 32879a2fa1..c2cb53ca47 100644 --- a/django/contrib/formtools/tests.py +++ b/django/contrib/formtools/tests/__init__.py @@ -1,23 +1,37 @@ +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']) diff --git a/django/contrib/formtools/tests/templates/formwizard/wizard.html b/django/contrib/formtools/tests/templates/formwizard/wizard.html new file mode 100644 index 0000000000..42b6e78be8 --- /dev/null +++ b/django/contrib/formtools/tests/templates/formwizard/wizard.html @@ -0,0 +1,9 @@ +
Step {{ step }} of {{ step_count }}
+ diff --git a/django/contrib/formtools/tests/urls.py b/django/contrib/formtools/tests/urls.py new file mode 100644 index 0000000000..b89b6e6ed1 --- /dev/null +++ b/django/contrib/formtools/tests/urls.py @@ -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])) + ) diff --git a/django/contrib/formtools/utils.py b/django/contrib/formtools/utils.py index e77931ffba..894178a616 100644 --- a/django/contrib/formtools/utils.py +++ b/django/contrib/formtools/utils.py @@ -4,8 +4,10 @@ except ImportError: import pickle from django.conf import settings -from django.utils.hashcompat import md5_constructor from django.forms import BooleanField +from django.utils.crypto import salted_hmac +from django.utils.hashcompat import md5_constructor + def security_hash(request, form, *args): """ @@ -15,7 +17,9 @@ def security_hash(request, form, *args): 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 @@ def security_hash(request, form, *args): 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() diff --git a/django/contrib/formtools/wizard.py b/django/contrib/formtools/wizard.py index 32e27df574..ea5843168e 100644 --- a/django/contrib/formtools/wizard.py +++ b/django/contrib/formtools/wizard.py @@ -8,12 +8,13 @@ import cPickle as pickle 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 @@ class FormWizard(object): # 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 @@ class FormWizard(object): # 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 @@ class FormWizard(object): # 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 @@ class FormWizard(object): 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): """ diff --git a/django/contrib/messages/storage/cookie.py b/django/contrib/messages/storage/cookie.py index 1fd3ed98d8..ffbfce157e 100644 --- a/django/contrib/messages/storage/cookie.py +++ b/django/contrib/messages/storage/cookie.py @@ -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 @@ class CookieStorage(BaseStorage): 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 @@ class CookieStorage(BaseStorage): 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. diff --git a/django/contrib/sessions/backends/base.py b/django/contrib/sessions/backends/base.py index b4cdeadd81..bb09cb657a 100644 --- a/django/contrib/sessions/backends/base.py +++ b/django/contrib/sessions/backends/base.py @@ -12,6 +12,7 @@ except ImportError: 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 @@ class SessionBase(object): 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) - pickled, tamper_check = encoded_data[:-32], encoded_data[-32:] - if 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 {} + # 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 not constant_time_compare(md5_constructor(pickled + settings.SECRET_KEY).hexdigest(), + tamper_check): + raise SuspiciousOperation("User tampered with session cookie.") + return pickle.loads(pickled) def update(self, dict_): self._session.update(dict_) diff --git a/django/contrib/sessions/tests.py b/django/contrib/sessions/tests.py index f9b66309cb..c02b8807ef 100644 --- a/django/contrib/sessions/tests.py +++ b/django/contrib/sessions/tests.py @@ -1,4 +1,6 @@ +import base64 from datetime import datetime, timedelta +import pickle import shutil import tempfile @@ -12,6 +14,7 @@ from django.contrib.sessions.models import Session 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,6 +240,24 @@ class SessionTestsMixin(object): 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): diff --git a/django/middleware/csrf.py b/django/middleware/csrf.py index 483510a1ae..7bef64edb9 100644 --- a/django/middleware/csrf.py +++ b/django/middleware/csrf.py @@ -15,6 +15,7 @@ from django.utils.cache import patch_vary_headers 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'(