Mattermost-server: 5.5.1: password max length (esp. hardcoded, unchangable) is too short

[lkraav]

Summary

Devices like Yubikey, in static key configuration mode, output 67 characters at once.

If you want 2FA "short prefix pin + key", it overflows Mattermost Server password limit.

Steps to reproduce

Try to set user password 80 characters long. Example:

X6kyAwdLeCkgb70c00KgK9oqORbQNgRNupt9UDQBEUtwqpZ9IzWOlEFB2cZxJjwPFL8QsbkBXh25wHxw

Expected behavior

I should be able to set passwords longer than 64 characters.

Observed behavior (that appears unintentional)

Hardcoded limit hit.

Possible fixes

Maybe increase USER_PASSWORD_MAX_LENGTH https://github.com/mattermost/mattermost-server/blob/09eae76c0066a1145d5059cd2b68744b3255e674/model/user.go#L52

Is there any reason to limit length of this field at all? Does arbitrary length password slow anything down?

[amyblais]

Hi @lkraav, after some research, I haven't been able to find information on whether there is a specific reason why there is a password length limit. I'll ask our team about this and get back to you.

[amyblais]

From one of our developers:

"Digging through the history, it seems to have been a bcrypt limitation: https://stackoverflow.com/questions/16594613/how-to-hash-long-passwords-72-characters-with-blowfish/16597402#16597402. We opted to stop the user at 72 characters vs. trying to mess with the hashing algorithm here."

[lkraav]

Thanks.

Per bcrypt implementation, only the first 72 characters of a string are used.

So why not allow longer entry and perhaps just inform the user, only 72 characters will be used? It'd solve this issue's use case.

EDIT it's also a bit confusing why UI talks about 64, when we now know the real limit is 72 characters?

[lieut-data]

Hey @lkraav! I don't have a great explanation for the difference between the advertised limit of 64 characters (on the system console) and the internal limitation of 72 characters, other than the latter is perhaps an oddity of the implementation vs. a business decision as such.

Truncating the password is definitely an option, though we'd have to give some thought as to how we might upgrade the hashing implementation in the future but still be able to accept the old password.

@crspeller, do you have any more context into this?

[crspeller]

I don't have any more context into why the client side limitation is 64 characters while the server side is 72.

Removing the client side limit would be fine by me. But I don't like the idea of truncating the password even with a user warning.

As for the Yubikey use case mentioned above, they can be configured to output a static key of any length so there is no issue here.

[lkraav]

Removing the client side limit would be fine by me. But I don't like the idea of truncating the password even with a user warning.

Any details available on the liking part? Shouldn't good security hygiene dictate the tempo?

Reducing key length to accommodate any app's random-looking password implementations - how would I (or anyone) explain this to project stakeholders? This key isn't used just for MM access.

Aside from the above, it so happens that the static key actually outputs 67 characters, which fits in MM server's current model boundaries. So maybe initial peace on earth could be reached simply by bumping the actual 72 characters limit in the UI.

[crspeller]

Shouldn't good security hygiene dictate the tempo?

Yup. We agree there. I am just unsure of what solution is the best one for security.

So maybe initial peace on earth could be reached simply by bumping the actual 72 characters limit in the UI

Yup. Let's start with that. @amyblais Can you make a JIRA ticket for that?

[amyblais]

Ticket here: https://mattermost.atlassian.net/browse/MM-13513

[dalcde]

As noticed in https://github.com/mattermost/mattermost-server/issues/13644 , the server also reports the error incorrectly when a password that is too long is rejected. It says

{"id":"model.user.is_valid.pwd.app_error","message":"Your password must contain at least 10 characters.","detailed_error":"","request_id":"...","status_code":400}