Fixed #19866 -- Added security logger and return 400 for SuspiciousOperation.
SuspiciousOperations have been differentiated into subclasses, and are now logged to a 'django.security.*' logger. SuspiciousOperations that reach django.core.handlers.base.BaseHandler will now return a 400 instead of a 500. Thanks to tiwoc for the report, and Carl Meyer and Donald Stufft for review.
This commit is contained in:
parent
36d47f72e3
commit
d228c1192e
|
@ -5,8 +5,9 @@ from django.utils.importlib import import_module
|
|||
from django.utils import six
|
||||
|
||||
|
||||
__all__ = ['handler403', 'handler404', 'handler500', 'include', 'patterns', 'url']
|
||||
__all__ = ['handler400', 'handler403', 'handler404', 'handler500', 'include', 'patterns', 'url']
|
||||
|
||||
handler400 = 'django.views.defaults.bad_request'
|
||||
handler403 = 'django.views.defaults.permission_denied'
|
||||
handler404 = 'django.views.defaults.page_not_found'
|
||||
handler500 = 'django.views.defaults.server_error'
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
from django.core.exceptions import SuspiciousOperation
|
||||
|
||||
|
||||
class DisallowedModelAdminLookup(SuspiciousOperation):
|
||||
"""Invalid filter was passed to admin view via URL querystring"""
|
||||
pass
|
|
@ -14,6 +14,7 @@ from django.utils.translation import ugettext, ugettext_lazy
|
|||
from django.utils.http import urlencode
|
||||
|
||||
from django.contrib.admin import FieldListFilter
|
||||
from django.contrib.admin.exceptions import DisallowedModelAdminLookup
|
||||
from django.contrib.admin.options import IncorrectLookupParameters
|
||||
from django.contrib.admin.util import (quote, get_fields_from_path,
|
||||
lookup_needs_distinct, prepare_lookup_value)
|
||||
|
@ -128,7 +129,7 @@ class ChangeList(six.with_metaclass(RenameChangeListMethods)):
|
|||
lookup_params[force_str(key)] = value
|
||||
|
||||
if not self.model_admin.lookup_allowed(key, value):
|
||||
raise SuspiciousOperation("Filtering by %s not allowed" % key)
|
||||
raise DisallowedModelAdminLookup("Filtering by %s not allowed" % key)
|
||||
|
||||
filter_specs = []
|
||||
if self.list_filter:
|
||||
|
|
|
@ -10,7 +10,6 @@ from django.conf import global_settings, settings
|
|||
from django.contrib.sites.models import Site, RequestSite
|
||||
from django.contrib.auth.models import User
|
||||
from django.core import mail
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.core.urlresolvers import reverse, NoReverseMatch
|
||||
from django.http import QueryDict, HttpRequest
|
||||
from django.utils.encoding import force_text
|
||||
|
@ -18,7 +17,7 @@ from django.utils.html import escape
|
|||
from django.utils.http import urlquote
|
||||
from django.utils._os import upath
|
||||
from django.test import TestCase
|
||||
from django.test.utils import override_settings
|
||||
from django.test.utils import override_settings, patch_logger
|
||||
from django.middleware.csrf import CsrfViewMiddleware
|
||||
from django.contrib.sessions.middleware import SessionMiddleware
|
||||
|
||||
|
@ -155,23 +154,28 @@ class PasswordResetTest(AuthViewsTestCase):
|
|||
# produce a meaningful reset URL, we need to be certain that the
|
||||
# HTTP_HOST header isn't poisoned. This is done as a check when get_host()
|
||||
# is invoked, but we check here as a practical consequence.
|
||||
with self.assertRaises(SuspiciousOperation):
|
||||
self.client.post('/password_reset/',
|
||||
with patch_logger('django.security.DisallowedHost', 'error') as logger_calls:
|
||||
response = self.client.post('/password_reset/',
|
||||
{'email': 'staffmember@example.com'},
|
||||
HTTP_HOST='www.example:dr.frankenstein@evil.tld'
|
||||
)
|
||||
self.assertEqual(response.status_code, 400)
|
||||
self.assertEqual(len(mail.outbox), 0)
|
||||
self.assertEqual(len(logger_calls), 1)
|
||||
|
||||
# Skip any 500 handler action (like sending more mail...)
|
||||
@override_settings(DEBUG_PROPAGATE_EXCEPTIONS=True)
|
||||
def test_poisoned_http_host_admin_site(self):
|
||||
"Poisoned HTTP_HOST headers can't be used for reset emails on admin views"
|
||||
with self.assertRaises(SuspiciousOperation):
|
||||
self.client.post('/admin_password_reset/',
|
||||
with patch_logger('django.security.DisallowedHost', 'error') as logger_calls:
|
||||
response = self.client.post('/admin_password_reset/',
|
||||
{'email': 'staffmember@example.com'},
|
||||
HTTP_HOST='www.example:dr.frankenstein@evil.tld'
|
||||
)
|
||||
self.assertEqual(response.status_code, 400)
|
||||
self.assertEqual(len(mail.outbox), 0)
|
||||
self.assertEqual(len(logger_calls), 1)
|
||||
|
||||
|
||||
def _test_confirm_start(self):
|
||||
# Start by creating the email
|
||||
|
@ -678,5 +682,7 @@ class ChangelistTests(AuthViewsTestCase):
|
|||
self.login()
|
||||
|
||||
# A lookup that tries to filter on password isn't OK
|
||||
with self.assertRaises(SuspiciousOperation):
|
||||
with patch_logger('django.security.DisallowedModelAdminLookup', 'error') as logger_calls:
|
||||
response = self.client.get('/admin/auth/user/?password__startswith=sha1$')
|
||||
self.assertEqual(response.status_code, 400)
|
||||
self.assertEqual(len(logger_calls), 1)
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
from django.core.exceptions import SuspiciousOperation
|
||||
|
||||
|
||||
class WizardViewCookieModified(SuspiciousOperation):
|
||||
"""Signature of cookie modified"""
|
||||
pass
|
|
@ -1,8 +1,8 @@
|
|||
import json
|
||||
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.core.signing import BadSignature
|
||||
|
||||
from django.contrib.formtools.exceptions import WizardViewCookieModified
|
||||
from django.contrib.formtools.wizard import storage
|
||||
|
||||
|
||||
|
@ -21,7 +21,7 @@ class CookieStorage(storage.BaseStorage):
|
|||
except KeyError:
|
||||
data = None
|
||||
except BadSignature:
|
||||
raise SuspiciousOperation('WizardView cookie manipulated')
|
||||
raise WizardViewCookieModified('WizardView cookie manipulated')
|
||||
if data is None:
|
||||
return None
|
||||
return json.loads(data, cls=json.JSONDecoder)
|
||||
|
|
|
@ -2,6 +2,8 @@ from __future__ import unicode_literals
|
|||
|
||||
import base64
|
||||
from datetime import datetime, timedelta
|
||||
import logging
|
||||
|
||||
try:
|
||||
from django.utils.six.moves import cPickle as pickle
|
||||
except ImportError:
|
||||
|
@ -14,7 +16,9 @@ from django.utils.crypto import constant_time_compare
|
|||
from django.utils.crypto import get_random_string
|
||||
from django.utils.crypto import salted_hmac
|
||||
from django.utils import timezone
|
||||
from django.utils.encoding import force_bytes
|
||||
from django.utils.encoding import force_bytes, force_text
|
||||
|
||||
from django.contrib.sessions.exceptions import SuspiciousSession
|
||||
|
||||
# session_key should not be case sensitive because some backends can store it
|
||||
# on case insensitive file systems.
|
||||
|
@ -94,12 +98,16 @@ class SessionBase(object):
|
|||
hash, pickled = encoded_data.split(b':', 1)
|
||||
expected_hash = self._hash(pickled)
|
||||
if not constant_time_compare(hash.decode(), expected_hash):
|
||||
raise SuspiciousOperation("Session data corrupted")
|
||||
raise SuspiciousSession("Session data corrupted")
|
||||
else:
|
||||
return pickle.loads(pickled)
|
||||
except Exception:
|
||||
except Exception as e:
|
||||
# ValueError, SuspiciousOperation, unpickling exceptions. If any of
|
||||
# these happen, just return an empty dictionary (an empty session).
|
||||
if isinstance(e, SuspiciousOperation):
|
||||
logger = logging.getLogger('django.security.%s' %
|
||||
e.__class__.__name__)
|
||||
logger.warning(force_text(e))
|
||||
return {}
|
||||
|
||||
def update(self, dict_):
|
||||
|
|
|
@ -2,10 +2,13 @@
|
|||
Cached, database-backed sessions.
|
||||
"""
|
||||
|
||||
import logging
|
||||
|
||||
from django.contrib.sessions.backends.db import SessionStore as DBStore
|
||||
from django.core.cache import cache
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.utils import timezone
|
||||
from django.utils.encoding import force_text
|
||||
|
||||
KEY_PREFIX = "django.contrib.sessions.cached_db"
|
||||
|
||||
|
@ -41,7 +44,11 @@ class SessionStore(DBStore):
|
|||
data = self.decode(s.session_data)
|
||||
cache.set(self.cache_key, data,
|
||||
self.get_expiry_age(expiry=s.expire_date))
|
||||
except (Session.DoesNotExist, SuspiciousOperation):
|
||||
except (Session.DoesNotExist, SuspiciousOperation) as e:
|
||||
if isinstance(e, SuspiciousOperation):
|
||||
logger = logging.getLogger('django.security.%s' %
|
||||
e.__class__.__name__)
|
||||
logger.warning(force_text(e))
|
||||
self.create()
|
||||
data = {}
|
||||
return data
|
||||
|
|
|
@ -1,8 +1,10 @@
|
|||
import logging
|
||||
|
||||
from django.contrib.sessions.backends.base import SessionBase, CreateError
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.db import IntegrityError, transaction, router
|
||||
from django.utils import timezone
|
||||
|
||||
from django.utils.encoding import force_text
|
||||
|
||||
class SessionStore(SessionBase):
|
||||
"""
|
||||
|
@ -18,7 +20,11 @@ class SessionStore(SessionBase):
|
|||
expire_date__gt=timezone.now()
|
||||
)
|
||||
return self.decode(s.session_data)
|
||||
except (Session.DoesNotExist, SuspiciousOperation):
|
||||
except (Session.DoesNotExist, SuspiciousOperation) as e:
|
||||
if isinstance(e, SuspiciousOperation):
|
||||
logger = logging.getLogger('django.security.%s' %
|
||||
e.__class__.__name__)
|
||||
logger.warning(force_text(e))
|
||||
self.create()
|
||||
return {}
|
||||
|
||||
|
|
|
@ -1,5 +1,6 @@
|
|||
import datetime
|
||||
import errno
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
|
@ -8,6 +9,9 @@ from django.conf import settings
|
|||
from django.contrib.sessions.backends.base import SessionBase, CreateError, VALID_KEY_CHARS
|
||||
from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured
|
||||
from django.utils import timezone
|
||||
from django.utils.encoding import force_text
|
||||
|
||||
from django.contrib.sessions.exceptions import InvalidSessionKey
|
||||
|
||||
class SessionStore(SessionBase):
|
||||
"""
|
||||
|
@ -48,7 +52,7 @@ class SessionStore(SessionBase):
|
|||
# should always be md5s, so they should never contain directory
|
||||
# components.
|
||||
if not set(session_key).issubset(set(VALID_KEY_CHARS)):
|
||||
raise SuspiciousOperation(
|
||||
raise InvalidSessionKey(
|
||||
"Invalid characters in session key")
|
||||
|
||||
return os.path.join(self.storage_path, self.file_prefix + session_key)
|
||||
|
@ -75,7 +79,11 @@ class SessionStore(SessionBase):
|
|||
if file_data:
|
||||
try:
|
||||
session_data = self.decode(file_data)
|
||||
except (EOFError, SuspiciousOperation):
|
||||
except (EOFError, SuspiciousOperation) as e:
|
||||
if isinstance(e, SuspiciousOperation):
|
||||
logger = logging.getLogger('django.security.%s' %
|
||||
e.__class__.__name__)
|
||||
logger.warning(force_text(e))
|
||||
self.create()
|
||||
|
||||
# Remove expired sessions.
|
||||
|
|
|
@ -0,0 +1,11 @@
|
|||
from django.core.exceptions import SuspiciousOperation
|
||||
|
||||
|
||||
class InvalidSessionKey(SuspiciousOperation):
|
||||
"""Invalid characters in session key"""
|
||||
pass
|
||||
|
||||
|
||||
class SuspiciousSession(SuspiciousOperation):
|
||||
"""The session may be tampered with"""
|
||||
pass
|
|
@ -1,3 +1,4 @@
|
|||
import base64
|
||||
from datetime import timedelta
|
||||
import os
|
||||
import shutil
|
||||
|
@ -15,14 +16,16 @@ from django.contrib.sessions.models import Session
|
|||
from django.contrib.sessions.middleware import SessionMiddleware
|
||||
from django.core.cache import get_cache
|
||||
from django.core import management
|
||||
from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
|
||||
from django.core.exceptions import ImproperlyConfigured
|
||||
from django.http import HttpResponse
|
||||
from django.test import TestCase, RequestFactory
|
||||
from django.test.utils import override_settings
|
||||
from django.test.utils import override_settings, patch_logger
|
||||
from django.utils import six
|
||||
from django.utils import timezone
|
||||
from django.utils import unittest
|
||||
|
||||
from django.contrib.sessions.exceptions import InvalidSessionKey
|
||||
|
||||
|
||||
class SessionTestsMixin(object):
|
||||
# This does not inherit from TestCase to avoid any tests being run with this
|
||||
|
@ -272,6 +275,15 @@ class SessionTestsMixin(object):
|
|||
encoded = self.session.encode(data)
|
||||
self.assertEqual(self.session.decode(encoded), data)
|
||||
|
||||
def test_decode_failure_logged_to_security(self):
|
||||
bad_encode = base64.b64encode(b'flaskdj:alkdjf')
|
||||
with patch_logger('django.security.SuspiciousSession', 'warning') as calls:
|
||||
self.assertEqual({}, self.session.decode(bad_encode))
|
||||
# check that the failed decode is logged
|
||||
self.assertEqual(len(calls), 1)
|
||||
self.assertTrue('corrupted' in calls[0])
|
||||
|
||||
|
||||
def test_actual_expiry(self):
|
||||
# Regression test for #19200
|
||||
old_session_key = None
|
||||
|
@ -411,12 +423,12 @@ class FileSessionTests(SessionTestsMixin, unittest.TestCase):
|
|||
# This is tested directly on _key_to_file, as load() will swallow
|
||||
# a SuspiciousOperation in the same way as an IOError - by creating
|
||||
# a new session, making it unclear whether the slashes were detected.
|
||||
self.assertRaises(SuspiciousOperation,
|
||||
self.assertRaises(InvalidSessionKey,
|
||||
self.backend()._key_to_file, "a\\b\\c")
|
||||
|
||||
def test_invalid_key_forwardslash(self):
|
||||
# Ensure we don't allow directory-traversal
|
||||
self.assertRaises(SuspiciousOperation,
|
||||
self.assertRaises(InvalidSessionKey,
|
||||
self.backend()._key_to_file, "a/b/c")
|
||||
|
||||
@override_settings(SESSION_ENGINE="django.contrib.sessions.backends.file")
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
"""
|
||||
Global Django exception and warning classes.
|
||||
"""
|
||||
import logging
|
||||
from functools import reduce
|
||||
|
||||
|
||||
|
@ -9,37 +10,56 @@ class DjangoRuntimeWarning(RuntimeWarning):
|
|||
|
||||
|
||||
class ObjectDoesNotExist(Exception):
|
||||
"The requested object does not exist"
|
||||
"""The requested object does not exist"""
|
||||
silent_variable_failure = True
|
||||
|
||||
|
||||
class MultipleObjectsReturned(Exception):
|
||||
"The query returned multiple objects when only one was expected."
|
||||
"""The query returned multiple objects when only one was expected."""
|
||||
pass
|
||||
|
||||
|
||||
class SuspiciousOperation(Exception):
|
||||
"The user did something suspicious"
|
||||
"""The user did something suspicious"""
|
||||
|
||||
|
||||
class SuspiciousMultipartForm(SuspiciousOperation):
|
||||
"""Suspect MIME request in multipart form data"""
|
||||
pass
|
||||
|
||||
|
||||
class SuspiciousFileOperation(SuspiciousOperation):
|
||||
"""A Suspicious filesystem operation was attempted"""
|
||||
pass
|
||||
|
||||
|
||||
class DisallowedHost(SuspiciousOperation):
|
||||
"""HTTP_HOST header contains invalid value"""
|
||||
pass
|
||||
|
||||
|
||||
class DisallowedRedirect(SuspiciousOperation):
|
||||
"""Redirect to scheme not in allowed list"""
|
||||
pass
|
||||
|
||||
|
||||
class PermissionDenied(Exception):
|
||||
"The user did not have permission to do that"
|
||||
"""The user did not have permission to do that"""
|
||||
pass
|
||||
|
||||
|
||||
class ViewDoesNotExist(Exception):
|
||||
"The requested view does not exist"
|
||||
"""The requested view does not exist"""
|
||||
pass
|
||||
|
||||
|
||||
class MiddlewareNotUsed(Exception):
|
||||
"This middleware is not used in this server configuration"
|
||||
"""This middleware is not used in this server configuration"""
|
||||
pass
|
||||
|
||||
|
||||
class ImproperlyConfigured(Exception):
|
||||
"Django is somehow improperly configured"
|
||||
"""Django is somehow improperly configured"""
|
||||
pass
|
||||
|
||||
|
||||
|
|
|
@ -8,7 +8,7 @@ import itertools
|
|||
from datetime import datetime
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.core.exceptions import SuspiciousFileOperation
|
||||
from django.core.files import locks, File
|
||||
from django.core.files.move import file_move_safe
|
||||
from django.utils.encoding import force_text, filepath_to_uri
|
||||
|
@ -260,7 +260,7 @@ class FileSystemStorage(Storage):
|
|||
try:
|
||||
path = safe_join(self.location, name)
|
||||
except ValueError:
|
||||
raise SuspiciousOperation("Attempted access to '%s' denied." % name)
|
||||
raise SuspiciousFileOperation("Attempted access to '%s' denied." % name)
|
||||
return os.path.normpath(path)
|
||||
|
||||
def size(self, name):
|
||||
|
|
|
@ -8,7 +8,7 @@ from django import http
|
|||
from django.conf import settings
|
||||
from django.core import urlresolvers
|
||||
from django.core import signals
|
||||
from django.core.exceptions import MiddlewareNotUsed, PermissionDenied
|
||||
from django.core.exceptions import MiddlewareNotUsed, PermissionDenied, SuspiciousOperation
|
||||
from django.db import connections, transaction
|
||||
from django.utils.encoding import force_text
|
||||
from django.utils.module_loading import import_by_path
|
||||
|
@ -170,11 +170,27 @@ class BaseHandler(object):
|
|||
response = self.handle_uncaught_exception(request,
|
||||
resolver, sys.exc_info())
|
||||
|
||||
except SuspiciousOperation as e:
|
||||
# The request logger receives events for any problematic request
|
||||
# The security logger receives events for all SuspiciousOperations
|
||||
security_logger = logging.getLogger('django.security.%s' %
|
||||
e.__class__.__name__)
|
||||
security_logger.error(force_text(e))
|
||||
|
||||
try:
|
||||
callback, param_dict = resolver.resolve400()
|
||||
response = callback(request, **param_dict)
|
||||
except:
|
||||
signals.got_request_exception.send(
|
||||
sender=self.__class__, request=request)
|
||||
response = self.handle_uncaught_exception(request,
|
||||
resolver, sys.exc_info())
|
||||
|
||||
except SystemExit:
|
||||
# Allow sys.exit() to actually exit. See tickets #1023 and #4701
|
||||
raise
|
||||
|
||||
except: # Handle everything else, including SuspiciousOperation, etc.
|
||||
except: # Handle everything else.
|
||||
# Get the exception info now, in case another exception is thrown later.
|
||||
signals.got_request_exception.send(sender=self.__class__, request=request)
|
||||
response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
|
||||
|
|
|
@ -360,6 +360,9 @@ class RegexURLResolver(LocaleRegexProvider):
|
|||
callback = getattr(urls, 'handler%s' % view_type)
|
||||
return get_callable(callback), {}
|
||||
|
||||
def resolve400(self):
|
||||
return self._resolve_special('400')
|
||||
|
||||
def resolve403(self):
|
||||
return self._resolve_special('403')
|
||||
|
||||
|
|
|
@ -11,7 +11,7 @@ import cgi
|
|||
import sys
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.core.exceptions import SuspiciousMultipartForm
|
||||
from django.utils.datastructures import MultiValueDict
|
||||
from django.utils.encoding import force_text
|
||||
from django.utils import six
|
||||
|
@ -370,7 +370,7 @@ class LazyStream(six.Iterator):
|
|||
if current_number == num_bytes])
|
||||
|
||||
if number_equal > 40:
|
||||
raise SuspiciousOperation(
|
||||
raise SuspiciousMultipartForm(
|
||||
"The multipart parser got stuck, which shouldn't happen with"
|
||||
" normal uploaded files. Check for malicious upload activity;"
|
||||
" if there is none, report this to the Django developers."
|
||||
|
|
|
@ -14,7 +14,7 @@ except ImportError:
|
|||
|
||||
from django.conf import settings
|
||||
from django.core import signing
|
||||
from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured
|
||||
from django.core.exceptions import DisallowedHost, ImproperlyConfigured
|
||||
from django.core.files import uploadhandler
|
||||
from django.http.multipartparser import MultiPartParser
|
||||
from django.utils import six
|
||||
|
@ -72,7 +72,7 @@ class HttpRequest(object):
|
|||
msg = "Invalid HTTP_HOST header: %r." % host
|
||||
if domain:
|
||||
msg += "You may need to add %r to ALLOWED_HOSTS." % domain
|
||||
raise SuspiciousOperation(msg)
|
||||
raise DisallowedHost(msg)
|
||||
|
||||
def get_full_path(self):
|
||||
# RFC 3986 requires query string arguments to be in the ASCII range.
|
||||
|
|
|
@ -12,7 +12,7 @@ except ImportError:
|
|||
from django.conf import settings
|
||||
from django.core import signals
|
||||
from django.core import signing
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.core.exceptions import DisallowedRedirect
|
||||
from django.http.cookie import SimpleCookie
|
||||
from django.utils import six, timezone
|
||||
from django.utils.encoding import force_bytes, iri_to_uri
|
||||
|
@ -452,7 +452,7 @@ class HttpResponseRedirectBase(HttpResponse):
|
|||
def __init__(self, redirect_to, *args, **kwargs):
|
||||
parsed = urlparse(redirect_to)
|
||||
if parsed.scheme and parsed.scheme not in self.allowed_schemes:
|
||||
raise SuspiciousOperation("Unsafe redirect to URL with protocol '%s'" % parsed.scheme)
|
||||
raise DisallowedRedirect("Unsafe redirect to URL with protocol '%s'" % parsed.scheme)
|
||||
super(HttpResponseRedirectBase, self).__init__(*args, **kwargs)
|
||||
self['Location'] = iri_to_uri(redirect_to)
|
||||
|
||||
|
|
|
@ -1,3 +1,5 @@
|
|||
from contextlib import contextmanager
|
||||
import logging
|
||||
import re
|
||||
import sys
|
||||
import warnings
|
||||
|
@ -401,3 +403,21 @@ class IgnoreDeprecationWarningsMixin(object):
|
|||
class IgnorePendingDeprecationWarningsMixin(IgnoreDeprecationWarningsMixin):
|
||||
|
||||
warning_class = PendingDeprecationWarning
|
||||
|
||||
|
||||
@contextmanager
|
||||
def patch_logger(logger_name, log_level):
|
||||
"""
|
||||
Context manager that takes a named logger and the logging level
|
||||
and provides a simple mock-like list of messages received
|
||||
"""
|
||||
calls = []
|
||||
def replacement(msg):
|
||||
calls.append(msg)
|
||||
logger = logging.getLogger(logger_name)
|
||||
orig = getattr(logger, log_level)
|
||||
setattr(logger, log_level, replacement)
|
||||
try:
|
||||
yield calls
|
||||
finally:
|
||||
setattr(logger, log_level, orig)
|
||||
|
|
|
@ -63,6 +63,11 @@ DEFAULT_LOGGING = {
|
|||
'level': 'ERROR',
|
||||
'propagate': False,
|
||||
},
|
||||
'django.security': {
|
||||
'handlers': ['mail_admins'],
|
||||
'level': 'ERROR',
|
||||
'propagate': False,
|
||||
},
|
||||
'py.warnings': {
|
||||
'handlers': ['console'],
|
||||
},
|
||||
|
|
|
@ -43,6 +43,21 @@ def server_error(request, template_name='500.html'):
|
|||
return http.HttpResponseServerError(template.render(Context({})))
|
||||
|
||||
|
||||
@requires_csrf_token
|
||||
def bad_request(request, template_name='400.html'):
|
||||
"""
|
||||
400 error handler.
|
||||
|
||||
Templates: :template:`400.html`
|
||||
Context: None
|
||||
"""
|
||||
try:
|
||||
template = loader.get_template(template_name)
|
||||
except TemplateDoesNotExist:
|
||||
return http.HttpResponseBadRequest('<h1>Bad Request (400)</h1>')
|
||||
return http.HttpResponseBadRequest(template.render(Context({})))
|
||||
|
||||
|
||||
# This can be called when CsrfViewMiddleware.process_view has not run,
|
||||
# therefore need @requires_csrf_token in case the template needs
|
||||
# {% csrf_token %}.
|
||||
|
|
|
@ -44,9 +44,24 @@ SuspiciousOperation
|
|||
-------------------
|
||||
.. exception:: SuspiciousOperation
|
||||
|
||||
The :exc:`SuspiciousOperation` exception is raised when a user has performed
|
||||
an operation that should be considered suspicious from a security perspective,
|
||||
such as tampering with a session cookie.
|
||||
The :exc:`SuspiciousOperation` exception is raised when a user has
|
||||
performed an operation that should be considered suspicious from a security
|
||||
perspective, such as tampering with a session cookie. Subclasses of
|
||||
SuspiciousOperation include:
|
||||
|
||||
* DisallowedHost
|
||||
* DisallowedModelAdminLookup
|
||||
* DisallowedRedirect
|
||||
* InvalidSessionKey
|
||||
* SuspiciousFileOperation
|
||||
* SuspiciousMultipartForm
|
||||
* SuspiciousSession
|
||||
* WizardViewCookieModified
|
||||
|
||||
If a ``SuspiciousOperation`` exception reaches the WSGI handler level it is
|
||||
logged at the ``Error`` level and results in
|
||||
a :class:`~django.http.HttpResponseBadRequest`. See the :doc:`logging
|
||||
documentation </topics/logging/>` for more information.
|
||||
|
||||
PermissionDenied
|
||||
----------------
|
||||
|
|
|
@ -270,6 +270,13 @@ Minor features
|
|||
stores active language in session if it is not present there. This
|
||||
prevents loss of language settings after session flush, e.g. logout.
|
||||
|
||||
* :exc:`~django.core.exceptions.SuspiciousOperation` has been differentiated
|
||||
into a number of subclasses, and each will log to a matching named logger
|
||||
under the ``django.security`` logging hierarchy. Along with this change,
|
||||
a ``handler400`` mechanism and default view are used whenever
|
||||
a ``SuspiciousOperation`` reaches the WSGI handler to return an
|
||||
``HttpResponseBadRequest``.
|
||||
|
||||
Backwards incompatible changes in 1.6
|
||||
=====================================
|
||||
|
||||
|
|
|
@ -231,3 +231,25 @@ same way you can for the 404 and 500 views by specifying a ``handler403`` in
|
|||
your URLconf::
|
||||
|
||||
handler403 = 'mysite.views.my_custom_permission_denied_view'
|
||||
|
||||
.. _http_bad_request_view:
|
||||
|
||||
The 400 (bad request) view
|
||||
--------------------------
|
||||
|
||||
When a :exc:`~django.core.exceptions.SuspiciousOperation` is raised in Django,
|
||||
the it may be handled by a component of Django (for example resetting the
|
||||
session data). If not specifically handled, Django will consider the current
|
||||
request a 'bad request' instead of a server error.
|
||||
|
||||
The view ``django.views.defaults.bad_request``, is otherwise very similar to
|
||||
the ``server_error`` view, but returns with the status code 400 indicating that
|
||||
the error condition was the result of a client operation.
|
||||
|
||||
Like the ``server_error`` view, the default ``bad_request`` should suffice for
|
||||
99% of Web applications, but if you want to override the view, you can specify
|
||||
``handler400`` in your URLconf, like so::
|
||||
|
||||
handler400 = 'mysite.views.my_custom_bad_request_view'
|
||||
|
||||
``bad_request`` views are also only used when :setting:`DEBUG` is ``False``.
|
||||
|
|
|
@ -394,7 +394,7 @@ requirements of logging in Web server environment.
|
|||
Loggers
|
||||
-------
|
||||
|
||||
Django provides three built-in loggers.
|
||||
Django provides four built-in loggers.
|
||||
|
||||
``django``
|
||||
~~~~~~~~~~
|
||||
|
@ -434,6 +434,35 @@ For performance reasons, SQL logging is only enabled when
|
|||
``settings.DEBUG`` is set to ``True``, regardless of the logging
|
||||
level or handlers that are installed.
|
||||
|
||||
``django.security.*``
|
||||
~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
The security loggers will receive messages on any occurrence of
|
||||
:exc:`~django.core.exceptions.SuspiciousOperation`. There is a sub-logger for
|
||||
each sub-type of SuspiciousOperation. The level of the log event depends on
|
||||
where the exception is handled. Most occurrences are logged as a warning, while
|
||||
any ``SuspiciousOperation`` that reaches the WSGI handler will be logged as an
|
||||
error. For example, when an HTTP ``Host`` header is included in a request from
|
||||
a client that does not match :setting:`ALLOWED_HOSTS`, Django will return a 400
|
||||
response, and an error message will be logged to the
|
||||
``django.security.DisallowedHost`` logger.
|
||||
|
||||
Only the parent ``django.security`` logger is configured by default, and all
|
||||
child loggers will propagate to the parent logger. The ``django.security``
|
||||
logger is configured the same as the ``django.request`` logger, and any error
|
||||
events will be mailed to admins. Requests resulting in a 400 response due to
|
||||
a ``SuspiciousOperation`` will not be logged to the ``django.request`` logger,
|
||||
but only to the ``django.security`` logger.
|
||||
|
||||
To silence a particular type of SuspiciousOperation, you can override that
|
||||
specific logger following this example::
|
||||
|
||||
'loggers': {
|
||||
'django.security.DisallowedHost': {
|
||||
'handlers': ['null'],
|
||||
'propagate': False,
|
||||
},
|
||||
|
||||
Handlers
|
||||
--------
|
||||
|
||||
|
|
|
@ -11,7 +11,6 @@ except ImportError: # Python 2
|
|||
|
||||
from django.conf import settings, global_settings
|
||||
from django.core import mail
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.core.files import temp as tempfile
|
||||
from django.core.urlresolvers import reverse
|
||||
# Register auth models with the admin.
|
||||
|
@ -30,6 +29,7 @@ from django.db import connection
|
|||
from django.forms.util import ErrorList
|
||||
from django.template.response import TemplateResponse
|
||||
from django.test import TestCase
|
||||
from django.test.utils import patch_logger
|
||||
from django.utils import formats, translation, unittest
|
||||
from django.utils.cache import get_max_age
|
||||
from django.utils.encoding import iri_to_uri, force_bytes
|
||||
|
@ -543,20 +543,21 @@ class AdminViewBasicTest(TestCase):
|
|||
self.assertContains(response, '%Y-%m-%d %H:%M:%S')
|
||||
|
||||
def test_disallowed_filtering(self):
|
||||
self.assertRaises(SuspiciousOperation,
|
||||
self.client.get, "/test_admin/admin/admin_views/album/?owner__email__startswith=fuzzy"
|
||||
)
|
||||
with patch_logger('django.security.DisallowedModelAdminLookup', 'error') as calls:
|
||||
response = self.client.get("/test_admin/admin/admin_views/album/?owner__email__startswith=fuzzy")
|
||||
self.assertEqual(response.status_code, 400)
|
||||
self.assertEqual(len(calls), 1)
|
||||
|
||||
try:
|
||||
self.client.get("/test_admin/admin/admin_views/thing/?color__value__startswith=red")
|
||||
self.client.get("/test_admin/admin/admin_views/thing/?color__value=red")
|
||||
except SuspiciousOperation:
|
||||
self.fail("Filters are allowed if explicitly included in list_filter")
|
||||
# Filters are allowed if explicitly included in list_filter
|
||||
response = self.client.get("/test_admin/admin/admin_views/thing/?color__value__startswith=red")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
response = self.client.get("/test_admin/admin/admin_views/thing/?color__value=red")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
try:
|
||||
self.client.get("/test_admin/admin/admin_views/person/?age__gt=30")
|
||||
except SuspiciousOperation:
|
||||
self.fail("Filters should be allowed if they involve a local field without the need to whitelist them in list_filter or date_hierarchy.")
|
||||
# Filters should be allowed if they involve a local field without the
|
||||
# need to whitelist them in list_filter or date_hierarchy.
|
||||
response = self.client.get("/test_admin/admin/admin_views/person/?age__gt=30")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
e1 = Employee.objects.create(name='Anonymous', gender=1, age=22, alive=True, code='123')
|
||||
e2 = Employee.objects.create(name='Visitor', gender=2, age=19, alive=True, code='124')
|
||||
|
@ -574,10 +575,9 @@ class AdminViewBasicTest(TestCase):
|
|||
ForeignKey 'limit_choices_to' should be allowed, otherwise raw_id_fields
|
||||
can break.
|
||||
"""
|
||||
try:
|
||||
self.client.get("/test_admin/admin/admin_views/inquisition/?leader__name=Palin&leader__age=27")
|
||||
except SuspiciousOperation:
|
||||
self.fail("Filters should be allowed if they are defined on a ForeignKey pointing to this model")
|
||||
# Filters should be allowed if they are defined on a ForeignKey pointing to this model
|
||||
response = self.client.get("/test_admin/admin/admin_views/inquisition/?leader__name=Palin&leader__age=27")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
def test_hide_change_password(self):
|
||||
"""
|
||||
|
|
|
@ -61,6 +61,7 @@ class TransactionsPerRequestTests(TransactionTestCase):
|
|||
connection.settings_dict['ATOMIC_REQUESTS'] = old_atomic_requests
|
||||
self.assertContains(response, 'False')
|
||||
|
||||
|
||||
class SignalsTests(TestCase):
|
||||
urls = 'handlers.urls'
|
||||
|
||||
|
@ -89,3 +90,11 @@ class SignalsTests(TestCase):
|
|||
self.assertEqual(self.signals, ['started'])
|
||||
self.assertEqual(b''.join(response.streaming_content), b"streaming content")
|
||||
self.assertEqual(self.signals, ['started', 'finished'])
|
||||
|
||||
|
||||
class HandlerSuspiciousOpsTest(TestCase):
|
||||
urls = 'handlers.urls'
|
||||
|
||||
def test_suspiciousop_in_view_returns_400(self):
|
||||
response = self.client.get('/suspicious/')
|
||||
self.assertEqual(response.status_code, 400)
|
||||
|
|
|
@ -9,4 +9,5 @@ urlpatterns = patterns('',
|
|||
url(r'^streaming/$', views.streaming),
|
||||
url(r'^in_transaction/$', views.in_transaction),
|
||||
url(r'^not_in_transaction/$', views.not_in_transaction),
|
||||
url(r'^suspicious/$', views.suspicious),
|
||||
)
|
||||
|
|
|
@ -1,5 +1,6 @@
|
|||
from __future__ import unicode_literals
|
||||
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.db import connection, transaction
|
||||
from django.http import HttpResponse, StreamingHttpResponse
|
||||
|
||||
|
@ -15,3 +16,6 @@ def in_transaction(request):
|
|||
@transaction.non_atomic_requests
|
||||
def not_in_transaction(request):
|
||||
return HttpResponse(str(connection.in_atomic_block))
|
||||
|
||||
def suspicious(request):
|
||||
raise SuspiciousOperation('dubious')
|
||||
|
|
|
@ -8,9 +8,10 @@ import warnings
|
|||
from django.conf import LazySettings
|
||||
from django.core import mail
|
||||
from django.test import TestCase, RequestFactory
|
||||
from django.test.utils import override_settings
|
||||
from django.test.utils import override_settings, patch_logger
|
||||
from django.utils.encoding import force_text
|
||||
from django.utils.log import CallbackFilter, RequireDebugFalse, RequireDebugTrue
|
||||
from django.utils.log import (CallbackFilter, RequireDebugFalse,
|
||||
RequireDebugTrue)
|
||||
from django.utils.six import StringIO
|
||||
from django.utils.unittest import skipUnless
|
||||
|
||||
|
@ -354,3 +355,22 @@ class SettingsConfigureLogging(TestCase):
|
|||
settings.configure(
|
||||
LOGGING_CONFIG='logging_tests.tests.dictConfig')
|
||||
self.assertTrue(dictConfig.called)
|
||||
|
||||
|
||||
class SecurityLoggerTest(TestCase):
|
||||
|
||||
urls = 'logging_tests.urls'
|
||||
|
||||
def test_suspicious_operation_creates_log_message(self):
|
||||
with self.settings(DEBUG=True):
|
||||
with patch_logger('django.security.SuspiciousOperation', 'error') as calls:
|
||||
response = self.client.get('/suspicious/')
|
||||
self.assertEqual(len(calls), 1)
|
||||
self.assertEqual(calls[0], 'dubious')
|
||||
|
||||
def test_suspicious_operation_uses_sublogger(self):
|
||||
with self.settings(DEBUG=True):
|
||||
with patch_logger('django.security.DisallowedHost', 'error') as calls:
|
||||
response = self.client.get('/suspicious_spec/')
|
||||
self.assertEqual(len(calls), 1)
|
||||
self.assertEqual(calls[0], 'dubious')
|
||||
|
|
|
@ -0,0 +1,10 @@
|
|||
from __future__ import unicode_literals
|
||||
|
||||
from django.conf.urls import patterns, url
|
||||
|
||||
from . import views
|
||||
|
||||
urlpatterns = patterns('',
|
||||
url(r'^suspicious/$', views.suspicious),
|
||||
url(r'^suspicious_spec/$', views.suspicious_spec),
|
||||
)
|
|
@ -0,0 +1,11 @@
|
|||
from __future__ import unicode_literals
|
||||
|
||||
from django.core.exceptions import SuspiciousOperation, DisallowedHost
|
||||
|
||||
|
||||
def suspicious(request):
|
||||
raise SuspiciousOperation('dubious')
|
||||
|
||||
|
||||
def suspicious_spec(request):
|
||||
raise DisallowedHost('dubious')
|
|
@ -7,7 +7,6 @@ from __future__ import unicode_literals
|
|||
import os
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.core.urlresolvers import reverse
|
||||
from django.template import (TemplateDoesNotExist, TemplateSyntaxError,
|
||||
Context, Template, loader)
|
||||
|
@ -20,6 +19,7 @@ from django.utils._os import upath
|
|||
from django.utils.translation import ugettext_lazy
|
||||
from django.http import HttpResponse
|
||||
|
||||
from .views import CustomTestException
|
||||
|
||||
@override_settings(
|
||||
TEMPLATE_DIRS=(os.path.join(os.path.dirname(upath(__file__)), 'templates'),)
|
||||
|
@ -619,7 +619,7 @@ class ExceptionTests(TestCase):
|
|||
try:
|
||||
response = self.client.get("/test_client_regress/staff_only/")
|
||||
self.fail("General users should not be able to visit this page")
|
||||
except SuspiciousOperation:
|
||||
except CustomTestException:
|
||||
pass
|
||||
|
||||
# At this point, an exception has been raised, and should be cleared.
|
||||
|
@ -629,7 +629,7 @@ class ExceptionTests(TestCase):
|
|||
self.assertTrue(login, 'Could not log in')
|
||||
try:
|
||||
self.client.get("/test_client_regress/staff_only/")
|
||||
except SuspiciousOperation:
|
||||
except CustomTestException:
|
||||
self.fail("Staff should be able to visit this page")
|
||||
|
||||
|
||||
|
|
|
@ -3,12 +3,15 @@ import json
|
|||
from django.conf import settings
|
||||
from django.contrib.auth.decorators import login_required
|
||||
from django.http import HttpResponse, HttpResponseRedirect
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.shortcuts import render_to_response
|
||||
from django.core.serializers.json import DjangoJSONEncoder
|
||||
from django.test.client import CONTENT_TYPE_RE
|
||||
from django.template import RequestContext
|
||||
|
||||
|
||||
class CustomTestException(Exception):
|
||||
pass
|
||||
|
||||
def no_template_view(request):
|
||||
"A simple view that expects a GET request, and returns a rendered template"
|
||||
return HttpResponse("No template used. Sample content: twice once twice. Content ends.")
|
||||
|
@ -18,7 +21,7 @@ def staff_only_view(request):
|
|||
if request.user.is_staff:
|
||||
return HttpResponse('')
|
||||
else:
|
||||
raise SuspiciousOperation()
|
||||
raise CustomTestException()
|
||||
|
||||
def get_view(request):
|
||||
"A simple login protected view"
|
||||
|
|
|
@ -516,7 +516,7 @@ class RequestURLconfTests(TestCase):
|
|||
b''.join(self.client.get('/second_test/'))
|
||||
|
||||
class ErrorHandlerResolutionTests(TestCase):
|
||||
"""Tests for handler404 and handler500"""
|
||||
"""Tests for handler400, handler404 and handler500"""
|
||||
|
||||
def setUp(self):
|
||||
from django.core.urlresolvers import RegexURLResolver
|
||||
|
@ -528,12 +528,14 @@ class ErrorHandlerResolutionTests(TestCase):
|
|||
def test_named_handlers(self):
|
||||
from .views import empty_view
|
||||
handler = (empty_view, {})
|
||||
self.assertEqual(self.resolver.resolve400(), handler)
|
||||
self.assertEqual(self.resolver.resolve404(), handler)
|
||||
self.assertEqual(self.resolver.resolve500(), handler)
|
||||
|
||||
def test_callable_handers(self):
|
||||
from .views import empty_view
|
||||
handler = (empty_view, {})
|
||||
self.assertEqual(self.callable_resolver.resolve400(), handler)
|
||||
self.assertEqual(self.callable_resolver.resolve404(), handler)
|
||||
self.assertEqual(self.callable_resolver.resolve500(), handler)
|
||||
|
||||
|
|
|
@ -4,5 +4,6 @@ from django.conf.urls import patterns
|
|||
|
||||
urlpatterns = patterns('')
|
||||
|
||||
handler400 = 'urlpatterns_reverse.views.empty_view'
|
||||
handler404 = 'urlpatterns_reverse.views.empty_view'
|
||||
handler500 = 'urlpatterns_reverse.views.empty_view'
|
||||
|
|
|
@ -9,5 +9,6 @@ from .views import empty_view
|
|||
|
||||
urlpatterns = patterns('')
|
||||
|
||||
handler400 = empty_view
|
||||
handler404 = empty_view
|
||||
handler500 = empty_view
|
||||
|
|
Loading…
Reference in New Issue