flask-session
flask-session copied to clipboard
pallets-eco
Added support for signer alternatives
We are interested in using flask-session as our session system but noticed a lack of support for a SHA256 HMAC. In the past, I have been informed that this is stronger than the current default of a SHA1 HMAC (mostly due to length -- SHA1 is 160 bits, SHA256 is 256 bits).
https://en.wikipedia.org/wiki/Hash-based_message_authentication_code#Security
This PR adds backwards compatible support to opt in to using a SHA256 HMAC. In this PR:
- Added test to verify HMAC exists in SHA1 variant
- Added test for verifying SHA256 HMAC provides a longer HMAC
- Added collection of various signers
- Added
signer_typeoption to all Session classes
Maybe we should add support for customizing a signer, which will make everyone happy. We can provide a config key SESSION_SIGNER, you can pass in any signer instance you want.
Could you provide a more detailed example of what using that code would look like?
While that works, I would be opposed to everyone rewriting/reimplementing the same SHA256 signer. As with anything related to security, the algorithm can be great but someone can mess up implementation and ruin it (e.g. what if someone forgets the salt).
We ship with the default one, mostly people wouldn't change it. If anyone does, there are endless possible demands, we cannot ship all of them, someone who changes the default behavior should be aware of what exactly he is doing.
While I understand the concern for maintenance, it's pretty irresponsible for something as sensitive as handling user sessions. If someone can forge someone else's user session, then they have full access as that user. Can you elaborate on what your main concern is?
There is no concern at all. Your solution solves your problem, not everyone's problem, it is that simple.
Right but it sets up a framework for everyone to reuse signers. I imagine that someone in the future might want sha512 and this would allow for them to reuse that code by adding yet another function.
On the other hand, I don't foresee anyone ever wanting to roll their own signer fully as that violates the rule of not implementing custom security solutions.
We can add one config SESSION_SIGNER_PARAMS, which should be the all kwargs that itsdangerous.Signer takes, then we check it and make sure every param is passed in. On the one hand, you can customize anything you want for any reason, on the other, everyone can use their own salt.
That seems more restricting than the originally proposed SESSION_SIGNER parameter itself. The concept of having multiple built-in options and a custom SESSION_SIGNER override aren't mutually exclusive. How about supporting both a SESSION_SIGNER_TYPE (this PR) and SESSION_SIGNER (which fully overrides the signer)?
Intent is to deprecate signer from 0.7.0 #212. The relevant entropy should be provided by the session identifier length. Flask-session is for only serverside sessions so I can see no point in signing only the session id and expiry, that is effectively just making the sid longer, which can be ajusted in SESSION_SID_LENGTH
If anyone can put forward an alternative case I would like to hear it
