CSRF / SafeForm
Luke Plant
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 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.
Regards,
Luke
== Notes ==
[1] http://code.djangoproject.com/changeset/9554
- automatic exceptions for AJAX
- manual exceptions possible
http://code.djangoproject.com/changeset/9553
- split middleware into two components.
[2] It will mean:
1) always doing a redirect when you display a login form: you
have to create a new session, send the session cookie with a
HttpRedirectResponse, then in the next request create a form that
has the csrf token in it. Decorators like @staff_member_required
would need changing.
OR
2) creating a new session and returning a form with a csrf token at
the same time. The mechanism that generates the token will need
to be aware that the session ID might not be in
request.COOKIES[SESSION_COOKIE_NAME], but in request.session.??
--
"Outside of a dog, a book is a man's best friend... inside of a dog,
it's too dark to read."
Luke Plant || http://lukeplant.me.uk/
Jacob Kaplan-Moss
> == 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
Bob Thomas
On Dec 3, 9:14 am, Luke Plant <L.Plant...@cantab.net> 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
> 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).
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 ?
Luke Plant
> On Dec 3, 9:14 am, Luke Plant <L.Plant...@cantab.net> 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/
Luke Plant
> 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/
Bob Thomas
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