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

Use Group instead of Curve when appropriate #1177

Open
daxpedda opened this issue Dec 24, 2022 · 8 comments
Open

Use Group instead of Curve when appropriate #1177

daxpedda opened this issue Dec 24, 2022 · 8 comments

Comments

@daxpedda
Copy link
Contributor

With dalek-cryptography/curve25519-dalek#473 hopefully getting done, it would be nice to use more types with Ristretto255.

Unfortunately types like PublicKey, SecretKey, NonZeroScalar, hash2curve::GroupDigest, and the ecdh module all work over Curve, which can not be implemented over Ristretto255 (as far as I understand).

So transitioning these types to use Group instead could allow us to use these types with Ristretto255 too.

@tarcieri
Copy link
Member

I think you're confusing elliptic_curve::Curve with group::Curve. The names are indeed confusing, however they serve two different functions.

elliptic_curve::Curve is pretty much the core trait of that respective crate, and provides a baseline level of functionality even for curves which may lack a full arithmetic implementation, such as ensuring scalars are in range of the curve's order. Notably the existence of that trait predates any integration with ff/group or perhaps less ambiguous names could have be selected.

As for getting everything to work with Ristretto255, that will probably take some refactoring and maybe some combinations of features just don't make sense. But it's definitely not just as simple as s/Curve/Group/.

@daxpedda
Copy link
Contributor Author

I think you're confusing elliptic_curve::Curve with group::Curve.

How so? Do you mean because I was proposing to use group::Group, which isn't a good replacement for elliptic_curve::Curve (now that I look at it)?

As for getting everything to work with Ristretto255, that will probably take some refactoring and maybe some combinations of features just don't make sense. But it's definitely not just as simple as s/Curve/Group/.

As far as I could tell, it would take some major restructuring and I'm clueless enough not to really be able to say what direction this should go.

Taking PublicKey for example, it relies on Curve and ProjectiveArithmetic.
ProjectiveArithmetic relies on Curve and AffineArithmetic.
AffineArithmetic relies on Curve and ScalarArithmetic.
ScalarArithmetic relies on Curve.

So removing Curve from these traits and replacing it with a new Group trait (not sure about the terminology here) doesn't seem too unreasonable to me here. But the fact that all of these trait chains usually rely on AffineArithmetic is a problem, as curve25519-dalek doesn't have an affine representation for both Ristretto255 and Curve25519.

I'm not sure how to circumvent this and my lack of cryptographic knowledge really hinders me to put a vision on this, I'm happy for any input about a direction to go for this.

Apologies in advance if I'm just getting something totally wrong here.

@tarcieri
Copy link
Member

tarcieri commented Dec 24, 2022

Yeah, elliptic_curve::Curve and group::{Curve, Group} do very different things.

The elliptic-curve crate uses ZSTs to identify particular curves (e.g. p256::NistP256, k256::Secp256k1), which is what makes type aliases like AffinePoint<C>, ProjectivePoint<C>, Scalar<C>, and NonZeroScalar<C> possible.

This is pretty different from group::Curve and group::Group, which are intended to be impl'd on ProjectivePoint in the context of elliptic curve groups.

The AffineArithmetic, ProjectiveArithmetic, and ScalarArithmetic traits provide the linkages between the ZSTs and the traits in the group crate.

This all could likely be refactored and simplified. The integration with ff/group was retrofitted after elliptic-curve had its own *Arithmetic traits (albeit underdeveloped) to express similar concepts. They could probably be combined into e.g. a CurveArithmetic trait now.

I think it makes sense to implement hash2curve in such a way you could plug Ristretto255, but probably not trying to make everything in elliptic-curve work with Ristretto255. Ristretto(255) isn't an elliptic curve in and of itself, so some impedance mismatch is to be expected there.

@daxpedda
Copy link
Contributor Author

Yes hash2curve also seemed like the easiest one to port to me.

Personally I would at least need: PublicKey, SecretKey and hash2curve. Do you think that PublicKey and SecretKey is not an option here?

I would like to point out that some refactoring might be needed for Ed25519/Curve25519 as well, as it doesn't have an affine representation in curve25519-dalek. What is going to be the move here? Actually implement an affine representation or is curve25519-dalek just not going to be compatible for the foreseeable future?
(I'm aware of the ed25519 crate, but that doesn't really cover the same use-case as all the elliptic-curve traits)

@tarcieri
Copy link
Member

tarcieri commented Dec 24, 2022

PublicKey and SecretKey are really intended for working with wire formats (e.g. PKCS#8/SPKI), and in that regard, seem orthogonal to hash2curve, which use a different class of wire formats (or can be a considered an alternative to those classical wire formats).

Is there a reason you can't use group::Group/group::Group::Scalar instead?

@daxpedda
Copy link
Contributor Author

daxpedda commented Dec 24, 2022

Is there a reason you can't use group::Group/group::Group::Scalar instead?

We can. If hash2curve is supported too, actually all our bases are covered, but we will have to re-implement all the nice types that elliptic-curve already offers.

Most of my requirements here come from voprf and opaque-ke, which do want to use Ristretto255 and Curve25519 but have to make their own types to wrap over the differences between the elliptic-curve and curve25519-dalek ecosystem. It would be nice not to have to make your own PublicKey and SecretKey type, or constantly write custom serialization for NonZeroScalar and NonIdentityPoint.

HPKE is another example, if you look at hpke, they had to make a new trait with an associated type for PublicKey and SecretKey to smooth over the differences between the two ecosystems and allow users to use both. Again, skipping over all these complexities and just use elliptic_curve::PublicKey would be a good improvement.

But generally speaking every time I'm working on cryptographic libraries and I want to use Ristretto255 or Curve25519, I have to re-implement a lot of great types provided by elliptic-curve. It would be nice if these two ecosystems fit together better.

For clarification though, this issue doesn't prevent anything right now. As you can see there are plenty of workarounds.

I understand that this is a very tall order and might not be appropriate for elliptic-curve in the first place.

@daxpedda
Copy link
Contributor Author

@tarcieri
Copy link
Member

tarcieri commented Jan 8, 2024

FWIW, I proposed renaming group::Curve to group::CurveGroup here, which I think would cut down on some of the confusion: zkcrypto/group#51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
@tarcieri @newpavlov @daxpedda and others