mirror of
				https://github.com/django/django.git
				synced 2025-10-26 07:06:08 +00:00 
			
		
		
		
	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:
		| @@ -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' | ||||
|   | ||||
							
								
								
									
										6
									
								
								django/contrib/admin/exceptions.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										6
									
								
								django/contrib/admin/exceptions.py
									
									
									
									
									
										Normal file
									
								
							| @@ -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/', | ||||
|                 {'email': 'staffmember@example.com'}, | ||||
|                 HTTP_HOST='www.example:dr.frankenstein@evil.tld' | ||||
|             ) | ||||
|         self.assertEqual(len(mail.outbox), 0) | ||||
|         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/', | ||||
|                 {'email': 'staffmember@example.com'}, | ||||
|                 HTTP_HOST='www.example:dr.frankenstein@evil.tld' | ||||
|             ) | ||||
|         self.assertEqual(len(mail.outbox), 0) | ||||
|         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) | ||||
|   | ||||
							
								
								
									
										6
									
								
								django/contrib/formtools/exceptions.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										6
									
								
								django/contrib/formtools/exceptions.py
									
									
									
									
									
										Normal file
									
								
							| @@ -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. | ||||
|   | ||||
							
								
								
									
										11
									
								
								django/contrib/sessions/exceptions.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										11
									
								
								django/contrib/sessions/exceptions.py
									
									
									
									
									
										Normal file
									
								
							| @@ -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') | ||||
|   | ||||
							
								
								
									
										10
									
								
								tests/logging_tests/urls.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										10
									
								
								tests/logging_tests/urls.py
									
									
									
									
									
										Normal file
									
								
							| @@ -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), | ||||
| ) | ||||
							
								
								
									
										11
									
								
								tests/logging_tests/views.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										11
									
								
								tests/logging_tests/views.py
									
									
									
									
									
										Normal file
									
								
							| @@ -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 | ||||
|   | ||||
		Reference in New Issue
	
	Block a user