stalwartlabs / mail-auth

DKIM, ARC, SPF and DMARC library for Rust
https://docs.rs/mail-auth/
Apache License 2.0
82 stars 13 forks source link

Requiring keys to be Clone #8

Closed djc closed 1 year ago

djc commented 1 year ago

In my next step working on ring-based crypto impls, I'm running into an issue that I'd like your thoughts on before I go off in the wrong direction. In 3a571edfdb454edc98077b7dcf664abd0307ba6c you added a Clone derive to the types that implement SigningKey for use in tests. However, ring's private key types generally don't implement Clone -- which is a kind of misuse-resistance that I value, making it harder to accidentally leak private material through a code base.

How do you think we should solve this? I could change ArcSealer/DkimSigner to take a Cow, which would allow the caller to decide whether they just want to just pass a reference instead of key ownership. Alternatively, I could change the tests to parse the key material once for every sealer/signer it wants to set up.

mdecimus commented 1 year ago

I just removed the Clone derive on the latest commit. It was implemented for RsaKey only (which contains just the public key) but not for Ed25519 keys as these do not implement Clone either.

Regarding using Cow , the only use case I can think of for passing a key by reference is when the same SigningKey needs to be shared by both ArcSealer and DkimSigner. However, since the signers/sealers are supposed to be allocated only once rather than each time a signature is needed, I think that creating two keys and passing their ownership makes it simpler.