Adding Origin header checking to CSRF middleware (#16010)

[Tim Graham]

Hello, I've updated this 10 year old patch but some more changes are needed. I'll target it for Django 4.0.

https://code.djangoproject.com/ticket/16010

https://github.com/django/django/pull/13829

Here are a few design decisions and questions that have come up:

1. It seems the main reason this wasn't merged 10 years ago is because the patch didn't consider cross-domain POSTs. At the time, there was only CSRF_COOKIE_DOMAIN to consider.

These days referer checking allows cross-domain POSTs by considering SESSION_COOKIE_DOMAIN,  CSRF_COOKIE_DOMAIN, and CSRF_TRUSTED_ORIGINS (along with the request's host) [0]. Unfortunately, these settings only include the domain or a wildcard for all subdomains like '*.example.com'. However, origin checking requires including the scheme and port (if non-default).

We could add another setting CSRF_ALLOWED_ORIGINS (taking naming inspiration from  CORS_ALLOWED_ORIGINS in django-cors-headers [1]) which would be a list of hosts, including the schema and port. For example:

CSRF_ALLOWED_ORIGINS = [
    "https://example.com",
    "https://sub.example.com",
    "http://localhost:8080",
    "http://127.0.0.1:9000",
]

Unfortunately such a name is very similar and not well differentiated from the already existing CSRF_TRUSTED_ORIGINS setting. That setting could possibly be deprecated as netlocs for referer checking could be parsed from CSRF_ALLOWED_ORIGINS.

(Another possibility would be to have a Django 4.0 upgrade step be modifying the hosts in CSRF_TRUSTED_ORIGINS to include the scheme. This would be backward incompatible if trying to run older versions of Django concurrently though.)

Following the pattern of django-cors-headers, another setting may be needed to support all subdomains.

CSRF_ALLOWED_ORIGIN_REGEXES = [
    r"^https://\w+\.example\.com$",
]

However, it's less straightforward (if possible at all) to extra netlocs from arbitrary regular expressions. I'm not sure that full regular expression support is really needed. Perhaps it would be enough to support asterisks in CSRF_ALLOWED_ORIGINS (e.g. '"https://*.example.com"). urlparse() can handle that case.

2. There's also a question of backward compatibility. Since CSRF_ALLOWED_ORIGINS would be empty by default, only same-origin requests will be allowed unless the new settings are set. I can't think of a useful deprecation path here, but perhaps a system check to flag an empty CSRF_ALLOWED_ORIGINS if CSRF_TRUSTED_ORIGINS isn't empty (or if CSRF_COOKIE_DOMAIN or SESSION_COOKIE_DOMAIN are used) could be helpful in giving a heads up.

3. OWASP Cheat Sheet Series [2] says, "If the Origin header is not present, verify the hostname in the Referer header matches the target origin." which suggests to me that referer checking can be skipped if the origin header can be verified. Agreed?

4. OWASP Cheat Sheet also has some discussion of when 'Origin' is 'null'. I'm not sure if Django's checking needs to consider this. Maybe it would be enough to discard a null header and fall back to referer checking.

Thanks for any feedback.

[0] https://github.com/django/django/blob/407d3cf39cd6a62f7277e401d646a4c7e8446879/django/middleware/csrf.py#L257-L281
[1] https://github.com/adamchainz/django-cors-headers
[2] https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#checking-the-referer-header

[Jacob Rief]

Shouldn't we consider to put the CSRF token onto the deprecation list anyway?

All browsers released later than 2017 support the 'SameSite' cookie attribute, making the CSRF token obsolete.

I don't know what kind of policy the Django Project follows in deprecating browsers, but we can expect 

that IE, Edge<16, Safari<12, Chrome<51, etc. won't play a major role when Django-4 (or maybe 5?) will be released.

Strictly speaking, the CSRF token is a hack/workaround which in an ideal world shouldn't be required anyway.

And it always has been painful having to fiddle with it in my Django Form Views.

Just my two cents,

Jacob 

[Tim Graham]

OWASP Cheat Sheet says, "It is important to note that [the SameSite Cookie] attribute should be implemented as an additional layer defense in depth concept. This attribute protects the user through the browsers supporting it, and it contains as well 2 ways to bypass it as mentioned in the following section. This attribute should not replace having a CSRF Token. Instead, it should co-exist with that token in order to protect the user in a more robust way."

[Adam Johnson]

Hi Tim,

Thanks for working on this. I've put together some replies to your points here.

On #1 - I think it's legitimate to have `CSRF_TRUSTED_ORIGINS` require the schemes. I think the setting should have included them all along, since a scheme is part of the definition of an origin. It's backwards incompatible but the workaround when supporting multiple Django versions would be fairly simple, for example:

```
CSRF_TRUSTED_ORIGINS = [
    "example.com",
]
if django.VERSION >= (4, 0):
    CSRF_TRUSTED_ORIGINS = [f"https://{origin}" for origin in CSRF_TRUSTED_ORIGINS]
```

We could also provide temporary, immediately-deprecated support for origins without schemes by interpreting them as "http" and "https" by default.

I didn't add the regex support to django-cors-headers and I don't like it, since a malformed regex could cause a security hole.

On #2 - see above, no new setting.

On #3 - agreed. This will allow sites to drop the Referer header completely for privacy, if they want, by setting SECURE_REFERRER_POLICY = 'no-referrer'.

On #4 - the "privacy-sensitive contexts" listed on OWASP all seem to be things that would only trigger GET requests. It might even be legitimate to fail CSRF if origin is null, since CSRF is skipped for GET's.

Thanks,

Adam

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/915ff447-502e-45c2-8d18-bf5bee848c52n%40googlegroups.com.

--

Adam