RSA icon indicating copy to clipboard operation
RSA copied to clipboard

Implement Keypair trait for the RSA keys

Open lumag opened this issue 3 years ago • 4 comments

This is rather intrusive implementation. It reworks the way RsaPrivateKey is organized. Unfortunately it also meant that default Serialize an Deserilialize derivations provide a list of tokens that are not backwards-compatible. Thus I had to implement those traits manually.

lumag avatar Sep 16 '22 19:09 lumag

@lumag I'm confused why you need to change RsaPublicKey/RsaPrivateKey at all

tarcieri avatar Sep 16 '22 23:09 tarcieri

@lumag I'm confused why you need to change RsaPublicKey/RsaPrivateKey at all

@tarcieri because I have to embed a VerifyingKey as a field into the SigningKey. Otherwise as_ref() will fail with the returning a reference for temp variable error. And putting VerifyingKey next to RsaPrivateKey results in information duplication. So...I ended up with RsaPrivateKeyParts,

lumag avatar Sep 18 '22 16:09 lumag

@lumag I opened a tracking issue for problems with Keypair and suggested a possible alternative: https://github.com/RustCrypto/traits/issues/1124

tarcieri avatar Sep 19 '22 21:09 tarcieri

@tarcieri as a side note: While this PR is intrusive, it removes the required usage of RsaPrivateKey, making all SigningKey implementations first class citizens. Also note, that ECDSA signing keys also incorporate VerifyingKey as a field.

lumag avatar Sep 24 '22 15:09 lumag

I have rebased this PR on top of RSA 0.7.0. Note, I had to disable AsRef<RsaPrivateKey> for SigningKey<D> implementations. Since RsaPrivateKey is not a field of SigningKey, I don't see a way to implement AsRef

lumag avatar Oct 22 '22 23:10 lumag

@lumag PTAL at the Keypair changes to the signature crate here:

https://github.com/RustCrypto/traits/pull/1141/files#diff-1eb80c5452df3ad684ce36b4bfb4590a600978c10f0135bcba713ca18617d9bc

It makes the AsRef support an optional optimization, and changes the default Keypair trait to return the public key by value.

This is a proposed breaking change which I think would help simplify this use case.

tarcieri avatar Oct 24 '22 19:10 tarcieri