Fix Montgomery reduction in repeated squaring

src/modular/fixed_monty_form/pow.rs

@@ -154,10 +154,10 @@ impl<const LIMBS: usize, const RHS_LIMBS: usize>
 
 #[cfg(test)]
 mod tests {
-    use crate::traits::MultiExponentiate;
     use crate::{
-        U256,
+        U64, U256,
         modular::{FixedMontyForm, FixedMontyParams},
+        traits::MultiExponentiate,
     };
 
     const PARAMS: FixedMontyParams<{ U256::LIMBS }> = FixedMontyParams::new_vartime(
@@ -230,6 +230,20 @@ mod tests {
         assert_eq!(res_vartime.retrieve(), expected);
     }
 
+    #[test]
+    fn issue_1343_regression_test() {
+        let modulus = U64::from_u64(0x7241_9be3_5d7d_03fb).to_odd().unwrap();
+
+        let params = FixedMontyParams::new_vartime(modulus);
+        let x = FixedMontyForm::new(&U64::from_u64(13480), &params);
+        let exponent = U64::from_u64(16);
+
+        let ct = x.pow(&exponent);
+        let vt = x.pow_vartime(&exponent);
+
+        assert_eq!(ct, vt);
+    }
+
     #[test]
     fn test_multi_exp_array() {
         let base = U256::from(2u8);

src/modular/mul.rs

@@ -173,6 +173,8 @@ pub(crate) const fn square_montgomery_form<const LIMBS: usize>(
 /// `a` is in Montgomery form.
 ///
 /// This method is variable time in `n`.
+///
+/// Assumes `a` is fully reduced, i.e. `a < modulus`.
 #[inline(always)]
 pub(crate) const fn square_repeat_montgomery_form<const LIMBS: usize>(
     a: &Uint<LIMBS>,
@@ -190,29 +192,21 @@ pub(crate) const fn square_repeat_montgomery_form<const LIMBS: usize>(
     let mut i = 0;
     let mut out = *a;
     let mut base;
-    let mut carry;
 
-    loop {
+    while i != n {
         (base, out) = (out, Uint::ZERO);
-        carry = montgomery_multiply_inner(
+        let carry = montgomery_multiply_inner(
             &base.limbs,
             &base.limbs,
             &mut out.limbs,
             &modulus.as_ref().limbs,
             mod_neg_inv,
         );
+        // Because `base < modulus` here, the raw product is below `2 * modulus` and a single
+        // conditional subtraction suffices.
+        out = out.try_sub_with_carry(carry, modulus.as_ref()).0;
         i += 1;
-        if i == n {
-            break;
-        }
-        // intermediate results are in "Almost Montgomery form", which is <= Uint::MAX
-        // but may require the modulus to be subtracted twice.
-        out = out
-            .conditional_borrowing_sub(modulus.as_ref(), carry.is_nonzero())
-            .0;
     }
 
-    // correct for "Almost Montygomery form"
-    (out, carry) = out.try_sub_with_carry(carry, modulus.as_ref());
-    out.try_sub_with_carry(carry, modulus.as_ref()).0
+    out
 }

src/uint/sub.rs

@@ -1,9 +1,7 @@
 //! [`Uint`] subtraction operations.
 
 use super::Uint;
-use crate::{
-    Checked, CheckedSub, Choice, CtOption, Limb, Sub, SubAssign, Wrapping, WrappingSub, word,
-};
+use crate::{Checked, CheckedSub, CtOption, Limb, Sub, SubAssign, Wrapping, WrappingSub, word};
 
 impl<const LIMBS: usize> Uint<LIMBS> {
     /// Computes `self - (rhs + borrow)`, returning the result along with the new borrow.
@@ -30,29 +28,6 @@ impl<const LIMBS: usize> Uint<LIMBS> {
         (Self { limbs }, borrow)
     }
 
-    /// Perform wrapping subtraction, returning the truthy value as the second element of
-    /// the tuple if an underflow has occurred.
-    #[inline]
-    #[must_use]
-    pub(crate) const fn conditional_borrowing_sub(
-        &self,
-        rhs: &Self,
-        choice: Choice,
-    ) -> (Self, Choice) {
-        let mut limbs = [Limb::ZERO; LIMBS];
-        let mask = Limb::select(Limb::ZERO, Limb::MAX, choice);
-        let mut borrow = Limb::ZERO;
-
-        let mut i = 0;
-        while i < LIMBS {
-            let masked_rhs = rhs.limbs[i].bitand(mask);
-            (limbs[i], borrow) = self.limbs[i].borrowing_sub(masked_rhs, borrow);
-            i += 1;
-        }
-
-        (Self { limbs }, borrow.lsb_to_choice())
-    }
-
     /// Perform saturating subtraction, returning `ZERO` on underflow.
     #[must_use]
     pub const fn saturating_sub(&self, rhs: &Self) -> Self {
@@ -137,8 +112,6 @@ impl<const LIMBS: usize> WrappingSub for Uint<LIMBS> {
 
 #[cfg(test)]
 mod tests {
-    use ctutils::Choice;
-
     use crate::{CheckedSub, Limb, U128};
 
     #[test]
@@ -156,27 +129,6 @@ mod tests {
         assert_eq!(borrow, Limb::MAX);
     }
 
-    #[test]
-    fn conditional_borrowing_sub_no_sub() {
-        let (res, borrow) = U128::ONE.conditional_borrowing_sub(&U128::ONE, Choice::FALSE);
-        assert_eq!(res, U128::ONE);
-        assert!(!borrow.to_bool());
-    }
-
-    #[test]
-    fn conditional_borrowing_sub_no_borrow() {
-        let (res, borrow) = U128::ONE.conditional_borrowing_sub(&U128::ZERO, Choice::TRUE);
-        assert_eq!(res, U128::ONE);
-        assert!(!borrow.to_bool());
-    }
-
-    #[test]
-    fn conditional_borrowing_sub_borrow() {
-        let (res, borrow) = U128::ZERO.conditional_borrowing_sub(&U128::ONE, Choice::TRUE);
-        assert_eq!(res, U128::MAX);
-        assert!(borrow.to_bool());
-    }
-
     #[test]
     fn saturating_sub_no_borrow() {
         assert_eq!(