removed legacy impl everything is working except signal testcases [skip ci]

Author: jmwample

curve25519-dalek/src/backend/serial/u64/field.rs

@@ -597,24 +597,4 @@ impl FieldElement51 {
 
         Choice::from(c_gt as u8)
     }
-
-    /// Returns 1 if self is greater than the other and 0 otherwise
-    // strategy: check if b-a overflows. if it does not overflow, then a was larger
-    pub(crate) fn mpn_sub_n(&self, other: &Self) -> Choice {
-        let mut _ul = 0_u64;
-        let mut _vl = 0_u64;
-        let mut _rl = 0_u64;
-
-        let mut cy = 0_u64;
-        for i in 0..5 {
-            _ul = self.0[i];
-            _vl = other.0[i];
-
-            let (_sl, _cy1) = _ul.overflowing_sub(_vl);
-            let (_rl, _cy2) = _sl.overflowing_sub(cy);
-            cy = _cy1 as u64 | _cy2 as u64;
-        }
-
-        Choice::from((cy != 0_u64) as u8)
-    }
 }

curve25519-dalek/src/edwards.rs

@@ -588,7 +588,7 @@ impl EdwardsPoint {
     where
         D: Digest<OutputSize = U64> + Default,
     {
-        use crate::elligator2::Legacy;
+        use crate::elligator2::RFC9380;
 
         let mut hash = D::new();
         hash.update(bytes);
@@ -601,7 +601,7 @@ impl EdwardsPoint {
         // rfc9380 should always result in a valid point since no field elements
         // are invalid. so unwrap should be safe.
         #[allow(clippy::unwrap_used)]
-        let fe1 = MontgomeryPoint::from_representative::<Legacy>(&res).unwrap();
+        let fe1 = MontgomeryPoint::from_representative::<RFC9380>(&res).unwrap();
         let E1_opt = fe1.to_edwards(sign_bit);
 
         E1_opt

curve25519-dalek/src/elligator2.rs

@@ -34,22 +34,38 @@
 //! [`EdwardsPoint`] objects themselves.
 //!
 //! ```rust
+//! # use hex::FromHex;
 //! use rand::RngCore;
 //! use curve25519_dalek::{MontgomeryPoint, EdwardsPoint, elligator2::{RFC9380, Randomized, MapToPointVariant}};
 //!
 //! // Montgomery Points can be mapped to and from elligator representatives
 //! // using any algorithm variant.
 //! let tweak = rand::thread_rng().next_u32() as u8;
-//! let mont_point = MontgomeryPoint::default(); // example point known to be representable
+//! let mont_point = MontgomeryPoint::default(); // example private key point known to be representable
 //! let r = mont_point.to_representative::<RFC9380>(tweak).unwrap();
+//! let pubkey = RFC9380::mul_base_clamped(mont_point.to_bytes()).to_montgomery();
 //!
-//! _ = MontgomeryPoint::from_representative::<RFC9380>(&r).unwrap();
+//! let derived_pubkey = MontgomeryPoint::from_representative::<RFC9380>(&r).unwrap();
+//! assert_eq!(pubkey, derived_pubkey);
 //!
-//! // Edwards Points can be transformed as well.
+//! // Points in byte format can be transformed
+//!
+//! // (example known representable point)
+//! let mut point = <[u8;32]>::from_hex("00deadbeef00deadbeef00deadbeef00deadbeef00deadbeef00deadbeef0124").unwrap();
+//! let tweak = rand::thread_rng().next_u32() as u8;
+//! let r = RFC9380::to_representative(&point, tweak).unwrap();
+//! let pubkey = RFC9380::mul_base_clamped(point).to_montgomery();
+//!
+//! let derived_pubkey = MontgomeryPoint::from_representative::<RFC9380>(&r).unwrap();
+//! assert_eq!(pubkey, derived_pubkey);
+//!
+//! // Edwards Points can be transformed as well. This example uses the Randomized variant.
 //! let edw_point = EdwardsPoint::default(); // example point known to be representable
 //! let r = edw_point.to_representative::<Randomized>(tweak).unwrap();
+//! let pubkey = Randomized::mul_base_clamped(edw_point.to_montgomery().to_bytes());
 //!
-//! _ = EdwardsPoint::from_representative::<Randomized>(&r).unwrap();
+//! let derived_pubkey = EdwardsPoint::from_representative::<Randomized>(&r).unwrap();
+//! assert_eq!(pubkey, derived_pubkey);
 //! ```
 //!
 //! ### Generating Representable Points.
@@ -131,8 +147,7 @@ use crate::EdwardsPoint;
 
 use cfg_if::cfg_if;
 use subtle::{
-    Choice, ConditionallyNegatable, ConditionallySelectable, ConstantTimeEq, ConstantTimeGreater,
-    CtOption,
+    Choice, ConditionallyNegatable, ConditionallySelectable, ConstantTimeEq, CtOption,
 };
 
 /// bitmask for a single byte when clearing the high order two bits of a representative
@@ -141,10 +156,6 @@ pub(crate) const MASK_UNSET_BYTE: u8 = 0x3f;
 pub(crate) const MASK_SET_BYTE: u8 = 0xc0;
 
 /// (p - 1) / 2 = 2^254 - 10
-pub(crate) const DIVIDE_MINUS_P_1_2_BYTES: [u8; 32] = [
-    0xf6, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x3f,
-];
 
 /// Common interface for the different ways to compute the elligator2 forward
 /// and reverse transformations.
@@ -230,67 +241,6 @@ impl MapToPointVariant for Randomized {
     }
 }
 
-#[cfg(feature = "digest")]
-/// In general this mode should **NEVER** be used unless there is a very specific
-/// reason to do so as it has multiple serious known flaws.
-///
-/// Converts between a point on elliptic curve E (Curve25519) and an element of
-/// the finite field F over which E is defined. Supports older implementations
-/// with a common implementation bug (Signal, Kleshni-C).
-///
-/// In contrast to the [`RFC9380`] variant, `Legacy` does NOT assume that input values are always
-/// going to be the least-square-root representation of the field element.
-/// This is divergent from the specifications for both elligator2 and RFC 9380,
-/// however, some older implementations miss this detail. This allows us to be
-/// compatible with those alternate implementations if necessary, since the
-/// resulting point will be different for inputs with either of the
-/// high-order two bits set. The kleshni C and Signal implementations are examples
-/// of libraries that don't always use the least square root.
-///
-// We return the LSR for to_representative values. This is here purely for testing
-// compatability and ensuring that we understand the subtle differences that can
-// influence proper implementation.
-pub struct Legacy;
-
-#[cfg(feature = "digest")]
-impl MapToPointVariant for Legacy {
-    fn from_representative(representative: &[u8; 32]) -> CtOption<EdwardsPoint> {
-        let representative = FieldElement::from_bytes(representative);
-        let (x, y) = map_fe_to_edwards(&representative);
-        let point = EdwardsPoint {
-            X: x,
-            Y: y,
-            Z: FieldElement::ONE,
-            T: &x * &y,
-        };
-        CtOption::new(point, Choice::from(1))
-    }
-
-    // There is a bug in the kleshni implementation where it
-    // takes a sortcut when computng greater than for field elemements.
-    // For the purpose of making tests pass matching the bugged implementation
-    // I am adding the bug here intentionally. Legacy is not exposed and
-    // should not be exposed as it is obviously flawed in multiple ways.
-    //
-    // What we want is:
-    //      If root - (p - 1) / 2 < 0, root := -root
-    // This is not equivalent to:
-    //      if root > (p - 1)/2 root := -root
-    //
-    fn to_representative(point: &[u8; 32], _tweak: u8) -> CtOption<[u8; 32]> {
-        let pubkey = EdwardsPoint::mul_base_clamped(*point);
-        let v_in_sqrt = v_in_sqrt_pubkey_edwards(&pubkey);
-
-        point_to_representative(&MontgomeryPoint(*point), v_in_sqrt.into())
-
-        // let divide_minus_p_1_2 = FieldElement::from_bytes(&DIVIDE_MINUS_P_1_2_BYTES);
-        // let did_negate = divide_minus_p_1_2.ct_gt(&b);
-        // let should_negate = Self::gt(&b, &divide_minus_p_1_2);
-        // FieldElement::conditional_negate(&mut b, did_negate ^ should_negate);
-        // CtOption::new(b.as_bytes(), c)
-    }
-}
-
 // ===========================================================================
 // Montgomery and Edwards Interfaces
 // ===========================================================================
@@ -577,8 +527,6 @@ pub(crate) fn point_to_representative(
     point: &MontgomeryPoint,
     v_in_sqrt: bool,
 ) -> CtOption<[u8; 32]> {
-    let divide_minus_p_1_2 = FieldElement::from_bytes(&DIVIDE_MINUS_P_1_2_BYTES);
-
     // a := point
     let a = &FieldElement::from_bytes(&point.0);
     let a_neg = -a;
@@ -602,7 +550,8 @@ pub(crate) fn point_to_representative(
 
     // If root > (p - 1) / 2, root := -root
     // let negate = divide_minus_p_1_2.ct_gt(&b);
-    let negate = divide_minus_p_1_2.mpn_sub_n(&b);
+    // let negate = divide_minus_p_1_2.mpn_sub_n(&b);
+    let negate = b.is_negative();
     FieldElement::conditional_negate(&mut b, negate);
 
     CtOption::new(b.as_bytes(), is_encodable)
@@ -678,8 +627,6 @@ pub(crate) fn v_in_sqrt(key_input: &[u8; 32]) -> Choice {
 /// Determines if `V <= (p - 1)/2` for an EdwardsPoint (e.g an x25519 public key)
 /// and returns a [`Choice`] indicating the result.
 pub(crate) fn v_in_sqrt_pubkey_edwards(pubkey: &EdwardsPoint) -> Choice {
-    let divide_minus_p_1_2 = FieldElement::from_bytes(&DIVIDE_MINUS_P_1_2_BYTES);
-
     // sqrtMinusAPlus2 is sqrt(-(486662+2))
     let (_, sqrt_minus_a_plus_2) = FieldElement::sqrt_ratio_i(
         &(&MONTGOMERY_A_NEG - &(&FieldElement::ONE + &FieldElement::ONE)),
@@ -697,8 +644,8 @@ pub(crate) fn v_in_sqrt_pubkey_edwards(pubkey: &EdwardsPoint) -> Choice {
 
     // is   v <= (q-1)/2  ?
     // divide_minus_p_1_2.ct_gt(&v)
-    // v.is_negative()
-    divide_minus_p_1_2.mpn_sub_n(&v)
+    // divide_minus_p_1_2.mpn_sub_n(&v)
+    !v.is_negative()
 }
 
 // ============================================================================
@@ -800,8 +747,3 @@ mod randomness;
 #[cfg(test)]
 #[cfg(feature = "elligator2")]
 mod subgroup;
-
-#[cfg(test)]
-#[cfg(feature = "elligator2")]
-#[cfg(feature = "digest")]
-mod legacy;

curve25519-dalek/src/elligator2/compatibility.rs

@@ -1,11 +1,23 @@
 use super::*;
-
 use hex::FromHex;
 
 ////////////////////////////////////////////////////////////
 // Ntor tests                                             //
 ////////////////////////////////////////////////////////////
 
+
+#[test]
+#[cfg(feature = "elligator2")]
+fn ident_is_representable() {
+    let m = MontgomeryPoint::default();
+    let repres = m.to_representative::<RFC9380>(0u8).expect("should succeed, I think");
+    let pubkey = RFC9380::mul_base_clamped(m.to_bytes()).to_montgomery();
+
+    // let orig = MontgomeryPoint::from_representative::<RFC9380>(&out).expect("should prob also work");
+    let orig = MontgomeryPoint::map_to_point(&repres);
+    assert_eq!(&pubkey, &orig);
+}
+
 #[test]
 #[cfg(feature = "elligator2")]
 fn pubkey_from_repres() {