Dear Flask, Please Fix Your “Secure” Cookies

http://stacksmashing.net/2012/08/10/dear-flask-please-fix-your-secure-cookies/

by mxey • 14 years ago

39 47 14 years ago

I would love to switch the default to JSON but people store too much stuff in their sessions that does not work on top of JSON (datetime objects and token objects are the most common offender).

In fact the python-openid extension is the most common offender. Also I can't just switch the default because it would invalidate everyone's currently issued sessions.

The issue is not new, changing it without breaking things is the hard part.

seunosewa• 14 years ago

It's a security issue. You're allowed to break things (or at least deprecate dangerous features)!

the_mitsuhiko8• 14 years ago

It's a security issue if people leak their secret key. At that point you have a huge problem anyways. Also breaking things assumes that afterwards you are left with an equally functioning system which I currently don't think I can guarantee.

phleet• 14 years ago

Why can't a custom JSON encoder be used with support for serializing/deserializing datetime objects?

the_mitsuhiko8• 14 years ago

It's still inline signalling and might conflict with what people have. Keep in mind that sessions are schemaless. It's not a huge problem changing that and I have a somewhat working implementation but it will break stuff, that's just how it works unfortunately.

moe4• 14 years ago

One thing that you could and should add is AES-encryption.

It does nothing for the pickle issue but it's quite a shame to send cookies in plain-text in this day and age.

david_shaw• 14 years ago

In information security, people used to think that web-based vulnerabilities were "lame" or "easy."

To some extent, they may have been correct: if you look at the relative complexity of HTTP header fuzzing or a SQL injection attack compared to finding 0day in, say, the Linux kernel, it's generally not even a contest.

That said, exploits like this demonstrate the carelessness that can go into many applications and frameworks on the web. In my years of application security, I've seen some "interesting" vulnerabilities like this, but I've seen thousands of people with standard, well-known security problems all over their applications.

"Broken Authentication & Session Management" is an extremely common finding for my team to write up. People think that, oh, we don't need this cookie to be marked Secure or HTTPOnly. "If the browser's owned, we're screwed anyway."

The problem with this uninformed stance on application security is that exploits do not always need to stand on their own to introduce a significant level of risk. For example, should a cookie not be marked HTTPOnly, the browser itself does not need to be compromised. Simple cross-site scripting (another extremely common issue in webapps) can easily access these cookies and throw them at malicious receivers.

This is just one example, but like the article referenced in the original post, it seems to me that some people just don't take security seriously.

That said, the research referenced here is pretty cool -- for those of you that missed it, you can read the Black Hat 2011 paper on this issue here: http://media.blackhat.com/bh-us-11/Slaviero/BH_US_11_Slavier...

kijin• 14 years ago

Wait a second, this is 2012. Why are we still storing session data in cookies, whether encrypted or not?

Unless the data is really simple and you don't care if it's stolen, session data should always be stored on the server side, and only a meaningless identifier sent to the client.

Managing sessions on the server side used to be a PITA, to be sure, especially if you had multiple servers. But nowadays there are lightening-fast distributed key-value stores that are ideal for storing session data. No need to encrypt, base64-encode, base64-decode, and decrypt multiple kilobytes of data with every request. Think of all the bandwidth and latency you could save by not sending those kilobytes back and forth!

the_mitsuhiko8• 14 years ago

> Why are we still storing session data in cookies, whether encrypted or not?

First of all they are not encrypted, they have a MAC. Secondly because you can verify the basic information of the request without hitting a database which is very convenient. In fact if it would not be for signed cookies Flask would not have any sessions because it does not dictate a data store. However you can easily change that: http://flask.pocoo.org/snippets/75/

> Think of all the bandwidth and latency you could save by not sending those kilobytes back and forth!

You are not saving anything there. A session should really only contain user id.

heretohelp4• 14 years ago

>You are not saving anything there. A session should really only contain user id.

This is all ours do. I'm not totally clear on where the actual security issue is here. Adding a level of indirection to a backend store doesn't seem like it would change much.

kanzure• 14 years ago

> You are not saving anything there. A session should really only contain user id.

So it looks like Flask is using something that just deliberately runs pickled strings. Even if I know my own secret key, I don't exactly need to be able to send myself arbitrary code. So JSON seems to make sense to me for my use cases.

    pickle.loads("cos\nsystem\n(S'whoami'\ntR.")

Right.. so anyway, does anyone know which session engine I want to use with Flask if I want to generate a token for a user's session, which I can then later revoke if I end up hating that user's session but not two other sessions?

mark_story2• 14 years ago

That's clearly not the case in all situations though. If its simply a userid there would be no reason to use pickle. JSON or simple string storage would be sufficient. Clearly people are storing far more than just a userid in their client side sessions.

moe4• 14 years ago

But nowadays there are lightening-fast distributed key-value stores that are ideal for storing session data.

"Lightning fast" is still a lot slower than no lookup at all.

A few bytes of cookie cost next to nothing in terms of bandwidth nor latency and can save a lot of hardware on the backend for large sites.

mark_story2• 14 years ago

Sure it costs nothing until you are compromised and any valuable data you have is taken. Remote code exploits are extremely dangerous. And mitigating them with a small amount of i/o overhead seems like a fair trade off.

jacques_chester2• 14 years ago

Security is more important. Storing anything in the cookie apart from a simple identifying token increases the attack surface.

If you use encryption and an HMAC to harden up said cookie, you need to a) not bugger up your scheme and b) pay the overhead the calculations. For the HMAC it's trivial, but ciphers are a bit more involved.

the_mitsuhiko8• 14 years ago

For what it's worth, I implemented JSON based sessions for testing purposes: https://github.com/mitsuhiko/flask/commit/4df3bf2058954624f9...

They are currently on a separate branch and they are known to break python-openid (and with that Flask-OpenID). I don't have a solution for that yet. If someone is really concerned regarding security you can copy/paste the code into your own project. The session interface in Flask is pluggable for a while already.

the_mitsuhiko8• 14 years ago

I should add that this will not be at all the final implementation for Flask 0.10 in case I want to change that. If the change of the implementation will happen I will make itsdangerous (separate Python module) have a way to serialize the custom objects properly and then add this as a dependency.

nezza-_-7• 14 years ago

Thank you for the new implementation and your fast reaction!

lotyrin• 14 years ago

Is there a reason why session storage isn't local, with clients only given the index into that storage?

Allows for any size/types of data but the worst case scenario-without code execution is that an attacker hijacks an existing but valid session.

greghinch• 14 years ago

Er, if someone has gotten access to your secret key, aren't they already into your server? Or is this just CYA for developer incompetence?

the_mitsuhiko8• 14 years ago

That was my thinking so far in regards to this issue. I do however understand the concerns so I am probably going to evaluate a few alternatives.

nezza-_-7• 14 years ago

Local file inclusions are quite often very hard or impossible to exploit. Also, as linked in the blogpost, some may just pushed them online by accident, someone may lost a notebook or whatever. So no, filesystem access is only one way.

btilly• 14 years ago

Not necessarily. They may have gotten access to an old backup of your server. This misfeature would let them escalate that to a compromise of your live server.

Attacks often involve multiple rounds of escalation as people keep on increasing their level of access. You want that escalation to be as hard as possible. Therefore you really don't want there to be a fact that could be floating around your organization in various ways that can be used to go straight to executing arbitrary code on your production site.

NelsonMinar• 14 years ago

No Python program should ever try to unpickle data from an untrusted source.

nezza-_-7• 14 years ago

It's signed, so the integrity is ensured as long as the secret key stays secret.

slurgfest2• 14 years ago

There was never any good reason to use pickle for this, though..

alxv• 14 years ago

There is a way to harden pickle to protect it against the most basic exploits: http://docs.python.org/py3k/library/pickle.html#restricting-...

It is not a complete solution, as an attacker could still DoS your service by making pickle allocate a huge amount of memory, but at least that's better than allowing arbitrary code execution.

nezza-_-7• 14 years ago

Another problem we saw (but didn't mentioned): You can't logout sessions on the server-side. If you forgot to logout somewhere, your session will stay valid. And an expiration-date for the sessions is optional. Neither changing your password nor contacting the site admins will help you.

the_mitsuhiko8• 14 years ago

That's not really true. You could easily add another piece of information to your session that is derived from something stored with the user (for instance a running counter). So if someone runs away with your session you add a button "logout everything" which increments the counter. When the session is accessed for the next time the counter values don't match and the user is logged out.

zzzeek• 14 years ago

On this topic, Beaker cookie sessions are encrypted and are great; however, neither Ben or I have much time/interest in maintaining Beaker, as the caching is now modernized in dogpile.cache and neither of us agree with Beaker's server side session approach.

Might someone adapt Beaker's secure cookie session into its own pypi package and include a flask adapter with it? It's time we all got behind a best practice for sessions (which IMHO should be cookie based, encrypted, and very small/only basic identiication with more significant data stored in the DB).

jacques_chester2• 14 years ago

The rule of thumb is that each distinct kind of operation should have its own secret key.

For example I developed a protocol with hash-chaining and HMACs for messages. I have two keys: one for performing the hash-chain calculation, and a different key for the HMAC of the one time message.

Could I have used a single key? Absolutely, yes. But the time it takes to perform both is trivial. And I've bought myself a little extra protection from any attacks that reveal a few bits of a key, because gathering a bit here and there of different keys won't help my attacker as much as gathering bits for the a universal key being used for everything.

Dear Flask, Please Fix Your “Secure” Cookies

Related Stories