-
Notifications
You must be signed in to change notification settings - Fork 15
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
C extraction for ML-DSA #710
base: main
Are you sure you want to change the base?
Conversation
and use it everywhere
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.
Some of my comments are nits, some a questions, but I think by and large this isn't controversial :)
} | ||
|
||
impl XofAbsorb<168> for Shake128Absorb { | ||
type Squeeze = Shake128Squeeze; | ||
impl Xof<168> for Shake128Xof { |
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.
Should these impls somehow prevent using this sponge in a duplex way (i.e. absorbing after squeezing)? My understanding is that a duplex is not an xof
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.
That's what it did before. But I don't think we should do that here. It's only creating more boilerplate when using it. This is not a general purpose sha3 implementation right now that people should be using (unless they know what they are doing).
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.
They did that on a type level, I was just thinking of setting a bit when squeezing and then panicking when trying to squeeze. But I hear you wrt. not a general purpose SHA3, so it should be fine, as long as we know that we use it correctly. Does that follow from our proofs?
pub(crate) trait Xof { | ||
/// An ML-DSA specific Xof trait | ||
pub(crate) trait DsaXof { |
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.
What's the difference betwen DsaXof and Xof?
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.
This (DsaXof) is not actually a full Xof implementation but opererates only on multiple of blocks. That's enough for the algorithm. But not enough to hash the messages. That's where we need a proper Xof. That's why we needed to add a proper Xof here for DSA. This needs to get cleaned up in sha3. As part of that we should clean up names here too.
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.
Got it. Can we add that as a comment?
This PR makes all necessary to extract C code from ML-DSA.
To generate the C code, run
Follow ups
Note that this PR is on top of #707.
This works towards #706 but doesn't close it yet (see follow ups).