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

Add and enforce password validation #1233

Closed
tw4l opened this issue Sep 30, 2023 · 7 comments · Fixed by #1266
Closed

Add and enforce password validation #1233

tw4l opened this issue Sep 30, 2023 · 7 comments · Fixed by #1266
Assignees

Comments

@tw4l
Copy link
Member

tw4l commented Sep 30, 2023

Requirements

  • Between 8-64 characters in length
  • Not one of the top n commonly used passwords
  • Doesn't include their name or email

Pages

  • Sign up
  • Reset password
  • Account settings
@tw4l tw4l moved this from Triage to Design In Progress in Webrecorder Projects Sep 30, 2023
@tw4l tw4l self-assigned this Sep 30, 2023
@ikreymer ikreymer moved this from Design In Progress to Todo in Webrecorder Projects Sep 30, 2023
@SuaYoo SuaYoo added this to the DRAFT Release 2023.10 milestone Oct 2, 2023
@tw4l
Copy link
Member Author

tw4l commented Oct 2, 2023

OWASP recommendations:

  • Password Length
    • Minimum length of the passwords should be enforced by the application. Passwords shorter than 8 characters are considered to be weak (NIST SP800-63B).
    • Maximum password length should not be set too low, as it will prevent users from creating passphrases. A common maximum length is 64 characters due to limitations in certain hashing algorithms, as discussed in the Password Storage Cheat Sheet. It is important to set a maximum password length to prevent long password Denial of Service attacks.
  • Do not silently truncate passwords. The Password Storage Cheat Sheet provides further guidance on how to handle passwords that are longer than the maximum length.
  • Allow usage of all characters including unicode and whitespace. There should be no password composition rules limiting the type of characters permitted.
  • Ensure credential rotation when a password leak occurs, or at the time of compromise identification.
  • Include password strength meter to help users create a more complex password and block common and previously breached passwords
    • zxcvbn-ts library can be used for this purpose.
    • Pwned Passwords is a service where passwords can be checked against previously breached passwords. You can host it yourself or use the API.

@Shrinks99
Copy link
Member

Shrinks99 commented Oct 2, 2023

Looks like zxcvbn (nice name very creative) has a haveIbeenpwned matcher built in which is cool!

Watching the USENIX talk about it, this seems like a great solution... A few points?

  1. Daniel mentions that he prefers UI that "lets users fix the password any way they like" and not imposing strict requirements. Considering that we will have strict length requirements, I do think those (and any others we absolutely need) should be explicitly listed along with the zxcvbn password improvement info. I personally find having to figure out password requirements as I go to be infurating (as somebody who's passwords are random character strings that are as long as I'm allowed to enter) :P
  2. As a follow up to that, their library seems to basically output a guess count and score that developers (us) can use to demand users enter a more complex password. In the talk, Daniel mentions that this could potentially be quite confusing to people if the demanded score is set too high (see previous point). Should we only require the character limit to be reached and use zxcvbn as a guideline? Should we set a low score as a requirement? He also mentions that the estimator produces metadata that can be used to provide more feedback to the user which would be nice to investigate further.

Fundamentally zxcvbn seems to be a reactive method of telling users what their password should be rather than a proactive one (requirements based). I guess this is mostly something for me to work within, how can we best convey the requirements and recommendations it sets forth to users? 🤔

@tw4l
Copy link
Member Author

tw4l commented Oct 4, 2023

First step (requiring passwords to be between 8-64 chars) merged. Seems like we could add zxcvbn to the frontend as a next step to show users the relative strength of their password - might need design from @Shrinks99

@tw4l tw4l moved this from PR In Review to Dev In Progress in Webrecorder Projects Oct 4, 2023
@tw4l
Copy link
Member Author

tw4l commented Oct 5, 2023

It seems that the Dropbox zxcvbn repo has been dead for a number of years, but there is an active typescript rewrite that might work better for us anyway: https://github.com/zxcvbn-ts/zxcvbn

@tw4l tw4l assigned SuaYoo and Shrinks99 and unassigned tw4l Oct 10, 2023
@ikreymer ikreymer moved this from Dev In Progress to Todo in Webrecorder Projects Oct 10, 2023
@Shrinks99
Copy link
Member

While zxcvbn itself is a scoring library that gives suggestions, we want to give users as many reasonable hard requirements that will result in low scores to reduce guesswork while creating a strong password. We should list these, use zxcvbn to check for them, and additionally list actionable suggestions it generates to help people choose stronger passwords. As a bonus check, it would be great to warn them if the password has been compromized! (zxcvbn also does this!)

Hard Requirements

  • Between 8-64 characters in length
  • Not one of the top n commonly used passwords
  • Doesn't include their name or email

@SuaYoo SuaYoo moved this from Todo to Ready for Dev in Webrecorder Projects Oct 10, 2023
@SuaYoo SuaYoo moved this from Ready for Dev to Dev In Progress in Webrecorder Projects Oct 10, 2023
@SuaYoo SuaYoo moved this from Dev In Progress to PR In Review in Webrecorder Projects Oct 11, 2023
@SuaYoo SuaYoo moved this from PR In Review to Dev In Progress in Webrecorder Projects Oct 11, 2023
@SuaYoo
Copy link
Member

SuaYoo commented Oct 11, 2023

Should we reject passwords on the frontend with a score less than 3? Or less than 4?

@Shrinks99
Copy link
Member

Shrinks99 commented Oct 11, 2023

Anyone with a score less than 4 is likely not using a password manager, it's tricky balancing security recommendations with the inherently flawed system of having to remember everything :\

IMO less than 3? We may want to also include a note recommending they use a password manager as a suggestion for passwords that aren't scored at a level 4, maybe link to https://www.privacyguides.org/en/passwords/ ??

@SuaYoo SuaYoo moved this from Dev In Progress to PR In Review in Webrecorder Projects Oct 11, 2023
@github-project-automation github-project-automation bot moved this from PR In Review to Done! in Webrecorder Projects Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done!
Development

Successfully merging a pull request may close this issue.

3 participants