Skip to content
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

impl Clone and PartialEq for keypair types #859

Open
jimmycuadra opened this issue Jun 20, 2019 · 3 comments
Open

impl Clone and PartialEq for keypair types #859

jimmycuadra opened this issue Jun 20, 2019 · 3 comments

Comments

@jimmycuadra
Copy link

ring::signature::Ed25519KeyPair is used in ruma-signatures in a type that I'd like to implement Clone and PartialEq. I'm wondering if there's a specific reason Ed25519KeyPair doesn't implement these traits, or if it was just never needed before.

I can work around this by keeping the public and private keys and just creating a new Ed25519KeyPair in a manual Clone impl, and comparing the public and private keys in a manual implementation of PartialEq, but I'm not sure if 1) there is some security implication to this 2) there is any sort of random state held in Ed25519KeyPair that would make two values not equal even if they were generated from the same key pair.

If it's in fact okay to implement these traits in ring, I'd be happy to submit a PR.

@est31
Copy link

est31 commented Jun 24, 2019

👍 this would be great for rcgen as well: rustls/rcgen#17

Optimally, all KeyPair structs would get Clone and PartialEq (maybe Eq even)

@Ralith
Copy link
Contributor

Ralith commented Jul 9, 2019

Possibly related prior discussion: #727

@briansmith briansmith changed the title impl Clone and PartialEq for ring::signature::Ed25519KeyPair impl Clone and PartialEq for keypair types Mar 27, 2020
@briansmith
Copy link
Owner

I'm still on the fence regarding Clone, for the reasons I stated in #727. I want to support keys that are stored in places that don't allow cloning. Also, going forward the keypairs might store state (especially EcdsaKeyPairs) that shouldn't be cloned for semantic reasons related to randomization. Most users can use Arc<XxxKeyPair> to get a good enough approximation to a clonable XxxKeyPair.

Regarding PartialEq, maybe it is fine to implement PartialEq for public keys and then implement PartialEq for keypairs by saying that two keypairs are partially equal if the public keys are partially equal. Presumably this would allow people to look up a key pair by the public key in a map, so that might be useful.

I think that we're better off waiting to do this until after the big refactoring of the KeyPair types is merged; I've started working on it for RSA and I hope to have it merged soon.

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

No branches or pull requests

4 participants