Permalink
Browse files

Initial origin checking implementation

Use origin header to check reject illegitimate requests. Cookie
domain checks need improvement.

Signed-off-by: Rohan Jain <crodjer@gmail.com>
  • Loading branch information...
crodjer committed May 27, 2012
1 parent c3b7905 commit 13ba1623cfa99d573c5d1d690887a89566f0f237
Showing with 40 additions and 1 deletion.
  1. +20 −1 django/middleware/csrf.py
  2. +20 −0 tests/regressiontests/csrf_tests/tests.py
View
@@ -20,6 +20,7 @@
REASON_BAD_REFERER = "Referer checking failed - %s does not match %s."
REASON_NO_CSRF_COOKIE = "CSRF cookie not set."
REASON_BAD_TOKEN = "CSRF token missing or incorrect."
+REASON_BAD_ORIGIN = "Origin checking failed - %s does not match %s."
CSRF_KEY_LENGTH = 32
@@ -104,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
@@ -112,6 +114,23 @@ def process_view(self, request, callback, callback_args, callback_kwargs):
# branches that call reject().
return self._accept(request)
+ host = request.META.get('HTTP_HOST', '')
+ origin = request.META.get('HTTP_ORIGIN')
+ cookie_domain = getattr(settings, 'CSRF_COOKIE_DOMAIN') or host
+
+ if origin is not None:
+ # TODO: Fix this check to be like browser cookie check
+ if not origin.endswith(cookie_domain):

This comment has been minimized.

Show comment Hide comment
@PaulMcMillan

PaulMcMillan Jun 5, 2012

This would allow bar.com to attack foobar.com, if it came down to the origin header being the deciding factor.

@PaulMcMillan

PaulMcMillan Jun 5, 2012

This would allow bar.com to attack foobar.com, if it came down to the origin header being the deciding factor.

This comment has been minimized.

Show comment Hide comment
@crodjer

crodjer Jun 5, 2012

Owner

I improved the behaviour of origin checking in a later commit 85f7037. That should take care of these cases.

@crodjer

crodjer Jun 5, 2012

Owner

I improved the behaviour of origin checking in a later commit 85f7037. That should take care of these cases.

+ reason = REASON_BAD_ORIGIN % (origin, cookie_domain)
+ logger.warning('Forbidden (%s): %s',
+ reason, request.path,
+ extra={
+ 'status_code': 403,
+ 'request': request,
+ }
+ )
+ return self._reject(request, reason)
+
if request.is_secure():
# Suppose user visits http://example.com/
# An active network attacker (man-in-the-middle, MITM) sends a
@@ -140,7 +159,7 @@ def process_view(self, request, callback, callback_args, callback_kwargs):
return self._reject(request, REASON_NO_REFERER)
# Note that request.get_host() includes the port.
- good_referer = 'https://%s/' % request.get_host()
+ good_referer = 'https://%s/' % host
if not same_origin(referer, good_referer):
reason = REASON_BAD_REFERER % (referer, good_referer)
logger.warning('Forbidden (%s): %s', reason, request.path,
@@ -333,3 +333,23 @@ 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',''))
+
+ def test_good_origin_header(self):
+ """
+ Test if a good origin header is accepted.
+ """
+ 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.
+ """
+ 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)

0 comments on commit 13ba162

Please sign in to comment.