Skip to content
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

Closed
mathieutu opened this issue Feb 6, 2023 · 3 comments
Closed

Input max length of 71 instead of 72 bytes. #43

mathieutu opened this issue Feb 6, 2023 · 3 comments

Comments

@mathieutu
Copy link
Contributor

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:

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.

@quinot
Copy link
Contributor

quinot commented Feb 6, 2023

Proposed PR: #44

@patrickfav
Copy link
Owner

Very fair point. Ive merged the PR and will see that I release a new version soon. Thank you!

@patrickfav
Copy link
Owner

Released with 0.10.0+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants