-
Notifications
You must be signed in to change notification settings - Fork 23
Add and implement KEM traits #1053
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the traits look pretty good, I have mostly minor comments.
But indeed, as you mentioned it would be good to have more documentation in the traits
crate, as well as tests that exercise the impls.
Review re-requested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it looks pretty good!
But could you still add some uses of the generic test in the KEM crates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! But we're not using it anywhere at the moment, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I inject it in the macro for implementing the slice variant. Not super clean, but it did the trick...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could put it into a second macro or rename the macro, not sure what the best way to do that is.
dk: &mut [u8; SECRET_KEY_SIZE], | ||
rand: &[u8; KEY_GENERATION_SEED_SIZE], | ||
) -> Result<(), libcrux_traits::kem::owned::KeyGenError> { | ||
if rand.iter().fold(0, crate::or) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In curve25519
we disallow zero randomness because it results in a bad key, but in ML-KEM I don't think there's a reason to disallow all zero randomness for keygen or encaps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I figured zero randomness is always a problem because it suggests that no randomness has been generated. If the zero key itself is the problem, then we should first clamp and then check that the key is valid eg here.
This PR adds a number of KEM traits and implements them for all our KEMs. All of them are derand-style.
Traits:
libcrux-traits::kem::arrayref::Kem
: has const generics for all the lengths. Contains associated functions (i.e. methods have no receiver). Functions take referenves to arrays of the correct length as arguments. Outputs are written to mutable references.libcrux-traits::kem::slice::Kem
: has no const generics. Generally similar to thearrayref
variant, but takes slices instead of array references. Error types contain variants for length mismatches.libcrux-traits::kem::owned::Kem
: similar to arrayref, but instead of writing to mutable references, outputs are returned as arrays. Has a blanket impl for all arrayref::Kem.libcrux-traits::kem::secrets::Kem
: similar to owned, but also classifies all returned values and consumes classified values. This is a step to get closer to secret independence. Has a blanket impl for all owned.slice does not have a blanket impl for all arrayref because that doesn't work:
That's why there is a macro that impls slice::Kem for any type that impls arrayref::Kem.
What is not in here (but maybe should?)
This might still be a little bit messy and not super well documented, but I hope it's good enough for a first look.