CSRF protection in rails 2.3.11
Mathijs
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
Jon Leighton
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
Mathijs
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
> < 1KViewDownload
Tsutomu Kuroda
I have a question.
> 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
In my understanding, Alice gets logged out from Facebook unintentionally
by visiting badsite.com. I think this is an ugly side effect. Am I wrong?
Michael Koziarski
> 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.
CSRF attacks are about using *session* data to perform an action
without the user's knowledge. The attack you're describing here,
which doesn't rely on session data, could also be performed just using
curl from the command line of the attackers laptop, there's no need to
involve the user's computer at all.
If there's no involvement of the session (or some other
client-browser-specific) vector, then it's not a CSRF attack.
However you're correct that we need a little more documentation than
we had previously because the attack vectors are a touch trickier. I
intend to post an FAQ blog post and update the guides in a few days to
ensure that it's crystal clear what's required.
The thing to remember is that we *can't* raise an exception in the
default case any more because *every api request would be rejected*.
For users who don't have any API, then the original behaviour was
probably fine. However a large portion of rails apps do and we can't
completely break them!
> I hope you see my point.
I see your point and agree that we need a little more documentation
around this. However it is nowhere near as simple as your suggestions
imply. Developers will have to understand the implications of their
choices.
I suspect we'll end up with an api like:
protect_from_forgery :on_failure=>:raise
protect_from_forgery :on_failure=>:reset
protect_from_forgery :on_failure=>:some_method
protect_from_forgery :on_failure=> proc { ... }
I realise that the previous situation where you could just ignore the
issue was greatly preferable, but that was based on some assumptions
which don't hold. The old rules don't apply anymore.
> --
> 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 rubyonra...@googlegroups.com.
> To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
>
>
--
Cheers
Koz
Mathijs
> CSRF attacks are about using *session* data to perform an action
> without the user's knowledge. The attack you're describing here,
> which doesn't rely on session data, could also be performed just using
> curl from the command line of the attackers laptop, there's no need to
> involve the user's computer at all.
IP address.
The attack allows him to command other people (his visitors) to
perform actions on my site.
In my opionion, the 'session riding' part of CSRF is not just about
the physical session/cookie object, just as much about starting new
sessions. (so session as in "the order in which requests are made", we
only allow a signup if you first asked for the signup form).
But no matter what we think CSRF protection is about, the old behavior
was about validating every potentially harmful request, the new one a
specific type of attack, which was already covered by the old one.
So we can conclude that the security got loosened somewhat (for
reasons you mention below).
>
> If there's no involvement of the session (or some other
> client-browser-specific) vector, then it's not a CSRF attack.
address counts as client-browser specific.
>
> However you're correct that we need a little more documentation than
> we had previously because the attack vectors are a touch trickier. I
> intend to post an FAQ blog post and update the guides in a few days to
> ensure that it's crystal clear what's required.
>
> The thing to remember is that we *can't* raise an exception in the
> default case any more because *every api request would be rejected*.
> For users who don't have any API, then the original behaviour was
> probably fine. However a large portion of rails apps do and we can't
> completely break them!
ApplicationController, and I don't put the CSRF protection in there.
Instead, api calls need an API key param to verify their origin. Also
there's "skip_before_filter :verify_authenticity_token" to bypass CSRF
protection on a controller or action basis.
This is probably what most apps with an API use, because - as you
point out - there are already a large number of rails apps out there
with an API, working with the old system. So I don't see how they
would be broken.
I agree it's nice we can now configure the behavior ourselves, instead
of having to fully bypass it, all I'm saying is that for the default
case making sure a request is dropped sounds best to me. The API case
should indeed be able to easily switch to other behavior (for certain
controllers probably).
> I see your point and agree that we need a little more documentation
> around this. However it is nowhere near as simple as your suggestions
> imply. Developers will have to understand the implications of their
> choices.
But since the default (rails generator) "application" already contains
'protect_from_forgery', which just doesn't do anything unless they
start using the session as a security measure, you can't really hold
every dev responsible for expecting that they are protected against
forged requests from other sites.
>
> I suspect we'll end up with an api like:
>
> protect_from_forgery :on_failure=>:raise
> protect_from_forgery :on_failure=>:reset
> protect_from_forgery :on_failure=>:some_method
> protect_from_forgery :on_failure=> proc { ... }
>
> I realise that the previous situation where you could just ignore the
> issue was greatly preferable, but that was based on some assumptions
> which don't hold. The old rules don't apply anymore.
controllers and actions, while the default was safe for all abuse-
cases. The new system is aimed towards applications with an api that
don't want to use skip_before_filter and that do use a session for
their security-measures.
What assumption does not hold anymore? Which rule changed?
Nicolás Sanguinetti
> On Feb 12, 8:05 am, Michael Koziarski <mich...@koziarski.com> wrote:
>> CSRF attacks are about using *session* data to perform an action
>> without the user's knowledge. The attack you're describing here,
>> which doesn't rely on session data, could also be performed just using
>> curl from the command line of the attackers laptop, there's no need to
>> involve the user's computer at all.
>
> Except for him not being able to vote twice using curl with the same
> IP address.
> The attack allows him to command other people (his visitors) to
> perform actions on my site.
> In my opionion, the 'session riding' part of CSRF is not just about
> the physical session/cookie object, just as much about starting new
> sessions. (so session as in "the order in which requests are made", we
> only allow a signup if you first asked for the signup form).
> But no matter what we think CSRF protection is about, the old behavior
> was about validating every potentially harmful request, the new one a
> specific type of attack, which was already covered by the old one.
> So we can conclude that the security got loosened somewhat (for
> reasons you mention below).
>
>>
>> If there's no involvement of the session (or some other
>> client-browser-specific) vector, then it's not a CSRF attack.
>
> As mentioned, I think starting a new sessions from a "clean" IP
> address counts as client-browser specific.
So you're saying people with dynamic IPs which rotate every 12 hours
(the majority of Uruguay, for example) should just log in into your
site each time the IP rotates, or am I reading it wrong?
Michael Koziarski
> was about validating every potentially harmful request, the new one a
> specific type of attack, which was already covered by the old one.
> So we can conclude that the security got loosened somewhat (for
> reasons you mention below).
The security didn't get loosened, it got changed in a manner that
preserves security for the bulk of use cases while preserving the
restful API support that so many people rely on.
> As mentioned, I think starting a new sessions from a "clean" IP
> address counts as client-browser specific.
IP addresses are cheap and plentiful and don't provide the security
you seem to think they do. For example the entire nation of qatar
makes internet requests from the same IP address. Would your
theoretical site only accept a single vote from qatar.
However that's beside the point, your application could either
override the default handler or require users view the poll question
before voting (making use of the session). Both are perfectly
acceptable answers, what's not acceptable would be rejecting each and
every API request to every rails app on the internet.
> I don't really see why. The old system was easy to bypass for specific
> controllers and actions, while the default was safe for all abuse-
> cases. The new system is aimed towards applications with an api that
> don't want to use skip_before_filter and that do use a session for
> their security-measures.
> What assumption does not hold anymore? Which rule changed?
I think you missed the details of the security vulnerability, our
previous method of whitelisting API requests has been broken.
Previously your API requests *didn't trigger CSRF protection*, now
they do. *that* is why the error handling had to change. Previously
we could raise 500 errors because the checking never fired for API
requests. Your entire model of using seperate api controllers for
CSRF protection was completely unnecessary under the old system.
Please take the time to read the advisory carefully as I think you're
misunderstanding why we undertook this change. Attackers can forge
arbitrary HTTP headers, previously this was not believed to be
possible. Because of this incorrect assumption (made by basically the
entire web security community, not just us) we were able to whitelist
certain requests. We can no longer whitelist anything, everything can
be forged.
It was not undertaken lightly and it was not done without a lot of
careful consideration. We consulted with security experts, analysed
various alternatives and chose the least-worse option out of a series
of bad options.
The setting as it is now will not be changed, we may enhance the api
in the future and we will improve the documentation to make sure
people can make informed choices. But the default won't change.
--
Cheers
Koz
Mathijs
Please don't go into details too much for the example case I came up
with.
I already mentioned in my first post, that if the poll is of any real
concern, it would of course have been put behind a login.
And while we're at it, SSL login, with a 2-tier authenticator device
and iris-scan, just to be sure.
And we need to verify their real ID before allowing signup, because
email-verification is not enough as people can create them on-the-fly.
I realize that protection-by-ip-address is not very safe, but for the
poll example I gave, it might be just enough.
I beg to differ that there are a lot of websites out there using
exactly that.
Working around it is easy, you can use a bunch of proxys, tunnel
request through tor, employ a botnet and probably more.
But by far the easiest (if you own a website) is to just have your
visitors perform the action on your behalf.
So I'm not saying anything about whether or not ip-protected polls are
good or bad practice, I'm just arguing it's a valid use case, which
was protected by default with the old CSRF protection.
Thanks,
Mathijs
On Feb 12, 9:45 am, Nicolás Sanguinetti <h...@nicolassanguinetti.info>
wrote:
>
> read more »
Mathijs
I did not miss the details of the vulnerability, it was just not part
of our discussion about the default behavior.
The changes to CSRF protection came in 3 parts.
1-The whitelisting part is clear and I understand I now need to use a
token for xhr request because they now are forgeable.
As is stated, every non-get request needs a token, which tightens
security. I'm certainly not opposing to this change.
2-The second change is that the handling is now fully configurable and
people can handle unverified requests in any way that is appropriate
for their application. This is a great change as well, indeed for APIs
and such.
3-The third change was the change of the default behavior. As far as I
can see, changing the default opened up some cases (mainly the empty
starter rails app with no authentication) that were protected before.
And as far as I can see, it didn't start protecting cases that were
unprotected before.
I would just have liked if the default stayed to raise (the safest,
most catchall), and have people change this to session-reset the
moment they implement authentication/login, or implement an API.
In other words, secure-by-default, but configurable enough to bind it
to any application-internal security/session/authentication logic
later on.
But as long as everybody is made fully aware of the implications, I
don't mind what the default is.
So indeed documentation will take away my concerns.
Thanks for the feedback.
Mathijs
Jimmy
Plz see inline:
> However you're correct that we need a little more documentation than
> we had previously because the attack vectors are a touch trickier. I
> intend to post an FAQ blog post and update the guides in a few days to
> ensure that it's crystal clear what's required.
FAQ blog post, because for me it's not crystal clear at the moment.
Specifically:
> CSRF attacks are about using *session* data to perform an action
> without the user's knowledge. The attack you're describing here,
> which doesn't rely on session data,
have a login form. Whether this login form has the authentication
token or not doesn't matter anymore, login always succeeds. Even with
cookies disabled. Previously it would not. Even though
+handle_unverified_request+ is called, resetting the session doesn't
matter, as it was a new session anyways. Code continues, and login
succeeds.
In the post at http://weblog.rubyonrails.org/ heading "Upgrade
Process", I think the suggestion to simply remove the remember_me
cookie isn't enough.
Wouldn't this be better advise? :
def handle_unverified_request
super # call the default behaviour which resets the session
redirect_to login_url
end
Cheers,
Jimmy
Mislav Marohnić
on our homepage we
have a login form. Whether this login form has the authentication
token or not doesn't matter anymore, login always succeeds. Even with
cookies disabled. Previously it would not.
A Wilson
How is this a security flaw? Login only succeeds if the credentials are correct. If someone has credientials, they can login to the site, and I don't see what role forged cross-site requests play in this case.