Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove examples and discourage password as key value #125

Closed
jsocol opened this issue Jul 30, 2017 · 2 comments
Closed

Remove examples and discourage password as key value #125

jsocol opened this issue Jul 30, 2017 · 2 comments
Milestone

Comments

@jsocol
Copy link
Owner

jsocol commented Jul 30, 2017

There are some potential risks around using key='post:password', especially with cache stores like Redis that make it possible to enumerate cache keys. @willkg and I decided to replace those examples with a note similar to

We don't recommend creating ratelimit keys using any kind of secrets like passwords, api keys, aws keys, private keys or things like that. The way ratelimit works means that those things would come in as strings, get md5 hashed and then stored in your cache backend creating a virtually-plain-text copy of secret things in your cache backend. That's a terrible situation to be in security-wise. Don't do it.

@jsocol jsocol added this to the 1.1 milestone Jul 30, 2017
@mlissner
Copy link

The current docs give basically the opposite conclusion and say:

Key values are never stored in a raw form, even as cache keys.

That tipped me off to this being an issue because I realized that pbkdf2 would be too slow to do with every request and that any fast solution would be a risk. From there I dug into the code to see how this was handled and was pretty dismayed to find the md5 code.

I hope this can be addressed urgently. It's only a documentation issue.

@willkg
Copy link
Collaborator

willkg commented Oct 31, 2017

@mlissner PRs welcome!

jsocol added a commit that referenced this issue Dec 29, 2018
Fix #125. Remove passwords from examples and recommended cache keys and
emphasize note in security section about fast hashes.

[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants