cachelib
cachelib copied to clipboard
sign cache values
I have a suggestion: It should be possible to sign (/ apply HMAC) to cache values in the same way werkzeug.contrib.securecookie
does already.
pickle
is used as serializer to serialize the content. While this is absolutely fine as long nobody can access the underlying cache back end (Redis, FS, Memcached), it may allow privilege escalation once an attacker gains access to it, as pickle
allows to store arbitrary code.
Proposal:
- Add a warning to the documentation.
- Add the option pass a signing key to sign the results and raise a warning if no signing key is passed at initialization.
- Deprecate not using a signing key and ultimately enforce using one.
Practically pallets' ItsDangerous
could be used here.
If wanted, I can create a pull request implementing my proposal.
Hey @Varbin, I would be happy to review a PR for this if you are still interested!
Sure! I think #11 should be solved first, to not have accidentally conflicting code.
Anyway, my proposed change will be like:
- Add
itsdangerous~=2.0.0
as installation requirement. - Add key kwarg to the class constructors
- Add
_load
and_dumps
methods to theBaseCache
class, which will replace calls topickle.loads
/pickle.dumps
- if a key is set this will call the itsdangerous instance, otherwise plain pickle. - Add a deprecation warning to calls without a key.
The only class that will not support the key argument will be memcached, as the memcache client libraries already do serialization.
Add a deprecation warning to calls without a key.
I'm not sure on that part. IMHO it should be up to the user. Not signing cache data is only an issue when you have an attacker with arbitrary write access to your cache backend - which means you are either already compromised (in that case the secret might no longer be secret anyway) or severely misconfigured stuff (redis bound to a public IP and not blocked by a local firewall and directly exposed to a malicious network/client)
Sure! I think #11 should be solved first, to not have accidentally conflicting code.
Don't let this hold you back @Varbin. If you feel like getting started, please go ahead. I'll just solve the conflicts when the time comes.
- Add a deprecation warning to calls without
I agree with @ThiefMaster on that one. I would rather add a very conspicuous note to our (soon to be written) documentation stating that using a key is strongly recommended.