CSRF Middlware and usage of request attributes (META, csrf_cookie_needs_reset)

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

CSRF Middlware and usage of request attributes (META, csrf_cookie_needs_reset)

Florian Apolloner
Hi there,

I am considering rewriting and (hopefully) simplifying the CSRF middleware. While looking through the code I realized that we put stuff into request.META as well as attributes on the request object itself (csrf_cookie_needs_reset) for instance. Is there any reason why we do not stick to one format?

Or more generally put: When should middlewares write into META as opposed to a attribute.

Cheers,
Florian

--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/43f6d8da-c66c-4100-a6d6-e85d4cef3684%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: CSRF Middlware and usage of request attributes (META, csrf_cookie_needs_reset)

Adam Johnson-2
It looks like it dates back to the Django 1.1 refactor and extension in https://github.com/django/django/commit/8e70cef9b67433edd70935dcc30c621d1e7fc0a0 ticket #9977, for which the referred wiki page ( https://code.djangoproject.com/wiki/CsrfProtection) is still up too with rationales about the design. I can't find any discussion about request.META vs attributes though, but since the move in the commit was from two middlewares to a unified one, maybe it was to do with that, since request.META is always there whilst attributes might not be if the middleware aren't set up properly? It seems like it could just be oversight. I would think that moving would require a standard deprecation period.

On Sat, 29 Dec 2018 at 11:47, Florian Apolloner <[hidden email]> wrote:
Hi there,

I am considering rewriting and (hopefully) simplifying the CSRF middleware. While looking through the code I realized that we put stuff into request.META as well as attributes on the request object itself (csrf_cookie_needs_reset) for instance. Is there any reason why we do not stick to one format?

Or more generally put: When should middlewares write into META as opposed to a attribute.

Cheers,
Florian

--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/43f6d8da-c66c-4100-a6d6-e85d4cef3684%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM2OPG%3DZo_NG5WScJNq4RwVgAttgdWHJ27u45onr8XQ5tw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: CSRF Middlware and usage of request attributes (META, csrf_cookie_needs_reset)

Florian Apolloner
Hi Adam,

On Saturday, December 29, 2018 at 7:59:44 PM UTC+1, Adam Johnson wrote:
since request.META is always there whilst attributes might not be if the middleware aren't set up properly?

request.META is always there, but the keys inside there might not be. In that sense you have to do "key in request.META" like we do with "hasattr(request, key)" if you are not sure on whether process_request of the middleware did actually set that stuff.
 
It seems like it could just be oversight. I would think that moving would require a standard deprecation period.

Unless we documented the attributes/keys I'd consider it an implementation detail of the middleware.

Cheers,
Florian

--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/fa76d5eb-68c6-4f9d-859d-3eb48011ae23%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: CSRF Middlware and usage of request attributes (META, csrf_cookie_needs_reset)

Adam Johnson-2
Unless we documented the attributes/keys I'd consider it an implementation detail of the middleware. 

Fair enough, I think I'm becoming a bit too conservative :)

On Sat, 29 Dec 2018 at 22:13, Florian Apolloner <[hidden email]> wrote:
Hi Adam,

On Saturday, December 29, 2018 at 7:59:44 PM UTC+1, Adam Johnson wrote:
since request.META is always there whilst attributes might not be if the middleware aren't set up properly?

request.META is always there, but the keys inside there might not be. In that sense you have to do "key in request.META" like we do with "hasattr(request, key)" if you are not sure on whether process_request of the middleware did actually set that stuff.
 
It seems like it could just be oversight. I would think that moving would require a standard deprecation period.

Unless we documented the attributes/keys I'd consider it an implementation detail of the middleware.

Cheers,
Florian

--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/fa76d5eb-68c6-4f9d-859d-3eb48011ae23%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM3aYbpiFvC_5ST%2B22KLN3O-SoBQQ34jm0Zu8g8iJM371Q%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: CSRF Middlware and usage of request attributes (META, csrf_cookie_needs_reset)

Luke Plant-2
In reply to this post by Florian Apolloner

Hi Florian,

My own instincts would be steer away from writing to request.META for most things, because request.META also contains things from the environment and indeed from the user request. You really don't want an attacker to be able to set an HTTP header and bypass security controls or directly influence anything which is supposed to be from application/framework code.

Of course, all arbitrary HTTP request headers are mapped to `request.META['HTTP_ …`, while some specific ones have special mappings,  but I'd rather not have to think about that kind of thing when doing security audits. With attributes it is always very clear where they have come from.

In addition, looking at the docs for request.META it is confusing to users if there are things in there that have not originated directly from the request.

As the original author of the CSRF stuff, I have no idea now why I used request.META for some things, if indeed it was me who did it that way — attributes seems much better for those usages.

Luke


On 29/12/2018 14:47, Florian Apolloner wrote:
Hi there,

I am considering rewriting and (hopefully) simplifying the CSRF middleware. While looking through the code I realized that we put stuff into request.META as well as attributes on the request object itself (csrf_cookie_needs_reset) for instance. Is there any reason why we do not stick to one format?

Or more generally put: When should middlewares write into META as opposed to a attribute.

Cheers,
Florian
--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/43f6d8da-c66c-4100-a6d6-e85d4cef3684%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9fb674dc-df9a-0c86-8e93-3465b8a596bb%40cantab.net.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: CSRF Middlware and usage of request attributes (META, csrf_cookie_needs_reset)

Adam Johnson-2
Thanks Luke for your look-again-later self code review :)

On Tue, 1 Jan 2019 at 16:51, Luke Plant <[hidden email]> wrote:

Hi Florian,

My own instincts would be steer away from writing to request.META for most things, because request.META also contains things from the environment and indeed from the user request. You really don't want an attacker to be able to set an HTTP header and bypass security controls or directly influence anything which is supposed to be from application/framework code.

Of course, all arbitrary HTTP request headers are mapped to `request.META['HTTP_ …`, while some specific ones have special mappings,  but I'd rather not have to think about that kind of thing when doing security audits. With attributes it is always very clear where they have come from.

In addition, looking at the docs for request.META it is confusing to users if there are things in there that have not originated directly from the request.

As the original author of the CSRF stuff, I have no idea now why I used request.META for some things, if indeed it was me who did it that way — attributes seems much better for those usages.

Luke


On 29/12/2018 14:47, Florian Apolloner wrote:
Hi there,

I am considering rewriting and (hopefully) simplifying the CSRF middleware. While looking through the code I realized that we put stuff into request.META as well as attributes on the request object itself (csrf_cookie_needs_reset) for instance. Is there any reason why we do not stick to one format?

Or more generally put: When should middlewares write into META as opposed to a attribute.

Cheers,
Florian
--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/43f6d8da-c66c-4100-a6d6-e85d4cef3684%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9fb674dc-df9a-0c86-8e93-3465b8a596bb%40cantab.net.
For more options, visit https://groups.google.com/d/optout.


--
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 [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM0OdDSFvO%2BWAdBgw4iSMQ0BMPiTnvtXX%2Byr8TR8Y9%3DAYA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.