Proposal: django.forms.SafeForm - forms with built in CSRF protection

[Simon Willison]

CSRF[1] is one of the most common web application vulnerabilities, but
continues to have very poor awareness in the developer community.
Django ships with CSRF protection in the form of middleware, but it's
off by default. I'm willing to bet most people don't turn it on.

I don't believe middleware is the right way to approach this. It's too
"magic" - it involves code that parses and re-writes your HTML as the
response is being returned. It also means CSRF failures can't be
gracefully handled - the middleware can throw up a big ugly error
page, but ideally a CSRF failure would be treated the same way as a
regular form validation error.

I propose django.forms should include a SafeForm class, which is a
subclass of Form that includes built-in protection against CSRF. I
imagine the interface looking something like this:

from django import forms

class AddArticleForm(forms.SafeForm):
headline = forms.CharField()
author_email = forms.EmailField()
# etc

The interface to the form needs to be slightly different as the form
needs access to the full request object - CSRF validation requires
inspecting a cookie or session variable. Rather than passing
request.POST I suggest passing just request:

def add_article(request):
if request.method == 'POST': # If the form has been submitted...
form = AddArticleForm(request)
if form.is_valid(): # All validation rules pass
# Process the data in form.cleaned_data
# ...
return HttpResponseRedirect('/done/')
else:
form = AddArticleForm()

response = render_to_response('add_article.html', {
'form': form,
})
form.protect(response)
return response

Note the "form.protect(response)" at the end - it's a bit ugly, but we
need some way of setting a cookie on the response that can later be
used to verify that the form submission did indeed come from our own
code.

The template will need to include the hidden CSRF token fields:

<form action="/add_article/" method="POST">
{{ form.non_field_errors }}
{% for field in form %}
<div class="fieldWrapper">
{{ field.errors }}
{{ field.label_tag }}: {{ field }}
</div>
{% endfor %}
{{ form.csrf_fields }}
<p><input type="submit" value="Send message" /></p>
</form>

A CSRF failure message (I still haven't worked out the ideal user-
facing copy for this, "Your form session has expired; please re-
submit" is my current favourite) will be included as one of the
non_field_errors.

I'm also not sure how to deal with generating valid HTML in the above
- {{ form.csrf_fields }} needs to be wrapped in a block level element
for irritating reasons.

I suggest using cookies for the additional token rather than sessions
as doing so removes the dependency on Django's session framework (a
good idea for large sites that don't want the overhead of storing
thousands of inactive sessions).

Why not build this in to django.forms.Form directly? Because CSRF is
only an issue for forms that are supposed to only be used by
authenticated users. Forms that don't require a cookie don't need to
be protected.

The other option would be for the django.forms.Form constructor to
take a "csrf_protection=True" keyword argument, but this doesn't feel
as neat as a subclass.

That's as far as my thinking has got at the moment. I don't see any
reason this needs to be developed from the start as a patch or branch
against Django - it can be done as an external project and merged in
to django.forms once the code is well tested and the API is finalized.

(I'm totally up for working on this, but I really, really need to ship
the new version of django_openid first.)

Cheers,

Simon

[1] Some CSRF links for the uninitiated:
* http://en.wikipedia.org/wiki/Cross-site_request_forgery
* http://simonwillison.net/2008/talks/amajax-security/
* http://shiflett.org/articles/cross-site-request-forgeries

[Brian Beck]

On Sep 22, 4:25 pm, Simon Willison <si...@simonwillison.net> wrote:
> CSRF[1] is one of the most common web application vulnerabilities, but
> continues to have very poor awareness in the developer community.
> Django ships with CSRF protection in the form of middleware, but it's
> off by default. I'm willing to bet most people don't turn it on.

+0.5, I've written almost the exact code for this in a project for a
client. I would be +1 if:

- CsrfMiddleware was marked as deprecated. You're right; it's ugly.
However:
-- What about third-party app forms that aren't SafeForms, but need to
be? The situation dictates this, not the form author.
- If there's some other way to spell form.protect(response).
-- What about situations where the developer does not have access to
the response; if the form is loaded from a template tag or filter, for
instance: {% with model_obj|get_the_right_form as obj_form %} (I know,
you probably don't recommend this ;)

[Simon Willison]

On Sep 22, 9:41 pm, Brian Beck <exo...@gmail.com> wrote:
> -- What about third-party app forms that aren't SafeForms, but need to
> be?  The situation dictates this, not the form author.

I think we keep CSRF middleware around to deal with that. We also very
actively encourage third party apps to adopt SafeForm as soon as 1.1
is out.

> - If there's some other way to spell form.protect(response)

Here's the thing: any kind of cookie will do for CSRF protection as
long as its unique to the current user and can't be guessed by an
attacker. This is important because the cookie will be used to derive
the CSRF token (probably as the salt for a hash, with a timestamp as
well to reduce the impact of replay attacks). Django's session cookie
would be ideal here, but I really want this to work in situations
where sessions aren't in use. form.protect(response) was the first
thing that came to mind, but there's almost certainly a less crufty
way of doing this - maybe with a view decorator, or even some
middleware? I'm very open to suggestions on this.

> -- What about situations where the developer does not have access to
> the response; if the form is loaded from a template tag or filter, for
> instance: {% with model_obj|get_the_right_form as obj_form %} (I know,
> you probably don't recommend this ;)

As long as you've set the per-user-unique-unguessable-cookie before
you get to that stage, you'll be fine.

Cheers,

Simon

[Brian Beck]

On Sep 22, 4:55 pm, Simon Willison <si...@simonwillison.net> wrote:
> > -- What about third-party app forms that aren't SafeForms, but need to
> > be? The situation dictates this, not the form author.
> I think we keep CSRF middleware around to deal with that. We also very
> actively encourage third party apps to adopt SafeForm as soon as 1.1
> is out.

But still, the situation dictates the need for SafeForm, not the form
author. If this becomes best practice, essentially *every* form will
need to be initialized with a request.

P.S. I just posted the code I was using if anyone is interested:
http://www.djangosnippets.org/snippets/1077/

[Brian Beck]

On Sep 22, 5:01 pm, Brian Beck <exo...@gmail.com> wrote:
> But still, the situation dictates the need for SafeForm, not the form
> author. If this becomes best practice, essentially *every* form will
> need to be initialized with a request.

What about something like:

def protect_form(form_class):
class SafeForm(form_class):
csrf_token = CharField(...)
def clean_csrf_token(self):
...
return SafeForm

protect_form(ThirdPartyForm)(request.POST, request=request)

[Simon Willison]

On Sep 22, 10:01 pm, Brian Beck <exo...@gmail.com> wrote:
> > > -- What about third-party app forms that aren't SafeForms, but need to
> > > be?  The situation dictates this, not the form author.
> > I think we keep CSRF middleware around to deal with that. We also very
> > actively encourage third party apps to adopt SafeForm as soon as 1.1
> > is out.
>
> But still, the situation dictates the need for SafeForm, not the form
> author.  If this becomes best practice, essentially *every* form will
> need to be initialized with a request.

One thing that might help out in this case would be the ability to
create a SafeForm from a regular Form (which might be an argument for
csrf protection as a feature of django.forms.Form rather than a
subclass). If the third party code is well written (it follows the
class-based generic view idiom for example, providing a get_form()
method that can be over-ridden) it should be straight forward to
intercept the form it creates and upgrade it to a SafeForm.

You've reminded me of another problem with SafeForm: how does it
interact with ModelForms? Is there a SafeModelForm as well? What about
FormSets?

Cheers,

Simon

[Brian Beck]

On Sep 22, 5:16 pm, Simon Willison <si...@simonwillison.net> wrote:
> One thing that might help out in this case would be the ability to
> create a SafeForm from a regular Form (which might be an argument for
> csrf protection as a feature of django.forms.Form rather than a
> subclass). If the third party code is well written (it follows the
> class-based generic view idiom for example, providing a get_form()
> method that can be over-ridden) it should be straight forward to
> intercept the form it creates and upgrade it to a SafeForm.

See my previous post I just slipped in while you were replying. :)

[Jacob Kaplan-Moss]

On Mon, Sep 22, 2008 at 3:25 PM, Simon Willison <si...@simonwillison.net> wrote:
> I propose django.forms should include a SafeForm class, which is a
> subclass of Form that includes built-in protection against CSRF. I
> imagine the interface looking something like this:

Yes, indeed -- we should do this, and do it quick. CSRFMiddleware is a
hack; this would be quite a bit nicer.

I've not thought this through nearly as much as you or Brian obvious
have, so I'll leave you two to discuss, however, I have one thought...

On Mon, Sep 22, 2008 at 4:16 PM, Simon Willison <si...@simonwillison.net> wrote:
> You've reminded me of another problem with SafeForm: how does it
> interact with ModelForms? Is there a SafeModelForm as well? What about
> FormSets?

This makes me think -- is it possible that CSRF protection at the form
level is too low? Perhaps it's something that needs to be happening
at, say, the view level? Some sort of decorator, and/or a tag to spit
out the CSRF token in the template...

Just a thought, and now I'll butt out and let you two actually get
some work done.

Jacob

[Simon Willison]

On Sep 22, 10:21 pm, "Jacob Kaplan-Moss" <jacob.kaplanm...@gmail.com>
wrote:

> This makes me think -- is it possible that CSRF protection at the form
> level is too low? Perhaps it's something that needs to be happening
> at, say, the view level? Some sort of decorator, and/or a tag to spit
> out the CSRF token in the template...

Interesting thought. It feels like the form is the right place for
this for a couple of reasons:

1. It involves adding an extra form field
2. When a CSRF check fails, it's polite to show a message. Form
validation is a good place for this.

The downside of doing it at the form level is the need to have access
to the request and (potentially) the response as well, for setting a
cookie.

Doing things at the view level (with a decorator) provides access to
both request and response objects, but doesn't provide access to form
fields or validation errors.

Maybe the answer is a combination of both - a form subclass and a
decorator on the view?

Will have to think about that.

Cheers,

Simon

[Jan Oberst]

On Sep 22, 10:25 pm, Simon Willison <si...@simonwillison.net> wrote:
> CSRF[1] is one of the most common web application vulnerabilities, but
> continues to have very poor awareness in the developer community.
> Django ships with CSRF protection in the form of middleware, but it's
> off by default. I'm willing to bet most people don't turn it on.

I agree that a middleware is the wrong place for this. And I'll
definately have to implement some kind of CSRF protection some time in
the future. I like it!

> Why not build this in to django.forms.Form directly? Because CSRF is
> only an issue for forms that are supposed to only be used by
> authenticated users. Forms that don't require a cookie don't need to
> be protected.

I'd protect all my forms if there's a neat way to do it. Why would it
only apply to logged-in users? I'm not using contrib.auth.

Jan

[Simon Willison]

On Sep 22, 11:09 pm, Jan Oberst <mik...@gmail.com> wrote:
> I'd protect all my forms if there's a neat way to do it. Why would it
> only apply to logged-in users? I'm not using contrib.auth.

It doesn't need to only apply to contrib.auth logged in users, but it
should only be used for forms which are behind some kind of cookie-
based protection (auth is the most obvious example, but if you've
rolled your own authentication scheme you should be able to use
SafeForm as well).

There's no point in protecting a form which anyone can use (e.g. a
public "contact us" form) as the purpose of CSRF is for an attacker to
force you to perform an authenticated action that you don't want to
perform - deleting something from a CMS for example. If anyone can use
the form in question the attacker can just go and submit it
themselves.

Cheers,

Simon

[Simon Willison]

On Sep 22, 9:41 pm, Brian Beck <exo...@gmail.com> wrote:
> - If there's some other way to spell form.protect(response).

Here's a crazy idea that might just work:

class AddArticleForm(forms.SafeForm):
headline = forms.CharField()

# ...

def add_article(request):

form = AddArticleForm(request)
if form.is_valid():

# Process the data in form.cleaned_data

return HttpResponseRedirect('/done/')
return form.render('add_article.html', {
'extra_context_args': 'Go here',
})

We're doing a bunch of interesting things here. First, it's
AddArticleForm.__init__ itself that looks at request.method and
decides if it's going to bind to the data (if request.method ==
'POST') or simply create a new blank form. Second, we're using
form.render() instead of render_to_response. form.render is a thin
wrapper around render_to_response that does the following:

1. Adds 'form' to the context - a "form_var='article_form'" keyword
argument could be used to change this default behaviour
2. Uses a RequestContext (with the request that was passed to
AddArticleForm.__init__) - I can't think of any reason not to
3. Creates the HttpResponse using render_to_response
4. Sets a CSRF cookie on the response, if necessary

This solves all of our problems in one shot (the need to sometimes set
a cookie, having access to the request, etc), with the added bonus of
reducing the amount of view boilerplate needed to use a form to the
absolute minimum.

The significant downside is that having a render() method on a form
that performs the same function as render_to_response feels really,
really strange. It's convenient, but it just doesn't feel right and
I'm not sure I can justify it.

Interesting option though.

Cheers,

Simon

[Simon Willison]

On Sep 23, 12:27 am, Simon Willison <si...@simonwillison.net> wrote:
>     return form.render('add_article.html', {
>         'extra_context_args': 'Go here',
>     })

Using render_response as the method name might make this a little more
palatable:

return form.render_response('add_article.html', {

[Tai Lee]

On Sep 23, 9:27 am, Simon Willison <si...@simonwillison.net> wrote:
> The significant downside is that having a render() method on a form
> that performs the same function as render_to_response feels really,
> really strange. It's convenient, but it just doesn't feel right and
> I'm not sure I can justify it.

How would this work when you have multiple forms/modelforms/formsets
on one page?

[Jan Oberst]

You could save a only verifier code in the cookie that is unique for
all forms rendered on a single request object. Then the form would
sign its csrf fields with that one unique code.

Storing that unique code on the request object seems strange, though.

[Simon Willison]

In that case, you'd have to fall back to the old way of doing things -
form.render() would be a shortcut, not the only way of doing this.
You'd probably end up with code that looked like this:

def complex_view(request):
form_one = FormOne(request)
form_two = FormTwo(request)
form_three = FormThree(request)
if form_one.is_valid() and form_two.is_valid() and
form_three.is_valid():
# Do something with their cleaned_data
return HttpResponseRedirect("/done/")
response = render_to_response('complex_view.html', {
'form_one': form_one,
'form_two': form_two,
'form_three': form_three
})
form_one.protect(response)
form_two.protect(response)
form_three.protect(response)
return response

Since all form.protect(response) actually does is ensure that there's
a csrf cookie, you should only actually have to do it once for one of
the forms. Maybe it shouldn't be a form method at all in that case -
maybe it should be a function called django.forms.protect(request,
response) (request so it can check if the cookie has been set already,
and response so it can set it if needed).

Cheers,

Simon

[Ville Säävuori]

> I propose django.forms should include a SafeForm class, which is a
> subclass of Form that includes built-in protection against CSRF. I
> imagine the interface looking something like this:

Why wouldn't the safe form class be the forms.Form class instead? We
do XSS-protection by default already, so why don't we encourage CSRF-
protection, too?

If for some reason you wouldn't want CSRF-magic to your forms, we
could have forms.UnsafeForm (or whatever). Of course, this would
propably mean backwards incompatible change, but I think that this is
something that we'd want in the long run. I'm not saying that we
should push this further from 1.1 but that we should definitely make
something like this (however it's implemented) as the default way in
the long run.

As a separate note, I'd also like some well documented low-level
helpers to make protecting AJAX-calls easier.

[oggie rob]

On Sep 22, 1:25 pm, Simon Willison <si...@simonwillison.net> wrote:
> CSRF[1] is one of the most common web application vulnerabilities, but
> continues to have very poor awareness in the developer community.
> Django ships with CSRF protection in the form of middleware, but it's
> off by default. I'm willing to bet most people don't turn it on.
>
> I don't believe middleware is the right way to approach this. It's too
> "magic" - it involves code that parses and re-writes your HTML as the
> response is being returned. It also means CSRF failures can't be
> gracefully handled - the middleware can throw up a big ugly error
> page, but ideally a CSRF failure would be treated the same way as a
> regular form validation error.

I just read this thread now, and by the end of it, things are looking
pretty complicated. Is it worth a gut check to make sure this is
worthwhile? Although the middleware might be a hack now, it seems
sensible that it fits in request/response areas rather than in forms:
you still need to go out of your way to add it anyway (so users won't
necessarily "turn it on"); it takes a lot more code; add in the
multiple forms per page question, and to me it seems like you've fixed
a problem by introducing another.
Finally, it doesn't take much to make a pretty message - something
like "You are under attack, close down your browser and try again"
with images of flaming people & such - for the (lets be realistic)
very rare cases when a CSRF attack occurs. All you need is a template,
right? (And I would consider sending an email to the admin notifying
them that at attack was attempted, at least to get an idea of how
common these issues are.)

-rob

[Simon Willison]

On Sep 23, 9:00 am, oggie rob <oz.robhar...@gmail.com> wrote:
> I just read this thread now, and by the end of it, things are looking
> pretty complicated. Is it worth a gut check to make sure this is
> worthwhile? Although the middleware might be a hack now, it seems
> sensible that it fits in request/response areas rather than in forms:
> you still need to go out of your way to add it anyway (so users won't
> necessarily "turn it on"); it takes a lot more code; add in the
> multiple forms per page question, and to me it seems like you've fixed
> a problem by introducing another.

Here's a useful case in point: the admin. Django's admin should ship
with CSRF protection turned on and baked in. Right now, I'm willing to
bet 95% of the Django admin sites out there are exploitable via CSRF
because the middleware wasn't turned on. This is really bad.

I'm positive we can figure out a better API for CSRF protection than
what we have at the moment. At the moment I'm focused on forms, but if
there's something we can do at the view level instead I'd love to see
some suggestions.

> Finally, it doesn't take much to make a pretty message - something
> like "You are under attack, close down your browser and try again"
> with images of flaming people & such - for the (lets be realistic)
> very rare cases when a CSRF attack occurs.

I'm worried about false positives. One example where this would happen
is if you were to change your SECRET_KEY (secret management is a whole
other issue we haven't addressed). That's why I like the validation
error approach - it's unobtrusive and doesn't unnecessarily scare
people. We should definitely log detected CSRF issues though (logging
= another issue).

Cheers,

Simon

[oggie rob]

On Sep 23, 3:26 am, Simon Willison <si...@simonwillison.net> wrote:
> On Sep 23, 9:00 am, oggie rob <oz.robhar...@gmail.com> wrote:
>> Is it worth a gut check to make sure this is worthwhile?
>

> Here's a useful case in point: the admin. Django's admin should ship
> with CSRF protection turned on and baked in. Right now, I'm willing to
> bet 95% of the Django admin sites out there are exploitable via CSRF
> because the middleware wasn't turned on. This is really bad.
>

I'm sorry, I used the wrong term here. I didn't mean that CSRF
protection isn't worthwhile, just that going the route of an extended
form might not be the best way to do it.
As for suggestions, I'm not sure I have one exactly, but I'm thinking
of perhaps overriding is_valid() and maybe using the RequestContext
object.. not sure yet.

-rob

[Brian Beck]

On Sep 23, 10:56 am, oggie rob <oz.robhar...@gmail.com> wrote:
> I'm sorry, I used the wrong term here. I didn't mean that CSRF
> protection isn't worthwhile, just that going the route of an extended
> form might not be the best way to do it.
> As for suggestions, I'm not sure I have one exactly, but I'm thinking
> of perhaps overriding is_valid() and maybe using the RequestContext
> object.. not sure yet.

The problem is that any token, no matter where we generate it, isn't
going to be submitted back with the POST request unless it's a field
in the form that was submitted. So the only options I see are
mangling the HTML to add these fields (CsrfMiddleware), or add them to
the form objects (SafeForm).

[oggy]

Could we just include something like a signed salt+timestamp
+REMOTE_ADDR in a hidden field? It's not exactly bulletproof because
of the possibility of a same-IP-CSRF (affecting people behind
proxies), but it's dead simple and doesn't require a lot of code
change: Form -> SafeForm + request as the first parameter to __init__.
Heck, I'd even trust sed to do it for me ;).

Alternatively, since the only thing we need is to make sure that we're
dealing with the same user across multiple requests, we could also
provide some kind of a middleware that sets a sid-like cookie and
include that instead of the REMOTE_ADDR. The obvious downside would be
that you'd need to include it in settings.py.

[Brian Beck]

On Sep 23, 12:13 pm, oggy <ognjen.ma...@gmail.com> wrote:
> Could we just include something like a signed salt+timestamp
> +REMOTE_ADDR in a hidden field? It's not exactly bulletproof because
> of the possibility of a same-IP-CSRF (affecting people behind
> proxies), but it's dead simple and doesn't require a lot of code
> change: Form -> SafeForm + request as the first parameter to __init__.
> Heck, I'd even trust sed to do it for me ;).

Timestamp and REMOTE_ADDR wouldn't make a proper token unless we also
included the timestamp and REMOTE_ADDR as hidden fields -- the server
needs to be able to *regenerate* the token when the form is submitted
in order to validate the POSTed token.

> Alternatively, since the only thing we need is to make sure that we're
> dealing with the same user across multiple requests, we could also
> provide some kind of a middleware that sets a sid-like cookie and
> include that instead of the REMOTE_ADDR. The obvious downside would be
> that you'd need to include it in settings.py.

Yes, this is what Simon suggested -- if sessions are not being used,
then send a cookie that is effectively a session ID, but only used for
CSRF protection. I don't see why that requires any settings.py
changes, though.

[Manuel Saelices]

We've implemented a similar aproach to minimize put same logic pattern
in views, and so maximize DRY philosophy:

https://tracpub.yaco.es/cmsutils/browser/trunk/forms/forms.py

In this URL you can check GenericForm, and its childs, GenericAddForm
and GenericEditForm. The only requirement is to pass request into
constructor, that maybe is a little more coupled solution that normal
Django forms (here DRY make conflict with "less coupled")

With a request passed to forms, you can do a lot of things in
reusability, like SignedForm, but it's true that is less explicit
solution ("explicit better than implicit", third philosophical
concept).

Regards,
Manuel Saelices.

[Jan Oberst]

On Sep 23, 6:13 pm, oggy <ognjen.ma...@gmail.com> wrote:
> Could we just include something like a signed salt+timestamp
> +REMOTE_ADDR in a hidden field? It's not exactly bulletproof because
> of the possibility of a same-IP-CSRF (affecting people behind
> proxies), but it's dead simple and doesn't require a lot of code
> change: Form -> SafeForm + request as the first parameter to __init__.
> Heck, I'd even trust sed to do it for me ;).

Adding a signed field with a timestamp would be a much easier way to
secure forms. But it's not nearly as as secure as having the token
signed with an additional cookie. By setting a signed cookie you can
verify that this very form was displayed to this very client. Also,
you don't want to expire a form too early for people who just type
slow. And if a token is available for too long someone can generate a
proper token and then use it for an attack for too long.

[Robert Coup]

On Wed, Sep 24, 2008 at 5:29 AM, Brian Beck <exo...@gmail.com> wrote:

On Sep 23, 12:13 pm, oggy <ognjen.ma...@gmail.com> wrote:
> Could we just include something like a signed salt+timestamp
> +REMOTE_ADDR in a hidden field? It's not exactly bulletproof because
> of the possibility of a same-IP-CSRF (affecting people behind
> proxies), but it's dead simple and doesn't require a lot of code
> change: Form -> SafeForm + request as the first parameter to __init__.
> Heck, I'd even trust sed to do it for me ;).

Timestamp and REMOTE_ADDR wouldn't make a proper token unless we also
included the timestamp and REMOTE_ADDR as hidden fields -- the server
needs to be able to *regenerate* the token when the form is submitted
in order to validate the POSTed token.

There are still ways to use these or other identifying values without the server having to store them somewhere.

csrf_token = "[timestamp]/[remote_addr]/[hash]"
where hash = sha1(timestamp + remote_addr + secret_key)

eg. 1222204129/123.123.123.123/40bd001563085fc35165329ea1ff5c5ecbdbbeef

base64 encode the csrf_token if you want to make it slightly more obscure.
eg.MTIyMjIwNDEyOS8xMjMuMTIzLjEyMy4xMjMvNDBiZDAwMTU2MzA4NWZjMzUxNjUzMjllYTFmZjVjNWVjYmRiYmVlZg==

then when you get a form submission, base64-decode it, split at "/", check the hash matches by recalculating it, then use the proximity-to-timestamp and the remote_addr to check the form validity.

Rob :)

[Simon Willison]

On Sep 23, 10:17 pm, "Robert Coup" <robert.c...@onetrackmind.co.nz>
wrote:

> then when you get a form submission, base64-decode it, split at "/", check
> the hash matches by recalculating it, then use the proximity-to-timestamp
> and the remote_addr to check the form validity.

Anything that relies on remote_addr is flawed, because IP addresses
change all the time. I frequently load up a Google Groups thread on my
laptop, compose a reply on the train to work and submit it when I get
there - and since I've moved networks my IP address changes in between
loading the form and submitting it. There's also the risk of proxies
that load balance traffic through different IP addresses, not to
mention IP addresses that are shared by many people (including a
potential attacker).

Cheers,

Simon

[Simon Willison]

On Sep 23, 1:06 am, Simon Willison <si...@simonwillison.net> wrote:
> Since all form.protect(response) actually does is ensure that there's
> a csrf cookie, you should only actually have to do it once for one of
> the forms.

Unsurprisingly, I've been totally over-engineering this. There's no
need for form.protect(response) at all - in fact, the form doesn't
need to have anything to do with the response object. This
dramatically simplifies things - in fact, it takes us back to the
original proposal of a SafeForm that just takes the request object as
an argument to its constructor.

CSRF attacks are a problem for systems where an action is only meant
to be available to a specific logged in user. This user is inevitably
identified by a unique cookie. This is normally a session cookie,
hence many CSRF protection mechanisms key their hidden form token off
the session cookie. That's also what Django's CSRF middleware does.

I like to be able to avoid sessions where possible, which is why I
didn't want SafeForm to use the session cookie and thought it would
need to set a cookie itself. But even if an application isn't using
sessions, it's still going to need to use one or more cookies that are
unique to the user.

I think we solve this with a setting, the default of which looks like
this:

CSRF_COOKIES = [SESSION_COOKIE_NAME]

SafeForm creates its csrf_token based on a hash of the cookie values
for the keys in that list. It's a list because some authentication
cases may rely on more than one cookie to uniquely identify the user
(rare but possible). By default, it will use the Django session
cookie. If you're not using sessions in your app (you're using a
signed cookie called 'current_user' for example) you can specify that
in the CSRF_COOKIES session.

So, the bit about the SafeForm needing access to the response was a
red herring.

Cheers,

Simon

[Amit Upadhyay]

On Tue, Sep 23, 2008 at 8:43 PM, Brian Beck <exo...@gmail.com> wrote:
> The problem is that any token, no matter where we generate it, isn't
> going to be submitted back with the POST request unless it's a field
> in the form that was submitted. So the only options I see are
> mangling the HTML to add these fields (CsrfMiddleware), or add them to
> the form objects (SafeForm).

There is another option, a template tag. I would implement it as a
middleware and a template tag. Template tag csrf_protect, will require
CSRFMiddleware and django.core.context_processors.request, will add a
input file containing something derived from {{ request }} and
middleware will check and raise HttpForbidden. Its so ugly that it
does not deserve a form validation error in my opinion. This will
require least amount of changes in existing sites.

--
Amit Upadhyay
Vakow! www.vakow.com
+91-9820-295-512

[Simon Willison]

On Sep 23, 11:23 pm, Simon Willison <si...@simonwillison.net> wrote:
> CSRF attacks are a problem for systems where an action is only meant
> to be available to a specific logged in user. This user is inevitably
> identified by a unique cookie. This is normally a session cookie,
> hence many CSRF protection mechanisms key their hidden form token off
> the session cookie.

There's another option that avoids the need for any cookies at all:
generating a persistent one-use-only token when a form is saved,
storing that in the database and only allowing submissions that
include a token that was previously assigned.

This avoids any need for cookies at all, but has the serious
disadvantage that you end up with potentially thousands of useless
tokens stored in your database. You can clear these out periodically
but it's still overhead that it would be nice to avoid.

Cheers,

Simon

[Simon Willison]

On Sep 24, 1:04 am, Simon Willison <si...@simonwillison.net> wrote:
> There's another option that avoids the need for any cookies at all:
> generating a persistent one-use-only token when a form is saved,
> storing that in the database and only allowing submissions that
> include a token that was previously assigned.

Scratch that - the tokens would still need to be assigned to an
individual user (almost certainly keyed off a cookie) as otherwise an
attacker could create their own tokens and use them to attack another
user.

It would work for sites protected using HTTP authentication rather
than cookies though, as you'd be able to attach each token to the HTTP
auth username. I don't think this is a case we need to address though.

[Simon Willison]

On Sep 23, 11:51 pm, "Amit Upadhyay" <upadh...@gmail.com> wrote:
> There is another option, a template tag. I would implement it as a
> middleware and a template tag. Template tag csrf_protect, will require
> CSRFMiddleware and django.core.context_processors.request, will add a
> input file containing something derived from {{ request }} and
> middleware will check and raise HttpForbidden.

Oddly enough that's exactly how ASP.NET MVC does it:

http://blog.codeville.net/2008/09/01/prevent-cross-site-request-forgery-csrf-using-aspnet-mvcs-antiforgerytoken-helper/

They use their equivalent of a view decorator rather than middleware,
which is what I'd suggest doing in this case as well (middleware in
Django is usually applied globally which isn't always what you want).

Cheers,

Simon

[oggy]

On Sep 23, 11:02 pm, Jan Oberst <jan.obe...@gmail.com> wrote:
> Adding a signed field with a timestamp would be a much easier way to
> secure forms. But it's not nearly as as secure as having the token
> signed with an additional cookie. By setting a signed cookie you can
> verify that this very form was displayed to this very client. Also,
> you don't want to expire a form too early for people who just type
> slow. And if a token is available for too long someone can generate a
> proper token and then use it for an attack for too long.

The additional cookie is pointless. A single random-enough cookie is
enough to differentiate between two users, and that's all you need.
You can then use this cookie to include it in the signature. Here's a
snippet to explain what I mean:
http://www.djangosnippets.org/snippets/1082/

You start with the assumption of a secret unique user cookie (which
I'll affectionately refer to as "the cookie"). The only two things
that the user gets in plain text are the timestamp and the salt. The
salt is probably unnecessary, but what the heck. Nonces are cool. To
this information you add the signature of [timestamp, salt, form class
name, the cookie], and that's your token. The form class name is
probably unnecessary as well.

So to CSRF the form now, you need to recreate the token -> you need to
recreate/acquire the above signature. Recreate is a no-go because you
don't know the site's SECRET_KEY. Acquiring would work using our site
as a black box, but only if you knew the cookie -> contradicts our
assumptions.

Uh, come to think about it, it seems the secret cookie is enough, the
rest is just superfluous. But it might be just my brain farting.

[oggy]

So I guess it's not my brain farting.

On Sep 24, 12:23 am, Simon Willison <si...@simonwillison.net> wrote:
> Unsurprisingly, I've been totally over-engineering this. There's no

...

> I think we solve this with a setting, the default of which looks like
> this:

At least you haven't been writing over-engineered code for the past
2hrs :)

[oggie rob]

> in fact, it takes us back to the
> original proposal of a SafeForm that just takes the request object as
> an argument to its constructor.

Well this seems much simpler, although there is still the requirement
to add the csrf_fields whenever you write out the form in the template
(which isn't much - I'm just looking for the shortest and most
failsafe path).

As for making a subclass - is there a problem having "request" as an
optional field in the BaseForm constructor? It could then at least be
included with as_table, etc automatically at the cost only of adding
an extra constructor parameter to existing forms, and ignored without
it.

-rob

[Rudolph]

I'd like to discuss if forms should try to be secure by default? It's
a bit like the autoescaping discussion.

The new Form class would need to accept dictionaries or request
objects as input. A dictionary is however only accepted if you
explicitly disable CSRF protection:
form = MyForm(request.POST, csrf_protection=False)

We can keep it backwards compatible by introducing a setting which
defaults to:
CSRF_PROTECTION = False
and putting CSRF_PROTECTION = True in the default settings.py file of
new projects. Also encourage the use in the documentation.

In Django 2.0 we set this to True. People would then need to
explicitly turn off CSRF protection globally or on a per form basis.

Thanks, Rudolph

[Simon Willison]

On Sep 23, 11:23 pm, Simon Willison <si...@simonwillison.net> wrote:

> CSRF attacks are a problem for systems where an action is only meant
> to be available to a specific logged in user. This user is inevitably
> identified by a unique cookie. This is normally a session cookie,
> hence many CSRF protection mechanisms key their hidden form token off
> the session cookie.

It turns out it's not that straight-forward after all:

http://icanhaz.com/csrfpdf (PDF link, "Robust Defenses for Cross-Site
Request Forgery")

The above paper introduces the "login CSRF" attack, where CSRF is used
to force a victim to log in to a site using /the attacker's/
credentials. The hope is that the user will then enter personally
sensitive information which the hacker can harvest later on.

Django's CSRF mechanism needs to be able to protect forms even in the
absence of a unique-to-the-user cookie, which means it needs a way of
setting its own cookies. We can either do this using the
form.protect() or form.render_response() methods I advocated earlier,
or we can use a middleware/view decorator combination. I think I'm
leaning towards the view decorator / middleware option now.

Cheers,

Simon

[zellyn]

I like the middleware/view decorator for setting the cookie.

Would it make sense to have the middleware/view decorator set a
property on the request, and pass the request to all forms, and have
*all* forms CSRF-protect themselves when the property is set? That
would make it easy to add protection to externally-developed apps.

Zellyn

[Simon Willison]

On Sep 24, 2:18 pm, zellyn <zel...@gmail.com> wrote:
> Would it make sense to have the middleware/view decorator set a
> property on the request, and pass the request to all forms, and have
> *all* forms CSRF-protect themselves when the property is set? That
> would make it easy to add protection to externally-developed apps.

That's an interesting idea. I'm a bit cautious of requiring ALL forms
to take a request object though - Django's current form library is
decoupled from Django's request object, which is actually a really
useful property. I've used a "form" class to import CSV data in to
Django ORM models in the past for example, using the form logic to
handle the necessary type conversions.

Keeping django.forms decoupled from HttpRequest also ensurse it can be
used by other Python web frameworks that don't have the same request/
response model.

I'm fine with SafeForm depending on HttpRequest, but I'd rather not
introduce that dependency to BaseForm and Form.

Cheers,

Simon

[zellyn]

On Sep 24, 10:00 am, Simon Willison <si...@simonwillison.net> wrote:

> That's an interesting idea. I'm a bit cautious of requiring ALL forms
> to take a request object though - Django's current form library is
> decoupled from Django's request object, which is actually a really
> useful property. I've used a "form" class to import CSV data in to
> Django ORM models in the past for example, using the form logic to
> handle the necessary type conversions.

I don't like the coupling either. I guess the request could be an
optional param - with template tags passing it by default - but I
agree - it feels like mixing things that shouldn't be mixed.

How about if the form-rendering template tags were aware of the CSRF-
protection value - passed explicitly, or in the template context... it
still feels a bit like mixing things, though.

Zellyn

[David Durham, Jr.]

My first reaction is that inheritance is not appropriate for this. I
would add something to a Form Meta nested class like 'safe_form =
False' with True becoming the default. True being the default has
potential to break older apps, but a setting could change the default,
and security is probably important enough to add the small
inconvenience. Forms would then need to be rendered with the hidden
__csfr_token input where the value is provided by middleware. Then
you need some setting that says use a session or separate signed
cookie to verify the token from the form submit. The default
behaviour could be to use session when available and fall back on
cookie otherwise (which can be overriden with a setting).

A form init could become something like MyForm(request.POST,
safe_token=csfr.get_token(request))

-Dave

[Luke Plant]

Hi Simon,

> CSRF[1] is one of the most common web application vulnerabilities, but
> continues to have very poor awareness in the developer community.
> Django ships with CSRF protection in the form of middleware, but it's
> off by default. I'm willing to bet most people don't turn it on.
>
> I don't believe middleware is the right way to approach this. It's too
> "magic" - it involves code that parses and re-writes your HTML as the
> response is being returned. It also means CSRF failures can't be
> gracefully handled - the middleware can throw up a big ugly error
> page, but ideally a CSRF failure would be treated the same way as a
> regular form validation error.
>

> I propose django.forms should include a SafeForm class, which is a
> subclass of Form that includes built-in protection against CSRF. I
> imagine the interface looking something like this:

I carefully prepared a reply to this, only to forget to transfer it to
my memory stick for sending (I have just moved house and I'm without
internet connection to my own computer...). Anyway, here is a shortened
version, hope it makes some kind of sense as it was written hurriedly.
I'm unlikely to be able to reply to any responses to this, for reasons
mentioned above.

I agree that the CsrfMiddleware is ugly, but before replacing it, here
are the things about it that I do like:

- It Just Works (after you turn it on).

- The only ugly bit (which is also the 'fragile' bit) is the post
processing HTML munging, and if this fails, you are secure by default.

- You don't have to remember to do *anything* for it to work, you just
turn it on once. This is a big deal -- this is why we have SQL escaping
in the ORM, and auto-escaping in the templates. If you leave something
to human error, humans will make an error.

- You don't have security related boilerplate code messing up every
view function. This means all the ugliness of having to deal with the
real world of CSRF etc has been confined to one ugly middleware.

- It works for non-Django forms, which I do have sometimes e.g. forms
with a dynamic number of controls, like tables with a checkbox in each
row for manipulating large numbers of objects. I have several instances
of this in my code already, I'm glad I don't have to re-implement CSRF
protection for these. It also Just Works for AJAX (you may have to do
extra work to get hold of the token client side, and of course you need
to use POST appropriately in your AJAX).

- If we moved to a SafeForm, at least the SafeForm code thus far
proposed, it would be more work to be CSRF safe than non-safe, and all
the example code for Django froms on tutorials acrosss the internet
would therefore be non-safe, just like all the example PHP code across
the web is vulnerable to all kind of hacks.

- There are ways of making the existing implementation nicer (e.g.
redirect to a view which handles the error more gracefully, with a link
(via REFERER header or something) to the original page, or something
like that), and we could turn it on by default in default settings -
SafeForm probably couldn't do that.

I'm not saying don't replace it, but let's make sure the cure is better
than the illness.

Regards,

Luke

[oggy]

On Sep 24, 11:17 am, Simon Willison <si...@simonwillison.net> wrote:
> It turns out it's not that straight-forward after all:
>

> http://icanhaz.com/csrfpdf(PDF link, "Robust Defenses for Cross-Site

> Request Forgery")
>
> The above paper introduces the "login CSRF" attack, where CSRF is used
> to force a victim to log in to a site using /the attacker's/
> credentials. The hope is that the user will then enter personally
> sensitive information which the hacker can harvest later on.

This doesn't strike me as too big of a deal - you just need to make
sure to equip your login form with the anti-CSRF token. The attacker
could try to submit his/her token to the server along with his
credentials, but could not tamper with the cookies due to the same-
origin policy. The server will detect the mismatch.

Regarding the interface for this, all of the suggested ones have their
pros and cons. Template tags are nice since there's no change in the
Python code - unless you're not passing the RequestContext that is -
but you need to use the middleware or a decorator + change your
TEMPLATE_CONTEXT_PROCESSORS. All the other ones require you to change
your Form instantiation. I'd also be -1 on coupling the forms with
HttpRequest by default (yeah I've seen Mark Ramm's talk on Youtube ;)

But Luke has a point - if the "unsafe" is the default, there's a good
chance that somewhere something will slip through and the current
middleware IS very convenient in that regard. However, the issues that
Simon raised are still there, and there is a slight performance
penalty to be paid.

Either way, there's something that definitely can be done soon -
improve the docs! The current ones really don't do too much to raise
the awareness of the CSRF threat. Do a search for "csrf" - IMHO it
needs to be mentioned at least in the tutorial and in the forms docs.

Actually, Django might well do with a separate "security" page
describing Web app security issues and how they're dealt with in
Django. One could then have the above pages link to the appropriate
sections in this one.

[Rudolph]

I like Luke's arguments.

A middleware seems like the right place because CSRF protection is
about requests and responses. CSRF protection is more about POST
requests in generic, with HTML forms being a very common type of POST
request.

IMHO the default settings.py file (generated with 'django-admin.py
startproject') should have the middleware enabled by default.

And wouldn't it be possible to enhance the current CSRFMiddleware to
be more flexible, like also work without Django's session middleware?
And add a template tag that inserts the token, for example to be used
in AJAX or forms generated by javascript:
<script type="text/javascript">
var token = '{% csrf_token %}';
</script>

Cheers,

Rudolph

[Brian Beck]

On Sep 25, 2:54 am, Rudolph <rudolph.fro...@gmail.com> wrote:
> I like Luke's arguments.
>
> A middleware seems like the right place because CSRF protection is
> about requests and responses. CSRF protection is more about POST
> requests in generic, with HTML forms being a very common type of POST
> request.
>
> IMHO the default settings.py file (generated with 'django-admin.py
> startproject') should have the middleware enabled by default.

I wouldn't mind keeping the middleware around and enabling it by
default, but including SafeForm in the same app (at
django.contrib.csrf.forms).