WoltLab / WCF

WoltLab Suite Core (previously WoltLab Community Framework)
https://www.woltlab.com
GNU Lesser General Public License v2.1
237 stars 144 forks source link

Password strength estimator #3378

Closed BurntimeX closed 4 years ago

BurntimeX commented 4 years ago

When entering a new password, the user should be shown a visual indicator of the estimated strength of the password.

We could use the following solution: https://github.com/dropbox/zxcvbn

I would also like to remove the existing password security settings (registerpassword*), as these do not always lead to better passwords.

TimWolla commented 4 years ago

I would also like to remove the existing password security settings (registerpassword*), as these do not always lead to better passwords.

Agreed. But I think a somewhat sane security level should be enforced by default then. As an example:

These do not disallow any obviously strong passwords, but prevent the worst of the bad ones.

The check could then be disabled in development + debug mode to allow stuff like test for testing on localhost.

BurntimeX commented 4 years ago

zxcvbn calculates a score for the password. We could require a minimum value for the score and reject all passwords with a score below 2.

TimWolla commented 4 years ago

That would require putting it both into the client and the PHP clone onto the server, keeping them properly in sync.

But yes, that would probably be the better solution if we're already going for zxcvbn.

cadeyrn commented 4 years ago

Either: At least 12 characters or: At least 10 characters with lowercase, uppercase and digits.

These are classic anti-patterns.

Reading recommendations:

dtdesign commented 4 years ago

I agree with @cadeyrn, enforcing arbitrary character classes is stupid. In case you forgot: https://xkcd.com/936/.

It should be sufficient to "validate" the password in the client and if the password is considered weak, notify the user. Maybe even prompt them if it is too weak, but if they want to use it, let them. A certain minimum length should be in place regardless, because we need at least some entropy.

TimWolla commented 4 years ago

I'm very well aware that length beats character classes, that's why my suggestion included the “free pass” with regard to classes once you reached 12 characters (e.g. allowing for diceware passwords or anything coming out of a generator).

For a smaller length at least requiring the three basic character classes (I intentionally left out special characters, because of the bad user experience on phones) ensures that there's some basic security level.

GangstaSunny commented 4 years ago

I agree to @TimWolla but this still wouldn't be a real good solution. Always keep in mind that the most users will then have to use passwords they can't remember. Because of that they will probably reuse their passwords a lot.

It may depend on community and even if its used internally or don't. So it should be - in my opinion - an administrator choice.

dtdesign commented 4 years ago

The biggest misconception about password rules are that they lead to better passwords. They do not, all they do is forcing users to slightly modify their password to match the rules: If password does not qualify, password1! isn't significantly safer. The extra character classes aren't improving the password because many users will use predictable prefixes/suffixes to match the rules.

I guess we could use zxcvbn's password score and add an option to enforce a minimum score, for example, > 0 or > 1. Anything above that is unrealistic and annoys the user. Do not enforce anything else, it wouldn't improve the security.