Erroneous HTTPS CSRF Referrer Failure
Joshua Kehn
I understand why the Referrer check for secure requests is in place. What is currently preventing cross-domain API requests is that the check is not configurable. I'm talking specifically about requests when request.is_secure() returns True and an unsafe but specifically cross-origin request is being made.
What currently exists is a simple referrer check.
good_referer = 'https://%s/' % request.get_host()
if not same_origin(referer, good_referer):
reason = REASON_BAD_REFERER % (referer, good_referer)
return self._reject(request, reason)
Let's discuss two domains, api.project.com and dashboard.project.com. Both are served over HTTPS. Both are fronted with a reverse proxy that performs TLS termination.
Inside the Django settings normally one would configure:
# Read the special header set by the reverse proxy that indicates a secure request
SECURE_PROXY_SSL_HEADER = ("HTTP_FORWARDED", "https")
# Set the cookies on the root domain, allows api-dev.project.com to be configured
CSRF_COOKIE_DOMAIN = ".project.com"
SESSION_COOKIE_DOMAIN = ".project.com"
# Configure hosts we allow
ALLOWED_HOSTS = [
"api.project.com"
]
Now a POST request comes in to the API from a user on dashboard.project.com/posts/new. Django sees the following:
POST not in ('GET', 'HEAD', 'OPTIONS', 'TRACE'), this is an "unsafe" requestrequest.is_secure()isTrueHostheader isapi.project.com- Referrer is
dashboard.project.com/posts/new
At this point, the same_origin(referer, good_referer) check fails on middleware/csrf.py:159 and the entire request breaks. This check is not configurable and does not allow any hooks to bypass or alter behavior short of turning CSRF protection off entirely. This check is not present on non-secure requests and can cause surprise and confusion as to this new behavior when secure requests are enabled.
What are some possible solutions?
Why not configure the Host header when sending the request?
Causes all absolute reversed URLs to use that Host header which is obviously incorrect.
What about serving behind the same reverse proxy and provide path based routing?
Path based routing is a separate discussion but it introduces cache problems and configuration problems. Nevertheless, Django should not be required to serve on the same Host as the originator with zero configuration.
Why not write some custom middleware that does your own CSRF checks?
Not DRY, complicated, prone to failure. Would prefer to not roll my own security code for obvious reasons.
What are you doing now to solve this issue?
Added a small piece of middleware the changes the Referer header to a "good_referer".
Suggested Solution
Allow specific referrer hosts to be accepted via the settings. The change could be as simple as:
good_referers = [ "https://%s/" % host for host in settings.ALLOWED_HOSTS ]
if not any([same_origin(referer, good_referer) for good_referer in good_referers]):
...
Alternatively introduce a new setting that bypasses this secure referer check. I've outlined the common scenario where this check fails to provide any usefulness. While I believe this check provides additional security under ideal and simple use cases I don't believe Django's security should be relegated to the simple case.
I'd greatly welcome any comments or feedback on this matter, including alternative suggestions to resolving the issue that don't require making changes in Django core.
Best,
-Josh
Carl Meyer
On 08/31/2015 09:38 AM, Joshua Kehn wrote:
> I understand why the Referrer check for secure requests is in place.
> What is currently preventing cross-domain API requests is that the check
> is not configurable. I'm talking specifically about requests when
> |request.is_secure()| returns |True| and an unsafe but specifically
> cross-origin request is being made.
request" here. I think the point is that the request is in fact safe,
because it's coming from an approved CORS source, but there's no way to
tell the CSRF middleware that.
> What currently exists is a simple referrer check.
>
> |good_referer = 'https://%s/' % request.get_host() if not
> same_origin(referer, good_referer): reason = REASON_BAD_REFERER %
> (referer, good_referer) return self._reject(request, reason) |
> not allow any hooks to bypass or alter behavior short of turning CSRF
> protection off entirely. This check is not present on non-secure
> requests and can cause surprise and confusion as to this new behavior
> when secure requests are enabled.
>
>
> What are some possible solutions?
>
> Added a small piece of middleware the changes the |Referer| header to a
> "|good_referer|".
used the same solution. Ideally the middleware should verify that the
request is a valid CORS request, from an approved host, before patching
the referer.
Note that the most popular CORS-headers package for Django that I'm
aware of (https://github.com/ottoyiu/django-cors-headers) even includes
a middleware to patch the referer to get around this problem:
https://github.com/ottoyiu/django-cors-headers/blob/master/corsheaders/middleware.py#L26
> Suggested Solution
>
> Allow specific referrer hosts to be accepted via the settings. The
> change could be as simple as:
>
> |good_referers = [ "https://%s/" % host for host in
> settings.ALLOWED_HOSTS ] if not any([same_origin(referer, good_referer)
> for good_referer in good_referers]): ... |
sufficient (though it would be a small improvement on the current
situation). `ALLOWED_HOSTS` is meant to be a list of host names that are
aliases for the _current_ server. It should not also include remote
servers that are authorized for CORS requests to this server. So in your
example, `dashboard.project.com` should _not_ be included in
`ALLOWED_HOSTS` on `api.project.com`, so this fix wouldn't help.
> Alternatively introduce a new setting that bypasses this secure referer
> check.
altogether, but I do think there should be a way to make CORS and the
referer check work together without having to monkey-patch the request
referer.
The simplest approach would be a setting which is a list of valid CSRF
referer hosts. The problem with this is that it doesn't imply any
validation of the request CORS headers, but maybe that's OK. If you're
declaring "I trust this particular remote host not to CSRF my users,"
that's probably sufficient.
Alternatives might include just adding CORS support to Django directly,
or providing a more generic hook for third-party CORS packages to take
over the referer check themselves.
Carl
Joshua Kehn
On 31 Aug 2015, at 12:35, Carl Meyer wrote:
I'm not sure what you mean by "unsafe but specifically cross-origin
request" here. I think the point is that the request is in fact safe,
because it's coming from an approved CORS source, but there's no way to
tell the CSRF middleware that.
Yes, exactly. However I avoided using the term CORS because (1) it is not supported in Django core and (2) specifically has use around OPTIONS requests and ensuring preflight browser requests succeed.
I've had this same problem in a CORS-accessible API scenario, and I've
used the same solution. Ideally the middleware should verify that the
request is a valid CORS request, from an approved host, before patching
the referer.
Exactly except the referrer should not be patched. The referrer may be used upstream in other code, what should change is the CSRF check.
Note that the most popular CORS-headers package for Django that I'm
aware of (https://github.com/ottoyiu/django-cors-headers) even includes
a middleware to patch the referer to get around this problem:
https://github.com/ottoyiu/django-cors-headers/blob/master/corsheaders/middleware.py#L26
I'm using that one at present, I did not know the middleware had that. Thanks!
Suggested Solution
Allow specific referrer hosts to be accepted via the settings. The
change could be as simple as:|good_referers = [ "https://%s/" % host for host in
settings.ALLOWED_HOSTS ] if not any([same_origin(referer, good_referer)
for good_referer in good_referers]): ... |
I don't think just piggybacking on
ALLOWED_HOSTSlike this is
sufficient (though it would be a small improvement on the current
situation).ALLOWED_HOSTSis meant to be a list of host names that are
aliases for the current server. It should not also include remote
servers that are authorized for CORS requests to this server. So in your
example,
dashboard.project.comshould not be included in
ALLOWED_HOSTSonapi.project.com, so this fix wouldn't help.
I used ALLOWED_HOSTS as a placeholder specifically because the connotation of allowing certain Host headers has a parallel with the Referer header. I think ALLOWED_REFERERS or similar, but definitely a separate setting, is needed.
Alternatively introduce a new setting that bypasses this secure referer
check.I don't think there should be a setting to disable referer checking
altogether, but I do think there should be a way to make CORS and the
referer check work together without having to monkey-patch the request
referer.
A setting that allows bypassing and off by default would be the easiest way to keep HTTP vs. HTTPS CSRF checks identical.
The simplest approach would be a setting which is a list of valid CSRF
referer hosts. The problem with this is that it doesn't imply any
validation of the request CORS headers, but maybe that's OK. If you're
declaring "I trust this particular remote host not to CSRF my users,"
that's probably sufficient.
Agreed with simplest approach. What additional validation on the referer host would be added? You wouldn't add it if you didn't control it, at least that's my mindset. If I control dashboard.project.com is the case.
The argument could be made that this widens the attack surface, if you accept Referer requests from a large number of hosts, if one of those hosts is compromised that could be a point of entry. It's all tradeoffs, specifically in this case I have a separate frontend from the Django API and I'm very close to turning CSRF off entirely for lack of proper configuration.
Alternatives might include just adding CORS support to Django directly,
or providing a more generic hook for third-party CORS packages to take
over the referer check themselves.
I haven't thought about this, so I don't have a strong idea. I think the CORS is specific enough to utilize a separate system, my bigger irk here is that HTTP vs. HTTPS processing for CSRF is different and not configurable. It speaks strongly to Django's security mindset which I appreciate, but patching a header to allow requests to work doesn't make me feel good.
--jk
Carl Meyer
I think it would make sense to just add a `CSRF_ALLOWED_REFERERS`
setting, defaulting to `None` (which would give the current behavior of
requiring a match with the `Host` header). If set, it would be a list of
valid referer hosts. Documentation needs to be extremely clear that you
should only include hosts that are under your control, or you trust
completely, to this setting.
Anyone else see a problem with that that I'm missing?
You up for filing a ticket and maybe a patch/pull-request too?
Carl
Joshua Kehn
Anyone else see a problem with that that I'm missing?
I think this sounds fine.
You up for filing a ticket and maybe a patch/pull-request too?
--
You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/eQeaNzSlSbw/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/55E48D02.8000301%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.
Carl Meyer
On 08/31/2015 11:22 AM, Joshua Kehn wrote:
> On 31 Aug 2015, at 13:21, Carl Meyer wrote:
> I think it would make sense to just add a |CSRF_ALLOWED_REFERERS|
> setting, defaulting to |None| (which would give the current behavior of
> requiring a match with the |Host| header). If set, it would be a list of
> valid referer hosts. Documentation needs to be extremely clear that you
> should only include hosts that are under your control, or you trust
> completely, to this setting.
implied trust.
2) If it's set, a match with the Host header (or maybe with any host in
`ALLOWED_HOSTS`) should still be allowed, so you aren't forced to
duplicate `ALLOWED_HOSTS` inside `CSRF_TRUSTED_REFERERS`.
Carl
Joshua Kehn
On 31 Aug 2015, at 13:25, Carl Meyer wrote:
A couple follow-up thoughts:
On 08/31/2015 11:22 AM, Joshua Kehn wrote:
On 31 Aug 2015, at 13:21, Carl Meyer wrote:
I think it would make sense to just add a |CSRF_ALLOWED_REFERERS|
setting, defaulting to |None| (which would give the current behavior of
requiring a match with the |Host| header). If set, it would be a list of
valid referer hosts. Documentation needs to be extremely clear that you
should only include hosts that are under your control, or you trust
completely, to this setting.1) Maybe
CSRF_TRUSTED_REFERERSis a better name, to emphasize the
implied trust.
That name does sound better.
2) If it's set, a match with the Host header (or maybe with any host in
ALLOWED_HOSTS) should still be allowed, so you aren't forced to
duplicateALLOWED_HOSTSinsideCSRF_TRUSTED_REFERERS.
So the check here would look something like (excuse any typos, I'm not writing this in an editor):
allowed_hosts = list(settings.ALLOWED_HOSTS) + list(settings.CSRF_TRUSTED_REFERERS)
if "*" in allowed_hosts:
# Skip further checks since Django has been configured to allow any host.
else:
good_referers = ["https://{0}".format(host) for host in allowed_hosts]
if not any([same_origin(referer, good_referer) for good_referer in good_referers]):
# Reject CSRF referer mismatch
I would imagine that the "*" host would be allowed in CSRF_TRUSTED_REFERERS just like it is in ALLOWED_HOSTS?
The next thought would be a separate extension but worth explorting: Should Django then enforce CSRF referrer checks outside of secure requests if we have a setting specifically for it?
--jk
Carl Meyer
> 2) If it's set, a match with the Host header (or maybe with any host in
>
> So the check here would look something like (excuse any typos, I'm not
> writing this in an editor):
>
> |allowed_hosts = list(settings.ALLOWED_HOSTS) +
> list(settings.CSRF_TRUSTED_REFERERS) if "*" in allowed_hosts: # Skip
> further checks since Django has been configured to allow any host. else:
> good_referers = ["https://{0}".format(host) for host in allowed_hosts]
> if not any([same_origin(referer, good_referer) for good_referer in
> good_referers]): # Reject CSRF referer mismatch |
>
> I would imagine that the |"*"| host would be allowed in
> |CSRF_TRUSTED_REFERERS| just like it is in |ALLOWED_HOSTS|?
don't think there is any scenario in which that is a safe or reasonable
configuration.
And I think that the fact that it's allowed in `ALLOWED_HOSTS` might be
a reason to just stick to "Host header or CSRF_TRUSTED_REFERERS", and
leave ALLOWED_HOSTS out of it.
> The next thought would be a separate extension but worth explorting:
> Should Django then enforce CSRF referrer checks outside of secure
> requests if we have a setting specifically for it?
offers zero additional security in the HTTP case, because HTTP is
wide-open to MITM attacks regardless.
Carl
Tim Graham
Joshua Kehn
On 31 Aug 2015, at 13:56, Carl Meyer wrote:
No, I don't think
*should be allowed inCSRF_TRUSTED_REFERERS; I
don't think there is any scenario in which that is a safe or reasonable
configuration.And I think that the fact that it's allowed in
ALLOWED_HOSTSmight be
a reason to just stick to "Host header or CSRF_TRUSTED_REFERERS", and
leave ALLOWED_HOSTS out of it.
I would agree. I'll draft a ticket and post here once I have.
No, that would be a backwards-incompatible change, and the REFERER check
offers zero additional security in the HTTP case, because HTTP is
wide-open to MITM attacks regardless.
Good points, agreed.
I'll get the ticket in and work on a patch later today for review
Joshua Kehn
On 31 Aug 2015, at 14:02, Tim Graham wrote:
Is this related or duplicate to https://code.djangoproject.com/ticket/24496?
That ticket has a patch that got stalled a bit, but might be worth reviving
first in case this new one causes it to go stale.
Carl Meyer
> On 31 Aug 2015, at 14:02, Tim Graham wrote:
>
> Is this related or duplicate to
> https://code.djangoproject.com/ticket/24496?
> That ticket has a patch that got stalled a bit, but might be worth
> reviving
> first in case this new one causes it to go stale.
>
> Looks related.
>
> 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.
>
> Sounds very similar to what we've discussed here.
think CSRF_TRUSTED_REFERERS -- or CSRF_TRUSTED_ORIGINS if we don't like
reproducing the RFC-codified mis-spelling of "referrer" -- is a better
name).
This solution is more powerful than just using CSRF_COOKIE_DOMAIN, since
it also allows for separate-domain CORS situations in addition to
cross-subdomain requests. So I would consider this to be a good fix for
#24496; I don't think we need another ticket.
Carl
Joshua Kehn
On 31 Aug 2015, at 14:24, Carl Meyer wrote:
This solution is more powerful than just using CSRF_COOKIE_DOMAIN, since
it also allows for separate-domain CORS situations in addition to
cross-subdomain requests. So I would consider this to be a good fix for
#24496; I don't think we need another ticket.
Great. I was able to get this together tonight.
- PR #5218 is open: https://github.com/django/django/pull/5218
- Ticket #24496 updated: https://code.djangoproject.com/ticket/24496#comment:14
- Direct link to the patch: https://code.djangoproject.com/attachment/ticket/24496/24496.diff
Feedback appreciated.
--jk