Check CSRF Referer against CSRF_COOKIE_DOMAIN

[Matt Robenolt]

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. The canonical source for where we think it should be from is CSRF_COOKIE_DOMAIN.

If we set our CSRF_COOKIE_DOMAIN to .example.com, that means our CSRF validation should work for www.example.com and foo.example.com. Not just strictly the domain the request is coming from.

[Tim Graham]

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?

[Matt Robenolt]

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

[Carl Meyer]

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.

[Matt Robenolt]

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

[Aymeric Augustin]

Looks reasonable to me.

[Tim Graham]

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

[Seth Gottlieb]

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.

[Joshua Kehn]

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.

[Carl Meyer]

The new patch looks good, thanks Josh! After giving it some more thought overnight, I think we should actually combine the two proposed patches for this ticket. CSRF_TRUSTED_ORIGINS is broader in scope for true cross-origin requests from different domains, but using CSRF_COOKIE_DOMAIN as the default (if set) is just sensible, and easier if you have quite a few subdomains that should all be trusted (since we don't have any sort of wildcarding in CSRF_TRUSTED_ORIGINS).

[Joshua Kehn]

Updated PR based on comments, I'll update the attached patch as well after this comment.

Regarding including the CSRF_COOKIE_DOMAIN, wouldn't that be required anyways for the request to pass ALLOWED_HOSTS? I'm thinking that should be checked after the CSRF_TRUSTED_ORIGINS check fails, correct? I'm attaching two flows.

[Joshua Kehn]

Existing flow (with proposed patch): ​http://l.kehn.io/image/0z2X3j2k082c

CSRF_COOKIE_DOMAIN flow: ​http://l.kehn.io/image/3p3J142b3j3H

[Joshua Kehn]

New CSRF_COOKIE_DOMAIN flow

[Joshua Kehn]

Existing (with patch) flow

[Carl Meyer]

I think the logic should look more like "construct a list of valid referers, where that list consists of X plus CSRF_TRUSTED_ORIGINS, where X is CSRF_COOKIE_DOMAIN if set and otherwise request.get_host()".

In other words, it's basically the same logic as in your current PR, except that if CSRF_COOKIE_DOMAIN is set, it replaces request.get_host() entirely. But you'll also need the other changes in Matt's pull request in order to get the correct host matching (where initial dot allows subdomains).

I would probably start with Matt's PR and layer the addition of CSRF_TRUSTED_ORIGINS on top of that.

We could even merge them as separate fixes? Maybe I was wrong to suggest doing them both in this ticket.

[Joshua Kehn]

I think the domain check complicates matters enough to warrant a separate fix. I've essentially repurposed this ticket to accommodate the CSRF_TRUSTED_ORIGINS additions so I'd suggest creating a new ticket.

I don't know about the get_host() vs. CSRF_COOKIE_DOMAIN check. Let's say CSRF_COOKIE_DOMAIN is set to .example.com and ALLOWED_HOSTS is set to ['api.example.com', 'api-dev.example.com']. To allow cross origin requests also set CSRF_TRUSTED_ORIGINS to ['dashboard.example.com'].

At this point Django would allow a request referred from badhost.example.com, since it's part of the CSRF_TRUSTED_ORIGINS + CSRF_COOKIE_DOMAIN. Is that acceptable?

[Carl Meyer]

Replying to joshkehn:

I think the domain check complicates matters enough to warrant a separate fix. I've essentially repurposed this ticket to accommodate the CSRF_TRUSTED_ORIGINS additions so I'd suggest creating a new ticket.

I think instead we should revert the changes to ticket title and description here, and open a new ticket for CSRF_TRUSTED_ORIGINS, since this ticket was originally about CSRF_COOKIE_DOMAIN and that's its history. Sorry I led you down the wrong path yesterday.

I don't know about the get_host() vs. CSRF_COOKIE_DOMAIN check. Let's say CSRF_COOKIE_DOMAIN is set to .example.com and ALLOWED_HOSTS is set to ['api.example.com', 'api-dev.example.com']. To allow cross origin requests also set CSRF_TRUSTED_ORIGINS to ['dashboard.example.com'].

At this point Django would allow a request referred from badhost.example.com, since it's part of the CSRF_TRUSTED_ORIGINS + CSRF_COOKIE_DOMAIN. Is that acceptable?

Yes, that's fine. ALLOWED_HOSTS is not relevant here. If CSRF_COOKIE_DOMAIN is set to .example.com, that's a declaration that all subdomains of .example.com are trusted to submit unsafe requests (well, they still have to pass the CSRF token check, but they're trusted enough to pass the referrer check).

[Joshua Kehn]

Right, my question was with CSRF_TRUSTED_ORIGINS explicitly set wouldn't someone reasonably assume that it's a filter on top of CSRF_COOKIE_DOMAIN?

Example: Set CSRF_COOKIE_DOMAIN to .example.com for convenience of configuration so that the cookie is shared among any subdomain. Set CSRF_TRUSTED_ORIGINS to only allow certain referers to pass the CSRF checks.

My suggestion would be to only use CSRF_COOKIE_DOMAIN if CSRF_TRUSTED_ORIGINS is empty and make that a separate branch of logic.

Carl, I'm not sure if there's an easy way to split out into a new ticket. Do you have one? If not, I can manually do it a bit later today.

[Carl Meyer]

Replying to joshkehn:

Right, my question was with CSRF_TRUSTED_ORIGINS explicitly set wouldn't someone reasonably assume that it's a filter on top of CSRF_COOKIE_DOMAIN?

Example: Set CSRF_COOKIE_DOMAIN to .example.com for convenience of configuration so that the cookie is shared among any subdomain. Set CSRF_TRUSTED_ORIGINS to only allow certain referers to pass the CSRF checks.

My suggestion would be to only use CSRF_COOKIE_DOMAIN if CSRF_TRUSTED_ORIGINS is empty and make that a separate branch of logic.

It's a reasonable point, but I don't think so. Any host you are willing to share the CSRF cookie with is implicitly trusted to make use of it. Sharing the cookie is actually already a much deeper level of trust than that provided by CSRF_TRUSTED_ORIGINS alone, because it allows that host to get past both parts of the CSRF check, referrer and token. CSRF_TRUSTED_ORIGINS is a technique for adding _additional_ hosts who can bypass the referrer check, it's not a mechanism for restriction.

With your proposed behavior, it would be impossible to both have many wildcarded subdomains AND an external CORS-using host. That would be pretty frustrating.

Certainly the interaction of the two settings should be documented clearly.

Carl, I'm not sure if there's an easy way to split out into a new ticket. Do you have one? If not, I can manually do it a bit later today.

No, there's no secret technique for that. Shouldn't be too hard; I wouldn't try to copy all the comments or anything. Just open a new ticket and say "some previous discussion in #24496", and re-title your pull request so it gets linked up with the new ticket.

[Carl Meyer]

Resetting the title/summary of this ticket to again be specific to the CSRF_COOKIE_DOMAIN fix. Also resetting the "needs docs" flag, since I think that's all that the original PR for that (​https://github.com/django/django/pull/4337) is waiting on.

Sorry again for causing confusion and extra work by mixing these two slightly different issues together in one ticket.

[Joshua Kehn]

Done, thanks Carl. New ticket #25334, PR and commit updated.

[Tim Graham]

Pending some release notes and a versionchanged annotation.

[Tim Graham]

Patch now doesn't merge cleanly either.

[Tim Graham]

​Updated PR

[Tim Graham <timograham@…>]

In b0c56b89:

Fixed #24496 -- Added CSRF Referer checking against CSRF_COOKIE_DOMAIN.

Thanks Seth Gottlieb for help with the documentation and
Carl Meyer and Joshua Kehn for reviews.