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

2019-01-01 Thread Adam Johnson 
Thanks Luke for your look-again-later self code review :)

On Tue, 1 Jan 2019 at 16:51, Luke Plant  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 django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> 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 django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> 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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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.

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

2019-01-01 Thread Luke Plant 

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 django-developers+unsubscr...@googlegroups.com 
.
To post to this group, send email to 
django-developers@googlegroups.com 
.

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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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.

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

2018-12-30 Thread Adam Johnson 
>
> 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 
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 django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> 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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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.

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

2018-12-29 Thread 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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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.

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

2018-12-29 Thread Adam Johnson 
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 
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 django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> 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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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.

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

2018-12-29 Thread 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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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.