(optional) CSRF protection in zope.formlib
Hi, I've been working on CSRF protection for zope.formlib. I have a "csrfprotection" branch in my zope.formlib fork on github. The changes against the current zope.formlib mainline can be found here: https://github.com/janwijbrand/zope.formlib/compare/csrfprotection When creating form components based on zope.formlib.form.FormBase, one can enable this protection just by setting the attribute ``protected`` to True on the component. This implementation is based on the following assumptions: * We do not want to keep server-side state(!) * An "attacker" that attempts CSRF cannot get to information stored in cookies that are meant for the domain of the (forged) request. * The token stored in the cookie is sufficiently random and long, to be practically "unguessable" by the attacker. * The form submit is deemed valid as long as the token in the cookie is identical to a hidden input value that is part of the form submit. My questions: * Do you find this feature useful enough to be, in principle, included in zope.formlib? * I'd like to kindly request someone to review my branch and provide feedback. The included test cases describe a few more questions and concerns about this implementation. Thank you in advance! kind regards, jw
Hi Jan-Wij, +1 for implementing convenient CSRF. I wonder if you could make your implementation more orthogonal by implementing a CSRF "field/widget", and make your `protected` attribute simply trigger the inclusion of this field implicitly. This way you wouldn't need to change the `*pageform.pt` templates like you do now, and `setupToken()`/`checkToken()` would move to the widget code. Cheers, Leo On Wed, Sep 18, 2013 at 11:41 AM, Jan-Wijbrand Kolman <janwijbrand@gmail.com
wrote:
Hi,
I've been working on CSRF protection for zope.formlib.
I have a "csrfprotection" branch in my zope.formlib fork on github. The changes against the current zope.formlib mainline can be found here:
When creating form components based on zope.formlib.form.FormBase, one can enable this protection just by setting the attribute ``protected`` to True on the component.
This implementation is based on the following assumptions:
* We do not want to keep server-side state(!)
* An "attacker" that attempts CSRF cannot get to information stored in cookies that are meant for the domain of the (forged) request.
* The token stored in the cookie is sufficiently random and long, to be practically "unguessable" by the attacker.
* The form submit is deemed valid as long as the token in the cookie is identical to a hidden input value that is part of the form submit.
My questions:
* Do you find this feature useful enough to be, in principle, included in zope.formlib?
* I'd like to kindly request someone to review my branch and provide feedback.
The included test cases describe a few more questions and concerns about this implementation.
Thank you in advance!
kind regards, jw
______________________________**_________________ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/**listinfo/zope-dev<https://mail.zope.org/mailman/listinfo/zope-dev> ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/**listinfo/zope-announce<https://mail.zope.org/mailman/listinfo/zope-announce> https://mail.zope.org/mailman/**listinfo/zope<https://mail.zope.org/mailman/listinfo/zope>)
On 9/18/13 5:26 PM, Leonardo Rochael Almeida wrote:
+1 for implementing convenient CSRF.
I wonder if you could make your implementation more orthogonal by implementing a CSRF "field/widget", and make your `protected` attribute simply trigger the inclusion of this field implicitly.
This way you wouldn't need to change the `*pageform.pt <http://pageform.pt>` templates like you do now, and `setupToken()`/`checkToken()` would move to the widget code.
I've considered and experimented with that approach. However, as soon as you do more complex things with setting up fields in your own form component, things potentially get hairy. Furthermore, the form machinery tries to get values from the context object (in edit forms for example), for each field and tries to set values for this field on the context object when handling the submit. This would make handling this field special in way I didn't like. But yes, the compromise in my implementation is, that you need to render the hidden input field "yourself" if you overwrite the default templates - and you most probably do. For example, grok.formlib does bring its own "default" templates for forms. I'd need to update that package in case this implementation is accepted and lands. regards, jw
On 9/18/13 4:41 PM, Jan-Wijbrand Kolman wrote:
I've been working on CSRF protection for zope.formlib.
Anyone else interested in this feature? We made an internal release of zope.formlib with this functionality and so far we found no issues in using it in production. After updating zope.formlib's tests to also run on Python 3, I could merge this feature soon. Unless of course, someone objects :-) kind regards, jw
Hi, I'd like to tie a loose end and have this PR merged: https://github.com/zopefoundation/zope.formlib/pull/4 Without objections, I'll do so somewhere today or tomorrow. regards, jw
On 11/18/13 12:23 PM, Jan-Wijbrand Kolman wrote:
I'd like to tie a loose end and have this PR merged:
https://github.com/zopefoundation/zope.formlib/pull/4
Without objections, I'll do so somewhere today or tomorrow.
Merged PR and about to create a release. regards, jw
On 11/20/13 1:40 PM, Jan-Wijbrand Kolman wrote:
On 11/18/13 12:23 PM, Jan-Wijbrand Kolman wrote:
I'd like to tie a loose end and have this PR merged:
https://github.com/zopefoundation/zope.formlib/pull/4
Without objections, I'll do so somewhere today or tomorrow.
Merged PR and about to create a release.
If there're no objections to it, could someone grant me release rights for the zope.formlib package on pypi? My handle is "jw". Thanks! regards, jw
2013/11/20 Jan-Wijbrand Kolman <janwijbrand@gmail.com>
On 11/20/13 1:40 PM, Jan-Wijbrand Kolman wrote:
On 11/18/13 12:23 PM, Jan-Wijbrand Kolman wrote:
I'd like to tie a loose end and have this PR merged:
https://github.com/zopefoundation/zope.formlib/pull/4
Without objections, I'll do so somewhere today or tomorrow.
Merged PR and about to create a release.
If there're no objections to it, could someone grant me release rights for the zope.formlib package on pypi? My handle is "jw".
Granted.
Thanks!
regards, jw
On 11/20/13 7:03 PM, Gediminas Paulauskas wrote:
If there're no objections to it, could someone grant me release rights for the zope.formlib package on pypi? My handle is "jw".
Granted.
Thanks! Released as 4.3.0a2 regards, jw
participants (3)
-
Gediminas Paulauskas -
Jan-Wijbrand Kolman -
Leonardo Rochael Almeida