An addendum to the escaping proposals
James Bennett
proposal, I'd like to suggest we also just go ahead and turn on the
CSRF middleware by default, because apparently the fact that it isn't
enabled by default is leading people to scream about security
vulnerabilities in Django[1], which in turn causes package maintainers
to email me because the existence of a CVE makes it appear that
there's an actual problem here (as opposed to the actual report[2]
which concludes that users should enable the CSRF middleware, and
whose author -- I thought -- concluded that there was no problem after
being pointed in the direction of that middleware).
[1] http://nvd.nist.gov/nvd.cfm?cvename=CVE-2007-5828
[2] http://www.securityfocus.com/archive/1/482983/100/0/threaded
--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."
SmileyChris
On Nov 8, 6:08 am, "James Bennett" <ubernost...@gmail.com> wrote:
> If/when we ever do finally sit down and implement an auto-escaping
> proposal, I'd like to suggest we also just go ahead and turn on the
> CSRF middleware by default
Why do those two have to be tied together?
How about we just default the CSRF middleware to on now?
Malcolm Tredinnick
> If/when we ever do finally sit down and implement an auto-escaping
> proposal, I'd like to suggest we also just go ahead and turn on the
> CSRF middleware by default, because apparently the fact that it isn't
> enabled by default is leading people to scream about security
> vulnerabilities in Django[1], which in turn causes package maintainers
> to email me because the existence of a CVE makes it appear that
> there's an actual problem here (as opposed to the actual report[2]
> which concludes that users should enable the CSRF middleware, and
> whose author -- I thought -- concluded that there was no problem after
> being pointed in the direction of that middleware).
I have quite a different view about the severity of this. Out of the
box, Django has a CSRF vulnerability if you use admin. I'm not inclined
to dismiss this as a nothing event that you can work around by somehow
magically divining that you need to include an optional package if
you're using admin.
Now, it's very disappointing that the original reported to SecurityFocus
didn't report it to us first, but that only makes the original reporter
irresponsible and unprofessional, not wrong.
I do agree with Chris, though. It's completely unrelated to
auto-escaping (which will land today, most likely, since I've been
merging it and updating it yesterday and the day before).
Not sure if we should build it into admin or make the middleware a
requirement for admin, but this isn't a "dismiss it with a wave of the
hand" situation for me.
>
> [1] http://nvd.nist.gov/nvd.cfm?cvename=CVE-2007-5828
> [2] http://www.securityfocus.com/archive/1/482983/100/0/threaded
Regards,
Malcolm
--
Telepath required. You know where to apply...
http://www.pointy-stick.com/blog/
James Bennett
> I have quite a different view about the severity of this. Out of the
> box, Django has a CSRF vulnerability if you use admin. I'm not inclined
> to dismiss this as a nothing event that you can work around by somehow
> magically divining that you need to include an optional package if
> you're using admin.
Well, I've been thinking about it since we got the original report (we
*did* get a report well before the SecurityFocus post, though it
didn't quite make it through the official channels), and I'm not
convinced it's of high severity. The number of things an attacker has
to know or get right by chance, all at the same time, in order to
mount a successful exploit, is something of a barrier to making it
practical. And even if the attacker *does* manage it, there's no
chance of compromising the user account whose password was changed,
because the attacker doesn't obtain the username in the course of
exploiting this; in order to obtain that, the attacker has to resort
to some other means which likely would have compromised the account
anyway, which renders the CSRF moot.
Which means that this basically boils down to an annoyance attack,
changing a user's password without their knowledge. But that's already
exposed to anyone who can guess the user's email address, so anyone
who simply wants to cause this sort of mischief already has a much
easier route to accomplish it.
> Now, it's very disappointing that the original reported to SecurityFocus
> didn't report it to us first, but that only makes the original reporter
> irresponsible and unprofessional, not wrong.
Again, it was reported to us, but when the reporter was made aware of
the CSRF middleware he seemed to think that the middleware made the
whole thing moot. Which is why I am a bit surprised at the
SecurityFocus post and the CVE candidate.
At any rate, this conflates multiple issues, which is always
problematic: the original reporter suggested that we should begin
enabling the CSRF middleware by default, and I'm starting to lean that
way, especially since it seems we're going to start enabling output
escaping by default. I bring them up together because it seems that if
we're going to change Django to mitigate one common class of security
problem out of the box, we should probably cover the other big obvious
one while we're at it.
James Bennett
> Which means that this basically boils down to an annoyance attack,
> changing a user's password without their knowledge. But that's already
> exposed to anyone who can guess the user's email address, so anyone
> who simply wants to cause this sort of mischief already has a much
> easier route to accomplish it.
Sent too soon; I was going to explain that this comes up in the
password reset view, which simply accepts an email address and resets
the account(s) associated with it.
Malcolm Tredinnick
> On Nov 7, 2007 7:08 PM, James Bennett <ubern...@gmail.com> wrote:
> > Which means that this basically boils down to an annoyance attack,
> > changing a user's password without their knowledge. But that's already
> > exposed to anyone who can guess the user's email address, so anyone
> > who simply wants to cause this sort of mischief already has a much
> > easier route to accomplish it.
>
> Sent too soon; I was going to explain that this comes up in the
> password reset view, which simply accepts an email address and resets
> the account(s) associated with it.
The fact that that is also broken (which is very much my view) doesn't
the other problem, though.
Malcolm
--
Quantum mechanics: the dreams stuff is made of.
http://www.pointy-stick.com/blog/
Amit Upadhyay
I just switched on the middleware and all my ajax post actions are broken now. I am going to dive in the source and try to figure out how to generate enough token that I can embed in the page on page load, so that all my requests go fine. I would appreciate some tips if someone can tell me if I can reuse the token or I will have to create more than one. Also if the token expire after sometime, that I have to worry about.
--
Amit Upadhyay
Vakow! www.vakow.com
+91-9820-295-512
Gary Wilson
> I do agree with Chris, though. It's completely unrelated to
> auto-escaping (which will land today, most likely, since I've been
> merging it and updating it yesterday and the day before).
w00t!
> Not sure if we should build it into admin or make the middleware a
> requirement for admin, but this isn't a "dismiss it with a wave of the
> hand" situation for me.
By build it into admin, do you mean build it into newforms?
Possibly changing BaseForm from:
class BaseForm(StrAndUnicode):
def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
initial=None, error_class=ErrorList, label_suffix=':'):
to:
class BaseForm(StrAndUnicode):
def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
initial=None, error_class=ErrorList, label_suffix=':',
request=None, csrf_token=True):
When csrf_token is True, a CSRFTokenField formfield is added to the form, and
gets displayed along with the form's other fields. The field's clean method
would ensure that the correct token came back.
The csrf middleware components could be factored out for use in both the forms
and in the middleware.
Thoughts?
Gary
Malcolm Tredinnick
On Sat, 2007-11-10 at 11:14 -0600, Gary Wilson wrote:
> Malcolm Tredinnick wrote:
> > I do agree with Chris, though. It's completely unrelated to
> > auto-escaping (which will land today, most likely, since I've been
> > merging it and updating it yesterday and the day before).
>
> w00t!
For values of "today" that mean "as soon as the real world stops
interfering". :-)
>
> > Not sure if we should build it into admin or make the middleware a
> > requirement for admin, but this isn't a "dismiss it with a wave of the
> > hand" situation for me.
>
> By build it into admin, do you mean build it into newforms?
[...]
> The csrf middleware components could be factored out for use in both the forms
> and in the middleware.
Yeah, I'm not really sure what I mean, design-wise. I feel a little
uncomfortable about requiring the csrf key all the time in form
submissions, but I can't pin down why yet. As a consequence of that, the
middleware doesn't quite do the trick for me, because it's always on
(you can't say "don't touch this form, I'm handling it manually").
I haven't had time to think about this too much yet, but your approach
doesn't look too bad. Introducing request into BaseForm is a little
annoying, orthogonality-wise. I realise why it's needed -- we need the
session to get the token -- but it's annoying me at the moment. Like I
said, though, haven't thought about it enough to sound coherent yet. I
feel we should be doing something without over-reacting; not sure where
the middle ground is, though.
Malcolm
--
A conclusion is the place where you got tired of thinking.
http://www.pointy-stick.com/blog/
Chris Green
> Yeah, I'm not really sure what I mean, design-wise. I feel a little
> uncomfortable about requiring the csrf key all the time in form
> submissions, but I can't pin down why yet. As a consequence of that, the
> middleware doesn't quite do the trick for me, because it's always on
> (you can't say "don't touch this form, I'm handling it manually").
I think the use case for when you don't want CSRF protection is when
you are trying to encourage someone to send you POSTS. Think a
"google search engine form" on your own home page where you are
implementing the "google" part or perhaps a piece of software that
posts to home regarding an error condition.
Robert Coup
Another use case - AJAX behaviours where a page sends the django app multiple post requests without having a "form" in the html page. I process ajax requests via Newforms like any other post, so having it as part of that would be nicer imho, and allow disabling it.
Rob :)
Luke Plant
> By build it into admin, do you mean build it into newforms?
>
> Possibly changing BaseForm from:
>
> class BaseForm(StrAndUnicode):
> def __init__(self, data=None, files=None, auto_id='id_%s',
> prefix=None, initial=None, error_class=ErrorList, label_suffix=':'):
>
> to:
>
> class BaseForm(StrAndUnicode):
> def __init__(self, data=None, files=None, auto_id='id_%s',
> prefix=None, initial=None, error_class=ErrorList, label_suffix=':',
> request=None, csrf_token=True):
>
> When csrf_token is True, a CSRFTokenField formfield is added to the
> form, and gets displayed along with the form's other fields. The
> field's clean method would ensure that the correct token came back.
>
> The csrf middleware components could be factored out for use in both
> the forms and in the middleware.
>
> Thoughts?
A problem with this is that it requires more vigilance on the part of
the developer -- they have to remember to call the clean() method,
directly or indirectly. With the current middleware, a CSRF attack
simply never gets as far as the view function.
There are alternative ways to address some of the other issues mentioned
with respect to the current middleware. You could add some special
flag or response header telling the middleware not to mess with it on
the way out, and have some kind of exception list for incoming
requests. You could possibly even implement it as a decorator:
@nocsrfprotection
def my_view(request):
# blah
nocsrfprotection() would decorate the decorate the function with a flag
that is recognised by the middleware, and decorate its output as well
in some way. You would have to change the middleware so that it does
its 'rejection' business in process_view() instead of
process_request() -- it would check the view for the flag, and require
the CSRF token if it wasn't found.
To me, this approach seems nicer than including stuff explicitly in
forms or views -- there is too much room for developer error if the
request has actually got as far as a view function.
Luke
--
"I washed a sock. Then I put it in the dryer. When I took it out, it
was gone." (Steven Wright)
Luke Plant || http://lukeplant.me.uk/
SmileyChris
> You would have to change the middleware so that it does
> its 'rejection' business in process_view() instead of
> process_request() -- it would check the view for the flag, and require
> the CSRF token if it wasn't found.
>
> To me, this approach seems nicer than including stuff explicitly in
> forms or views -- there is too much room for developer error if the
> request has actually got as far as a view function.
Agreed. I like this solution.
Also, I think it would be good to have access to the CSRF token on the
context (via a context processor) - this way you can easily stuff your
ajax methods with the token too, if need be.