Skip to content

Csrf Enhancements #95

Closed
wants to merge 11 commits into from

3 participants

@crodjer
crodjer commented May 28, 2012

Pull request for GSoC project on Security Enhancements. This doesn't need to be merged anytime soon.

crodjer added some commits May 18, 2012
@crodjer crodjer Add myself to authors
Signed-off-by: Rohan Jain <crodjer@gmail.com>
54e3cf0
@crodjer crodjer Remove unused imports
Signed-off-by: Rohan Jain <crodjer@gmail.com>
c3b7905
@crodjer crodjer 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>
13ba162
@PaulMcMillan

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

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

@spookylukey
Django member

We should have a properly implemented and tested 'same_origin' function, that implements an RFC exactly, not adhoc domain name parsing and dot counting in the flow of another function.

@crodjer
crodjer added some commits May 29, 2012
@crodjer crodjer Emulate browser's cookie domains behaviour
For compatibility with cookie domain setting, origin check emulates
the behaviour of browser cookie-domain validator.

Signed-off-by: Rohan Jain <crodjer@gmail.com>
46a2f7b
@crodjer crodjer Enable referer/origin checks in non https requests
Instead of having differential mechanisms of CSRF checks for
http/https, generalise the referer/origin header for both.

This negates the CSRF_COOKIE_DOMAIN setting, i.e. cross-subdomain
requests are not allowed at all.

Signed-off-by: Rohan Jain <crodjer@gmail.com>
28d35a0
@crodjer crodjer Setting for cross site permitted domains
`PERMITTED_DOMAINS`, is a setting, which can take a list of domain
patterns in unix glob format.
Administrators will explicitly mention which domains are allowed to
make requests to the site.

This method is safer then using cookie domain which is vulnerable to
various MITM attacks.

Signed-off-by: Rohan Jain <crodjer@gmail.com>
8d8bebe
@crodjer crodjer Comments for the generalized referer checking
Signed-off-by: Rohan Jain <crodjer@gmail.com>
5df40b4
@crodjer crodjer Some more tests for permitted domains
Signed-off-by: Rohan Jain <crodjer@gmail.com>
ee52142
@crodjer crodjer Permitted domains settings to include csrf
`s/PERMITTED_DOMAINS/CSRF_PERMITTED_DOMAINS`, to express what this
setting directly affects.

Signed-off-by: Rohan Jain <crodjer@gmail.com>
cd6d781
@crodjer crodjer Use referer checking in absence of origin header
Instead of doing checks for both origin and referer header all the
time, do referer checks only in case of origin header's absence.

Given the purpose of the origin header, it can be relied upon at least
to a level equivalent (or even more) than referer header.

Signed-off-by: Rohan Jain <crodjer@gmail.com>
97733ce
@crodjer crodjer Split out port from host
The host header also gives out port with it, so it should be split out
of it. Since in a url, the string before the last colon (:) is the
host domain, we grab that one as host.

Signed-off-by: Rohan Jain <crodjer@gmail.com>
e49531f
@spookylukey spookylukey commented on the diff Oct 13, 2012
django/middleware/csrf.py
+ 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
Django member
spookylukey added a note 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 added a note Oct 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@spookylukey
Django member

Closing for the reasons described by Rohan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.