Re: Adding Origin header checking to CSRF middleware (#16010)

[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

On Tue, 12 Jan 2021 at 12:18, Tim Graham  wrote:

> 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."
> On Tuesday, January 12, 2021 at 5:44:56 AM UTC-5 jacob...@gmail.com wrote:
>
>> 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
>>
> --
> 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-developers+unsubscr...@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

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM2W4oqC4ZcC29YbeJ6uPP%2BV-q2B6Y%2Bg%3DCJver5ygArw8Q%40mail.gmail.com.

Re: Adding Origin header checking to CSRF middleware (#16010)

[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."
On Tuesday, January 12, 2021 at 5:44:56 AM UTC-5 jacob...@gmail.com wrote:

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

-- 
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-developers+unsubscr...@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.

Re: Adding Origin header checking to CSRF middleware (#16010)

[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 

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/311bb8a8-5d84-4111-91ee-619ae8e8782an%40googlegroups.com.

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

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/953b044e-655c-4edc-a4ca-31bd82bacf77n%40googlegroups.com.