Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Use secure default values for the session store. #7879

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

dosire commented Oct 9, 2012

This improves security by complying with OWASP recommendations: https://www.owasp.org/index.php/Transport_Layer_Protection_Cheat_Sheet#Rule-Use.22Secure.22_Cookie_Flag

If administrators enable config.force_ssl this code automatically tells clients to only send cookies over SSL. If config.force_ssl is not set there will be no effect.

The problem this solves is that when you force SSL to be true you might forget to set your cookies to HTTPS only. If you forget to do this then your cookies are vulnerable when your users are on a unsafe connection, for example public wifi. This attack happens in the wild with Firesheep (http://codebutler.com/firesheep/). You can read more about this attack at http://en.wikipedia.org/wiki/HTTP_cookie#Cookie_theft_and_session_hijacking

Of course some people will remember and configure the cookie correctly to prevent this attack. However I feel it is important to be secure by default. Default in this case means when a user forces SSL, if that is not set this PR will not change anything.

Contributor

dosire commented Oct 9, 2012

Please let me know if there are any comments or suggestions to improve this. If it is merged I will update the guides.

Great idea, I use force_ssl but completely missed/forgot to do anything with the cookies.

Contributor

dosire commented Oct 9, 2012

@bittersweet Thank you for the positive response, I appreciate it!

Owner

rafaelfranca commented Oct 9, 2012

cc/ @NZKoz

Member

NZKoz commented Oct 9, 2012

not sure I like the idea of handling this in generated code, you could instead change the cookie store itself to turn secure on when force_ssl is on.

In security terms there's no harm to making this change, but hsts will make more of a difference in most cases.

Contributor

dosire commented Oct 10, 2012

@NZKoz Thank you for your feedback. I'm open to changing the cookie store itself instead. I agree it is cleaner to do it in the cookie store than in a template. However, I think that doing it in a template allows for some situations that are hard to do when the logic is coded in the cookie store. For example, you might run an billing application and a webstore application on the same domain that share the same cookie so employees only have to login once. In this case you want to force_ssl for the admin application but the cookie should not be ssl only since the webstore also uses regular http. Does this seem a valid reason to do it in the template or should I modify this PR to put the logic in the cookie store?

Member

NZKoz commented Oct 12, 2012

no, still should be done in the cookie store, in the case you describe the user can just explicitly set :secure=>false, the default should only be used if there's no value provided for :secure in the options

Contributor

dosire commented Oct 13, 2012

@NZKoz Ok, thank you for the feedback, I'll have a look at adapting the the cookie store.

Contributor

dosire commented Oct 15, 2012

The cookie store was already changed nine months ago by José Valim: d6933a1
Closing this PR

@dosire dosire closed this Oct 15, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment