-
-
Notifications
You must be signed in to change notification settings - Fork 365
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
Blacklist common passwords #832
Comments
Probably (definitely?) overkill, but https://haveibeenpwned.com/Passwords has a known exposed passwords list. All encoded in SHA1 (cause there are people out there that have passwords that identify them 🤦♂️). Pity it has no frequency with them. |
@Silic0nS0ldier awesome idea. nextcloud/password_policy#60 This is a good example / an easy way of getting started. |
Since you've reminded me, there is a password manager that has since integrated with the service. And based on Troy Hunt's blog posts, the API endpoints are highly optimised (most requests don't get past the CDN which Cloudflare footed the bill of) so any potential bandwidth theft implications are essentially eliminated. Hmm... What would be the best place to perform the check? Server-side or client-side? |
If you rely on a 3rd party API, we should consider caching the info on our side using the cache service. Then it's either a question or doing the check in either form validation (client or server side). |
Update regarding the API: only anonymous requests will be accepted in the future. The anonymous API works by giving the first few characters of the hash, and receiving hashes that match in return (minus the characters that were sent originally). |
On the topic of caching, we'd need some system to keep the size to a reasonable amount. We could also limit caching to common results, since each hashed password is paired with a number indicating the number of items its appeared. I'm sure one of Troy's blogs has a graph we can use to get an idea of what a good number would be. |
Some things to consider are mentioned here https://twitter.com/alexjamesbrown/status/1063552695133487104 |
I have been playing around with this idea to see what I can come up with. I am having a hard time trying to determine if it would really be worth using this with the cache service or not? Would the purpose of that just be for server-side validation? |
Cache service would be to minimise the total amount of bandwidth consumed (have to consider HIBP and the site making the request if server side) and cut out an extra request where we can. As for if it should be server side or client side enforced... Sounds like something that should be configurable. Personally a requirement is my preference. Also worth considering if we want to do the check at login, to keep accounts having a secure password. |
Would the best place to store configuration options for something like this be inside the |
Depends if you want to store the base config (on /off switch) or the whole list in that config. If the list is long, or dynamically loaded, it might not be the best place. The I think for this kind of feature, server-side only check could be enough. We need to be careful with the HIBP list. More then 7 GB of data 😱 |
Yes I think just the base config would be stored there. Just as an example I have this so far:
A short refresher on how the HIBP API works: It uses a k-anonymity model. So, you send the first 5 characters of SHA1 hash of password, and get back a list of hash suffix. Then you compare the long list of hash suffix with your actual password hash suffix. Their website states the smallest result is 381, the largest 584. |
Oh, so you don't have to fetch the whole list... makes sense... |
I can recommend the Nextcloud example. As being said they also use haveibeenpwned |
I just noticed that the PR related to this doesn't appear to be linked here. #974 |
To harken back to what Alex mentioned initially, we probably still want to keep https://blog.codinghorror.com/password-rules-are-bullshit/ in mind as I'm sure there are lessons to be learned and applied from that article. |
As mentioned in 5. of the article, I think there would be benefit from also restricting use of these two:
And possibly these two:
|
Open to suggestion, but I think it would be a good community sprinkle instead. Not priority for now IMO. |
Together with minimum length, blacklisting common passwords is the most effective known password policy.
Not sure how many we should blacklist by default.
Probably, we should also enforce an entropy requirement and blacklist
password = username
andpassword = email
. See https://blog.codinghorror.com/password-rules-are-bullshit/The text was updated successfully, but these errors were encountered: