-
Notifications
You must be signed in to change notification settings - Fork 50
Description
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:
- [Question] What is the purpose of
LongPasswordStrategies.none()? #42 - Interoperability issue with PHP implementation (and others) when truncating long password #22
- Hash password strategy for overlong password and NUL bytes handling #27
- password must not be longer than 71 bytes plus null terminator encoded in utf-8, was 80 #13
- Allow for custom max password length in 'Version' #28
- Add long-password strategy to verifier #23
From 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:
case 'b':
/* strlen() returns a size_t, but the function calls
* below result in implicit casts to a narrower integer
* type, so cap key_len at the actual maximum supported
* length here to avoid integer wraparound */
key_len = strlen(key);
if (key_len > 72)
key_len = 72;
key_len++; /* include the NUL */
break;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.