Change requirements for ISession and make UnencryptedCookieSessionFactoryConfig a little bit more secure

[digitalresistor]

There recently was a blog post regarding "insecure" session cookies in Python web frameworks: http://vudang.com/2013/01/python-web-framework-from-lfr-to-rce/

This mainly has to do with the fact that people leave their secret keys ("itsaseekreet") easily accessible by posting source code in Github or other places, thereby destroying the security that an HMAC provides.

What I suggest is replacing signed_serialize and signed_deserialize with an alternate implementation that does not have the inherent flaws that pickle has, so that if the hmac key is leaked the worst that an attacker can do is change their session information.

Other changes that should be made is that ISession shouldn't need to be pickable by default, as this makes it too easy for users to write code that uses pickle to store the data from their session. Although I am sure that change will be unpopular, pickle makes it damn easy, but if we store pickle into a database for example and an attacker is able to SQL inject data, it is entirely possible for them to get code execution when the object is being depickled from the database.

Unfortunately Beaker's cookie session mechanism also uses pickle, so we can't simply update the documentation to tell people to use a third-party library.

I wonder if JSON might be a better option here (for the unsecured cookie anyway).

[mcdonc]

We won't take the existing one out, although it's probably a good idea to tell people the risks in the docs. I would not be averse to includiing a separate cookie-based session implementation that uses JSON.

[Sapphire64]

By the way, I was thinking about this not a long time ago. First of all, I don't like how it looks right now in a docs:

session.key = mykey
session.secret = mysecret

Also, itsaseekreet is quite confusing and not telling that this place is critical and should be changed. Maybe it is quite clear for pro's, but for beginners it is not obvious (and yes, I think Pyramid is one of the most beginners-friendly framework out there).

So, what can we do?

1. Update docs with RED-coloured notifications :)
2. What about generating random hash on each user's page request (by JS or by Sphinx somehow) and note that this hash should be unique and it was generated unique for you by docs.

If we will have any scaffolds with sessions we also should generate unique hash for each installation.

[digitalresistor]

The biggest problem still is that a lot of people will commit their work to Github and other places, or otherwise allow that key to leak. At that point people can do Very Bad Things™ and for example gain shell access to the server it is hosted on. Besides changing the documentation to make it very clear that while secure so long as the secret is never leaked it is a HUGE security hole. No matter whether the Pyramids project generates a random hash on each user's page request or not.

A JSON replacement would be a fantastic start, that won't allow for code execution if the HMAC key is found, and the docs should make it very clear that if you are going to use cookie based sessions that using the JSON one would be MUCH more preferable over using the Pickle based one.

While personally I suggest deprecating the pickle based on, and eventually removing it all together @mcdonc said that taking the existing one out is a no-go.

[mmerickel]

Considering the cookies are signed this is a lot less vulnerable than things like the yaml vulnerabilities recently discovered in rails. We'd added a hook already to the unencrypted session factory to support your own signed_serialize and signed_deserialize. @bertjwregeer does that turn this into a docs-issue at all for you?

Pyramid actively recommends separate ini files for production and deployment. We really can only stop users from shooting themselves in their feet to a certain point before it's on them.

[digitalresistor]

The issue is that the session stuff still requires that it is pickle-able. This means there can't be as simple recommendation of using json for example. Using json would solve the big problem that exists if the key is leaked. It is nice to say that the key should never be leaked because it is private, but real world has shown us that is not the case...

---

Another thing, I would like to see the documentation change to have stuff like the secrets be pulled from the registry/settings instead of directly input into the code. This would also make it clear that is a configuration knob and one that should be set outside of the code itself, making it less likely to be leaked (unless someone checks in their .conf).

I don't currently have time to go through the documentation and make that change :-(.

[mmerickel]

Hopefully most of this is fixed by #1142. I don't see us modifying the ISession interface until at least 1.6 for JSON support.

[digitalresistor]

Most of this has been fixed with doc changes, still not a big fan of Pickle support (and I am sure I sound like a broken record). Would like to keep this open until we do have JSON or another alternate more secure serializer support.

[digitalresistor]

With the ease that one can now switch out the serialiser used for the newer SignedCookieSessionFactory there is no need to keep this around.