Re: SecureForm in newforms

2008-04-19 Thread Simon Willison

On Apr 17, 5:04 pm, "Jeremy Dunck" <[EMAIL PROTECTED]> wrote:
> Middleware is easy to set and forget.  Is there a reason not to make
> SecureForm the default, and InsecureForm for people using Ajax?  ;-)

I'm pretty sure we can handle the Ajax case by not doing CSRF token
checks if request.is_ajax() is True. As far as I can tell there's no
way to make a CSRF attack with a custom "X-Requested-By:
XMLHttpRequest" HTTP header (unless your application has an XSS hole,
in which case CSRF tokens are useless anyway).

I've been thinking about this a LOT recently. Here are a few notes:

1. You only need a SafeForm (I like that term as it has similarity to
the "safe" temple filter) if you are dealing with a POST /and/ your
functionality is restricted to identified users in some way (generally
users that have an active session).
2. The constructor for a bound form needs to take the request object,
not an explicit request.POST. This is because the CSRF token
validation will need access to the request session or cookies in order
to confirm that the returned token is valid for the current user.
Since SafeForm is only intended for request.POST I see no problem in
just taking the request object:

bound_form = MySafeForm(request)

3. This is the point I'm stuck on: should the implementation add a new
hidden form field to the field definition or insert it in some other
way? I'd like to make everything completely transparent to anyone
using SafeForm, so I'd rather not even add a new field to form.fields
or form.base_fields.
4. How does Django know where to put the hidden field in the generated
HTML? Do we force people to add {{ form.csrf_token }} somewhere in
their form HTML? The alternative would be to add {% form ... %} and {%
endform %} template tags which replace explicit  tags and ensure
a CSRF token is included. Added bonus: if our form object has
knowledge of its action="" attribute we can include that in the
generation of the CSRF token, thus reducing the damage possible if a
token leaks to an attacker somehow (because a leaked token will be
valid for a specific user, action URL and time period).
5. There's a UI design problem here: what message do we show people if
they trip our CSRF detection? If our tokens are limited to a 24 hour
period for example then there's a chance someone might open a page in
a browser tab, then actually submit the form more than 24 hours later.
We need to show a regular form validation error along the lines of
"Please re-submit the form." This introduces another problem: where do
we display this error? Do we have to ensure people remember to add
{{ form.non_field_errors }} to their template (which they'll certainly
forget to do). Again, having a {% form %} tag helps here as it gives
us somewhere to cram the error in.

Cheers,

Simon
--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: SecureForm in newforms

2008-04-17 Thread Luke Plant


On Thu, 17 Apr 2008 09:21:18 -0700 (PDT), "mrts" <[EMAIL PROTECTED]>
said:
> 
> > Middleware is easy to set and forget.  Is there a reason not to make
> > SecureForm the default, and InsecureForm for people using Ajax?  ;-)
> 
> Doesn't fit my workflow. In my case some POSTs are handled internally
> by other libraries (think OpenID).

I don't like the idea that CrsfMiddleware is going to be replaced by
SecureForm, precisely for this reason -- that POSTs are often not going
to be handled by forms, so even if it is possible to always inherit from
SecureForm, you still have to remember to handle other POSTs.  I much
prefer the current 'secure by default', like we have with auto-escaping.

So it seems to be that what we need first of all is the option to have
'secure by default, but with exceptions', again like auto-escape.  That
would require some setting for excluding certain paths/views from the
CsrfMiddleware, or decorators on views that signal 'don't CSRF protect
this' (I prefer the latter, if implementation isn't too bad).

Yes, I agree that CsrfMiddleware is a bit scary, but other security
measures are also somewhat hairy (autoescape is not particularly simple
in implementation, or even in usage).  Actually, the only part of
CsrfMiddleware that is scary is the auto-inserting of the
csrfmiddlewaretoken into the output.  Perhaps it could be changed to not
do this, and instead provide some interfaces for the developer to get
and insert the token (e.g. get_csrf_token() and {% csrf_token %} ). 
SecureForm would include this by default, in other situations you might
want to manually include it.  If you forget, and you haven't excluded
your view from CsrfMiddleware, you will get an obvious error, but not a
vulnerability.

Luke
-- 
Luke Plant - L.Plant.98 at cantab.net


--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: SecureForm in newforms

2008-04-17 Thread mrts

> Middleware is easy to set and forget.  Is there a reason not to make
> SecureForm the default, and InsecureForm for people using Ajax?  ;-)

Doesn't fit my workflow. In my case some POSTs are handled internally
by other libraries (think OpenID).
--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: SecureForm in newforms

2008-04-17 Thread Jeremy Dunck

On Thu, Apr 17, 2008 at 11:00 AM, Jacob Kaplan-Moss
<[EMAIL PROTECTED]> wrote:
>
>  On Thu, Apr 17, 2008 at 10:08 AM, mrts <[EMAIL PROTECTED]> wrote:
>  >  This is cumbersome and error-prone, thus I propose that a SecureForm
>  >  or CSRFSecureForm be added to newforms that would automate the steps
>  >  given above (like CsrfMiddleware does).
>
>  Agreed -- I was just talking with Simon the other day about adding a
>  SecureForm to django.contrib.csrf, and perhaps even de-emphasizing the
>  middleware (which is a bit scary, frankly) in favor of the more
>  explicit form.
>
>  Jacob

Middleware is easy to set and forget.  Is there a reason not to make
SecureForm the default, and InsecureForm for people using Ajax?  ;-)

--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: SecureForm in newforms

2008-04-17 Thread Jacob Kaplan-Moss

On Thu, Apr 17, 2008 at 10:08 AM, mrts <[EMAIL PROTECTED]> wrote:
>  This is cumbersome and error-prone, thus I propose that a SecureForm
>  or CSRFSecureForm be added to newforms that would automate the steps
>  given above (like CsrfMiddleware does).

Agreed -- I was just talking with Simon the other day about adding a
SecureForm to django.contrib.csrf, and perhaps even de-emphasizing the
middleware (which is a bit scary, frankly) in favor of the more
explicit form.

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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---