Re: CSRF / SafeForm

[Bob Thomas]

I added a ticket (with patch) for implementing the template tag:
http://code.djangoproject.com/ticket/9977
It also adds a CSRF context processor, which is used by the tag.

The diff doesn't look quite right. There obviously needs to be an
empty __init__.py file added to the templatetags folder for it to
work.

-bob

On Jan 5, 12:17 pm, Luke Plant  wrote:
> I wrote:
> > If you want to implement any of this, I'm not planning on working
> > on it for this next week, I'll get in touch when I start in case
> > you've made some progress.
>
> I'm now not going to be able to implement this for the 1.1 deadline.  
> I could review and commit if someone else implemented it, but
> remember that Jacob also wanted to see the patch complete with docs
> etc. before then, so I'm guessing this will not make 1.1 now.
>
> We would need to also ensure that all apps in contrib use the template
> tag, otherwise we wouldn't be able to make the new method a
> recommendation.  This in turn will require
> TEMPLATE_CONTEXT_PROCESSORS to
> contain 'django.core.context_processors.request' (or some other
> method for the template tag to get hold of session id/cookies).
>
> Finally, most importantly:
>
> I think we really need CSRF protection for the admin by default for
> 1.1.  The CSRF middleware in its current state, while not perfect, is
> mature enough to be on by default IMO (as you can now manually add
> exceptions where needed, and AJAX is automatically excluded).  So I'd
> recommend adding it to the default MIDDLEWARE_CLASSES in
> global_settings, or at least the skeleton settings file created
> by 'manage.py startproject'.
>
> Luke
>
> --
> Noise proves nothing.  Often a hen who has merely laid an egg cackles
> as if she laid an asteroid.
>         -- Mark Twain
>
> Luke Plant ||http://lukeplant.me.uk/
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: CSRF / SafeForm

[Luke Plant]

I wrote:

> If you want to implement any of this, I'm not planning on working
> on it for this next week, I'll get in touch when I start in case
> you've made some progress.

I'm now not going to be able to implement this for the 1.1 deadline.  
I could review and commit if someone else implemented it, but 
remember that Jacob also wanted to see the patch complete with docs 
etc. before then, so I'm guessing this will not make 1.1 now.

We would need to also ensure that all apps in contrib use the template 
tag, otherwise we wouldn't be able to make the new method a 
recommendation.  This in turn will require 
TEMPLATE_CONTEXT_PROCESSORS to 
contain 'django.core.context_processors.request' (or some other 
method for the template tag to get hold of session id/cookies).

Finally, most importantly:

I think we really need CSRF protection for the admin by default for 
1.1.  The CSRF middleware in its current state, while not perfect, is 
mature enough to be on by default IMO (as you can now manually add 
exceptions where needed, and AJAX is automatically excluded).  So I'd 
recommend adding it to the default MIDDLEWARE_CLASSES in 
global_settings, or at least the skeleton settings file created 
by 'manage.py startproject'.

Luke

-- 
Noise proves nothing.  Often a hen who has merely laid an egg cackles 
as if she laid an asteroid.
-- Mark Twain

Luke Plant || http://lukeplant.me.uk/

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: CSRF / SafeForm

[Luke Plant]

On Tuesday 23 December 2008 16:51:46 Bob Thomas wrote:

> On Dec 3, 9:14 am, Luke Plant  wrote:
> > At the moment, once you've factored everything in, I think 'view
> > middleware' + template tag is the way to go, with some more
> > custom solution for loginCSRF.  The SafeForm ends up having an
> > unwieldly API, which means it won't be used or could be used
> > incorrectly, it will often require changing a template anyway,
> > and it's specific to Django forms.  The template tag solution
> > would basically require a single line being added to the template
> > for each form (plus some settings, once).
>
> I probably should have said something earlier, since I semi-
> volunteered to work on this before. I like the csrf_except
> decorator you added. Making it a view middleware also seems like it
> would be possible to make a decorator with
> decorator_from_middleware, so users can have both an opt-in and
> opt-out way of using the middleware, without requiring an awkward
> SafeForm class.
>
> I definitely think that it should be included in the default
> middleware, but without even requiring the template tag to use it.
> Maybe add a setting for CsrfMiddleware to inject the hidden field
> which is on by default but can be disabled if someone wants to use
> the template tag. This is an area where Django can easily be secure
> by default, as long as we add the flexibility for the users who
> need more control.

My plan is this: first, more work needs to be done to the current 
solution:

 - the csrf_exempt decorator should also disable the post processing
   i.e. the function of what is now the CsrfResponseMiddleware.
   This can probably be done by tagging the response object with some
   custom attribute.  There might be some complications here with
   serialising the response object, as happens with caching (but then
   again, in practice there might not, I haven't thought it through).

 - the csrf_exempt decorator should be decomposed into two decorators, 
   csrf_view_exempt and csrf_response_exempt, which can be applied
   separately (their functions should be obvious from the names).
   csrf_exempt becomes a shortcut for applying both these.

With this in place, you can disable post-processing on a case by case 
basis, or globally by using CsrfViewMiddleware instead of 
CsrfMiddleware.

Then, the template tag needs to be implemented.

Then, all the docs need updating/rewriting. The recommended usage is:

 - CsrfViewMiddleware enabled (*not* CsrfMiddleware)
 - use of the template tag in all forms that need it.

The docs should also inform about the quick-and-dirty solution, which 
is just using the CsrfMiddleware (i.e. the Django 1.0 solution), 
preferably with a migration path to the proper way i.e. start adding 
the template tag to the templates, use the csrf_response_exempt on 
those views, then switch to CsrfViewMiddleware and remove all 
csrf_response_exempt decorators.

As you suggest, an 'opt in' solution using decorator_from_middleware 
should also be documented.  I think the 'recommended' way above is 
much better though, as it's too easy to forget to opt in.

Also in the docs there probably needs to be some mention, under 
the 'limitations' section of the 'corner case' described in a comment 
in the current module [1], which becomes relevant once the 
CsrfResponseMiddleware is no longer recommended.

A further modification that would be nice:  remove the direct 
dependency on the session framework.  This can be done by having a 
setting CSRF_SESSION_COOKIE_NAMES.  This defaults to 
[SESSION_COOKIE_NAME].  The csrf token is based on a hash of all 
cookies specified by this setting. I think we *probably* want to say 
that if one or more of the cookies is not found, then we assume no 
session exists.

If you want to implement any of this, I'm not planning on working on 
it for this next week, I'll get in touch when I start in case you've 
made some progress.

Regards,

Luke

[1] 
http://code.djangoproject.com/browser/django/trunk/django/contrib/csrf/middleware.py#L70

-- 
"Yes, wearily I sit here, pain and misery my only companions. And 
vast intelligence of course. And infinite sorrow. And..." (Marvin the 
paranoid android)

Luke Plant || http://lukeplant.me.uk/

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: CSRF / SafeForm

[Bob Thomas]

On Dec 3, 9:14 am, Luke Plant  wrote:
>
> At the moment, once you've factored everything in, I think 'view
> middleware' + template tag is the way to go, with some more custom
> solution for loginCSRF.  The SafeForm ends up having an unwieldly
> API, which means it won't be used or could be used incorrectly, it
> will often require changing a template anyway, and it's specific to
> Django forms.  The template tag solution would basically require a
> single line being added to the template for each form (plus some
> settings, once).


I probably should have said something earlier, since I semi-
volunteered to work on this before. I like the csrf_except decorator
you added. Making it a view middleware also seems like it would be
possible to make a decorator with decorator_from_middleware, so users
can have both an opt-in and opt-out way of using the middleware,
without requiring an awkward SafeForm class.

I definitely think that it should be included in the default
middleware, but without even requiring the template tag to use it.
Maybe add a setting for CsrfMiddleware to inject the hidden field
which is on by default but can be disabled if someone wants to use the
template tag. This is an area where Django can easily be secure by
default, as long as we add the flexibility for the users who need more
control.

-bob

PS - Don't really want to hijack this, but you seem to be the only one
currently working on CsrfMiddleware. Would you mind looking at
http://code.djangoproject.com/ticket/9356 ?
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: CSRF / SafeForm

[Jacob Kaplan-Moss]

On Wed, Dec 3, 2008 at 8:14 AM, Luke Plant  wrote:
> == Conclusion ==
>
> At the moment, once you've factored everything in, I think 'view
> middleware' + template tag is the way to go, with some more custom
> solution for login CSRF.  The SafeForm ends up having an unwieldly
> API, which means it won't be used or could be used incorrectly, it
> will often require changing a template anyway, and it's specific to
> Django forms.  The template tag solution would basically require a
> single line being added to the template for each form (plus some
> settings, once).
>
> I also suggest we add CsrfMiddleware or CsrfViewMiddleware to the
> default middleware and put a note about it in the release notes.

Realized I never responded to this, so, for the record, I agree with
this conclusion. I'd like to see a bit of code -- and, more
importantly for me, the documentation -- before it goes in, but  think
this sounds like the best solution.

Jacob

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

CSRF / SafeForm

[Luke Plant]

Hi all,

I'm thinking about 'championing' this thing, having just done a bit 
more work on the existing CsrfMiddleware [1], and I've done some more 
thinking about the different issues. LONG email, sorry, this is quite 
complex stuff.

== First ==
Simon suggested that the current middleware displays scary messages 
when people's sessions' expire, but that doesn't seem to be the case 
(normally).  The middleware simply checks for a session cookie and 
corresponding token, and only if they don't match does it throws a 
nasty page back.  The token itself does not expire, and even if the 
session has, the middleware doesn't know and doesn't care.  The 
*only* way I have succeeded in getting to the scary "Cross Site 
Request Forgery detected" message is like this:

 - open a page that returns a POST form (either in a view that
   requires authentication or one that doesn't).
 - aquire a new session *in a different tab*.  This might mean:
   1) you had no session when you opened the first page, and you
  log in in a different tab.
   2) you manually delete the session cookie, and do something
  that triggers a new sesssion
   3) you are doing something in a different tab, your session
  expires by itself and you are given a new one.
 - in the first tab, try to submit the form. It fails because the
   token in the page disagrees with the new session cookie.

Note that:
 - logging out and logging in in another tab is not enough (unless
   this causes a new session to be created).
 - waiting for the cookie to expire, or deleting the session from the 
   database is not enough.
In both these cases, you will be presented by the normal 'your session 
has expired' screen for views that require authentication, or you 
will simply carry on as normal if the view doesn't require 
authentication.

The above cases that trigger the screen seem to be quite unusual - 
I've never seen it myself when actually using a site with the 
CsrfMiddleware installed.

So what I'm saying is that the 'view protection' part of the 
middleware seems to work well enough for the vast majority of cases.  
If necessary we could always allow the error page to be customised 
via a setting (i.e. allow for a dotted path to a view function), and 
we could improve the default implementation to do something sensible, 
like display a message and then redirect to the original page after 5 
seconds.

I like the middleware for this part, because it means you are safe by 
default, and it works for any POST form -- no special action 
required.

Note that the recent changes to this middleware allow you to 
selectively turn it off for chosen views using a decorator.

== Second ==
There is the issue of how to insert a CSRF token into the forms.  Two 
proposals:

 - SafeForm
   - this takes a request, so it can generate the required field,
 removing the need for the 'html munging' part of CsrfMiddleware.
   - it could also have a validate method that would remove the need
 for the 'view protection' part of CsrfMiddleware
   - If the developer is manually specifying each field in their forms
 rather than using form.as_table() etc, they will have to edit
 the template.
   
 - a template tag
   - it would require using django.core.context_processors.request
   - it would depend on the 'view protection' part of CsrfMiddleware
   - it would always require changing the template to insert this tag
   - the major advantage is that it can be used with any POST form,
 so that no matter how your form works, there is one way to sort
 out the CSRF problem - add a template tag (and ensure the 
 middleware is on).

== Third ==
There is the 'login CSRF' problem.
One solution:
 - simply start a session whenever a login page is accessed if there 
   is no session already, and then use the normal mechanism above.
   - There are some issues with this in terms of implementation [2]
   - bots that hit the login page will generate sessions.

Second solution:
 - have a special 'login cookie' - a randomly generated id, not stored
   in the database, that is accompanied by a corresponding token.
   Requires some custom logic in every login view (you have to  
   generate the cookie, add it to the response, and also tell the 
   form/template about the value of the cookie/token).
   It may be possible to abstract some of this out into helper
   functions.

Third solution:
 - Add the functionality of the second solution to SafeForm.  It would
   require SafeForm to be able to set cookies and therefore to access
   the response, so the API gets a bit ugly.

== Conclusion ==

At the moment, once you've factored everything in, I think 'view 
middleware' + template tag is the way to go, with some more custom 
solution for login CSRF.  The SafeForm ends up having an unwieldly 
API, which means it won't be used or could be used incorrectly, it 
will often require changing a template anyway, and it's specific to 
Django forms.  The template tag solution