ed448-goldilocks: account for oddness in Scalar divisions (#1369)

- Re-use `Scalar::halve()` to compute `div_by_four()`
- Rename `Scalar::halve()` to `Scalar::div_by_2()`

Author: daxpedda

ed448-goldilocks/src/decaf/scalar.rs

@@ -153,9 +153,9 @@ mod test {
         let eight = DecafScalar::from(8u8);
         let four = DecafScalar::from(4u8);
         let two = DecafScalar::from(2u8);
-        assert_eq!(eight.halve(), four);
-        assert_eq!(four.halve(), two);
-        assert_eq!(two.halve(), DecafScalar::ONE);
+        assert_eq!(eight.div_by_2(), four);
+        assert_eq!(four.div_by_2(), two);
+        assert_eq!(two.div_by_2(), DecafScalar::ONE);
     }
 
     #[test]

ed448-goldilocks/src/edwards/extended.rs

@@ -334,38 +334,10 @@ impl EdwardsPoint {
     /// Generic scalar multiplication to compute s*P
     pub fn scalar_mul(&self, scalar: &EdwardsScalar) -> Self {
         // Compute floor(s/4)
-        let mut scalar_div_four = *scalar;
-        scalar_div_four.div_by_four();
+        let scalar_div_four = scalar.div_by_2().div_by_2();
 
         // Use isogeny and dual isogeny to compute phi^-1((s/4) * phi(P))
-        let partial_result = variable_base(&self.to_twisted(), &scalar_div_four).to_untwisted();
-        // Add partial result to (scalar mod 4) * P
-        partial_result.add(&self.scalar_mod_four(scalar))
-    }
-
-    /// Returns (scalar mod 4) * P in constant time
-    pub(crate) fn scalar_mod_four(&self, scalar: &EdwardsScalar) -> Self {
-        // Compute compute (scalar mod 4)
-        let s_mod_four = scalar[0] & 3;
-
-        // Compute all possible values of (scalar mod 4) * P
-        let zero_p = EdwardsPoint::IDENTITY;
-        let one_p = self;
-        let two_p = one_p.double();
-        let three_p = two_p.add(self);
-
-        // Under the reasonable assumption that `==` is constant time
-        // Then the whole function is constant time.
-        // This should be cheaper than calling double_and_add or a scalar mul operation
-        // as the number of possibilities are so small.
-        // XXX: This claim has not been tested (although it sounds intuitive to me)
-        let mut result = EdwardsPoint::IDENTITY;
-        result.conditional_assign(&zero_p, Choice::from((s_mod_four == 0) as u8));
-        result.conditional_assign(one_p, Choice::from((s_mod_four == 1) as u8));
-        result.conditional_assign(&two_p, Choice::from((s_mod_four == 2) as u8));
-        result.conditional_assign(&three_p, Choice::from((s_mod_four == 3) as u8));
-
-        result
+        variable_base(&self.to_twisted(), &scalar_div_four).to_untwisted()
     }
 
     /// Add two points

ed448-goldilocks/src/edwards/scalar.rs

@@ -169,9 +169,9 @@ mod test {
         let eight = EdwardsScalar::from(8u8);
         let four = EdwardsScalar::from(4u8);
         let two = EdwardsScalar::from(2u8);
-        assert_eq!(eight.halve(), four);
-        assert_eq!(four.halve(), two);
-        assert_eq!(two.halve(), EdwardsScalar::ONE);
+        assert_eq!(eight.div_by_2(), four);
+        assert_eq!(four.div_by_2(), two);
+        assert_eq!(two.div_by_2(), EdwardsScalar::ONE);
     }
 
     #[test]

ed448-goldilocks/src/field/scalar.rs

@@ -13,7 +13,7 @@ use elliptic_curve::{
         Array, ArraySize,
         typenum::{Prod, Unsigned},
     },
-    bigint::{Limb, NonZero, U448, U896, Word, Zero},
+    bigint::{Integer, Limb, NonZero, U448, U896, Word, Zero},
     consts::U2,
     ff::{Field, helpers},
     ops::{Invert, Reduce, ReduceNonZero},
@@ -658,13 +658,6 @@ impl<C: CurveWithScalar> Scalar<C> {
         self.scalar.is_zero()
     }
 
-    /// Divides a scalar by four without reducing mod p
-    /// This is used in the 2-isogeny when mapping points from Ed448-Goldilocks
-    /// to Twisted-Goldilocks
-    pub(crate) fn div_by_four(&mut self) {
-        self.scalar >>= 2;
-    }
-
     // This method was modified from Curve25519-Dalek codebase. [scalar.rs]
     // We start with 14 u32s and convert them to 56 u8s.
     // We then use the code copied from Dalek to convert the 56 u8s to radix-16 and re-center the coefficients to be between [-16,16)
@@ -778,8 +771,12 @@ impl<C: CurveWithScalar> Scalar<C> {
     }
 
     /// Halves a Scalar modulo the prime
-    pub const fn halve(&self) -> Self {
-        Self::new(self.scalar.shr_vartime(1))
+    pub fn div_by_2(&self) -> Self {
+        let is_odd = self.scalar.is_odd();
+        let if_odd = self.scalar + *ORDER;
+        let scalar = U448::conditional_select(&self.scalar, &if_odd, is_odd);
+
+        Self::new(scalar >> 1)
     }
 
     /// Attempt to construct a `Scalar` from a canonical byte representation.