575947 - Users with passwords length less than 6 characters can't login after migration from 3.4.x or older to 3.6 or newer

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, defect)

Description

[Andrey Buranov]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6
Build Identifier: 3.6.1

I just finished the test migration from version 2.22.3 to 3.6.1 and discovered that I can't login with my password that have less than 6 characters. For Users with password length more than 6 characters all looks fine.

The problem is sufficiently important because I can't check the passwords for all several hundred users and change it manually. And it was no any words about this issue during migration process while checksetup.pl script worked.

How can I avoid this problem during migration or remove this security requirement just after?

Reproducible: Always

Steps to Reproduce:
1. You have not customized bugzilla installation based on version 2.2.23 (may be some other) and user with password length less than 6 characters
2. Migrate it to version 3.6.1
3. Try to login by any user with password length less than 6 characters
Actual Results:  
Now it's impossible to login for users with password length less than 6 characters.

Expected Results:  
It should be possible to login for users with password length less than 6 characters. Or may be they should be somehow automatically informed about this requirement...

Comment 1

[Frédéric Buclin]

When logging in, there is no check about the password length, so old passwords are still working with 3.6.1. This check is only done when changing your password.

Comment 2

[Frédéric Buclin]

OK, I just realized what's going on. When upgrading from 3.2 or older to 3.6 or newer, we move from a DB storing passwords encrypted with crypt() to a DB storing passwords encrypted with SHA-256. When Bugzilla detects that the stored password has been encrypted with crypt(), it encrypts it to SHA-256, see Bugzilla::Auth::Verify::DB::check_credentials(). To do that, it calls $user->set_password(), which itself calls validate_password() via _check_password(). But validate_password() checks the password length, and realizes that it's now smaller than the increased (from 3 to 6) minimum length. As a consequence, the password is rejected despite it's not a new one.

There are two alternatives here:
1) pass a 2nd argument NO_CHECK to set_password() so that it doesn't call validate_password();
2) or force the user to enter a new password because it's too short.

Any preference?

Comment 3

[Reed Loden [:reed]]

The latter. Should present an intermediary form requiring the user to create a new password. This form will be useful for future changes as well, such as bug 558803.

Comment 4

[Frédéric Buclin]

Taking! I have a patch ready. I need to test it carefully first before attaching it here.

Comment 5

[Frédéric Buclin]

Attachment: patch, v1

Rather than displaying an intermediate page, which wouldn't work with WS, I prefer to use the traditional way to change the password, i.e. by email. So if the password is too short, an error is thrown, asking the user to follow the URL in the email he got to enter a new password. Refactoring the code would be too invasive for 3.6 and 4.0.

Comment 6

[Max Kanat-Alexander]

Comment on attachment 475121 [details] [diff] [review]
patch, v1

Why not just return AUTH_ERROR with the error password_too_short, and have that error message link to the reset password page?

Comment 7

[Frédéric Buclin]

(In reply to comment #6)
> Why not just return AUTH_ERROR with the error password_too_short, and have that
> error message link to the reset password page?

You mean only changing the error thrown? Because you still need a new constant for this kind of problem. My idea was to avoid a useless extra step, and send an email immediately.

Comment 8

[Max Kanat-Alexander]

Ah, okay. But at some point, this code will pretty much become obsolete (because everybody will have upgraded their passwords), and it will only apply to a small number of users to start with (people who have passwords shorter than six characters). So it would be best to keep it as simple and as small as possible, and I think the simplest fix is just to link them to the password reset form, no?

Comment 9

[Frédéric Buclin]

(In reply to comment #8)
> Ah, okay. But at some point, this code will pretty much become obsolete

An admin is free to change the USER_PASSWORD_MIN_LENGTH constant. Also, assuming we move to 8 characters in the future, the code would still be applicable. I will simply change the error message to let the user ask for a new password, to make the patch less invasive.

Comment 10

[Max Kanat-Alexander]

Okay, that sounds good.

Comment 11

[Frédéric Buclin]

Attachment: patch, v2

Comment 12

[Max Kanat-Alexander]

Comment on attachment 482040 [details] [diff] [review]
patch, v2

Cool, this is looking better.

Could you explain why you can't use AUTH_ERROR?

Comment 13

[Frédéric Buclin]

(In reply to comment #12)
> Could you explain why you can't use AUTH_ERROR?

AUTH_ERROR is for code errors, which calls ThrowCodeError(). Here, we want ThrowUserError(), as the error is not a code error.

Comment 14

[Max Kanat-Alexander]

Comment on attachment 482040 [details] [diff] [review]
patch, v2

Oh, you're right, I didn't realize that.

I'm not sure that a new constant is the best way to go, though. I'm going to think about it for a bit.

Comment 15

[Max Kanat-Alexander]

Comment on attachment 482040 [details] [diff] [review]
patch, v2

Okay, so here's my thoughts on it:

Instead of calling it AUTH_PASSWORD_TOO_SHORT, call it AUTH_USER_ERROR, and make it work just like AUTH_ERROR but throw user errors instead. Or alternately, allow passing a particular hash key back along with AUTH_ERROR to do UserError instead of CodeError.

The error's id should be password_too_short instead of password_force_change.

The WebService description of the error should just say that the user is required to change their password for one reason or another (this will let customizers re-use this if they want to force password changes).

Comment 16

[Frédéric Buclin]

Attachment: patch, v3

Comment 17

[Max Kanat-Alexander]

Comment on attachment 483021 [details] [diff] [review]
patch, v3

Yeah, this looks great! :-) Thanks!!

Comment 18

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Auth.pm
modified Bugzilla/Auth/Verify/DB.pm
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
modified template/en/default/account/email/confirm-new.html.tmpl
modified template/en/default/account/password/set-forgotten-password.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7541.


Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified Bugzilla/Auth.pm
modified Bugzilla/Auth/Verify/DB.pm
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
modified template/en/default/account/email/confirm-new.html.tmpl
modified template/en/default/account/password/set-forgotten-password.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7440.

Committing to: bzr+ssh://[e-mail]/bugzilla/3.6/
modified Bugzilla/Auth.pm
modified Bugzilla/Auth/Verify/DB.pm
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
modified template/en/default/account/email/confirm-new.html.tmpl
modified template/en/default/account/password/set-forgotten-password.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7187.

Comment 19

[stanislav.seltser]

how do i change this constant USER_PASSWORD_MIN_LENGTH? i could not find a place in the code where it can be changed. it it stored in db table?

i had upgraded to bugzilla 4.0 and now none of my users can login including myself because passwords were too short

Comment 20

[stanislav.seltser]

another question is bugzilla-4.0 distribution on the web site reflects the patch you guys had made?

Comment 21

[stanislav.seltser]

even worst problem is that my email server is not yet setup on upgraded bugzilla instance so i cannot even get email with new password.

Comment 22

[Frédéric Buclin]

(In reply to comment #21)
> even worst problem is that my email server is not yet setup on upgraded
> bugzilla instance so i cannot even get email with new password.

This is a question for the support mailing-list, so additional questions or problems should be redirected there. But in short: fix your email server, then click the forgot password link to set a new one. We intentionnaly reject too short passwords for security reasons. *If and only if* you don't care too much about security can you edit the USER_PASSWORD_MIN_LENGTH constant in Bugzilla/Constants.pm.