RustCrypto / traits

Collection of cryptography-related traits
590 stars 191 forks source link

`password_hash`: Doesn't expect `=` char in base64 #1699

Closed onkoe closed 1 month ago

onkoe commented 1 month ago

It seems that, when creating your own salt and forming it into base64, password_hash::SaltString currently does not account for the allowed = characters in base64 strings.

Here's a repro using argon2:

use argon2::{password_hash::SaltString, PasswordHasher};
use base64::Engine;

fn main() {
    let a2 = argon2::Argon2::default();
    let password = b"bobrulez2024";

    // assume we have another random source that isn't comptaible with the
    // `SaltString::generate` method. we'll need to use our own!
    let totally_safe_salt = "abcdefghijlmnopqrstuvwxyz";
    let base64_salt = base64::prelude::BASE64_STANDARD.encode(totally_safe_salt);
    let salt_string = SaltString::from_b64(&base64_salt).unwrap();

    // now we'll try generating the hash
    a2.hash_password(password, &salt_string).unwrap();
}

This example fails with the following:

barrett@fedora ~/D/p/g/libghr (main)> cargo run --example a2_test
   Compiling libghr v0.1.0 (/home/barrett/Documents/projects/ghr/libghr)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.16s
     Running `target/debug/examples/a2_test`
thread 'main' panicked at examples/a2_test.rs:12:58:
called `Result::unwrap()` on an `Err` value: SaltInvalid(InvalidChar('='))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
barrett@fedora ~/D/p/g/libghr (main) [101]> 

I believe it is expecting an undocumented invariant to be upheld: any base64 input should have its equal signs removed.

Accordingly, removing them addresses the problem:

let salt_string = SaltString::from_b64(&base64_salt.replace('=', "")).unwrap();
barrett@fedora ~/D/p/g/libghr (main) [101]> cargo run --example a2_test
   Compiling libghr v0.1.0 (/home/barrett/Documents/projects/ghr/libghr)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.20s
     Running `target/debug/examples/a2_test`
barrett@fedora ~/D/p/g/libghr (main)> 

You can also use the encode_b64 constructor on SaltString, though this feels a bit off for my project. I need these hashes to be deterministic, and I have little control over password_hash internals.

That might just be my familiarity with base64, though. :)

tarcieri commented 1 month ago

The PHC string format implemented by the password-hash crate deliberately uses a subset of Base64 called "B64", which is defined here:

https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md#b64

The B64 encoding is the standard Base64 encoding (RFC 4648, section 4) except that the padding = signs are omitted, and extra characters (whitespace) are not allowed

The lack of support for padding with = is intentional and by design/specification.

onkoe commented 1 month ago

@tarcieri Thank you for your reply. Would a PR mentioning this in the documentation be accepted?

tarcieri commented 1 month ago

Yes, although note it is already documented here: https://docs.rs/password-hash/latest/password_hash/enum.Encoding.html#variant.B64

Some additional documentation in other places might be helpful.