ed448-goldilocks: account for oddness in Scalar divisions

Author: daxpedda

ed448-goldilocks/src/edwards/extended.rs

@@ -338,34 +338,7 @@ impl EdwardsPoint {
         scalar_div_four.div_by_four();
 
         // 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/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},
@@ -662,6 +662,18 @@ impl<C: CurveWithScalar> Scalar<C> {
     /// This is used in the 2-isogeny when mapping points from Ed448-Goldilocks
     /// to Twisted-Goldilocks
     pub(crate) fn div_by_four(&mut self) {
+        let s_mod_4 = self[0] & 3;
+
+        let s_plus_l = self.scalar + ORDER;
+        let s_plus_2l = s_plus_l + ORDER;
+        let s_plus_3l = s_plus_2l + ORDER;
+
+        self.scalar.conditional_assign(&s_plus_l, s_mod_4.ct_eq(&1));
+        self.scalar
+            .conditional_assign(&s_plus_2l, s_mod_4.ct_eq(&2));
+        self.scalar
+            .conditional_assign(&s_plus_3l, s_mod_4.ct_eq(&3));
+
         self.scalar >>= 2;
     }
 
@@ -778,8 +790,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 halve(&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.