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

x/crypto/bcrypt: GenerateFromPassword returns nil error on empty password #42230

Closed
ivanjaros opened this issue Oct 27, 2020 · 3 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ivanjaros
Copy link

I just found out that if I use nil slice or empty string to generate bcrypt hash, bcrypt will simply generate one without complaining. I would expect to get error but nope, I just get a real hash. This bit me while creating API layer and my front-end did not properly map fields and I was getting empty passwords but all requests just passed so I ended up with new users in db with random passwords. Luckily it was just testing in development.

Example
https://play.golang.org/p/DBnWUjn0YPX

@gopherbot gopherbot added this to the Unreleased milestone Oct 27, 2020
@dmitshur dmitshur changed the title x/crypto/bcrypt will hash empty data without error x/crypto/bcrypt: GenerateFromPassword returns nil error on empty password Oct 28, 2020
@dmitshur
Copy link
Contributor

As I understand, there are no restrictions on the password input to GenerateFromPassword, so a password of length 0 is considered valid input and this is working as intended. But maybe something else should be done.

CC @FiloSottile, @katiehockman, @rolandshoemaker.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 28, 2020
@rolandshoemaker
Copy link
Member

Unfortunately there isn't really a bcrypt specification, all we have to go off is the original paper from the OpenBSD folks, which doesn't specify what you do in this case, and other implementations.

The OpenBSD libc implementation appears to accept empty input, and can probably be considered the reference implementation. PHP does the same thing, and is likely the place where bcrypt is most often used in the wild.

I don't think there is a particularly strong security argument to reject empty inputs, but there is perhaps a UX one to be made. Given we appear to match the community implementation consensus on this matter though, I would lean towards just saying this works as intended.

@FiloSottile
Copy link
Contributor

Not the safest interface, but I think here backwards compatibility and matching the ecosystem are good enough reasons not to make a change.

@golang golang locked and limited conversation to collaborators Dec 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants