-
-
Notifications
You must be signed in to change notification settings - Fork 677
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
2.1.14 needs changing after researching this issue #1055
Comments
What defines the "breached password list"? |
https://auth0.com/breached-passwords describes is briefly. What do you think of: `[ADDED, SPLIT FROM 2.1.7, LEVEL L1 > L3] Verify that passwords submitted during account registration or password changes are checked against a list of breached passwords discovered in third party data breaches. (C6) |
I meant "who" defines the passwords in the "breached password list". I believe we're on the same page to not declare a commercial but free service as the standard similar to @jmanico stance on BugCrowd et al. Below is the list of breaches defined by Google's Password Manager which is a commercial but free service too:
As far as I am aware @HaveIBeenPwned doesn't publish the same corresponding list of breaches for "Pwned Passwords" as their methodology differs. |
Adding some kind of reference to OSS breached password lists seems reasonable. I also think it's ok to just list "breached password lists" and not mention commercial or other sources.
|
A standard needs to be measured against otherwise the minimum will become acceptable. Perhaps we should make this into an OWASP Project? |
I'll give interim approval to PwnedPasswordsTop100k.txt as it is published by NCSC @jmanico until their dataset is public domain. |
https://www.ncsc.gov.uk/static-assets/documents/PwnedPasswordsTop100k.txt
is certainly a good resource to add to ASVS.
|
Related issue and discussion: #841 |
I agree. I think this is an improvement. I think this requirement can be more clear on that passwords that are in the list should be denied immediately. I don't think we need to be specific which list should be used. For non-English sites it may make sense to use a non-English list. Some developers may prefer using an API over a static word list. |
I agree with all of your comments. I want this password list added as a reference at the bottom of this section as a secondary reference, not in a requirement itself.
|
PwnedPasswordsTop100k.txt isn't limited to English. Will ASVS mitigate https://cablej.io/blog/k-anonymity/ of the upstream API too? |
I am not sure, and I think the password list is a good resource to add at the end of the section. There are MANY ways to address credential stuffing other than the |
I believe ASVS should mandate a minimum standard but have actioned #1061 in the interim. |
Christian I'm closing this out now as the original issue is solved, but I'll take a PR where https://www.ncsc.gov.uk/blog-post/passwords-passwords-everywhere#:~:text=PwnedPasswordsTop100k.txt is listed as a reference at the end of the section. If you give me an ok I'll take it on as well. |
I can rename #1061 Pull Request and then resubmit #1061 Pull Request from a different branch @jmanico Also #1071 is missing the list of Google's Password Manager above i.e. #1055 (comment) |
Please just submit a new PR if that’s ok, I’m new to administering GIT it’s not intentional.
I also opened a new issue for this as well.
#1071
Is this ok for now, Christian?
|
2.1.14 is currently:
[ADDED, SPLIT FROM 2.1.7, LEVEL L1 > L3] Verify that passwords submitted during account registration or password changes are checked against a set of breached username/password pairs. (C6)
After reviewing the various services and talking to both @tghosth and Troy Hunt on this issue, I suggest we change this to:
`[ADDED, SPLIT FROM 2.1.7, LEVEL L1 > L3] Verify that passwords submitted during account registration or password changes are checked against a list of breached passwords. (C6)
The text was updated successfully, but these errors were encountered: