Sessions are unnecessarily complex

[Paul McMillan]

Django includes ​several backends for storing session data. None of these store data in a manner that is accessible to clients. The backends rely on trusted infrastructure: the database, the cache, or the filesystem.

For all of recorded history (since the earliest svn checkins), Django has ​stored session data in an obfuscated format. We pickle the data, hash it, and store the hash + base64(data). To retrieve session data, ​we do the opposite: pull the string from our trusted backend, hash the data, compare to the stored hash, and then if it matches, return it.

My best guess is that this is a holdover from an earlier era when Ellington stored session data client side in cookies. None of the default backends support this, and it ​may not currently be possible.

This means that we are running significant extra code every time we hit the session store. We trust the supplied backends, but we don't know that a user hasn't subclassed ​the base backend to depend on this hashing behavior.

I propose that we rework the base backend to include an InsecureSessionBase which our provided backends subclass. This new class will provide simple key/value behavior without the hashing. We will retain the current hashing behavior in the SessionBase class for backwards compatibility, and it will inherit from InsecureSessionBase for all other methods.

[Gabriel Hurley]

This seems like a reasonable assessment to me, and I'm +1 on not doing unnecessary checks... as for the solution, it sounds alright, though I haven't looked deeply into the code there.

[Luke Plant]

Personally I'm in favour of fixing #14579 to enable a cookie backend.

There is also a pretty good argument for retaining this check even when storing the data in a 'trusted' datastore - essentially that not all pieces of infrastructure are equally trusted or equally open to attack, and layers of security are good.

Consider a situation where the attacker has write access to the database, but not read or write access to the source code of the application. In many situations, that will be 'game over' as far as security is concerned, but not all. Alternatively, consider the case where the attacker has write access to part of the filesystem, or the cache backend, but nothing else, and the app is using the filesystem or cache session backend. Now, it is relatively easy to create 'pickled' values that will execute arbitrary commands when unpickled (not just provide arbitrary values). So, without this tamper check, a breach of filesystem/cache/database will escalate into the ability to execute arbitrary commands.

A real life situation where this might occur is a shared hosting environment where the '/tmp' directory is used for the filesystem session backend — which is the default directory for the filesystem session backend. Since this directory is often world-writable (even if the files created there are not world readable), an attacker could well use this to do arbitrary code injection. A quick check on a server I know suggests this might well be a very practical attack since I can see hundreds of files in /tmp that appear to be PHP session files. Although I can't read them, I think I can delete them easily, and if they were Django session files with no tamper check then I could replace them with some malicious data. The owners of the corresponding web apps certainly have some security issues - I can delete all of their sessions - but with a tamper check in place, it is no worse than that.

So, I'm pretty strongly in favour of keeping this layer of protection. It is a worthwhile check given the escalation of privileges it could prevent in some fairly likely situations.

[Paul McMillan]

This ticket does not intersect with #14579, which should indeed be fixed.

The proposed fix would allow users to retain granularity with relation to the existing checksum behavior. Your proposed situation with an attacker who has DB write permissions is not relevant. An attacker who has write access to the database may modify user permissions to grant a limited account superuser status. We don't hash and checksum those.

More generally, what makes session data worthy of extra protection within a framework that we already trust with other highly sensitive data? Is your position that all pickled values in Django must be protected with HMAC to prevent potential code execution? It might not be a bad option.

I will concede that it would be best to retain the check for sessions stored on the filesystem. That is not an especially high performance option, and it is a likely area for security misconfiguration.

I will send you a private email with a few more points.

[Paul McMillan]

I'm closing this as wontfix. In light of the recent security release where this extra checking did in fact prevent a more serious problem, I now agree that the tradeoff is reasonable.

[Jacob]

Milestone 1.3 deleted