RustCrypto / password-hashes

Password hashing functions / KDFs
677 stars 84 forks source link

Consistent salt validation #527

Closed huy-yearnings closed 1 month ago

huy-yearnings commented 2 months ago

Currently password_hash::Salt::from_b64 and Argon2::verify_inputs verify salt values differently. password_hash::Salt::from_b64 is checked at Argon2 initialization while Argon2::verify_inputs is checked at password hash step.

It does not make sense if I can instantiate a Argon2 hasher successfully but then the hasher fails to hash any password.

tarcieri commented 2 months ago

I'm not quite sure what you're asking for or what scenario you have in mind.

It does not make sense if I can instantiate a Argon2 hasher successfully but then the hasher fails to hash any password.

Salts should be unique to a given password. Specifying a salt at algorithm initialization time is conceptually flawed.

huy-yearnings commented 1 month ago

The issue is demonstrated in code snippet below. Though it is not a big deal, the inconsistency could make people confused sometimes.

#[test]
fn hash_error() {
    use argon2::{password_hash::{PasswordHasher, SaltString}, Argon2};

    let password = b"mypassword";

    // Create a salt with 5 bytes length
    // Any salt with 4 - 7 bytes length cause error at hash step
    let salt = SaltString::from_b64("bG9yZW0")
        .expect("Salt initialization failure"); // salt initialized successfully

    Argon2::default()
        .hash_password(password, &salt)
        .expect("Hash failure"); // SaltInvalid(TooShort)

    // stdout:
    // thread 'hash_error' panicked at password_hash/src/lib.rs:14:10:
    // Hash failure: SaltInvalid(TooShort)
}

Salts should be unique to a given password. Specifying a salt at algorithm initialization time is conceptually flawed.

I realized that I did it wrong. Thank you. Much appreciated!

Closing the issue.