Csrf Enhancements #95

Closed
wants to merge 11 commits into
from
View
1 AUTHORS
@@ -566,6 +566,7 @@ answer newbie questions, and generally made Django that much better:
Gasper Zejn <zejn@kiberpipa.org>
Jarek Zgoda <jarek.zgoda@gmail.com>
Cheng Zhang
+ Rohan Jain <crodjer@gmail.com>
A big THANK YOU goes to:
View
69 django/middleware/csrf.py
@@ -5,23 +5,22 @@
against request forgeries from other sites.
"""
-import hashlib
import re
-import random
from django.conf import settings
from django.core.urlresolvers import get_callable
from django.utils.cache import patch_vary_headers
-from django.utils.http import same_origin
+from django.utils.http import domain_permitted
from django.utils.log import getLogger
from django.utils.crypto import constant_time_compare, get_random_string
logger = getLogger('django.request')
REASON_NO_REFERER = "Referer checking failed - no Referer."
-REASON_BAD_REFERER = "Referer checking failed - %s does not match %s."
+REASON_BAD_REFERER = "Referer checking failed - %s is not permitted."
REASON_NO_CSRF_COOKIE = "CSRF cookie not set."
REASON_BAD_TOKEN = "CSRF token missing or incorrect."
+REASON_BAD_ORIGIN = "Origin checking failed - %s is not permitted."
CSRF_KEY_LENGTH = 32
@@ -106,6 +105,7 @@ def process_view(self, request, callback, callback_args, callback_kwargs):
# Assume that anything not defined as 'safe' by RC2616 needs protection
if request.method not in ('GET', 'HEAD', 'OPTIONS', 'TRACE'):
+
if getattr(request, '_dont_enforce_csrf_checks', False):
# Mechanism to turn off CSRF checks for test suite.
# It comes after the creation of CSRF cookies, so that
@@ -114,22 +114,37 @@ def process_view(self, request, callback, callback_args, callback_kwargs):
# branches that call reject().
return self._accept(request)
- if request.is_secure():
- # Suppose user visits http://example.com/
- # An active network attacker (man-in-the-middle, MITM) sends a
- # POST form that targets https://example.com/detonate-bomb/ and
- # submits it via JavaScript.
- #
- # The attacker will need to provide a CSRF cookie and token, but
- # that's no problem for a MITM and the session-independent
- # nonce we're using. So the MITM can circumvent the CSRF
- # protection. This is true for any HTTP connection, but anyone
- # using HTTPS expects better! For this reason, for
- # https://example.com/ we need additional protection that treats
- # http://example.com/ as completely untrusted. Under HTTPS,
- # Barth et al. found that the Referer header is missing for
- # same-domain requests in only about 0.2% of cases or less, so
- # we can use strict Referer checking.
+ host = request.META.get('HTTP_HOST', '')
+ # Note that host includes the port, so we split that out.
+ # If the host has port specified (checked with ':') then, grab the
+ # host domain from the string before the last ':'.
+ host = host.split(':')[-2] if ':' in host else host
+
+ origin = request.META.get('HTTP_ORIGIN')
+ permitted_domains = getattr(settings, 'CSRF_PERMITTED_DOMAINS', [host])
+
+ # If origin header exists, use it to check for csrf attacks.
+ # Origin header is being compared to None here because we need to
+ # reject requests with origin header as '' too, which otherwise is
+ # treated as null.
+ if origin is not None:
+ if not domain_permitted(origin, permitted_domains):
+ reason = REASON_BAD_ORIGIN % (origin)
+ logger.warning('Forbidden (%s): %s',
+ reason, request.path,
+ extra={
+ 'status_code': 403,
+ 'request': request,
+ }
+ )
+
+ return self._reject(request, reason)
+ else:
+ # Do a strict referer check in case an origin check succeds.
+ # As far as CSRF is concerned, attackers who are in a position
+ # to perform CSRF attack are not in a position to fake referer
+ # headers.
+
@spookylukey
spookylukey Oct 13, 2012

This is doing a strict referer check if there is no Origin header. That's going to cause lots of incorrect failures for the case where the browser is configured not to send a Referer header, or where the network strips the header. According to Barth et al. [1], not sending the Referer header is very rare for same-domain HTTPS (which is why we had it like that before), but not that rare for HTTP - probably because the network cannot strip the header for HTTPS, but can for HTTP. This could be anywhere between 0.5% - 7%.

[1] http://seclab.stanford.edu/websec/csrf/csrf.pdf

@crodjer
crodjer Oct 14, 2012
referer = request.META.get('HTTP_REFERER')
if referer is None:
logger.warning('Forbidden (%s): %s',
@@ -139,12 +154,13 @@ def process_view(self, request, callback, callback_args, callback_kwargs):
'request': request,
}
)
+
return self._reject(request, REASON_NO_REFERER)
- # Note that request.get_host() includes the port.
- good_referer = 'https://%s/' % request.get_host()
- if not same_origin(referer, good_referer):
- reason = REASON_BAD_REFERER % (referer, good_referer)
+ # Make sure that the http referer matches the permitted domains
+ # pattern.
+ if not domain_permitted(referer, permitted_domains):
+ reason = REASON_BAD_REFERER % (referer)
logger.warning('Forbidden (%s): %s', reason, request.path,
extra={
'status_code': 403,
@@ -153,6 +169,10 @@ def process_view(self, request, callback, callback_args, callback_kwargs):
)
return self._reject(request, reason)
+ # Legacy token checking method.
+ # TODO: Handle this with permitted domains. Cookies won't work
+ # there, invalidating the whole point of permitted domains
+ # functionality.
if csrf_token is None:
# No CSRF cookie. For POST requests, we insist on a CSRF cookie,
# and in this way we can avoid all CSRF attacks, including login
@@ -184,6 +204,7 @@ def process_view(self, request, callback, callback_args, callback_kwargs):
'request': request,
}
)
+
return self._reject(request, REASON_BAD_TOKEN)
return self._accept(request)
View
15 django/utils/http.py
@@ -10,6 +10,8 @@
from django.utils.encoding import smart_str, force_unicode
from django.utils.functional import allow_lazy
+from fnmatch import fnmatch
+
ETAG_MATCH = re.compile(r'(?:W/)?"((?:\\.|[^"])*)"')
MONTHS = 'jan feb mar apr may jun jul aug sep oct nov dec'.split()
@@ -213,3 +215,16 @@ def same_origin(url1, url2):
"""
p1, p2 = urlparse.urlparse(url1), urlparse.urlparse(url2)
return (p1.scheme, p1.hostname, p1.port) == (p2.scheme, p2.hostname, p2.port)
+
+def domain_permitted(url, permitted_domains):
+ """
+ Check if the url submitted is from a permitted domain
+ """
+ domain = urlparse.urlparse(url).hostname
+
+ for permitted_domain in permitted_domains:
+ # This uses the unix glob filename pattern matching, documented here:
+ # http://docs.python.org/library/fnmatch.html
+ if fnmatch(domain, permitted_domain):
+ return True
+ return False
View
113 tests/regressiontests/csrf_tests/tests.py
@@ -7,6 +7,9 @@
from django.template import RequestContext, Template
from django.test import TestCase
from django.views.decorators.csrf import csrf_exempt, requires_csrf_token, ensure_csrf_cookie
+from django.test.utils import override_settings
+
+settings.DEBUG = True
# Response/views used for CsrfResponseMiddleware and CsrfViewMiddleware tests
@@ -51,16 +54,21 @@ class CsrfViewMiddlewareTest(TestCase):
_csrf_id = "1"
def _get_GET_no_csrf_cookie_request(self):
+
return TestingHttpRequest()
def _get_GET_csrf_cookie_request(self):
req = TestingHttpRequest()
req.COOKIES[settings.CSRF_COOKIE_NAME] = self._csrf_id_cookie
+ req.META['HTTP_HOST'] = 'www.example.com'
+ req.META['HTTP_REFERER'] = 'http://www.example.com'
+
return req
def _get_POST_csrf_cookie_request(self):
req = self._get_GET_csrf_cookie_request()
req.method = "POST"
+
return req
def _get_POST_no_csrf_cookie_request(self):
@@ -95,6 +103,8 @@ def test_process_response_get_token_used(self):
patched.
"""
req = self._get_GET_no_csrf_cookie_request()
+ req.META['HTTP_HOST'] = 'www.example.com'
+ req.META['HTTP_REFERER'] = 'http://www.exmaple.com'
# Put tests for CSRF_COOKIE_* settings here
with self.settings(CSRF_COOKIE_NAME='myname',
@@ -333,3 +343,106 @@ def view(request):
resp2 = CsrfViewMiddleware().process_response(req, resp)
self.assertTrue(resp2.cookies.get(settings.CSRF_COOKIE_NAME, False))
self.assertTrue('Cookie' in resp2.get('Vary',''))
+
+ @override_settings(CSRF_PERMITTED_DOMAINS=['www.example.com'])
+ def test_good_origin_header(self):
+ """
+ Test if a good origin header is accepted for across subdomain settings.
+ """
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'www.example.com'
+ req.META['HTTP_ORIGIN'] = 'http://www.example.com'
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(None, req2)
+
+ @override_settings(CSRF_PERMITTED_DOMAINS=['example.com'])
+ def test_good_origin_header_3(self):
+ """
+ Test if a good origin header is accepted for a no subdomain.
+ """
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'example.com'
+ req.META['HTTP_ORIGIN'] = 'http://example.com'
+ req.META['HTTP_REFERER'] = 'http://example.com'
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(None, req2)
+
+ def test_good_origin_header_4(self):
+ """
+ Test if a good origin header is accepted for no cookie setting.
+ """
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'www.example.com'
+ req.META['HTTP_ORIGIN'] = 'http://www.example.com'
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(None, req2)
+
+ def test_bad_origin_header(self):
+ """
+ Test if a bad origin header is rejected for different domain.
+ """
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'www.example.com'
+ req.META['HTTP_ORIGIN'] = 'http://www.evil.com'
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(403, req2.status_code)
+
+ @override_settings(CSRF_PERMITTED_DOMAINS=['example.com'])
+ def test_bad_origin_header_2(self):
+ """
+ Test if a bad origin header is rejected for subdomains.
+ """
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'www.example.com'
+ req.META['HTTP_ORIGIN'] = 'http://www.example.com'
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(403, req2.status_code)
+
+ def test_bad_origin_header_3(self):
+ """
+ Test if a bad origin header is rejected with no cookie setting.
+ """
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'www.example.com'
+ req.META['HTTP_ORIGIN'] = 'http://www.evil.com'
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(403, req2.status_code)
+
+ @override_settings(CSRF_PERMITTED_DOMAINS=['crossdomain.com'])
+ def test_permitted_domains_cross(self):
+ '''
+ Test if permitted cross domains requests work
+ '''
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'example.com'
+ req.META['HTTP_ORIGIN'] = 'http://crossdomain.com'
+ req.META['HTTP_REFERER'] = 'http://crossdomain.com'
+
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(None, req2)
+
+ @override_settings(CSRF_PERMITTED_DOMAINS=['example.com', '*.crossdomain.com'])
+ def test_permitted_domains_cross_glob(self):
+ '''
+ Test if permitted cross domains specified in glob foramt work
+ '''
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'example.com'
+ req.META['HTTP_ORIGIN'] = 'http://test.crossdomain.com'
+ req.META['HTTP_REFERER'] = 'http://test.crossdomain.com'
+
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(None, req2)
+
+ @override_settings(CSRF_PERMITTED_DOMAINS=['example.com', 'valid.crossdomain.com'])
+ def test_permitted_domains_cross_invalid(self):
+ '''
+ Test if permitted cross domains invalid check works
+ '''
+ req = self._get_POST_request_with_token()
+ req.META['HTTP_HOST'] = 'example.com'
+ req.META['HTTP_ORIGIN'] = 'http://invalid.crossdomain.com'
+ req.META['HTTP_REFERER'] = 'http://invalid.crossdomain.com'
+
+ req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {})
+ self.assertEqual(403, req2.status_code)