Re: html email through from django.core.mail.send_mail and html email for password resets?

2013-07-27 Thread Justin Michalicek
Awesome, thank you.  I am guessing it is preferred that these be done as 
two separate tickets and patches since they are technically separate 
features.  I'm not seeing an obvious way to mark one as relying on another, 
though.  Should I just note in the trac ticket for #2 that it relies on #1 
when I create the tickets, wait for #1 to be accepted and merged then do 
the PasswordResetForm, or just do them both in one feature branch and pull 
request due to the PasswordResetForm change relying on the change to 
send_mail()?

On Saturday, July 27, 2013 11:17:48 AM UTC-4, Justin Michalicek wrote:
>
> I would like to make my first contribution to the Django code, but these 
> both are pretty easy and at least to me seem pretty obvious, so I thought 
> there might be some good reason they have not yet already been done.  
> Depending on the response, I would do these in different ways as well, so I 
> want to check here before getting to work.
>
> 1) Adding an html_message parameter to django.core.mail.send_mail(), the 
> same as mail_admins() and mail_managers() have.  Seems useful and like a 
> good idea, but it's dead simple, so it also seems like it would already be 
> there unless it for some reason had been decided that this is absolutely 
> not going to happen.
>
> 2) Adding the ability for django.contrib.auth.views.password_reset() to 
> take an optional parameter for an html email template as well as 
> django.contrib.auth.forms.
>
> PasswordResetForm.save() so that html password reset emails may be sent.  
> Again, simple enough that I worry that there's a reason this isn't already 
> done that I'm unaware of.
> It is also somewhat dependent on #1 there in that either #1 has to be done 
> first or this has to be done by using EmailMultiAlternatives directly rather 
> than send_mail().
>
> Any input on if I should proceed or if there's some reason I should not touch 
> these would be greatly appreciated.  I'd love to make a contribution to 
> Django and these look
> like a safe, simple, and useful place to start.
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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 http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.




Re: html email through from django.core.mail.send_mail and html email for password resets?

2013-07-27 Thread Russell Keith-Magee
Hi Justin,

On Sat, Jul 27, 2013 at 11:17 PM, Justin Michalicek
wrote:

> I would like to make my first contribution to the Django code, but these
> both are pretty easy and at least to me seem pretty obvious, so I thought
> there might be some good reason they have not yet already been done.
> Depending on the response, I would do these in different ways as well, so I
> want to check here before getting to work.
>
> 1) Adding an html_message parameter to django.core.mail.send_mail(), the
> same as mail_admins() and mail_managers() have.  Seems useful and like a
> good idea, but it's dead simple, so it also seems like it would already be
> there unless it for some reason had been decided that this is absolutely
> not going to happen.
>

I believe the reason for this omission is mostly historical. Once upon a
time, we were trying to avoid building specialist mail APIs, preferring to
encourage people to use Python's native mail facilities. Extensions that
added features to Django's mail APIs were shunned in favour of relying on
native Python functionality.

However, at some point, simple practicality takes over. Given that
mail_admins() and mail_managers() both accept html_message as an argument,
I can't see any good reason why send_mail shouldn't accept the same.


> 2) Adding the ability for django.contrib.auth.views.password_reset() to
> take an optional parameter for an html email template as well as
> django.contrib.auth.forms.
>
> PasswordResetForm.save() so that html password reset emails may be sent.  
> Again, simple enough that I worry that there's a reason this isn't already 
> done that I'm unaware of.
> It is also somewhat dependent on #1 there in that either #1 has to be done 
> first or this has to be done by using EmailMultiAlternatives directly rather 
> than send_mail().
>
> Again, this likely a historical oversight. I can't see any reason that we
shouldn't provide HTML mail as an option here.

The only caveat I would place on both these feature requests is that you
need to be careful about backwards compatibility -- existing functionality
should not change as a result of adding these new features. However, this
should be fairly easy to manage - you just need to make sure that you
provide default arguments that are ignored, and that output for the current
supported cases isn't altered.

> Any input on if I should proceed or if there's some reason I should not touch 
> these would be greatly appreciated.  I'd love to make a contribution to 
> Django and these look
> like a safe, simple, and useful place to start.
>
>
> Indeed -- you've picked two relatively simple extensions to existing APIs.
Make sure you've read Django's contribution guidelines [1], open a ticket
on Trac [2], and submit a patch or pull request. Can't wait to see what you
can contribute!

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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 http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.




Proposal: Modifying the CSRF protection scheme

2013-07-27 Thread Shai Berger
Hi everybody,

TL;DR: A simple change can make Django's CSRF protection a little better; an 
additional, slightly less simple one, can also make it look better.

Django's CSRF protection scheme is a bit unusual; unlike most such schemes, it 
does not rely on a value stored in the server that needs to be matched by a 
submitted token and is replaced with every submission, but rather on a 
constant value stored in a cookie. This generally works (for details of how 
and under what conditions exactly, see [1]), but has two minor problems:

1) It is unusual, and in particular diverges from what OWASP[2] recommends[3]; 
as a result, security analysts often think it is not secure. They have been 
proven wrong in all cases members of core are aware of, but proving it again 
and again is a nuisance, and there may be bad PR related to this.

2) It carries a "second-order" vulnerability: If your site has been 
compromised (XSS, Man-in-the-middle, or server compromise) then you become 
persistently vulnerable to CSRF. All of these vulnerabilities are way worse 
than CSRF and render all CSRF protection schemes worthless while they last;  
the point is *not* that they allow CSRF, but rather that they allow CSRF to be 
performed after the main hole has been plugged. This is because the attacker 
can use the main vulnerability to "steal", or even set, csrftoken cookie 
values, which they can then use later. After a successful attack of this 
magnitude, you need to reset the csrftoken cookies of all users, and this is 
neither obvious nor straightforward to do.

Django's unique scheme does have two advantages over the more common 
solutions, which we would like to keep:

1) It is not tied to sessions, users, or site-stored per-user data, allowing 
CSRF protection to a wider range of users

2) It avoids the problem of having only one "current" token, which causes the 
submission of one form to invalidate forms open in other browser tabs.

To improve on both problem issues, while keeping the advantages, I suggest the 
following modifications:

a) Use a signed cookie for csrftoken -- using Django's existing signing 
facility[4], this means signing the cookie with the SECRET_KEY from the 
settings; so that an attacker cannot set arbitrary cookies, and changing the 
SECRET_KEY after a compromise immeiately invalidates csrftoken cookies.

b) Optionally allowing time-limited CSRF tokens. Such tokens will be generated 
by adding a parameter of maximum age to the csrftoken tag, and by marking view 
methods (specifically with a decorator, or globally with a setting) as 
requiring timed tokens. When this is used, the posted token value will need to 
be different from the cookie value -- to keep advantage 2, the cookie will 
still be constant, and expiry time will only be present in the submitted 
token[5]. This method breaks the current way we do CSRF-protected AJAX, so it 
will likely stay optional (and opt-in).

As you may guess, signing the cookie adds an actual iota of security. Adding 
expiry adds very little -- if an attacker has access to the cookie, they can 
usually just ask the site to generate valid tokens for them, so getting any 
real protection will require annoyingly short expiry times. But the fact that 
an attacker needs this extra step makes it a tiny bit harder for them and 
makes their actions a tiny bit more detectable; and having a constantly-
changing CSRF token may make the whole thing look a little better to naive 
analysts.

I had some help and guidance in drafting this proposal -- you can credit 
Donald Stufft, mostly, for any egregious blunder I didn't make. I am still 
responsible for the ones I did make.

Your comments are welcome,

Shai.


[1]  https://docs.djangoproject.com/en/dev/ref/contrib/csrf/ -- in particular, 
"how it works" and "limitations"

[2] https://www.owasp.org

[3] 
https://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet

[4] https://docs.djangoproject.com/en/dev/topics/signing/

[5] django.core.signing.TimestampSigner signs content with the time of the 
signature, and then takes a max_age in its unsign() method; the suggested 
method would go the other way around, timestamping the token with the time of 
expiry, to allow checking without using data stored on the server (and to 
allow different forms to use different max-age values).

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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 http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [GSoC] Revamping validation framework and merging django-secure once again

2013-07-27 Thread Shai Berger
On Thursday 25 July 2013 08:37:06 Russell Keith-Magee wrote:
> 
> Could I get you to open this as a ticket so that it isn't forgotten?
> 
https://code.djangoproject.com/ticket/20814

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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 http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.