Skip to content

signature: add MultipartSigner and MultipartVerifier #1880

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

Merged
merged 5 commits into from
Jun 2, 2025

Conversation

daxpedda
Copy link
Contributor

@daxpedda daxpedda commented Jun 1, 2025

This PR adds new traits for multipart messages: MultipartSigner, RandomizedMultipartSigner, RandomizedMultipartSignerMut and MultipartVerifier.

The idea here is to allow non-contiguous bytes to be passed, which is necessary when the message has to be constructed from multiple sources without wanting to allocate memory for a contiguous message. E.g. for no_std environments or when the message is rather big but pre-hashing is not applicable, e.g. PureEdDSA, ML-DSA or SLH-DSA.

I know this is a rather big breaking change, so let me know what you think!

These new traits can be implemented by a bunch of crates:

Resolves RustCrypto/signatures#959.

@newpavlov
Copy link
Member

If we are to support such messages, we probably should do it in separate methods or maybe even in a separate trait.

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

If we are to support such messages, we probably should do it in separate methods or maybe even in a separate trait.

I will go ahead and move it into a separate method then. I don't think a separate trait is necessary as the &[u8] method will just use &[&[u8]] method. But let me know if you have more thoughts on this.

@daxpedda daxpedda changed the title signature: use &[&[u8]] for messages signature: use Iterator<Item = AsRef<[u8]> for messages Jun 1, 2025
@daxpedda daxpedda marked this pull request as draft June 1, 2025 11:03
@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

I went ahead and added a sign/verify_segments_* trait method to every relevant trait.
Now that its separate, I also went ahead and used Iterator<Item: AsRef<[u8]>>, because the more convenient method is still present.

Will update the other PRs today and see how it looks before moving this out of draft again.

@daxpedda daxpedda changed the title signature: use Iterator<Item = AsRef<[u8]> for messages signature: use Iterator<Item: AsRef<[u8]> for messages Jun 1, 2025
@tarcieri
Copy link
Member

tarcieri commented Jun 1, 2025

This adds a lot of code surface but really doesn’t add any new functionality.

We already support IUF signing via DigestSigner and PrehashSigner. So this would be yet another API for such cases.

Where I guess this could maybe help are for constructions that don’t fit into a strictly IUF API like streaming Ed25519 (non-Ed25519ph) or ML-DSA with external mu. But it’s unclear to me we can build traits which actually abstract over such APIs. If we could, I guess this would be close, but that should probably be the design rationale.

@tarcieri
Copy link
Member

tarcieri commented Jun 1, 2025

Also note that the original proposed &[&[u8]] is probably the safest approach here. Ed25519 needs to do multiple passes over the input message to compute a signature, and the input message can't change. I'm not sure how you'd express that with Iterator

@tarcieri
Copy link
Member

tarcieri commented Jun 1, 2025

FWIW, here's discussion on this sort of API for Ed25519. It was actually removed from this PR, but prior to that, it was suggested to use &[&[u8]] for the API:

https://github.com/dalek-cryptography/curve25519-dalek/pull/735/files/bfdaa86b1820596d893f69390a6204f19afde306#diff-bc1db46f3164930c55100377fea321e44637c4ca4575d3960c179f4ffbaa6703

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

Where I guess this could maybe help are for constructions that don’t fit into a strictly IUF API like streaming Ed25519 (non-Ed25519ph) or ML-DSA with external mu. But it’s unclear to me we can build traits which actually abstract over such APIs. If we could, I guess this would be close, but that should probably be the design rationale.

The intention is not a streaming interface, but a streaming interface would address the problem here indeed as well.

Also note that the original proposed &[&[u8]] is probably the safest approach here. Ed25519 needs to do multiple passes over the input message to compute a signature, and the input message can't change. I'm not sure how you'd express that with Iterator

Ah right ... I forgot why I had planned it with &[&[u8]] to begin with.


So if the approach with &[&[u8]] is acceptable, I would prefer to move forward with that. If not, we can continue with discussing a potential streaming API.

Just to clarify: the intention here is to support non-contiguous byte slices for Ed25519/448, ML-DSA and SLH-DSA.

@daxpedda daxpedda marked this pull request as ready for review June 1, 2025 17:18
@tarcieri
Copy link
Member

tarcieri commented Jun 1, 2025

I agree with the earlier comments that this should be a new trait, like MultipartSigner. Implementing it requires support from the underlying signature algorithm, which doesn't currently exist in ed25519-dalek or in ml-dsa, for example.

I think it would probably also be good to get everything prototyped in PRs to those crates first before we merge, so we know it's actually a good abstraction.

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

I agree with the earlier comments that this should be a new trait, like MultipartSigner. Implementing it requires support from the underlying signature algorithm (which doesn't currently exist in ed25519-dalek, for example, or in ml-dsa)

I added support for ML-DSA and SLH-DSA in RustCrypto/signatures#981. I'm happy to make a PR to the curve25519-dalek repo as well.

Just my 2¢: if you take a look at RustCrypto/signatures#981, the changes are pretty minimal. I really don't believe that going from &[u8] to &[&[u8]] is worth a separate trait. However, in the meantime I will move forward with MultipartSigner, let me know if you change your mind though, no problem at all.

@tarcieri
Copy link
Member

tarcieri commented Jun 1, 2025

@daxpedda if you look at dalek-cryptography/curve25519-dalek#735, the same cannot be said of Ed25519.

I don't want to force signature implementations to have to implement this interface as their only possible choice. We can't even guarantee this interface is possible for all signature algorithms (at least in an unbuffered, no_std-friendly manner)

@daxpedda daxpedda changed the title signature: use Iterator<Item: AsRef<[u8]> for messages signature: use &[&[u8]] for messages Jun 1, 2025
@tarcieri
Copy link
Member

tarcieri commented Jun 1, 2025

Alright, that was easier than expected, so let me say this:

  • Most people don't need or want this functionality
  • It's incredibly uncommon to define APIs this way, IMO
  • Changing this will break all current users of these traits

I still think it should be a separate trait to keep it out of the way.

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

Sounds good, on it!

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

Let me know what you think.

We may want to implement traits on demand, for OPAQUE I will only need MultiPartSigner, RandomizedMultiPartSigner and MultiPartVerifier.

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

Should I add a multipart_* pre-fix to the methods to prevent conflict?

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

I went ahead and did both: removed MultiPartSigner & co. and added prefixed method names with multi_part.

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

Implementation for ML-DSA and SLH-DSA done in RustCrypto/signatures#982.

I hope that dalek-cryptography/curve25519-dalek#763 was sufficient, so I will wait until we are ready to upgrade RustCrypto dependencies there before I make a PR. Probably dalek-cryptography/curve25519-dalek#735 will be relevant as well, as that could be used to implement the MultiPart traits.

Let me know if there is something else you want me to implement/explore/discuss.

@daxpedda daxpedda changed the title signature: use &[&[u8]] for messages signature: add MultiPartSigner and MultiPartVerifier Jun 1, 2025
@tarcieri
Copy link
Member

tarcieri commented Jun 1, 2025

@daxpedda regarding rsa, it's another case like ecdsa where it already impls DigestSigner/PrehashSigner and needs whatever glue (macro or blanket impl) we choose to use there

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

FYI: the blanket implementation didn't work because I can't bind D to the MultipartSigner trait. And I feel like the implementation is so short it doesn't deserve a macro, but let me know if you think otherwise (and where the macro should live in that case).

Will go ahead and make the PR in rsa.

@tarcieri
Copy link
Member

tarcieri commented Jun 1, 2025

Yeah, just go ahead and hand write impls for now

@daxpedda daxpedda changed the title signature: add MultiPartSigner and MultiPartVerifier signature: add MultipartSigner and MultipartVerifier Jun 1, 2025
@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 1, 2025

Done: RustCrypto/signatures#982.

Let me know if you want me to implement it for any other specific crate or for the complete rest.

@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 2, 2025

I went ahead and implemented it for the remaining crates.

@daxpedda daxpedda requested a review from tarcieri June 2, 2025 07:50
Copy link
Member

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can tentatively approve this, but it would be good to see it integrated into ed25519-dalek as well before a final release. It doesn't look like it will be a problem.

@tarcieri tarcieri merged commit f73c1a2 into RustCrypto:master Jun 2, 2025
18 checks passed
@daxpedda
Copy link
Contributor Author

daxpedda commented Jun 2, 2025

Done in dalek-cryptography/curve25519-dalek#764.

tarcieri pushed a commit to RustCrypto/RSA that referenced this pull request Jun 3, 2025
Implementation of `MultipartSigner` and `MultipartVerifier` added in
RustCrypto/traits#1880.
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.

Signing and Verifying Message from Non-Contiguous Byte Slices
3 participants