Fix bcrypt errors preventing users from being able to log in
#19101
Conversation
This was causing users with too long a password, or too long a server-configured pepper, to be unable to log in. The same fix applied to `hashpw` in #19078, but for `checkpw` as well.
As it's technically the password + the pepper that's checked for length.
| bytes_to_hash = pw.encode("utf8") + password_pepper.encode("utf8") | ||
| if len(bytes_to_hash) > 72: | ||
| # bcrypt only looks at the first 72 bytes | ||
| logger.debug( | ||
| "Password + pepper is too long; truncating to 72 bytes for bcrypt. " | ||
| "This is expected behaviour and will not affect a user's ability to log in. 72 bytes is " | ||
| "sufficient entropy for a password." | ||
| ) | ||
| bytes_to_hash = bytes_to_hash[:72] |
There was a problem hiding this comment.
Just noting that it sucks that we have this pattern splatted across a few places now. But it's not unreasonable to have it this way ⏩
| logger.debug( | ||
| "Password + pepper is too long; truncating to 72 bytes for bcrypt. " | ||
| "This is expected behaviour and will not affect a user's ability to log in. 72 bytes is " | ||
| "sufficient entropy for a password." | ||
| ) |
There was a problem hiding this comment.
This looks reasonable in terms of fixing the problem. The "why" reasoning sounds great!
| pw = unicodedata.normalize("NFKC", password) | ||
| password_pepper = self.hs.config.auth.password_pepper | ||
|
|
||
| bytes_to_hash = pw.encode("utf8") + password_pepper.encode("utf8") |
There was a problem hiding this comment.
In terms of my own learning about using a pepper, https://stackoverflow.com/questions/16891729/best-practices-salting-peppering-passwords was good read.
Combining both the password and pepper before hashing with bcrypt seems like a good approach 👍 vs feeding one hash into another. The only better pattern I see people recommend is to use an hmac (and we can't switch to a new pattern).
Matches the previous behavior in bcrypt (truncating to 72 bytes) and is what they recommend when encountering this error "password cannot be longer than 72 bytes, truncate manually if necessary (e.g. my_password[:72])"
Once again fixes #19063.
This was causing users with too long a password, or too long a server-configured pepper, to be unable to log in.
The same fix was applied to
bcrypt.hashpw(...)in #19078, butbcrypt.checkpw(...)was missed.Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.