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

Generate a random, universally unique salt automatically #36

Open
shssoichiro opened this issue Apr 29, 2018 · 6 comments
Open

Generate a random, universally unique salt automatically #36

shssoichiro opened this issue Apr 29, 2018 · 6 comments

Comments

@shssoichiro
Copy link

shssoichiro commented Apr 29, 2018

To be secure, a salt is required to be universally unique across all passwords everywhere. It is very easy for a developer to do this incorrectly, which is why password hashing libraries typically generate a salt internally to take away the risk of the caller doing it incorrectly. This library should follow this pattern and generate a universally unique salt for each password internally, rather than asking the caller to provide one.

@burdges
Copy link

burdges commented Apr 29, 2018

There are no mechanisms for adding a salt specified in the argon2 specs, so maybe another crate should provide that functionality. And another crate would simplify users auditing their own code.

In fact, you could do this generically across some hash function interface, not unlike what the RustCrypto project does. It might look nicer if one waits for const generics though.

@bryant
Copy link
Owner

bryant commented May 1, 2018 via email

@burdges
Copy link

burdges commented May 1, 2018

There are applications for the low-level calls beyond simple password hashing, including PAKE schemes, PoW schemes, usage in low-entropy environments, etc., so they should definitely remain exposed. Also, salts might require std, unless the recent addition of jitter to rand avoids this.

@bryant
Copy link
Owner

bryant commented May 1, 2018

Unless I've misunderstood, rand would not ensure universal uniqueness.

@shssoichiro
Copy link
Author

shssoichiro commented May 1, 2018

The universal uniqueness rule comes from the idea that if a salt is shared between multiple passwords, an attacker can build tables to attack all of those passwords at once. In practice, the salt doesn't need to be universally unique (though it is preferable), but it should be difficult to predict and not shared between multiple passwords.

Allowing developers to generate their own salts is a footgun, because developers will commonly do things such as using the same salt for every password in their database, or basing the salt off of something easily guessable such as the username or an auto-incrementing user ID.

I agree with leaving the salt parameter for the low level calls, but the *_simple functions in particular should take only a password, to protect developers in the common use-case (e.g. "I want to hash a password to store in my database").

Also, this library already depends on std modules, so I don't think that is a concern in this case.

@burdges
Copy link

burdges commented May 1, 2018

I think salts serve two purposes, first to reduce the entropy users must supply in their passwords, ala protection from tables, and second to prevent revealing the same password was used in multiple places. Associated data helps against the first, but not necessarily against the second. And "serious" deployments should record a "version" that encapsulates their particular parameter choices anyways, so they should store their salt that way probably.

You need two separate functions when managing the salt, so reusing the _simple postfix does not work so well. Analogs look more like:

struct Salted(pub [u8; 32]);  // 16 byte salt plus 16 byte result, not sure what these should be really
pub fn argon2i_salted(password: &str) -> Salted { .. }
pub fn argon2i_verify_salted(password: &str, salted: &Salted) -> bool { .. }

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