▲ 84 points
back
56 comments
This feels less like a framework bug for me than an app bug -- an impressively common app bug, I'll grant you, but an app bug nonetheless. Rails doesn't ship with user management. It's pretty upfront about that, and the docs tell you to roll your own since it is easy, and the community will tell you to use Devise since that will save you time since you'll be doing repetitive work for substantially all apps otherwise.
The typical way beginning Rails developers (including me, on my first Rails apps) would implement logins is stuffing the user ID in session[:user_id] and setting the currently logged in user to that when it is both present and in the DB.
It is true that this approach will make sessions replayable. It will also:
1) Make it impossible to reliably expire session, since the user's browser is not under your control and may treat the expiration date as advisory only.
2) Make it impossible to forcibly expire session, which you'll want to do in event of a user changing or resetting their password, changing their username/email address, etc. (Generic security advice for all web apps, by the way. Otherwise compromise of user accounts, via losing password, XSS, or what have you, is impossible to reverse.)
3) Makes it impossible to implement Sign Me Out Of All Devices, which is a very desirable capability to have for many applications. For example, if you have a more-important-than-cat-videos app, somebody losing their phone should probably not result in their account on your service being irrevocably compromised. You could provide a Dropbox-esque UI to manage their state across multiple devices or, in a pinch, just say "If you hit the log out button on any device, we log you out of all devices."
4) The simplest possible Rails login system is facially non-compliant with a number of security regimes you might be under, including HIPAA among others.
n.b. Switching to ActiveRecordStore doesn't by itself solve anything but #1. You do need app-side logic.
I actually regard this as a framework bug. Rails has been promoting the Cookie-Storage for at least two major releases. There's a couple of upsides to cookie storage that make it appealing, but the downside is the given lack of control. Some of those points you mention could be tackled even with cookie storage, but not all of them:
2) in the case of a compromise you can change the app secret. That's a pretty big sledgehammer and will log out all users from the app, but it will reliable smash that fly.
1) could be tackled the way gitbub proposed: just include the expiry date in the signed cookie. Discard any cookie that's past expiry.
3) could be tackled by using some sort of per-user cookie secret. That however would require a storage again and then you could as well store the whole session in said storage. Not much to win here.
So using Devise gets around this issue right?
I also wondered whether Devise handled this issue and looked at the source. Devise doesn't address this issue. It relies on warden underneath and serializes the user into the session without any sort of expiry (https://github.com/plataformatec/devise/blob/master/lib/devi...). There are a lot of options for remediation, though, like overriding the aforementioned serialization hash to attach an expiry, or setting an expiry in a separate key in the session.
If you use Devise make sure to read this, too:
http://stackoverflow.com/questions/11281141/after-logout-if-...
(I was quite surprised a long while back :-)
This is interesting...still doesn't answer my question though :)
This is a very common problem in a lot of signed cookie based session stores. Some frameworks get it right, some don't.
The best remediation is to include an expire timestamp within the content of the signed cookie and to check this on the server - you can't rely on the client deleting the cookie (never trust the client).
The guys at GitHub fixed this particular issue in their rails stack, and submitted a pull request 4 months ago:
https://github.com/rails/rails/pull/11168
It started out well, and their proposed solution would have fixed this problem. The thread got bogged down in discussing backwards compatibility and the original submitter just gave up on it.
I hope this might trigger somebody to pick that pull request up again and get it sorted and merged.
Since signed cookies are now a popular form of persisting session state I have been meaning to run through all the popular web app frameworks to check this issue.
edit: check out TimedSerializer from the itsdangerous Python library that Flask uses:
https://github.com/mitsuhiko/itsdangerous/blob/master/itsdan...
The giving up statement[1]:
Giving up. The inheritance and mixins approach in the chained and legacy cookie jar code is absurd beyond my patience.
Someone else is free to run with this code as a start and take the credit.
[1] https://github.com/rails/rails/pull/11168#issuecomment-22353...
Just as an example, Django uses a default SESSION_COOKIE_AGE of 2 weeks [1]
[1] - https://docs.djangoproject.com/en/dev/ref/settings/#std:sett...
The solution I use in my own application is for each cookie that gets set to be a distinct, random shared secret that is mirrored in a database table.
The shared secret is the combination of a near-random string of bytes generated by a hash of numerous things, plus a unique ID generated upon insert into the database table. The secret survives in the table until the user chooses to sign out, or until the cookie expires. Once the secret is expired, the cookie is useless.
This also means users can go into their account page to see on which devices their account has been remembered, but also to log out any/all other sessions.
>..random shared secret that is mirrored in a database table.
Isn't the point of using a cookiestore to avoid hitting the database and thus eliminating the need for the painful session/state maintenance across multiple nodes? Your solution looks more like a DIY security solution to me, (please correct me if I'm wrong) and I would take major precautions in gambling with security-related functions of a framework.
P.S I am confused, please explain if I am wrong, I will gladly rather be corrected than make a possibly serious security mistake in one of my own applications.
> eliminating the need for the painful session/state maintenance across multiple nodes
If session/state maintenance across nodes is painful, the solution isn't to trust the web browser, it's to make such maintenance much less painful. For example, you can use a cache server (like ehcache) to store session state. Or you can use smarter load balancers (like haproxy) to minimize node hopping.
A cookie is an authentication credential, just like a username and password combination. If you can't invalidate an authentication credential on the server side, your solution is an instant fail.
I like to ensure that the key used to sign the sessions is rotated on a regular basis. On Heroku, for example, there is the Secure Key[0] addon which takes care of this for you.
FWIW, I just posted this which contains useful code for CookieStore (not tested):
This is in the Ruby On Rails Security Guide: http://guides.rubyonrails.org/security.html#session-fixation
The author isn't telling people anything new, so it shouldn't take the form of an announcement.
Different concern.
Misleading:
>Separately, it is a good design for your Web app to require that the user supply their current password before changing sensitive fields such as password or email address. If the CookieStore-stored session were to be hijacked, the malicious user could change the user’s password:
I don't see why that's the case - presumably, if I'm requiring the user's password to perform sensitive account changes, then the hijacking user will not be able to perform those changes. It's not like the CookieStore contains a plaintext password or anything.
This is ultimately a simple replay attack, which any application using a server-side session would be equally vulnerable to. The effects of that replay attack are definitely worse with CookieStore, because a user cannot subsequently invalidate their session from the server side (although the app owner can obviously invalidate every session).
This could be solved relatively easily by providing a per-session token which can be invalidated, but at that point I guess you'd have to question why you're using a cookie-stored session anyway.
Pretty sure this can be fixed with a timestamp in the session data. You can either expire the timestamp by time or by keeping a datetime field on the user and updating the timestamp after a password change. Or you can use a combination of these two.
This is not a new issue. It is a well known limitation of the cookie store. I've been working around this with timestamps for years.
Can you share maybe the code you're using to do that?
As well: rotating the secret token every night could probably be used to discard all existing cookies, right? (although this is a bit brute force and will kick out anyone currently using the app).
Nope. The last version I wrote is buried in a rails app at a company I no longer work for.
It's a three or four line before filter in the application controller. I reimplement it every time since it's relatively simple.
I don't have it on any public code since it's also the last thing I implement before launching.
From the Open Source Vulnerability Database : http://osvdb.org/show/osvdb/97726
Versions concerned: RoR 2.x, 3.x, 4.x
Description : Ruby on Rails contains a flaw in its design that may allow attackers to more easily access applications. The issue is due to the CookieStore mechanism storing cookies on the client side, while not maintaining a corresponding entry on the server side. When an application terminates a session, Ruby on Rails has no method to track this and truly invalidate the cookie with the default configuration. This means that cookies persist "for life" and can be used to access an application even after it is thought to be terminated in many cases.
Solution: Currently, there are no known upgrades or patches to correct this vulnerability. It is possible to temporarily mitigate the flaw by implementing the following workaround: switch to a more secure authentication management systems (e.g. ActiveRecordStore).
Misleading. It's not a problem of CookieStore or Rails, it's the problem of HTTP. Changing session store to a database or memory store won't fix it: a session id is still required on the client side, then your session is still hijacked because the attacker still gets the session id. You may want to make the session id change after each request, then you are re-inventing TCP and ruining user experience. If the attacker can steal CookieStore, that means the attacker can also see everything else in the page, just securing the session does little help -- please stop worrying and use HTTPS (and add "secure" option for the cookie entry).
CookieStore does its own job correctly: it has an "httponly" option which prevents being stolen by injected javascript, and it checks signature which prevents content being modified or forged. But it's not CookieStore's job to prevent sensitive content being watched. It's HTTPS's job.
Changing session store to a database or memory store won't
fix it: a session id is still required on the client side,
then your session is still hijacked because the attacker
still gets the session id.
When you store a record of the session on the server side (with the session ID you mention), you clear that session record during logout; the session is now gone, and cookie isn't valid anymore.
(With the cookie-only approach, the server will continue to accept a cookie that you wanted to have cleared.)
When I click log out, I know I'm logging out on this browser but not other browsers. I usually want to keep my session on other devices... But you can still achieve "log me out on every devices" with CookieStore, it doesn't limit you from storing and checking things on the server-side.
When I click log out, I know I'm logging out on this
browser but not other browsers.
... it doesn't limit you from storing and checking
things on the server-side.
Got it, filing a bug at HTTPs github page. On a more serious note, this is why we have frameworks, and dont let developers roll their own because its "super easy".
I'm surprised that this issue is being raised now. It seems like it should have been obvious.
It looks like a fundamental design problem with a client side token... You can limit intrusions by adding an expires_at value to the cookie data. But I don't see any way to actually expire a token without some server side tracking.
How about simply storing a logout timestamp in the database when a user logs out? Whenever a user logs in again the timestamp field is cleared and as long as that timestamp field is empty the session cookie is accepted, otherwise ignored and the user is forced to log in again.
Basically we still get benefits from storing the session data client side (much less db access) and only store minimal data on the server side.
This could be problematic if the same user is logged in from multiple machines. Logging out from one client will invalidate all of the other sessions. For some web applications this would be seen as a bad experience to the user.
Another issue is old sessions may be deactivated once the user logs out, but all those old sessions suddenly become valid again when the user logs in again, since the time stamp field is now present. It doesn't prevent the issue the author is highlighting (that sessions can last forever).
Would that not mean that the cookie could still be re-used by a malicious user who had intercepted it, as long as the real user had subsequently logged in again?
I guess it reduces the attack surface (the genuine user must be logged in at the same time as the attacker) but doesn't completely solve the problem.
Yes, I think you are right. Additionally the cookie could have a creation timestamp and in the db the last login time is stored. So when the cookie timestamp is older than the last login the cookie is actually outdated and ignored. But again maybe it's simpler to just use serverside session storage ...
This seems like a really elegant solution, I've not given it much thought, so it could turn out to be flawed, but definitely worth pursuing.
The session data should always be on the server side usually store on memcached or redis. The cookie should just be an id used as a handle to the session on the server side. The server side data should always be deleted on logout.
Nowadays most services work only on HTTPS. In that case is not possible stealing the cookies observing the traffic but there is always malware able to grab the cookies. A mechanism to invalidate the session should always be available.
If you moving data back and forward on HTTP and you are using the cookie to store the variable in the session, there is always the risk to expose sensitive data stored in the session to the outside world.
Hi HN, G. S. McNamara (author) here. Glad to see awareness brought to this issue.
I posted about this on HN originally here: https://news.ycombinator.com/item?id=6458433
Web host took the MaverickBlogging.com site offline due to all the traffic; it's back up now.
Time to switch web hosts.
Is this valid even if you use session instead of cookie?
Yes.
I sincerely hope that nobody was using the CookieStore in deployment. I think everybody should know by now that cookies are not safe or secure storage for data.
Cookie-based sessions have many, very reasonable, use cases.
You are also clearly neglecting the fact that proper session cookies are _always_ cryptographically signed and cannot be tampered with, if properly implemented.
They do, and they are signed, but they still have issues like this which are so easily mitigated by just using a goddamn server.
Using a server based session storage is simple as long as your whole app lives in one datacenter and all frontend hosts can reach said server. Once you have app servers in multiple datacenters (e.g. for geo loadbalancing) and want to provide a seamless login no matter which server the user ends up on server based session storage just gets a lot harder. (Apart from having to handle a massive write load which used to cause major pain with mysql, myisam and database based session storage, but these times are luckily over).
What your forgetting is the reason people use frameworks such as Rails - so they don't have to care about these types of details.
I agree that simply storing everything in the cookie is wrong, and Rails should never have been doing this - hell, even CodeIgniter (PHP) has DB based sessions.
But most Rails devs (myself included) are just about getting shit done, we don't know everything that's going on under the bonnet, and we don't want to.
Please don't speak for others, especially when you claim that you don't know what you're talking about in the same sentence.
And FYI CookieStore is just the default, because it's convenient and require no dependency. But you're juste a line of configuration away to switch the sessions inside your DB or Memcache or whatever.
> But most Rails devs (myself included) are just about getting shit done, we don't know everything that's going on under the bonnet, and we don't want to.
I love getting shit done.
It's my job as a professional to know everything that's going on "under the bonnet."
I'm still learning.
But it's my job to try.
It's a pretty widespread practice. I think it's the Rails default.
Sure, it's the default, but don't people realize never to trust clientside data? I don't know if CookieStore is signed or not, but I generally assume even if I sign the data it's not safe.
It's not that hard to just set up a Redis or whatever store to handle stuff like this, I never understood why people whouldn't bother.
Cookie store has always been signed and in rails 4 it's encrypted.