accounts: Make password security rules more clear

[garrett]

From the top tasks UX findings: "Several participants struggled with setting a password that fit the security rules, hitting error states over and over again."

The change password dialog should state the requirements which it is looking for.

[marusak]

The change password dialog should state the requirements which it is looking for

Right, but this will require some advanced parsing of some configs and stuff. But I am fully aware that it is far from ideal that you pick passwd and it is denied due to reason X, you fix it, try again and denied due to Y, try again... If pwscore could write all failed checks at once... But it cannot :man_shrugging:

and it should probably still allow setting the password even if it does not pass

We already have that :) https://cockpit-project.org/blog/cockpit-228.html

[garrett]

and it should probably still allow setting the password even if it does not pass

We already have that :) https://cockpit-project.org/blog/cockpit-228.html

I thought so. Was racing against the clock last night and was more concerned about recording issues than checking this detail. :wink: I'll adjust the description.

[garrett]

Checking the password on the fly would be another approach.

We should probably have a show/hide toggle for the password as well, so people can see what they typed that finally makes the check happy.

I could imagine errors like:

1. password is too short; user makes it longer
2. password is a dictionary word; user adds more words
3. password does not have symbols; user adds symbols
4. password needs an upper case letter; user adds an upper case letter

Then they went from bob to Xx9z Mr#.!? tPeXz@Za4_4917=2p%v — but without a toggle, they're not going to remember it or hope to add it to a password manager (as it'd need copy/paste and normal password fields don't usually allow it).

[garrett]

By-the-way, here's an excellent article on the topic: https://www.nngroup.com/articles/password-creation/

When a system doesn’t show the requirements beforehand, users are essentially being asked to play a game where the rulebook is hidden. That’s neither fun, nor fair.

...and more about password masking/unmasking: https://www.nngroup.com/articles/stop-password-masking/

[garrett]

I've gone through the man page for /etc/security/pwquality.conf and categorized every directive.

Next step is to work this into how we should display it. We can either go with a checklist of requirements, a sentence, or some hybrid (such as a min/max check being shown a one requirement).

Long descriptions for column headers:

1. directive
2. whether or not we should display it by default — or do not display it at all
3. a very short description
4. value it expects; the max/off/min corresponds to a positive number / 0 / negative number
5. default value, according to the man page (for reference; we should ensure we actually get the values from the system, of course)
6. description from the man page

directive	display, if applies	short	value	default	description
difok	after failing check	Too similar to old pw	min/off	1	Number of characters in the new password that must not be present in the old password. (default 1) The special value of 0 disables all checks of similarity of the new password with the old password except the new password being exactly the same as the old one.
minlen	yes	Minimum size, in characters	min (>= 6, default 8)	8	Minimum acceptable size for the new password (plus one if credits are not disabled which is the default). (See pam_pwquality(8).) Cannot be set to lower value than 6. (default 8)
dcredit	yes	Numbers	max/off/min	0	The maximum credit for having digits in the new password. If less than 0 it is the minimum number of digits in the new password. (default 0)
ucredit	yes	Uppercase letters	max/off/min	0	The maximum credit for having uppercase characters in the new password. If less than 0 it is the minimum number of uppercase characters in the new password. (default 0)
lcredit	yes	Lowercase letters	max/off/min	0	The maximum credit for having lowercase characters in the new password. If less than 0 it is the minimum number of lowercase characters in the new password. (default 0)
ocredit	yes	Symbols	max/off/min	0	The maximum credit for having other characters in the new password. If less than 0 it is the minimum number of other characters in the new password. (default 0)
minclass	yes	Types of characters	min/off	0	The minimum number of required classes of characters for the new password (digits, uppercase, lowercase, others). (default 0)
maxrepeat	after failing check	Repeat	max/off	0	The maximum number of allowed same consecutive characters in the new password. The check is disabled if the value is 0. (default 0)
maxsequence	after failing check	Sequential	max/off	0	The maximum length of monotonic character sequences in the new password. Examples of such sequence are '12345' or 'fedcb'. Note that most such passwords will not pass the simplicity check unless the sequence is only a minor part of the password. The check is disabled if the value is 0. (default 0)
maxclassrepeat	after failing check	Type, next to each other	max/off	0	The maximum number of allowed consecutive characters of the same class in the new password. The check is disabled if the value is 0. (default 0)
gecoscheck	after failing check	Does not include user info	on/off	0	If nonzero, check whether the words longer than 3 characters from the GECOS field of the user's passwd(5) entry are contained in the new password. The check is disabled if the value is 0. (default 0)
dictcheck	after failing check	Is not dictionary word	on/off	1	If nonzero, check whether the password (with possible modifications) matches a word in a dictionary. Currently the dictionary check is performed using the cracklib library. (default 1)
usercheck=N	after failing check	Does not contain username	on/off	1	If nonzero, check whether the password (with possible modifications) contains the user name in some form. It is not performed for user names shorter than 3 characters. (default 1)
usersubstr=N	after failing check	Does not contain substring	value/off	0	If greater than 3 (due to the minimum length in usercheck), check whether the password contains a substring of at least N length in some form. (default 0)
enforcing=N	no	Error vs. Warning (only for PAM)	on/off	1	If nonzero, reject the password if it fails the checks, otherwise only print the warning. This setting applies only to the pam_pwquality module and possibly other applications that explicitly change their behavior based on it. It does not affect pwmake(1) and pwscore(1). (default 1)
badwords	after failing check	Additional forbidden words	Space-separated list		Space separated list of words that must not be contained in the password. These are additional words to the cracklib dictionary check. This setting can be also used by applications to emulate the gecos check for user accounts that are not created yet.
dictpath	no	Path to dictionary	String	System default	Path to the cracklib dictionaries. Default is to use the cracklib default.
retry=N	no	Times to retry	max	1	Prompt user at most N times before returning with error. The default is 1.
enforce_for_root	no	Admin override		off	The module will return error on failed check even if the user changing the password is root. This option is off by default which means that just the message about the failed check is printed but root can change the password anyway. Note that root is not asked for an old password so the checks that compare the old and new password are not performed.
local_users_only	no	Users in /etc/passwd		off	The module will not test the password quality for users that are not present in the /etc/passwd file. The module still asks for the password so the following modules in the stack can use the use_authtok option. This option is off by default.

[garrett]

Just to clarify:

I'm only suggesting we show the following by default — and only if they apply (which only minlen does by default):

• minlen
• dcredit
• ucredit
• lcredit
• ocredit
• minclass

And we'd then show the errors too, after a check which should happen after a keystroke (debounced) and on a password form blur.

The table above is my analysis of the man page. It's here to explain how I arrived at that decision, and so others could double-check my work (in case I made a mistake or misunderstood something). It can also serve as a quick reference for development purposes.

[marusak]

Thank you @garrett for going through it! I think it makes sense to pick only some values and the rest show on submission (or debouncing key strokes and run this check while user types). Of course there is no sensible way how to describe dictionary or some other rules.

The table above is my analysis of the man page

Maybe we could also show forbidden words in form '... and cannot contain the following words: "foo", "bar".'

We can either go with a checklist of requirements, a sentence, or some hybrid (such as a min/max check being shown a one requirement).

I would really prefer sentence. I think the logic for constructing sentence would not be the hard. Assuming that we only would use up to 6 rules for it, the sentence would not even become too unwieldy. Not sure at this moment if we would need to have a separate code branch for every rule or we could try to generalize it. (at least 3 digits, at least 6 characters long, at least 1 number..., maybe we could even generalize it.)

checks = [
cockpit.ngettext("at least $0 character", "at least $0 characters", 6),
cockpit.ngettext("at least $0 digit", "at least $0 digits", 3),
cockpit.ngettext("at least $0 number", "at least $0 numbers", 1)
]

desc = ""
if len(checks):
    desc = _("The password must contain ")
    desc += ", ".join(checks[:-1])
if len(checks) > 1:
    desc += _(" and ") + checks[-1]

This code would produce The password must contain at least 6 characters, at least 3 digits and at least 1 number and ti would be even localized. We could treat minlen as a special case and have it as The password must be at least $0character(s) long and the continue with other values. We however need to count with case when minlen is not defined.

Note about cockpit.ngettext - it will pick the correct form for singular or plural dynamically based on the value. It also works correctly in other languages where rules for string forms are more complicated, like in Slovak there are 3 forms, one for 1, one for 2 to 4 and then for the rest, or in some Asian languages there is only one form for all strings.

[garrett]

Maybe we could also show forbidden words in form '... and cannot contain the following words: "foo", "bar".'

Yes, after it's checked. I was assuming it was part of the error. We shouldn't say that upfront; there are a lot of forbidden words. We probably shouldn't show the words up-front, but if a word is setting off the warning, then it's most likely obvious to which one, right?

I would really prefer sentence.

Sentence might look better, however a list can be a checklist and when the errors are met, they can get checked or go away. If we dynamically update the sentence, then someone needs to read the sentence again.

That said, I think the pure # of characters with a minimum / maximum bound can be treated as one error. Hence my suggestion of hybrid.

correct form for singular or plural dynamically based on the value

Super happy this is the case! Thanks for using it and pointing it out, especially how it's solved for additional languages, like Slovak.

I've seen (s) way too many times in other software. Also incorrect plural/singular too, such as the infamous "[something] 1 times".

[marusak]

I'm only suggesting we show the following by default — and only if they apply (which only minlen does by default):

• minlen
• dcredit
• ucredit
• lcredit
• ocredit
• minclass

So the issue is that minlen is not just minimum length of password if any *credit has positive value. Also positive values of *credit are difficult to explain in easy sentence but I don't think we need to do it

Example:

• minlen=20
• dscore=4

Then password bgh_Kl".ggh fails with The password is shorter than 20 characters. If I add one digit into this password bgh_Kl".ggh1 it now fails with The password is shorter than 19 characters. If I add 3 digits it fails with 17 characters, with 4 it is 16 characters, 5 and more it is still 16 characters.

The same is true for any other *credit. So we cannot say Minimum required lenght 20 characters as when they type in digit it would change to 19 etc. Or is it something we want to do? To update this minimum number as they type? Or don't show minimum length if any credit is > 0?

Also we need to parse /etc/security/pwquality.conf.d/*.

Note: We can use a password pattern, using PatternFly's helper-text.

@garrett WDYT?

[garrett]

So the issue is that minlen is not just minimum length of password if any credit has positive value. Also positive values of credit are difficult to explain in easy sentence but I don't think we need to do it [...] Or is it something we want to do? To update this minimum number as they type? Or don't show minimum length if any credit is > 0?

What if we flip it upside-down and say what the weights are? It could look something like this:

⚠ Minimum length is not met. (Currently: 12 of approximately 20 characters.) – Increase password strength using: numbers, uppercase letters, lowercase numbers, and other characters. – Try to minimize use of: repeating characters

The idea is that we'd have a phrase at the beginning and then take all the various credits that are enabled, sort by number (so the more weighted ones come first), and turn them into a list. We have one list for positives, another for negatives. Each part of the list to join would be translated as a string and those strings would be joined together using Intl.ListFormat so it looks correct for each language.

"Other characters" could be problematic. Punctuation? Symbols? :shrug:

Of course, leave out any credits that are 0. And if there are no negatives, don't show that. And if there are even no positive credits, don't show that either.

Is this a silly idea?

If there are no credits whatsoever, it's more straightforward:

⚠ Minimum length is not met. (Currently: 12 of 20 characters.)

[marusak]

Is this a silly idea?

I like it! I see some issues that we might encounter like translating Increase password strength using: numbers, uppercase letters, lowercase numbers, and other characters. when we want it to only contain enabled credits. Also which rule says Try to minimize use of: repeating characters? Or is that just general advice?

Maybe if dcredit and lcredit and enabled we could do something like: (second and third row would be moved to right to show they are just subrules for the first one)

⚠ Minimum length is not met. (Currently: 12 of approximately 18 characters.) `⚠ Try using more digits ` :heavy_check_mark: Try using more lower case numbers ⚠ At least 3 out of: digits, lowercase, uppercase and other characters

It would start with approx. 20 chars and all would be ⚠. Then when they type in some lowercase numbers, we edit approx 18 chars and add check when there are more or equal then credit value of lcredit.

The last rule represents minclass=3

Also, it can become rather long. Do we want to fit it all into the dialog or we want to make it collapsible or something?

[garrett]

Credits can be 0, positive, or negative. Must have / must not have depend on positive or negative. One line for positive credits, one line for negative credits. Don't show when credits are 0. (And when no credits are positive, don't show the positive credit line. When no credits are negative, don't show the negative credits line.) "Repeating characters is a credit, and it's usually negative, so prevent # of repeating characters in a row, hence the example.

Also, it can become rather long. Do we want to fit it all into the dialog or we want to make it collapsible or something?

We're removing confirm and adding a toggle to password. It should then take up the same amount of vertical space.

We definitely don't want to make this collapsable/expandable.

---

Also, related to #16917, the alignment of the "Lock account" option is off.