From 45bd376c083a9930c5f9fe078534f87edd97d04a Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 28 Aug 2025 22:31:33 -0400 Subject: [PATCH] Fix handling of prime/composite-order curves within short-weierstrass --- crypto/evrf/embedwards25519/src/lib.rs | 4 ++++ crypto/short-weierstrass/src/affine.rs | 4 ++-- crypto/short-weierstrass/src/lib.rs | 13 +++++++------ crypto/short-weierstrass/src/projective.rs | 15 ++++++++++++--- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/crypto/evrf/embedwards25519/src/lib.rs b/crypto/evrf/embedwards25519/src/lib.rs index a0a96d57..9db6423b 100644 --- a/crypto/evrf/embedwards25519/src/lib.rs +++ b/crypto/evrf/embedwards25519/src/lib.rs @@ -67,6 +67,10 @@ impl ShortWeierstrass for Embedwards25519 { (repr, odd_y) } + // No points have a torsion element as this a prime-order curve + fn has_torsion_element(_point: Projective) -> Choice { + 0.into() + } } pub type Point = Projective; diff --git a/crypto/short-weierstrass/src/affine.rs b/crypto/short-weierstrass/src/affine.rs index afc58822..3bd8e118 100644 --- a/crypto/short-weierstrass/src/affine.rs +++ b/crypto/short-weierstrass/src/affine.rs @@ -79,8 +79,8 @@ impl Affine { /// Create an affine point from `x, y` coordinates, without performing any checks. /// /// This should NOT be used. It is solely intended for trusted data at compile-time. It MUST NOT - /// be used with any untrusted/unvalidated data. Providing any off-curve point may produce - /// completely undefined behavior. + /// be used with any untrusted/unvalidated data. Providing any point not within the largest + /// prime-order subgroup has completely undefined behavior. pub const fn from_xy_unchecked(x: C::FieldElement, y: C::FieldElement) -> Self { Self { x, y } } diff --git a/crypto/short-weierstrass/src/lib.rs b/crypto/short-weierstrass/src/lib.rs index ca7ca6ca..74e7fb1d 100644 --- a/crypto/short-weierstrass/src/lib.rs +++ b/crypto/short-weierstrass/src/lib.rs @@ -15,10 +15,6 @@ mod projective; pub use projective::Projective; /// An elliptic curve represented in short Weierstrass form, with equation `y^2 = x^3 + A x + B`. -/// -/// This elliptic curve is expected to be of prime order. If a generator of the elliptic curve has -/// a composite order, the elliptic curve is defined solely as its largest odd-prime-order -/// subgroup, further considered the entire group/elliptic curve. pub trait ShortWeierstrass: 'static + Sized + Debug { /// The field the elliptic curve is defined over. type FieldElement: Zeroize + PrimeField; @@ -26,9 +22,9 @@ pub trait ShortWeierstrass: 'static + Sized + Debug { const A: Self::FieldElement; /// The constant `B` from the curve equation. const B: Self::FieldElement; - /// A generator of this elliptic curve. + /// A generator of this elliptic curve's largest prime-order subgroup. const GENERATOR: Affine; - /// The scalar type. + /// The scalar type for the elliptic curve's largest prime-order subgroup. /// /// This may be omitted by specifying `()`. type Scalar; @@ -45,4 +41,9 @@ pub trait ShortWeierstrass: 'static + Sized + Debug { /// /// This is expected to return the `x` coordinate and if the `y` coordinate is odd. fn decode_compressed(bytes: &Self::Repr) -> (::Repr, Choice); + + /// If the point is outside the largest prime-order subgroup and isn't the identity point. + /// + /// This may immediately return `Choice::new(0)` for curves of prime order. + fn has_torsion_element(point: Projective) -> Choice; } diff --git a/crypto/short-weierstrass/src/projective.rs b/crypto/short-weierstrass/src/projective.rs index 203084e7..58c59ffa 100644 --- a/crypto/short-weierstrass/src/projective.rs +++ b/crypto/short-weierstrass/src/projective.rs @@ -308,12 +308,20 @@ impl GroupEncoding for Projective { let (x, odd_y) = C::decode_compressed(bytes); - // Parse x, recover y, return the result - C::FieldElement::from_repr(x).and_then(|x| { + let result = C::FieldElement::from_repr(x).and_then(|x| { + // Parse x and recover y let non_identity_on_curve_point = Affine::decompress(x, odd_y).map(Projective::from); + // Set the identity, if the identity let identity = CtOption::new(Projective::IDENTITY, identity); non_identity_on_curve_point.or_else(|| identity) - }) + }); + + let mut result_is_valid = result.is_some(); + let result = result.unwrap_or(Projective::IDENTITY); + // Constrain points to the prime-order subgroup + result_is_valid &= !C::has_torsion_element(result); + + CtOption::new(result, result_is_valid) } fn from_bytes_unchecked(bytes: &C::Repr) -> CtOption { Self::from_bytes(bytes) @@ -341,6 +349,7 @@ impl GroupEncoding for Projective { } } +/// We implement `PrimeGroup` due to constraining to a prime-order subgroup impl> PrimeGroup for Projective {} #[cfg(feature = "alloc")]