Opened 10 years ago

Last modified 9 years ago

#24496 closed New feature

Check CSRF Referer against CSRF_TRUSTED_ORIGINS — at Version 14

Reported by: Matt Robenolt Owned by: Joshua Kehn
Component: CSRF Version: dev
Severity: Normal Keywords: csrf 1.9
Cc: django@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Joshua Kehn)

Right now, if you try to share a CSRF token across a subdomain without https, everything works great since the Referer header isn't validated.

But over https, we want to be a bit more strict and make sure that the Referer is from another secure site, and also that the Referer matches where we think it should be coming from. Django should validate that the Referer header matches one of the domains listed in CSRF_TRUSTED_ORIGINS, including the currently responding ALLOWED_HOST.

Change History (14)

comment:1 by Tim Graham, 10 years ago

Could you provide a reference for "we want to be a bit more strict ..."? For example, is this part of an RFC or something that other frameworks implement?

comment:2 by Matt Robenolt, 10 years ago

I think the comment in this file explains it best:

# 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.

https://github.com/django/django/blob/668d53cd125175eb708cc0af143f47b42cd42153/django/middleware/csrf.py#L135-L149

comment:3 by Carl Meyer, 10 years ago

Tim, the "we want to be more strict" is something Django already implements (checking REFERER against the request host on an HTTPS request). Matt's feature request is for that check to be against CSRF_COOKIE_DOMAIN rather than the request host, to allow for cross-subdomain requests. I think this is reasonable, and probably the way it should have worked from the beginning.

comment:4 by Matt Robenolt, 10 years ago

I've opened a discussion on the mailing list as suggested by timgraham since this is related to security: https://groups.google.com/forum/#!topic/django-developers/tEEw02RhV0M

comment:5 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

comment:6 by Chris Streeter, 10 years ago

Cc: django@… added

comment:7 by Aymeric Augustin, 10 years ago

Looks reasonable to me.

comment:8 by Tim Graham, 10 years ago

Patch needs improvement: set

Marking as "patch needs improvement" per Luke's comments.

comment:9 by Tim Graham, 10 years ago

Patch needs improvement: unset

comment:10 by Tim Graham, 10 years ago

Patch needs improvement: set

comment:11 by Tim Graham, 10 years ago

Patch needs improvement: unset

comment:12 by Tim Graham, 9 years ago

Needs documentation: set

comment:13 by Seth Gottlieb, 9 years ago

Submitted a pull request to mattrobenolt that added some documentation for his fix.

If we decide to go with Troy Grosfield's suggestion of adding a CSRF_WHITELIST_ORIGINS setting (which I like), I can document that instead.

comment:14 by Joshua Kehn, 9 years ago

Description: modified (diff)
Needs documentation: unset
Owner: changed from nobody to Joshua Kehn
Status: newassigned
Summary: Check CSRF Referer against CSRF_COOKIE_DOMAINCheck CSRF Referer against CSRF_TRUSTED_ORIGINS

Based on the mailing list discussion here https://groups.google.com/forum/#!topic/django-developers/eQeaNzSlSbw I'm updating the ticket description and including a PR with tests and updated documentation that allows expanding the allowed HTTP Referer domains via a setting called CSRF_TRUSTED_ORIGINS.

PR is open: https://github.com/django/django/pull/5218

A patch has been added to this ticket as well.

Last edited 9 years ago by Joshua Kehn (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top