Implement CofactorGroup for all relevant curves (#1394)

- `DecafPoint` is always in the subgroup
-  Impelement `CofactorGroup` for `primeorder` curves
- Use `CofactorGroup` for `MapToCurve`

Author: daxpedda

ed448-goldilocks/src/decaf/points.rs

@@ -226,11 +226,11 @@ impl CofactorGroup for DecafPoint {
     type Subgroup = DecafPoint;
 
     fn clear_cofactor(&self) -> Self::Subgroup {
-        self.double().double()
+        *self
     }
 
     fn into_subgroup(self) -> CtOption<Self::Subgroup> {
-        CtOption::new(self.clear_cofactor(), self.is_torsion_free())
+        CtOption::new(self, Choice::from(1))
     }
 
     fn is_torsion_free(&self) -> Choice {

ed448-goldilocks/src/field/element.rs

@@ -16,7 +16,6 @@ use elliptic_curve::{
         consts::{U56, U64, U84, U88},
         modular::ConstMontyParams,
     },
-    group::cofactor::CofactorGroup,
     zeroize::DefaultIsZeroes,
 };
 use hash2curve::MapToCurve;
@@ -199,10 +198,6 @@ impl MapToCurve for Ed448 {
     fn map_to_curve(element: FieldElement) -> EdwardsPoint {
         element.map_to_curve_elligator2().isogeny().to_edwards()
     }
-
-    fn map_to_subgroup(point: EdwardsPoint) -> EdwardsPoint {
-        point.clear_cofactor()
-    }
 }
 
 impl MapToCurve for Decaf448 {
@@ -213,10 +208,6 @@ impl MapToCurve for Decaf448 {
     fn map_to_curve(element: FieldElement) -> DecafPoint {
         DecafPoint(element.map_to_curve_decaf448())
     }
-
-    fn map_to_subgroup(point: DecafPoint) -> DecafPoint {
-        point
-    }
 }
 
 impl Sum for FieldElement {

hash2curve/src/group_digest.rs

@@ -1,7 +1,9 @@
 //! Traits for handling hash to curve.
 
 use super::{ExpandMsg, MapToCurve, hash_to_field};
-use elliptic_curve::{ProjectivePoint, array::typenum::Unsigned};
+use elliptic_curve::ProjectivePoint;
+use elliptic_curve::array::typenum::Unsigned;
+use elliptic_curve::group::cofactor::CofactorGroup;
 
 /// Hash arbitrary byte sequences to a valid group element.
 pub trait GroupDigest: MapToCurve {
@@ -38,7 +40,7 @@ pub trait GroupDigest: MapToCurve {
         let [u0, u1] = hash_to_field::<2, X, _, Self::FieldElement, Self::FieldLength>(msg, dst)?;
         let q0 = Self::map_to_curve(u0);
         let q1 = Self::map_to_curve(u1);
-        Ok(Self::add_and_map_to_subgroup(q0, q1))
+        Ok((q0 + q1).clear_cofactor())
     }
 
     /// Computes the encode to curve routine.
@@ -67,7 +69,7 @@ pub trait GroupDigest: MapToCurve {
     {
         let [u] = hash_to_field::<1, X, _, Self::FieldElement, Self::FieldLength>(msg, dst)?;
         let q0 = Self::map_to_curve(u);
-        Ok(Self::map_to_subgroup(q0))
+        Ok(q0.clear_cofactor())
     }
 
     /// Computes the hash to field routine according to

hash2curve/src/map2curve.rs

@@ -2,12 +2,18 @@
 
 use elliptic_curve::array::typenum::NonZero;
 use elliptic_curve::array::{Array, ArraySize};
+use elliptic_curve::group::cofactor::CofactorGroup;
 use elliptic_curve::ops::Reduce;
 use elliptic_curve::{CurveArithmetic, ProjectivePoint};
 
 /// Trait for converting field elements into a point via a mapping method like
 /// Simplified Shallue-van de Woestijne-Ulas or Elligator.
-pub trait MapToCurve: CurveArithmetic<Scalar: Reduce<Array<u8, Self::ScalarLength>>> {
+pub trait MapToCurve:
+    CurveArithmetic<
+        ProjectivePoint: CofactorGroup<Subgroup = Self::ProjectivePoint>,
+        Scalar: Reduce<Array<u8, Self::ScalarLength>>,
+    >
+{
     /// The field element representation for a group value with multiple elements.
     type FieldElement: Reduce<Array<u8, Self::FieldLength>> + Default + Copy;
     /// The `L` parameter as specified in the [RFC](https://www.rfc-editor.org/rfc/rfc9380.html#section-5-6) for field elements.
@@ -17,17 +23,4 @@ pub trait MapToCurve: CurveArithmetic<Scalar: Reduce<Array<u8, Self::ScalarLengt
 
     /// Map a field element into a curve point.
     fn map_to_curve(element: Self::FieldElement) -> ProjectivePoint<Self>;
-
-    /// Map a curve point to a point in the curve subgroup.
-    /// This is usually done by clearing the cofactor, if necessary.
-    fn map_to_subgroup(point: ProjectivePoint<Self>) -> ProjectivePoint<Self>;
-
-    /// Combine two curve points into a point in the curve subgroup.
-    /// This is usually done by clearing the cofactor of the sum.
-    fn add_and_map_to_subgroup(
-        lhs: ProjectivePoint<Self>,
-        rhs: ProjectivePoint<Self>,
-    ) -> ProjectivePoint<Self> {
-        Self::map_to_subgroup(lhs + rhs)
-    }
 }

k256/src/arithmetic/hash2curve.rs

@@ -143,10 +143,6 @@ impl MapToCurve for Secp256k1 {
         }
         .into()
     }
-
-    fn map_to_subgroup(point: ProjectivePoint) -> ProjectivePoint {
-        point
-    }
 }
 
 impl Reduce<Array<u8, U48>> for Scalar {
@@ -267,6 +263,7 @@ mod tests {
         array::Array,
         bigint::{ArrayEncoding, NonZero, U384},
         consts::U48,
+        group::cofactor::CofactorGroup,
         ops::Reduce,
     };
     use hash2curve::{GroupDigest, MapToCurve};
@@ -374,7 +371,7 @@ mod tests {
             assert_eq!(aq1.x.to_bytes().as_slice(), test_vector.q1_x);
             assert_eq!(aq1.y.to_bytes().as_slice(), test_vector.q1_y);
 
-            let p = Secp256k1::add_and_map_to_subgroup(q0, q1);
+            let p = (q0 + q1).clear_cofactor();
             let ap = p.to_affine();
             assert_eq!(ap.x.to_bytes().as_slice(), test_vector.p_x);
             assert_eq!(ap.y.to_bytes().as_slice(), test_vector.p_y);

k256/src/arithmetic/projective.rs

@@ -12,6 +12,7 @@ use elliptic_curve::{
     BatchNormalize, CurveGroup, Error, Result,
     group::{
         Group, GroupEncoding,
+        cofactor::CofactorGroup,
         prime::{PrimeCurve, PrimeCurveAffine, PrimeGroup},
     },
     rand_core::TryRngCore,
@@ -451,6 +452,22 @@ impl GroupEncoding for ProjectivePoint {
     }
 }
 
+impl CofactorGroup for ProjectivePoint {
+    type Subgroup = ProjectivePoint;
+
+    fn clear_cofactor(&self) -> Self::Subgroup {
+        *self
+    }
+
+    fn into_subgroup(self) -> CtOption<Self::Subgroup> {
+        CtOption::new(self, Choice::from(1))
+    }
+
+    fn is_torsion_free(&self) -> Choice {
+        Choice::from(1)
+    }
+}
+
 impl PrimeGroup for ProjectivePoint {}
 
 impl CurveGroup for ProjectivePoint {

p256/src/arithmetic/hash2curve.rs

@@ -70,10 +70,6 @@ impl MapToCurve for NistP256 {
             .unwrap()
             .into()
     }
-
-    fn map_to_subgroup(point: ProjectivePoint) -> ProjectivePoint {
-        point
-    }
 }
 
 impl Reduce<Array<u8, U48>> for Scalar {

p384/src/arithmetic/hash2curve.rs

@@ -72,10 +72,6 @@ impl MapToCurve for NistP384 {
             .unwrap()
             .into()
     }
-
-    fn map_to_subgroup(point: ProjectivePoint) -> ProjectivePoint {
-        point
-    }
 }
 
 impl Reduce<Array<u8, U72>> for Scalar {
@@ -107,6 +103,7 @@ mod tests {
         array::Array,
         bigint::{ArrayEncoding, CheckedSub, NonZero, U384, U576},
         consts::U72,
+        group::cofactor::CofactorGroup,
         ops::Reduce,
         sec1::{self, ToEncodedPoint},
     };
@@ -239,7 +236,7 @@ mod tests {
             let q1 = NistP384::map_to_curve(u[1]);
             assert_point_eq!(q1, test_vector.q1_x, test_vector.q1_y);
 
-            let p = NistP384::add_and_map_to_subgroup(q0, q1);
+            let p = (q0 + q1).clear_cofactor();
             assert_point_eq!(p, test_vector.p_x, test_vector.p_y);
 
             // complete run

p521/src/arithmetic/hash2curve.rs

@@ -75,10 +75,6 @@ impl MapToCurve for NistP521 {
             .unwrap()
             .into()
     }
-
-    fn map_to_subgroup(point: ProjectivePoint) -> ProjectivePoint {
-        point
-    }
 }
 
 impl Reduce<Array<u8, U98>> for Scalar {
@@ -110,6 +106,7 @@ mod tests {
         array::Array,
         bigint::{ArrayEncoding, CheckedSub, NonZero, U576, U896},
         consts::U98,
+        group::cofactor::CofactorGroup,
         ops::Reduce,
         sec1::{self, ToEncodedPoint},
     };
@@ -242,7 +239,7 @@ mod tests {
             let q1 = NistP521::map_to_curve(u[1]);
             assert_point_eq!(q1, test_vector.q1_x, test_vector.q1_y);
 
-            let p = NistP521::add_and_map_to_subgroup(q0, q1);
+            let p = (q0 + q1).clear_cofactor();
             assert_point_eq!(p, test_vector.p_x, test_vector.p_y);
 
             // complete run

primeorder/src/projective.rs

@@ -16,6 +16,7 @@ use elliptic_curve::{
     bigint::{ArrayEncoding, ByteArray},
     group::{
         Group, GroupEncoding,
+        cofactor::CofactorGroup,
         prime::{PrimeCurve, PrimeGroup},
     },
     ops::{BatchInvert, LinearCombination},
@@ -274,6 +275,30 @@ where
     }
 }
 
+impl<C> CofactorGroup for ProjectivePoint<C>
+where
+    C: PrimeCurveParams,
+    CompressedPoint<C>: Send + Sync,
+    FieldBytes<C>: Copy,
+    FieldBytesSize<C>: ModulusSize,
+    CompressedPoint<C>: Copy,
+    <UncompressedPointSize<C> as ArraySize>::ArrayType<u8>: Copy,
+{
+    type Subgroup = ProjectivePoint<C>;
+
+    fn clear_cofactor(&self) -> Self::Subgroup {
+        *self
+    }
+
+    fn into_subgroup(self) -> CtOption<Self::Subgroup> {
+        CtOption::new(self, Choice::from(1))
+    }
+
+    fn is_torsion_free(&self) -> Choice {
+        Choice::from(1)
+    }
+}
+
 impl<C> CurveGroup for ProjectivePoint<C>
 where
     C: PrimeCurveParams,