search

RustCrypto / hashes

Collection of cryptographic hash functions written in pure Rust
1.76k stars 239 forks source link

blake2: Refuse empty keys in keyed hash construction #510

Closed edward-shen closed 6 months ago

edward-shen commented 9 months ago

Fixes #509.

Blake2 0.10 splits keyed hashing into *Mac struct variants. However, this permits empty keys to be provided to new_with_salt_and_personal, while the implementation assumes a non-empty key. This leads to an invalid construction of Blake2, as the kk argument of the parameter block will now be 0x00, but buffer is initialized as if kk > 0x00.

This change introduces a guard to the function, failing if the key was empty.

edward-shen commented 8 months ago

Hey @newpavlov! Sorry for the ping; I know your time is limited but I just wanted to check if this is on your or someone else's radar.

I believe this is a pretty easy-to-trigger API hazard (especially for non-experts) so I wanted to make sure this will be addressed sooner than later. Let me know if it's not the case.

Thanks!

tarcieri commented 8 months ago

I’d personally like to get #228 over the finish line

edward-shen commented 8 months ago

Hey @tarcieri, #228 still requires more discussion right? While that also does resolve #509 from what I can tell, the pace of discussion there feels like it won't be merged soon. Of course, I also might not be privy to discussion elsewhere.

This is a very small change that shouldn't create merge conflicts with #228, since as far as I can tell git is treating your blake2 implementation as completely new files.

I'm pushing for this pretty hard because I feel like preventing incorrect construction of hashes is pretty important, but if the project isn't in the state where it can accept PRs or is not accepting new PRs until #228 is merged, please let me know so I don't annoy you or other maintainers!

newpavlov commented 6 months ago

I will merge this PR, but we probably will replace implementation based on #228 before publishing v0.11.