Signature traits

[wysiwys]

This PR implements the Signature traits.

• keygen()
• sign()
• verify()

Resolves #1034

Includes implementations for:

• ed25519 (arrayref, slice, owned)
• libcrux-ml-dsa (owned)
• ecdsa (arrayref, slice, owned)

[keks]

I think this goes in a good direction, nice!

I am not sure what to best do with the Aux, it looks like it could be confusing to users. I wonder if being a bit less generic could help here.

Otherwise I left a few comments. I have one remark on it but then realize it affects a lot of things: There are a lot of doc comments missing.

[wysiwys]

Thanks for the review and feedback @keks! I addressed most of the feedback points, and added some comments for the other points.

For the trait organization (specifically how to handle randomness and auxiliary data), I think there is room for improvement, too, and I think there is likely a combination of the different options from our discussion above that will address many of the concerns around ergonomics/complexity. I will take another look at the different combinations of input data we need to support, and look at the pros and cons of different approaches.

[jschneider-bensch]

I think the traits look good!

I have some comments:

• There are some TODOs left in different places. Are these for follow-ups, or do you want to address them in this PR?
• Could you try and add a generic test for some of the APIs, e.g. like with the KEM trait, s.t. we know the implementation works?
• Could you summarize in the PR description which scheme now implements which APIs? Or are they all covered via blanket impls?

[keks]

What I am wondering is whether our Signature traits should support all uses of context. We also don't support all the features of the blake2 hash (e.g. personalisation or keying of the hash).

We could pretty easily support the use of contexts known at compile time:

• Add a trait Context { context() -> &'static [u8]; } to the mldsa crate
• Then impl<Ctx: Context> RandomizedSignature for MlDsa<Ctx>, where we call the context function to get the value
• The context function should be marked inline(always). Maybe we can define a macro that defines a context struct and implements the trait for it, so the user doesn't forget to set it?

Review re-requested

[jschneider-bensch]

This doesn't link correctly for me. Maybe something like [slice](libcrux_traits::signature::slice) would work?

[wysiwys]

Thanks for catching that! I updated the link to the suggestion. Could you let me know if the link works correctly on your side now?

[keks]

Nice, even RSA is looking better and better :)

I noticed some files had weird whitespace. Did you run rustfmt? I also found a bunch of TODOs that note that something doesn't show up in documentation. What exactly isn't showing up? The comments read like the trait doesn't show up, but that makes sense because the block only contains the impl.

[franziskuskiefer]

Converting this to a draft for now. See #1107.

[keks]

I think this looks pretty good! I have a few questions on some design details that I would like to discuss before we proceed, but the fundamentals look like they are in place!

[keks]

I am not sure this is the right time to do this, but we could make Signer or the per-algorithm struct generic over a type that implements a new trait Context { context()->&'static [u8]; } and use that as the context.

[wysiwys]

Added an implementation of this because I think it's a good abstraction for the context, and a good way to not exclude any sign/verify functionality. It can always be taken out later, too

[wysiwys]

I added an initial implementation of keygen() to the traits in this PR.