After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 691907 - passwords of sufficient length are rejected as too short
passwords of sufficient length are rejected as too short
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: User Accounts
3.6.x
Other Linux
: Normal normal
: ---
Assigned To: Michael Catanzaro
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-01-17 00:35 UTC by Michael Catanzaro
Modified: 2016-09-20 22:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix sensitivity of pw change button (2.51 KB, patch)
2013-01-17 00:35 UTC, Michael Catanzaro
committed Details | Review

Description Michael Catanzaro 2013-01-17 00:35:02 UTC
Created attachment 233642 [details] [review]
fix sensitivity of pw change button

Also, passwords that are "not good enough" are accepted. We should use the password quality level to determine the sensitivity of the "Change Password" button.

For example, lkjsdfk is rated "Too short" by the strength bar. The Change button is insensitive and reads "The new password is too short". However, add one letter - lkjsdfkr - and the strength bar jumps to "weak," but the button remains insensitive and the tooltip still says "too short" even though it should now allow the password. A couple of characters later, the password magically becomes long enough, even though the strength bar hasn't changed. 

But we also should deal with the "not good enough" passwords being accepted. For example, "passwords" is accepted just fine - even though the strength bar says "not good enough". So this patch resolves both issues by using the pwquality strength level to determine whether the button can be sensitive. It should probably be combined with the patch attached to https://bugzilla.gnome.org/show_bug.cgi?id=691265

Although that patch solves both bugs, I haven't found the root cause of the first one, since the code is attempting to get the min password length from pwquality.  Perhaps there is a problem with the call to pw_min_length in update_sensitivity (um-password-dialog.c) and perhaps that should be investigated more.
Comment 1 Ondrej Holy 2013-01-18 15:46:31 UTC
You are right. In addition there are probably some bugs in the libpwquality, because for passwords:

"a 1" -> The password is shorter than 6 characters
"a b" -> The password is shorter than 7 characters
"abc" -> The password is shorter than 8 characters

However minimal length should be 9 according:

pwquality_get_int_value (pwquality_default_settings (), PWQ_SETTING_MIN_LENGTH, &value)

But e.g. 8 characters "krutodemo" doesn't return any error...
Comment 2 Matthias Clasen 2013-01-19 04:18:31 UTC
If you look at check.c in libpwquality, around line 260, you'll find that it doesn't actually return the minimal length, but something more like
min_length - required_digits - required_other. Clearly a bug in libpwquality.

By my count, krutodemo actually has 9 characters: kru tod emo
Comment 3 Ondrej Holy 2013-01-19 18:47:06 UTC
(In reply to comment #2)
> If you look at check.c in libpwquality, around line 260, you'll find that it
> doesn't actually return the minimal length, but something more like
> min_length - required_digits - required_other. Clearly a bug in libpwquality.

Yes, I know, I was looking in the code, but it is weird and checking password in user account is wrong as Michael Catanzaro wrote.
 
> By my count, krutodemo actually has 9 characters: kru tod emo

Sorry I have mistyped it should be only 8 characters e.g. "krutodeo"...
Comment 4 Michael Catanzaro 2013-02-12 16:49:02 UTC
Even though it's important that we still need to fix libpwquality, I think my patch is still correct for gnome-control-center. Remember that we shouldn't count on only the password length to determine if we can change the password, as libpwquality can reject a password for other reasons, so fixing this in libpwquality alone is not enough, we still need this patch. Otherwise, the password strength bar can say "Not good enough" because your new password is almost the same as the old password, or based on a dictionary word, but the "change password" button will still work.
Comment 5 Michael Catanzaro 2013-02-12 16:50:12 UTC
Slight urgency here, as there's a tooltip involved and the string freeze is coming.
Comment 6 Matthias Clasen 2013-02-14 03:23:16 UTC
I wasn't able to try it, but the patch looks right to me
Comment 7 Bastien Nocera 2013-02-14 09:10:54 UTC
Review of attachment 233642 [details] [review]:

Subject line is missing "user-accounts:"

::: panels/user-accounts/um-password-dialog.c
@@ +230,3 @@
         const gchar *tooltip;
         gboolean can_change;
+        gint strength_level;

Use int, not gint.

@@ +235,3 @@
         verify = gtk_entry_get_text (GTK_ENTRY (um->verify_entry));
         old_password = gtk_entry_get_text (GTK_ENTRY (um->old_password_entry));
+        strength_level = update_password_strength(um);

Missing space before bracket.

@@ +341,3 @@
         gtk_widget_set_tooltip_text (um->strength_indicator, long_hint);
         gtk_widget_set_tooltip_text (um->strength_indicator_label, long_hint);
+        

Trailing spaces.
Comment 8 Bastien Nocera 2013-02-14 09:11:47 UTC
After fixes.
Comment 9 Michael Catanzaro 2013-02-14 14:52:16 UTC
Thanks Bastien.

I made a new bug for the libpwquality issue that we've now hidden: https://bugzilla.redhat.com/show_bug.cgi?id=911196
Comment 10 Michael Catanzaro 2015-12-31 20:35:42 UTC
This was not quite right. We should not reject passwords with strength 0; we should only reject the password if pwquality returns an error.
Comment 11 Michael Catanzaro 2016-09-20 22:33:55 UTC
(In reply to Michael Catanzaro from comment #10)
> This was not quite right. We should not reject passwords with strength 0; we
> should only reject the password if pwquality returns an error.

It is right; I got confused because the strength level we get from pw_strength() is not the same as returned by pwquality. Strength level 0 from pw_strength() corresponds to a negative return value (quality error) from pwquality_check(), which is all we "want" to block.