Notes about caching pages with a CSRF Form #4141

ricardclau · 2014-08-17T13:33:05Z

Q	A
Doc fix?	no
New docs?	no
Applies to	all
Fixed tickets	#1216

wouterj · 2014-08-17T13:56:01Z

book/forms.rst

+    In Symfony, the default CSRF token provider uses the PHP session id to
+    generate the actual token. This is why you should not try to cache pages
+    with forms including this protection. For more information, see the
+    cookbook ":doc:`/cookbook/security/csrf_in_login_form`"


please use references to link to the exact section

ricardclau · 2014-08-17T15:13:29Z

Fixed as per @wouterj comments

ricardclau · 2014-08-17T15:14:43Z

cookbook/security/csrf_in_login_form.rst

+the actual token with this piece of code::
+
+    // src/Symfony/Component/Form/Extension/Csrf/CsrfProvider/DefaultCsrfProvider.php
+    // ...


@wouterj Is this following the current doc standards? I have seen some different ways of doing it

weaverryan · 2014-08-19T21:53:18Z

cookbook/cache/form_csrf_caching.rst

+.. note::
+
+    A `Security CSRF Component`_ was introduced in Symfony 2.4 which provides
+    a class CsrfTokenManager for generating and validating CSRF tokens


Someone may have said to put this in, but I don' think it adds anything here.

Well, I did not know this new component existed until @stof mentioned it, but happy to take it out

well, I think the doc is missing the fact that the symfony/security component is actually distributed as 4 components now (symfony/security replacing them all for BC reasons). But this is a separate topic

I think this component splitting deserves a nice book entry in the components section.

What do you want to do with this cookbook?
Maybe we can remove this note now and add a link when this piece of doc is added, or perhaps we can rephrase it some way?

Unless you want to spend some time on the extension of the book chapter, I'd finish this pull request first. This can then be reworded or reorganised later on when someone is working on #4151.

👍 removing it for now then

ricardclau · 2014-08-20T21:51:25Z

So, the only thing missing would be clarifying if Symfony HTTPCache works like Varnish or not and in case it does not, rephrase that part a bit, right? @weaverryan @wouterj @stof @xabbuh

dbu · 2014-08-25T01:41:44Z

cookbook/cache/form_csrf_caching.rst

+via an uncached AJAX request but cache the rest of the HTML response.
+
+Or you can even load just the CSRF token with an AJAX request and replace the
+form field value with it.


would you have an example for this? it would mean not rendering the csrf hidden form element, and do an ajax call to retrieve the current csrf token?

Well, either get the full form markup, just the hidden field markup or even just the token string via an uncached AJAX request.

It may be tricky to explain it with words and make it clear for someone who is not familiar with all these concepts...

i think the symfony code example would be helpful. how do i render the form (no form_rest()?) and how do i render just the csrf for the ajax call? and how is this handled on the post, do we then keep the session cookie so the form layer just sees the csrf token automatically?

i think a closely related problem to what we discuss here is how to make sure a session is only present on requests that actually need it (csrf ajax request). is there a way on symfony side to prevent the session from being started, and to unset the cookie right away after successful form submission? or do you use varnish config with knowledge of which url needs or needs not a cookie? (the later would seem more error prone to me)

once merged, we should also link to #4661 - this PR and #4661 are pieces of the same puzzle

I'm ok with merging this without the example for now - we've given them 3 options, and I actually think the first 2 are probably the best (include the form in an un-cached ESI or load it via AJAX entirely).

Added the latest comments and tweaks and rebased with 2.3, anything else missing @weaverryan @dbu ?

dbu · 2014-08-25T01:51:17Z

regarding the built-in cache, there is this in the book

private_headers: Set of request headers that trigger "private" Cache-Control behavior on responses
that don't explicitly state whether the response is public or private via a Cache-Control directive.
(default: Authorization and Cookie);

so for our purposes, they work the same i would assume. private means the content is not to be cached

dbu · 2014-08-25T01:55:54Z

btw, this also relates to #3881 . we talk a lot about removing cookies in varnish - but this is always very hackish (the application knows when to cache or not, varnish should not know about specific urls). is there a way to have symfony only start a session when its actually needed e.g. for a form, and remove the cookie as soon as its not needed anymore? if so we should talk about that here, rather than propose to hack around in vcl.

for admin areas and the like, the path option of the session cookie should be used to only receive a cookie for that area.

i don't want to explode the scope of this PR - we can also open a new issue about that and work on it once this PR has been merged.

weaverryan · 2014-09-15T18:35:20Z

@dbu I'm very interested in your idea, because this is hard, and I don't like that :).

I don't remember if Varnish decides to not cache a response because the request has cookies attached to it or if the response has a set-cookie header. It's probably the latter, in which case your idea definitely has some merit! But I'm also not sure about things like google analytics headers - we would want these sent back in the response, but would not want Varnish to ignore caching because of them.

Thanks for the conversation - this stuff makes my head spin a bit... and I'm probably not alone ;).

dbu · 2014-09-16T04:32:16Z

varnish by default does not cache when the request has any cookie, as the cookie could have an effect on the response, and it certainly does not cache the set-cookie header response as that would be stupid most of the time. both can be overwritten in vcl when you really know what you are doing.

the google analytics case is a standard example of using vcl to fix up the request: https://www.varnish-cache.org/docs/4.0/users-guide/increasing-your-hitrate.html#cookies

the question for the doc in this case would be how far we want to duplicate varnish config. i guess the symfony doc must avoid becoming kind of a general varnish doc, but focus on symfony specific topics, and how to tailor symfony to optimally interact with varnish. (eg. not starting sessions when not needed). but we are disgressing from the content of this PR. ryan, do you want to collect all varnish related shortcomings of the doc in a new issue so we can work on that and make sure this PR can get merged without ending up in a major doc refactoring?

dbu · 2014-09-16T04:33:58Z

cookbook/cache/form_csrf_caching.rst

+them because the expected token is stored in their session and different
+for each user.
+
+How to Cache most of the Page and still Be Able to Use CSRF Protection


upper/lower case of this title is off. afaik in english everything in title is uppercase

Looks good. We decided to uppercase only non-close-class words. Though maybe we can find a headline that is a bit shorter and describes the following section as good as the current one.

Hm, actually I think "most" has to start with an uppercase "M" though.

dbu · 2014-12-10T15:47:24Z

any chance to wrap this up?

ricardclau · 2014-12-12T10:20:03Z

It's been a while since this discussion happened, so we should refresh the missing bits

The text explanation looks ok to me, what sort of coding example (if any) would we need to add? I remember @weaverryan wanted to do some further research, not sure if he found some time to do it

dbu · 2014-12-12T14:13:54Z

i would love to have an example how that ajax call to get the csrf would look: #4141 (comment) : controller that provides the token, js puts it into the field i guess?, how does that then work when submitting the form?

timglabisch · 2014-12-18T21:25:56Z

book/forms.rst

@@ -1765,6 +1765,13 @@ section.
    The ``intention`` option is optional but greatly enhances the security of
    the generated token by making it different for each form.

+.. caution::
+
+    CSRF tokens are meant to be different for every user. This is why you


for every request?

no. If the same user (user being related to a session here, so the same guy using 2 browsers will count as 2 users) is doing 2 requests, the same token will be used (for a given token id). Otherwise it would be impossible to check tokens later

ricardclau · 2014-12-29T12:39:11Z

The idea would be to have the form disabled in the cached page and have the javascript method enable the form once the token is retrieved from the server.

Plus, the user should be aware when using {{ form_rest() }} or similar functions in the page generation and check the output because this can potentially screw the whole procedure.

Anyway, I honestly think that unless you are youporn or have similar traffic you won't get much benefit on caching these pages. And it can be tricky for a newbie to understand all these concepts.

I don't have much time to provide a full working example, but anyone feel free to pick it up.

timglabisch · 2014-12-29T16:00:02Z

i think it's a good read even without the example. if you need such caching stragegies, it's more important to get ideas how to cache, instead of copy&paste a fully working example.

dbu · 2014-12-31T11:54:32Z

cookbook/cache/form_csrf_caching.rst

+For more information about how this protection works in Symfony, please
+check :ref:`CSRF Protection <forms-csrf>`.
+
+Why Varnish does not Cache these Pages by Default


this is about caching in general. this also applies to the symfony HttpCache and to any sane caching reverse proxy.

dbu · 2014-12-31T12:04:42Z

i agree that it does not need to be a complete ready-made thing you can just copy-paste. but seeing the code examples on server side what to do about form_rest, and how to provide the correct csrf token in the ajax call would make this a lot more applicable. the details will vary, but the basic things should be possible to explain here. otherwise everybody doing this has to dig it up himself and might end up with dangerous or hacky solutions.

weaverryan · 2015-01-03T19:37:47Z

cookbook/cache/form_csrf_caching.rst

+CSRF - or `Cross-site request forgery`_ - is a method by which a malicious
+user attempts to make your legitimate users unknowingly submit data that
+they don't intend to submit. Fortunately, CSRF attacks can be prevented by
+using a CSRF token inside your forms.


I don't think we should repeat this here. Instead, we should put something similar to this: https://github.com/symfony/symfony-docs/pull/4141/files#diff-da27b730a6f0f3e3f23c45088b6113a0R1770 and tell them that CSRF is going to cause issues when caching, hence this article.

weaverryan · 2015-01-03T19:48:21Z

@ricardclau I've just added a few minor notes. For me, with a little re-wording here and there (see my comments), this is ready to go :).

Thanks!

This PR was merged into the 2.3 branch. Discussion ---------- Notes about caching pages with a CSRF Form | Q | A | ------------- | --- | Doc fix? | no | New docs? | no | Applies to | all | Fixed tickets | #1216 Commits ------- 1bc7ef2 cache_csrf_form

weaverryan · 2015-01-04T22:31:32Z

Thanks Ricard! I opened #4772 with a few more tweaks to language, but I think the "guts" are solid here.

Cheers!

ricardclau · 2015-01-04T22:36:51Z

👍 @weaverryan

This PR was merged into the 2.3 branch. Discussion ---------- Tweaks to the new form csrf caching entry | Q | A | ------------- | --- | Doc fix? | no | New docs? | no | Applies to | all | Fixed tickets | n/a Hi guys! After merging this nice new entry in #4141, I wanted to make a few minor language tweaks (the diff looks bigger than the changes really are). This included shortening a few sections and talking less about how *all* reverse proxies refuse to cache pages with a session, because I don't (for example) believe this is true with Symfony's reverse proxy. Thanks! Commits ------- cc40b5c Adding missing words thanks to javiereguiluz 1c568e1 [#4141] Tweaks to the new form csrf caching entry

* 2.3: [#4857] Adding missing word thanks to xabbuh Fixing bad english thanks to xabbuh Adding missing words thanks to javiereguiluz [#4643] Minor english changes to make things even smoother (though they were correct before) replace API link for SwiftmailerBundle Update security.rst Update routing.rst don't output message from AuthenticationException Add custom link labels where Cookbook articles titles looked wrong Removed a leftover comma in security config sample [#4141] Tweaks to the new form csrf caching entry How to override vendor directory location - fix How to override vendor directory location - fix How to override vendor directory location

* 2.5: [#4857] Adding missing word thanks to xabbuh Fixing bad english thanks to xabbuh Adding missing words thanks to javiereguiluz [#4643] Minor english changes to make things even smoother (though they were correct before) replace API link for SwiftmailerBundle Update security.rst Update routing.rst don't output message from AuthenticationException Add custom link labels where Cookbook articles titles looked wrong Fix code example Removed a leftover comma in security config sample [#4141] Tweaks to the new form csrf caching entry How to override vendor directory location - fix How to override vendor directory location - fix How to override vendor directory location

wouterj reviewed Aug 17, 2014
View reviewed changes

wouterj added the In progress label Aug 17, 2014

ricardclau reviewed Aug 17, 2014
View reviewed changes

ricardclau force-pushed the cache_csrf_form branch 2 times, most recently from 68be3b3 to 3c66d5c Compare August 19, 2014 21:52

weaverryan reviewed Aug 19, 2014
View reviewed changes

dbu reviewed Aug 25, 2014
View reviewed changes

dbu mentioned this pull request Aug 25, 2014

Incomplete cookbook entry about Varnish + ESI ? #3881

Closed

dbu reviewed Sep 16, 2014
View reviewed changes

timglabisch reviewed Dec 18, 2014
View reviewed changes

dbu mentioned this pull request Dec 29, 2014

Varnish cookbook session cookie handling #4628

Merged

dbu reviewed Dec 31, 2014
View reviewed changes

weaverryan reviewed Jan 3, 2015
View reviewed changes

cache_csrf_form

1bc7ef2

ricardclau force-pushed the cache_csrf_form branch from 1936cff to 1bc7ef2 Compare January 4, 2015 19:48

weaverryan merged commit 1bc7ef2 into symfony:2.3 Jan 4, 2015

weaverryan added a commit that referenced this pull request Jan 4, 2015

[#4141] Tweaks to the new form csrf caching entry

1c568e1

weaverryan mentioned this pull request Jan 4, 2015

Tweaks to the new form csrf caching entry #4772

Merged

Notes about caching pages with a CSRF Form #4141

Notes about caching pages with a CSRF Form #4141

Conversation

ricardclau commented Aug 17, 2014

Choose a reason for hiding this comment

ricardclau commented Aug 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricardclau commented Aug 20, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbu commented Aug 25, 2014

dbu commented Aug 25, 2014

weaverryan commented Sep 15, 2014

dbu commented Sep 16, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbu commented Dec 10, 2014

ricardclau commented Dec 12, 2014

dbu commented Dec 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricardclau commented Dec 29, 2014

timglabisch commented Dec 29, 2014

Choose a reason for hiding this comment

dbu commented Dec 31, 2014

Choose a reason for hiding this comment

weaverryan commented Jan 3, 2015

weaverryan commented Jan 4, 2015

ricardclau commented Jan 4, 2015