search

sru-systems / rust-argon2

Rust library for hashing passwords using Argon2.
Apache License 2.0
221 stars 27 forks source link

SaltToShort check should be droppped. #53

Open AlexanderSchuetz97 opened 11 months ago

AlexanderSchuetz97 commented 11 months ago

Hello,

I am trying to write a program that derrives a AES key from a password. The entire program has to be compatible with existing java software. I.e the java code and the rust application have to get the same key from the same input.

The java application uses a 4 byte salt that is set to a constant value for all inputs. This step cannot be repated using your rust library as it will always fail with Error SaltToShort.

I have verified that all other parameters are compatible and rust and java arrive at the same result by changeing the java program to use 8 byte salt. This is however not feasable as this obvieusly changes the resulting AES key for the same password. I can obvieusly not "keep" this change in the java program as there is tons of existing data that was encrypted using 4 byte salt keys that the new rust application now has to decrypt.

I have checked the bouncy castle java library that the java application uses for argon2 and it appears to have no check whatsoever in terms of length. It even supports passing no salt at all. This java library is very mature and has a wide adoption. (Its older than rust itself)

My suggestion is to drop all salt length checks and ensure that the outputs are the same than what the bouncy castle java library would produce.

For me personally I think just changeing the constant from 8 to 4 would already solve all my issues... However I think there are other people out there that may have used any number of bytes smaller than 4 as a "fake" salt to derrive keys from a password.

This is the relevant java code from the bouncy castle library. As you can see there are no checks concerning length. grafik

The "null == octets" check can be ignored as you can just pass a 0 length array as parameter to bypass it.

AlexanderSchuetz97 commented 11 months ago

After removing the salt check from your library the library it works perfectly in sync with the java application. All i did was comment out the check in context.rs. I would prefer not having to build the rust-argon2 crate locally. If at all possible I would be happy if this could be done upstream...

I think this lenght < 8 check is based on a bogus stackoverflow comment of some guy saying that he would use 8 bytes for salt when hashing a password. While this is surely good advice I don't see any mention of a "hard" limit in the spec of argon2.

mrijkeboer commented 10 months ago

Hi @AlexanderSchuetz97,

Thanks for opening this issue. This library is based on the original (reference) Argon2 implementation. This original implementation defines a minimum salt length of 8 characters (code) and fails with an error when the salt is less than this minimum (code). Since this behavior is present in the reference implementation I'm not in favor of changing this.

AlexanderSchuetz97 commented 10 months ago

Hi @mrijkeboer This doesn't help me with making the rust application compatible with the java application.

As mentioned before the argon2 scientific paper does not mention a minimum size of the salt it only makes a recommedation for password hashing. I don't use argon2 for password hashing! A salt is pointless when argon is used as a key derivation function without any user context and I have to be compatible with existing implementation that in my case uses a fixed 4 byte value.

If you do not want remove this check then I will be forced to fork this library. Please close this issue if your decision on not removing this check is final so I can begin uploading the forked crates.