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

blake2: Refuse empty keys in keyed hash construction #510

Merged
merged 1 commit into from
Jan 11, 2024
Merged

blake2: Refuse empty keys in keyed hash construction #510

merged 1 commit into from
Jan 11, 2024

Conversation

edward-shen
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Member

tarcieri commented Nov 3, 2023

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

@edward-shen
Copy link
Contributor Author

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
Copy link
Member

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

@newpavlov newpavlov merged commit 6243d29 into RustCrypto:master Jan 11, 2024
15 checks passed
@edward-shen edward-shen deleted the edward-shen/blake2-mac-no-empty-key branch January 11, 2024 16:01
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

Successfully merging this pull request may close these issues.

blake2: MAC variants should not be constructable with an empty key?
3 participants