search

backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

[SR] Update the password strength indicator for longer/stronger passwords #4602

Closed jenlampton closed 3 years ago

jenlampton commented 4 years ago

(Written by @herbdool ) As per recommendations for lengthy passwords by the NIST and other security experts we should tweak the password strength algorithm so that a "fair" password is at 12 rather than at effectively 8. According to NIST:

Password length has been found to be a primary factor in characterizing password strength. Passwords that are too short yield to brute force attacks as well as to dictionary attacks using words and commonly chosen passwords. ... Users should be encouraged to make their passwords as lengthy as they want, within reason.

8 is not set explicitly but rather it's the shortest length possible if someone adds all the variety of characters that add to the entropy: e.g "Aa1$". After those four characters only length matters (at least it seems so in testing by @herbdool).

As @quicksketch mentioned in #293:

I realized that it makes no sense to coorece people into strictly adding numbers or special characters when it really the level of entropy that matters. In that case, a ton of lowercase letters or even numbers are okay, given that they are sufficiently long.

Ref:

Related issues:

PR by @herbdool: https://github.com/backdrop/backdrop/pull/3291

jenlampton commented 4 years ago

I think this is a great idea, I wonder if it would qualify to go in a bug-fix release? adding label to see.

indigoxela commented 4 years ago

@jenlampton and @herbdool many thanks for starting this issue. I'd like to get some more structure into this and extended the description.

Let's decide first, then implement. :wink:

jenlampton commented 4 years ago

I think setting the minimum length to 12 is a great first step, and I'd be really happy if we could get even that much into 1.17.

I'd like to get some more structure into this and extended the description.

I agree that we should continually re-evaluate how Backdrop measures password strength. But can we create another issue to do the more in-depth evaluation? I wouldn't want a longer discussion and evaluation of the whole problem-space to delay this improvement, or possibly prevent us from making any improvements at all.

Let's decide first, then implement. 😉

We already know that a 12 character password is better than an 8 character password. Sine we've already "decided" that much, this issue can be used for this first implementation.

In the other issue, we can take the time necessary to discuss the bigger picture, and we can make more decisions. There's no reason for us not to make this improvement while we also consider other improvements. :)

indigoxela commented 4 years ago

As per recommendations for a minimum length of 12 by the NIST and other security experts we should tweak the password strength algorithm so that a "fair" password is at 12

Just for completeness: what publication does this refer to? A link would be cool.

I only found that one an that recommends "at least 8"...

indigoxela commented 4 years ago

I did some experiments in the sandbox.

Here's a list of "passwords" that get a "fair" status:

11111111111111111111111111
aaaaaaaaaaaaaaaaa
asdfasdfasdfasdfa
correcthorsebatte
password1234567
happily eatin
ageich5Xiephp
ud9aech8ur8HW
ki"uW0che:g
po3De[o2if>
w5%Basdfqwe

If complexity is high, at least 11 characters are needed.

domaingood commented 4 years ago

I don't think minimum length to 12 will be good.I think minimum length 8 is fine.12 is too high. But Password Include Symbols:( e.g. @#$% ) + Numbers:( e.g. 123456 )+Lowercase Characters:( e.g. abcdefgh ) Must in 8 All We need Otherwise it not take as password.

docwilmot commented 4 years ago

@domaingood I think the idea here is that on https://howsecureismypassword.net/ no password less than 10 characters is considered good (the form turns blue) regardless of the complexity. All passwords of 8 characters are considered poor.

So if blue screen is the next step from poor, I'd say 10 based on that site because all complex passwords (caps numbers non-standard chars) turn the screen blue. It would be unreasonable to tell a user that a complex 10 character password is poor.

indigoxela commented 4 years ago

I think minimum length 8 is fine

@domaingood I'm afraid that's not really the case anymore. No matter what complexity a password has, if the length is 8, it could get cracked within hours.

Background: Passwords weaken dramatically as technologies evolve, a password that was strong back in 2010 could be weak nowadays and we should consider that.

12 is too high

Possibly, but anything shorter than 10 (or maybe 9?) is too weak nowadays.

jenlampton commented 4 years ago

I don't mind setting a "fair" password at 12 characters rather than at 8, but even bumping to 10 would be an improvement. If we can't get majority agreement or consensus on 12, can we shoot for 10 instead?

herbdool commented 4 years ago

We should err on the side of longer for a couple reasons:

herbdool commented 4 years ago

I was reading the Dropbox article https://dropbox.tech/security/zxcvbn-realistic-password-strength-estimation, where @quicksketch got the algorithm. But I couldn't figure out why it mentions doing a log base 2 of the possible combinations. I think it has to do with information theory and entropy: https://en.m.wikipedia.org/wiki/Entropy_(information_theory)

herbdool commented 4 years ago

I've taken a simpler approach. I've left the entropy calculation as-is since it matches that of the zxcvbn basic entropy calculation. But since the absolute entropy number is not as important, and our strength indicator goes from 1 to 100, we can just add a modifier so that we can change what is considered weak or strong. As computers get faster, the modifier should probably adapt so that passwords need to be longer.

~I made it so that the modifier can be changed per site, though I'm not sure if that's the best approach. Maybe it should just be a constant so that the number can change over time.~ The modifier is a constant that can be adjusted in future updates. I didn't want to make it directly a config setting since then we wouldn't be able to tweak it in the future since we wouldn't want to override others. Perhaps there could be a hook so others can adjust it.

indigoxela commented 4 years ago

Some more experiments in the sandbox. Here's a list of passwords that get a "fair" status:

1234123412341234123123
asdfasdfasdfasd
righthorsebatte
WONDERWALLSTREE
password123456
phez4aY1ietr
Ag5%ddddddd
Ag5%He8+Zk3

If only using digits, it needs 22 of them. If only using uppercase or lowercase, it needs 15. If using upper/lower/digit but no punctuation, it needs 12. If using all groups, it needs 11.

So the minlength is 11. Is this the desired result?

UPDATE: forgot to flush the caches before testing, examples have been updated.

herbdool commented 4 years ago

@indigoxela I was aiming for 11 or 12. I can tweak the constant a bit to make it 12. (I guess we're fighting against people potentially using passwords like: Ag5%password which gets a good grade but really shouldn't, but that's for a different issue.)

indigoxela commented 4 years ago

I was aiming for 11 or 12

Then it does exactly what it's supposed to do. :grinning:

I can tweak the constant a bit to make it 12

That constant comes handy - a simple switch to change length/strength. Maybe a little more explanation in the comment makes sense, for instance what it needs to get a stronger password.

One thought: "fair" vs. "strong" - do we already enforce "strong" with a minlength of 12? If comparing to results on howsecureismypassword.net I get the impression that we do.

herbdool commented 4 years ago

I'm not sure about fair vs strong. Why do you ask?

indigoxela commented 4 years ago

I'm not sure about fair vs strong. Why do you ask?

Because I belief that a minlength of 11 (or 12) leads to "strong" passwords, while the indicator shows "fair". We didn't decide (yet) what the meaning of "fair" or "strong" should be (how hard to crack...), that's why I ask.

herbdool commented 4 years ago

Since we added ability to reject weak passwords, by default we're accepting all passwords that are fair or stronger. So in the terminology we've got so far "fair" = "strong enough".

I think 11-12 is just barely strong enough since people tend to use patterns that are easily brute forced, so I think describing it as "fair" is accurate.

jenlampton commented 4 years ago

I also think 'fair' is accurate for 12 chars or less. I recommend at least 20-character passwords on projects where security is a concern.

On Fri, Sep 11, 2020, 7:07 AM Herb notifications@github.com wrote:

Since we added ability to reject weak passwords, by default we're accepting all passwords that are fair or stronger. So in the terminology we've got so far "fair" = "strong enough".

I think 11-12 is just barely strong enough since people tend to use patterns that are easily brute forced, so I think describing it as "fair" is accurate.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop/backdrop-issues/issues/4602#issuecomment-691116378, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBER6MGCA2Y26RPZ3I33TSFIVLFANCNFSM4Q3HP4CQ .

herbdool commented 3 years ago

I'm just getting back to this. It still needs updated labels for testing and review. @indigoxela you mentioned you tested and it worked for you. Want to do a code review too?

Regarding: "Maybe a little more explanation in the comment makes sense, for instance what it needs to get a stronger password." If I understand your suggestion it seems like a more general suggestion about the whole password indicator. If so, then we can create a new issue to improve that I think?

indigoxela commented 3 years ago

@herbdool I totally lost this issue out of sight. :wink: Sure, mention me again, when it's ready for more testing.

Regarding "explanation in the comment": I actually meant the code comments. They're sort of outdated and the logic isn't explained very well. Show mercy to the next developer touching this. :laughing:

herbdool commented 3 years ago

@indigoxela I've tried to improve the code comments. Want to take another look?

indigoxela commented 3 years ago

I've tried to improve the code comments.

@herbdool Excellent, very helpful. I'll have a closer look to your PR soon (testing again and review).

indigoxela commented 3 years ago

Some final testing. With this PR we get:

FAIR

GOOD

EXCELLENT

That all seems reasonable.

I also played a bit with the USER_PASSWORD_STRENGTH_MODIFIER constant locally. Works like a charm.

The code is also looking good. RTBC! @herbdool many thanks for all your work and, most of all, patience. :+1:

quicksketch commented 3 years ago

This hadn't yet been assigned a milestone, but based on the change in behavior, I thought it to be a better fit for 1.18.0 vs. 1.17.5. I merged https://github.com/backdrop/backdrop/pull/3291 into 1.x only. Since both releases are coming out tomorrow, I doubt that will cause any concern. Thanks @herbdool, @indigoxela, @docwilmot, @jenlampton, and @domaingood for your work on this issue!