Closed bramvbilsen closed 3 months ago
Added functionality as discussed in https://github.com/alexedwards/scs/issues/195#issuecomment-1926452646
Awesome! I'll review and merge this as soon as I get time :+1:
@alexedwards Any updates on this one? Is there anything I can do to help with the review? Or something you'd like me to improve?
@bramvbilsen Thanks for doing this, and sorry for the delay in getting it reviewed. It all looks good to me, I'll merge it now and push a new release.
No worries, thanks for taking the time to review! 😊
If tokens are sensitive, then wouldn't a hash be the wrong tool to use[1]? I would think argon2, scrypt, bcrypt, et. al. would be the more recommended options if session tokens were considered sensitive enough to require "enhanced security measures".
[1]: Just as they are the wrong tool for storing credentials in a database
@dropwhile You are correct, do not store SHA256 password hashes in your database! Use something like bcrypt instead.
However, as tokens are being used in this library to do lookups, we can't use bcrypt as hashing the same input twice would yield in different results due to salting. Thus we would never be able to perform a lookup once stored. Now, the salting protects against lookups in rainbow tables. Gigabytes (if not more) of passwords can be found in these tables. BUT we are not storing passwords! We are storing completely randomly generated tokens. So a rainbow table (I believe) would not help as they'd have to just try out all different combinations. Thus, cracking the token is now a matter of brute-forcing. But it is statistically improbable that someone would be able to do so in the short time the token is alive.
Of course, if there's any doubt about what I mentioned here. Do let me know! I'd love to discuss. I could totally be overlooking something. I think discussing this will lead to better security 😊
Summary
This PR introduces an opt-in feature that allows storing session tokens as hashes in the storage system. The goal of this update is to enhance security measures by ensuring that session tokens, which are sensitive in nature, are stored in a hashed format (SHA256). This mitigates risks associated with the exposure of raw tokens. Alongside the core functionality, tests have been added to validate the implementation and ensure its reliability.
Changes
The
SessionManager
is extended to accept a boolean value for the newHashTokenInStore
field.HashTokenInStore == false
: The behavior from before this PR is used. I.e. session tokens will not be hashed before getting saved in the storeHashTokenInStore == true
: Session tokens will be hashed before being saved to the store. Subsequent lookups (whether it be finds, deletes...) will first hash the provided token before using the hashed value in the lookup.This PR should not bring any breaking changes as it defaults to the old behavior. Rather, it extends functionality by providing extra opt-in security features.