Hi Jon, Thanks for your extensive reply.
I understand the use case you describe and I agree it gets the job done. My concern is more about the change in behavior itself. The old way didn't have anything to do with a logged-in user or a session. It was about making sure that any request that was able to make changes to the system, was checked to have originated from a form rendered by the application itself. I agree that the case you describe is by far the most common one, as most applications will have some kind of session. A use case that I can think of that is now unprotected (without extra measures): The front page of a website displays a poll, which any visitor is allowed to fill in. You don't have to be logged in for this, but your ip address is recorded so you can't vote twice. With the old system, if badsite.com (from your example) posted to this poll (to get the poll result more to his favor), CSRF protection would raise an exception, which you could handle yourself using rescue_from. If you didn't do any special handling yourself, it would still make sure the bad request was not processed. Now with the new system, this is no longer the case. All that happens is that the user that visits badsite.com has his/her session reset on my site, on which he/she might not even had a login. In other words, badsite.com can now craft an attack, which makes any visitor (even ones that have never visited my site) make changes to my site's state. I agree this is far less of a problem than abusing a session, and if the poll is of any real value, it would indeed have been put behind a login, but still it is a concern. But CSRF protection, as the name implies, should be about preventing bad requests triggered from other sites. The new system only focuses on preventing session abuse, and should thus be called something like session-hijacking protection. I understand I can just override handle_unverified_request to get the behavior I want, but I think that the announcement did not highlight this enough. Sure, it mentions that the session will be reset, and no error will be raised. It also mentions that you should override the default if you need to clear more than just the session (remember-me functionality). But it doesn't mention that if you want to make sure the request isn't processed (old behavior), you need to take extra steps, because by default, it will just go through. Since rails is about convention over configuration, and the "protect_from_forgery" line is in ApplicationController in a freshly- generated rails app, I think it is strange that you need to do something yourself to make it protect requests and this might give a false sense of security. At the very least, a comment next to the "protect_from_forgery" line, saying "this will only protect you if you setup login/logout functionality based on sessions" would make people aware of this. I hope you see my point. Mathijs On Feb 11, 9:51 pm, Jon Leighton <[email protected]> wrote: > Hi there, > > As you've identified, the difference between 2.3.10 and 2.3.11, and > between 3.0.3 and 3.0.4, in how they handle invalid csrf tokens is that > the former will raise ActionController::InvalidAuthenticityToken, but > the latter will reset the session. > > What we are trying to protect against is the following situation: > > * Alice is logged in to Facebook > * Alice visits badsite.com > * Mallory, who owns badsite.com has added some code into the page which > makes a request to facebook.com and posts on Alice's wall. > * Alice visits badsite.com and without her intending it to be, a comment > is posted on her wall > > With the current CSRF protection, the following will happen: > > * Alice is logged in to Facebook > * Alice visits badsite.com > * Mallory, who owns badsite.com has added some code into the page which > makes a request to facebook.com and posts on Alice's wall. > * Alice visits badsite.com and without her intending it to be, a request > is made to post on her wall > * Facebook detects that there is no CSRF token associated with the > request, and so logs her out by resetting the session > * Then, based on the authorisation rules within the application, > Facebook denies to post on the wall, because the user is not logged in > > With the old CSRF protection, the following will happen: > > * Alice is logged in to Facebook > * Alice visits badsite.com > * Mallory, who owns badsite.com has added some code into the page which > makes a request to facebook.com and posts on Alice's wall. > * Alice visits badsite.com and without her intending it to be, a request > is made to post on her wall > * Facebook detects that there is no CSRF token associated with the > request and so refuses to serve the request in any way (returns 500) > * So nothing gets posted on the wall > > The point is, they are different but both have the effect of preventing > the wall post. > > If for some reason you specifically want an exception to be raised in > this situation, you can customise handle_unverified_request, but it > doesn't compromise the aim of the CSRF protection to no raise an > exception, so long as the request is not allowed to go through as > authenticated by the existing session. > > Jon > > > > > > > > > > On Fri, 2011-02-11 at 11:28 -0800, Mathijs wrote: > > Hi all, > > > I think CSFR protection broke in rails 2.3.11. > > As in: it's turned off now. > > > I tried this in rails 2.3.10 and in 2.3.11 and 2.3.11 seems broken. > > > >rails csrftest > > >cd csrftest > > >script/generate scaffold post title:string > > >rake db:migrate > > > now I visit /posts/new in my browser, use firebug to delete or change > > the authenticity token, and submit the form. > > > rails 2.3.11: all fine, new post saved > > rails 2.3.10: ActionController::InvalidAuthenticityToken > > > I checked ApplicationController to see if it still contained > > "protect_from_forgery", which is the case. > > I read the announcement for the csrf changes in 2.3.11 and they talk > > about overriding handle_unverified_request for special cases where > > there are other ways for authenticating a user. In this simple case I > > demonstrated though, there is no concept of a user or logging in (or a > > session), so the default action of resetting the session is not very > > useful. > > In my opinion, CSRF protection is about verifying a request. It > > doesn't have anything to do with users/sessions/authentication. > > By default, a faulty request (unprotected/wrong token) should not > > reach the normal controller action code at all, so the main action to > > take when a non-verified request comes in, is to have the > > before_filter chain halt. nothing more, nothing less. > > Extra stuff (like destroying a session) is up to the user (or nice to > > leave in as a default). > > So I think the behavior in 2.3.11 is just wrong, because in the > > example I gave, the forms just get submitted and stuff gets persisted > > to the database. > > It's not clear from the announcement at all that you should now make > > sure the filter chain halts, or that you must protect your actions by > > something that's stored in the session (because that's all that gets > > done when a faulty request hits). > > > Maybe I'm just doing something wrong here, please let me know. > > Mathijs > > --http://jonathanleighton.com/ > > signature.asc > < 1KViewDownload -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
