GNOME Bugzilla – Bug 691907
passwords of sufficient length are rejected as too short
Last modified: 2016-09-20 22:33:55 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.
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...
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
(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"...
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.
Slight urgency here, as there's a tooltip involved and the string freeze is coming.
I wasn't able to try it, but the patch looks right to me
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.
After fixes.
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
This was not quite right. We should not reject passwords with strength 0; we should only reject the password if pwquality returns an error.
(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.