KeGroup trait implementation

[daxpedda]

Currently KeGroup is implemented on what represents the public key of a curve, the RistrettoPoint for ristretto255, ProjectivePoint for P-256 and MontgomaryPoint for X25519. When I switched to higher-level implementation for #250 I realized that we potentially don't even need a custom trait anymore.

elliptic_curve::PublicKey requires a type that implements Curve and ProjectiveArithmetic for example.

We can't actually use those because the current ristretto255 implementation we use doesn't implement any of these traits. So instead I'm proposing the following:

• Change KeGroup to not be implemented on the public key, but on a trait that is supposed to implement Curve and ProjectiveArithmetic. We introduce two associated types for a public and secret key.
• We can't provide a blanket implement of KeGroup for anything that satisfies the right trait, because we lack specialization to also provide a custom implementation for RistrettoPoint, but we can provide a simple template as an example.
• In the future, when curve25519-dalek implements the necessary traits, we can do a blanket implementation and basically remove all curve dependencies and associated crate features. Unless Rust stabilizes specialization first, lol.

This idea can also be implemented for voprf.
This is a proposal, I am planning to do the implementation as soon as this is approved.

[kevinlewi]

Hmm, interesting! So if I am understanding correctly, the main benefit for doing this would be that the consumer of the library doesn't need to provide definitions for our current custom traits KeGroup and OprfGroup, right?

One worry that I have is that we are coupling closely to how elliptic_curve and curve25519_dalek are doing things, and if in future versions they change, this might result in this library having to become more complex. Whereas if we are OK with defining the custom trait like how we are already doing, then we can maybe keep those future changes a bit more isolated...

I also don't expect that most consumers of this library will really need to worry about providing their own definitions of KeGroup and OprfGroup, since they will likely just use the existing supported ones we have (p256, ristretto255, x25519), so the pain point seems to be minor to me. Please correct me if I am wrong on this though!

[daxpedda]

Hmm, interesting! So if I am understanding correctly, the main benefit for doing this would be that the consumer of the library doesn't need to provide definitions for our current custom traits KeGroup and OprfGroup, right?

I was actually more concerned about us having to provide implementations. This way we are up-streaming the whole implementation, they probably have better testing and hopefully less errors. Certainly there are more eyes and users on elliptic-curve then on opaque-ke.

One worry that I have is that we are coupling closely to how elliptic_curve and curve25519_dalek are doing things, and if in future versions they change, this might result in this library having to become more complex. Whereas if we are OK with defining the custom trait like how we are already doing, then we can maybe keep those future changes a bit more isolated...

I'm not really worried about these libraries changing, they still follow the same RFCs and specs we follow. If you mean their API, I believe we should still use the same wrappers we currently have, just internally, so the code really shouldn't change much.

I also don't expect that most consumers of this library will really need to worry about providing their own definitions of KeGroup and OprfGroup, since they will likely just use the existing supported ones we have (p256, ristretto255, x25519), so the pain point seems to be minor to me. Please correct me if I am wrong on this though!

I agree, I was more hoping to upstream our work then users work. But it's definitely nice to be able to just get all kinds of implementations for free: P-256 and P-384 and hopefully in the future P-521 and decaf448.

See RustCrypto/elliptic-curves#495 for upstream hash to curve and RustCrypto/elliptic-curves#489 for P-384 arithmetic implementation that will be available soon.

[daxpedda]

Another thought: being able to just depend on traits means we won't need to expose any features anymore either, I think that's a nice bonus.

[daxpedda]

I did some rework based on our discussion here: facebook/voprf#49.