Skip to content

Sodium warning to Exception #5880

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

Closed
wants to merge 2 commits into from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jul 21, 2020

This promotes all the empty PWD warnings to exception.

I'm not sure if the last remaining warning about the "wrong size for the hashed password" should be converted too.

@Girgias Girgias force-pushed the sodium-warning-to-error branch from 288a4cf to dd4ed93 Compare September 3, 2020 15:40
@Girgias
Copy link
Member Author

Girgias commented Sep 4, 2020

@jedisct1 any opinion on this?

@jedisct1
Copy link
Contributor

jedisct1 commented Sep 4, 2020

Hi!

I'm not in favor of turning this into an error.

There are perfectly valid scenarios where an empty password can be fine, especially in local, command-line applications. It can also be acceptable to use Argon2 as a KDF with an empty password, but a salt with enough entropy.

The warning was a little addition for PHP, but other bindings don't treat empty passwords differently, so maybe we should keep things that way, if only for consistency.

@nikic
Copy link
Member

nikic commented Sep 4, 2020

Agree with @jedisct1 on this. Personally I'd also drop the warning.

@nikic
Copy link
Member

nikic commented Sep 8, 2020

As empty password handling is the only change here, I'm closing this per above.

@nikic nikic closed this Sep 8, 2020
@Girgias Girgias deleted the sodium-warning-to-error branch September 8, 2020 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants