Security fix. Announcement forthcoming.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@8877 bcc190cf-cafb-0310-a4f2-bffc1f526a37
This commit is contained in:
Jacob Kaplan-Moss 2008-09-02 21:10:00 +00:00
parent ecb5f4c693
commit 0e5faf225c
4 changed files with 19 additions and 112 deletions

View File

@ -1,7 +1,5 @@
import base64 import base64
import cPickle as pickle
import re import re
from django import http, template from django import http, template
from django.contrib.admin import ModelAdmin from django.contrib.admin import ModelAdmin
from django.contrib.auth import authenticate, login from django.contrib.auth import authenticate, login
@ -24,19 +22,6 @@ class AlreadyRegistered(Exception):
class NotRegistered(Exception): class NotRegistered(Exception):
pass pass
def _encode_post_data(post_data):
pickled = pickle.dumps(post_data)
pickled_md5 = md5_constructor(pickled + settings.SECRET_KEY).hexdigest()
return base64.encodestring(pickled + pickled_md5)
def _decode_post_data(encoded_data):
encoded_data = base64.decodestring(encoded_data)
pickled, tamper_check = encoded_data[:-32], encoded_data[-32:]
if md5_constructor(pickled + settings.SECRET_KEY).hexdigest() != tamper_check:
from django.core.exceptions import SuspiciousOperation
raise SuspiciousOperation, "User may have tampered with session cookie."
return pickle.loads(pickled)
class AdminSite(object): class AdminSite(object):
""" """
An AdminSite object encapsulates an instance of the Django admin application, ready An AdminSite object encapsulates an instance of the Django admin application, ready
@ -239,7 +224,7 @@ class AdminSite(object):
# If this isn't already the login page, display it. # If this isn't already the login page, display it.
if not request.POST.has_key(LOGIN_FORM_KEY): if not request.POST.has_key(LOGIN_FORM_KEY):
if request.POST: if request.POST:
message = _("Please log in again, because your session has expired. Don't worry: Your submission has been saved.") message = _("Please log in again, because your session has expired.")
else: else:
message = "" message = ""
return self.display_login_form(request, message) return self.display_login_form(request, message)
@ -275,15 +260,7 @@ class AdminSite(object):
else: else:
if user.is_active and user.is_staff: if user.is_active and user.is_staff:
login(request, user) login(request, user)
if request.POST.has_key('post_data'): return http.HttpResponseRedirect(request.get_full_path())
post_data = _decode_post_data(request.POST['post_data'])
if post_data and not post_data.has_key(LOGIN_FORM_KEY):
# overwrite request.POST with the saved post_data, and continue
request.POST = post_data
request.user = user
return self.root(request, request.path.split(self.root_path)[-1])
else:
return http.HttpResponseRedirect(request.get_full_path())
else: else:
return self.display_login_form(request, ERROR_MESSAGE) return self.display_login_form(request, ERROR_MESSAGE)
login = never_cache(login) login = never_cache(login)
@ -345,19 +322,9 @@ class AdminSite(object):
def display_login_form(self, request, error_message='', extra_context=None): def display_login_form(self, request, error_message='', extra_context=None):
request.session.set_test_cookie() request.session.set_test_cookie()
if request.POST and request.POST.has_key('post_data'):
# User has failed login BUT has previously saved post data.
post_data = request.POST['post_data']
elif request.POST:
# User's session must have expired; save their post data.
post_data = _encode_post_data(request.POST)
else:
post_data = _encode_post_data({})
context = { context = {
'title': _('Log in'), 'title': _('Log in'),
'app_path': request.get_full_path(), 'app_path': request.get_full_path(),
'post_data': post_data,
'error_message': error_message, 'error_message': error_message,
'root_path': self.root_path, 'root_path': self.root_path,
} }

View File

@ -21,7 +21,6 @@
<div class="form-row"> <div class="form-row">
<label for="id_password">{% trans 'Password:' %}</label> <input type="password" name="password" id="id_password" /> <label for="id_password">{% trans 'Password:' %}</label> <input type="password" name="password" id="id_password" />
<input type="hidden" name="this_is_the_login_form" value="1" /> <input type="hidden" name="this_is_the_login_form" value="1" />
<input type="hidden" name="post_data" value="{{ post_data }}" /> {#<span class="help">{% trans 'Have you <a href="/password_reset/">forgotten your password</a>?' %}</span>#}
</div> </div>
<div class="submit-row"> <div class="submit-row">
<label>&nbsp;</label><input type="submit" value="{% trans 'Log in' %}" /> <label>&nbsp;</label><input type="submit" value="{% trans 'Log in' %}" />

View File

@ -1,5 +1,4 @@
import base64 import base64
import cPickle as pickle
try: try:
from functools import wraps from functools import wraps
except ImportError: except ImportError:
@ -11,41 +10,18 @@ from django.contrib.auth.models import User
from django.contrib.auth import authenticate, login from django.contrib.auth import authenticate, login
from django.shortcuts import render_to_response from django.shortcuts import render_to_response
from django.utils.translation import ugettext_lazy, ugettext as _ from django.utils.translation import ugettext_lazy, ugettext as _
from django.utils.hashcompat import md5_constructor
ERROR_MESSAGE = ugettext_lazy("Please enter a correct username and password. Note that both fields are case-sensitive.") ERROR_MESSAGE = ugettext_lazy("Please enter a correct username and password. Note that both fields are case-sensitive.")
LOGIN_FORM_KEY = 'this_is_the_login_form' LOGIN_FORM_KEY = 'this_is_the_login_form'
def _display_login_form(request, error_message=''): def _display_login_form(request, error_message=''):
request.session.set_test_cookie() request.session.set_test_cookie()
if request.POST and 'post_data' in request.POST:
# User has failed login BUT has previously saved post data.
post_data = request.POST['post_data']
elif request.POST:
# User's session must have expired; save their post data.
post_data = _encode_post_data(request.POST)
else:
post_data = _encode_post_data({})
return render_to_response('admin/login.html', { return render_to_response('admin/login.html', {
'title': _('Log in'), 'title': _('Log in'),
'app_path': request.get_full_path(), 'app_path': request.get_full_path(),
'post_data': post_data,
'error_message': error_message 'error_message': error_message
}, context_instance=template.RequestContext(request)) }, context_instance=template.RequestContext(request))
def _encode_post_data(post_data):
pickled = pickle.dumps(post_data)
pickled_md5 = md5_constructor(pickled + settings.SECRET_KEY).hexdigest()
return base64.encodestring(pickled + pickled_md5)
def _decode_post_data(encoded_data):
encoded_data = base64.decodestring(encoded_data)
pickled, tamper_check = encoded_data[:-32], encoded_data[-32:]
if md5_constructor(pickled + settings.SECRET_KEY).hexdigest() != tamper_check:
from django.core.exceptions import SuspiciousOperation
raise SuspiciousOperation, "User may have tampered with session cookie."
return pickle.loads(pickled)
def staff_member_required(view_func): def staff_member_required(view_func):
""" """
Decorator for views that checks that the user is logged in and is a staff Decorator for views that checks that the user is logged in and is a staff
@ -54,10 +30,6 @@ def staff_member_required(view_func):
def _checklogin(request, *args, **kwargs): def _checklogin(request, *args, **kwargs):
if request.user.is_authenticated() and request.user.is_staff: if request.user.is_authenticated() and request.user.is_staff:
# The user is valid. Continue to the admin page. # The user is valid. Continue to the admin page.
if 'post_data' in request.POST:
# User must have re-authenticated through a different window
# or tab.
request.POST = _decode_post_data(request.POST['post_data'])
return view_func(request, *args, **kwargs) return view_func(request, *args, **kwargs)
assert hasattr(request, 'session'), "The Django admin requires session middleware to be installed. Edit your MIDDLEWARE_CLASSES setting to insert 'django.contrib.sessions.middleware.SessionMiddleware'." assert hasattr(request, 'session'), "The Django admin requires session middleware to be installed. Edit your MIDDLEWARE_CLASSES setting to insert 'django.contrib.sessions.middleware.SessionMiddleware'."
@ -65,7 +37,7 @@ def staff_member_required(view_func):
# If this isn't already the login page, display it. # If this isn't already the login page, display it.
if LOGIN_FORM_KEY not in request.POST: if LOGIN_FORM_KEY not in request.POST:
if request.POST: if request.POST:
message = _("Please log in again, because your session has expired. Don't worry: Your submission has been saved.") message = _("Please log in again, because your session has expired.")
else: else:
message = "" message = ""
return _display_login_form(request, message) return _display_login_form(request, message)
@ -98,16 +70,7 @@ def staff_member_required(view_func):
else: else:
if user.is_active and user.is_staff: if user.is_active and user.is_staff:
login(request, user) login(request, user)
# TODO: set last_login with an event. return http.HttpResponseRedirect(request.get_full_path())
if 'post_data' in request.POST:
post_data = _decode_post_data(request.POST['post_data'])
if post_data and LOGIN_FORM_KEY not in post_data:
# overwrite request.POST with the saved post_data, and continue
request.POST = post_data
request.user = user
return view_func(request, *args, **kwargs)
else:
return http.HttpResponseRedirect(request.get_full_path())
else: else:
return _display_login_form(request, ERROR_MESSAGE) return _display_login_form(request, ERROR_MESSAGE)

View File

@ -4,7 +4,7 @@ from django.test import TestCase
from django.contrib.auth.models import User, Permission from django.contrib.auth.models import User, Permission
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.contrib.admin.models import LogEntry from django.contrib.admin.models import LogEntry
from django.contrib.admin.sites import LOGIN_FORM_KEY, _encode_post_data from django.contrib.admin.sites import LOGIN_FORM_KEY
from django.contrib.admin.util import quote from django.contrib.admin.util import quote
from django.utils.html import escape from django.utils.html import escape
@ -136,31 +136,31 @@ class AdminViewPermissionsTest(TestCase):
Section._meta.get_delete_permission())) Section._meta.get_delete_permission()))
# login POST dicts # login POST dicts
self.super_login = {'post_data': _encode_post_data({}), self.super_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'super', 'username': 'super',
'password': 'secret'} 'password': 'secret'}
self.super_email_login = {'post_data': _encode_post_data({}), self.super_email_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'super@example.com', 'username': 'super@example.com',
'password': 'secret'} 'password': 'secret'}
self.super_email_bad_login = {'post_data': _encode_post_data({}), self.super_email_bad_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'super@example.com', 'username': 'super@example.com',
'password': 'notsecret'} 'password': 'notsecret'}
self.adduser_login = {'post_data': _encode_post_data({}), self.adduser_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'adduser', 'username': 'adduser',
'password': 'secret'} 'password': 'secret'}
self.changeuser_login = {'post_data': _encode_post_data({}), self.changeuser_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'changeuser', 'username': 'changeuser',
'password': 'secret'} 'password': 'secret'}
self.deleteuser_login = {'post_data': _encode_post_data({}), self.deleteuser_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'deleteuser', 'username': 'deleteuser',
'password': 'secret'} 'password': 'secret'}
self.joepublic_login = {'post_data': _encode_post_data({}), self.joepublic_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'joepublic', 'username': 'joepublic',
'password': 'secret'} 'password': 'secret'}
@ -271,17 +271,6 @@ class AdminViewPermissionsTest(TestCase):
self.failUnlessEqual(Article.objects.all().count(), 3) self.failUnlessEqual(Article.objects.all().count(), 3)
self.client.get('/test_admin/admin/logout/') self.client.get('/test_admin/admin/logout/')
# Check and make sure that if user expires, data still persists
post = self.client.post('/test_admin/admin/admin_views/article/add/', add_dict)
self.assertContains(post, 'Please log in again, because your session has expired.')
self.super_login['post_data'] = _encode_post_data(add_dict)
post = self.client.post('/test_admin/admin/admin_views/article/add/', self.super_login)
# make sure the view removes test cookie
self.failUnlessEqual(self.client.session.test_cookie_worked(), False)
self.assertRedirects(post, '/test_admin/admin/admin_views/article/')
self.failUnlessEqual(Article.objects.all().count(), 4)
self.client.get('/test_admin/admin/logout/')
# 8509 - if a normal user is already logged in, it is possible # 8509 - if a normal user is already logged in, it is possible
# to change user into the superuser without error # to change user into the superuser without error
login = self.client.login(username='joepublic', password='secret') login = self.client.login(username='joepublic', password='secret')
@ -489,31 +478,31 @@ class SecureViewTest(TestCase):
def setUp(self): def setUp(self):
# login POST dicts # login POST dicts
self.super_login = {'post_data': _encode_post_data({}), self.super_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'super', 'username': 'super',
'password': 'secret'} 'password': 'secret'}
self.super_email_login = {'post_data': _encode_post_data({}), self.super_email_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'super@example.com', 'username': 'super@example.com',
'password': 'secret'} 'password': 'secret'}
self.super_email_bad_login = {'post_data': _encode_post_data({}), self.super_email_bad_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'super@example.com', 'username': 'super@example.com',
'password': 'notsecret'} 'password': 'notsecret'}
self.adduser_login = {'post_data': _encode_post_data({}), self.adduser_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'adduser', 'username': 'adduser',
'password': 'secret'} 'password': 'secret'}
self.changeuser_login = {'post_data': _encode_post_data({}), self.changeuser_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'changeuser', 'username': 'changeuser',
'password': 'secret'} 'password': 'secret'}
self.deleteuser_login = {'post_data': _encode_post_data({}), self.deleteuser_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'deleteuser', 'username': 'deleteuser',
'password': 'secret'} 'password': 'secret'}
self.joepublic_login = {'post_data': _encode_post_data({}), self.joepublic_login = {
LOGIN_FORM_KEY: 1, LOGIN_FORM_KEY: 1,
'username': 'joepublic', 'username': 'joepublic',
'password': 'secret'} 'password': 'secret'}
@ -597,17 +586,6 @@ class SecureViewTest(TestCase):
# Login.context is a list of context dicts we just need to check the first one. # Login.context is a list of context dicts we just need to check the first one.
self.assert_(login.context[0].get('error_message')) self.assert_(login.context[0].get('error_message'))
# Check and make sure that if user expires, data still persists
data = {'foo': 'bar'}
post = self.client.post('/test_admin/admin/secure-view/', data)
self.assertContains(post, 'Please log in again, because your session has expired.')
self.super_login['post_data'] = _encode_post_data(data)
post = self.client.post('/test_admin/admin/secure-view/', self.super_login)
# make sure the view removes test cookie
self.failUnlessEqual(self.client.session.test_cookie_worked(), False)
self.assertContains(post, "{'foo': 'bar'}")
self.client.get('/test_admin/admin/logout/')
# 8509 - if a normal user is already logged in, it is possible # 8509 - if a normal user is already logged in, it is possible
# to change user into the superuser without error # to change user into the superuser without error
login = self.client.login(username='joepublic', password='secret') login = self.client.login(username='joepublic', password='secret')