-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Input max length of 71 instead of 72 bytes. #43
Comments
Proposed PR: #44 |
Very fair point. Ive merged the PR and will see that I release a new version soon. Thank you! |
Released with 0.10.0+ |
This was referenced Feb 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi,
Thanks for your work.
This issue to raise a point about the character limit you considered for password, and the way the null terminator byte is handled.
It is following:
LongPasswordStrategies.none()
? #42From what I've understood from my readings, a password must be >= 72 bytes, and it should contain a null terminator byte since
$2a$
.In your implementation, you consider this null byte as part of the 72, so put a hard limit of 71 bytes.
Like others before us, we're trying to make compatible this package to other implementations because we're using the password in several apps in other languages. However, all our passwords are prehashed, and their max length is exactly 72 bytes.
It worked for years with other implementations, but break with yours, because of the Strict Strategy error.
However, when using the pass-through strategy, or setting the limit to 72, everything is working great. We also tried to use the custom
VERSION_2Y_NO_NULL_TERMINATOR
to avoid this 71 limitation. However, if the generated passwords work for the 72 bytes inputs, they don't for smaller ones.The OpenBSD original implementation itself, adds the null terminator byte to the 72 byte limit:
Some examples of implementations that work with 72 bytes inputs and add a null terminator byte at the end:
We can avoid the issue with the
LongPasswordStrategies.none
strategy, but as we seem to respect the standard, I think we shouldn't need it, and that the package would benefit increasing the max length by 1.What do you think?
Thanks.
Matt'.
cc @Indigo744 because I think you worked a lot on this topic.
The text was updated successfully, but these errors were encountered: