Make BoxedUint::rem_vartime infallible using NonZero (#336)

Now that #335 has been merged allowing `NonZero<BoxedUint>` to be
expressed, we can use it as the operand for remainder, eliminating the
need to handle division by zero at the type system level.

This dramatically simplifies the implementation of
`BoxedResidueParams::new` since now all of the remainder calculations
are infallible, and is more consistent with the other remainder
functions.

src/modular/boxed_residue.rs

@@ -4,8 +4,8 @@
 mod mul;
 
 use super::reduction::montgomery_reduction_boxed;
-use crate::{BoxedUint, Limb, Word};
-use subtle::{Choice, CtOption};
+use crate::{BoxedUint, Limb, NonZero, Word};
+use subtle::CtOption;
 
 #[cfg(feature = "zeroize")]
 use zeroize::Zeroize;
@@ -37,19 +37,23 @@ impl BoxedResidueParams {
         let bits_precision = modulus.bits_precision();
         let is_odd = modulus.is_odd();
 
-        let r = BoxedUint::ct_map(
-            bits_precision,
-            BoxedUint::max(bits_precision).rem_vartime(&modulus), // TODO(tarcieri): constant time
-            |r| r.wrapping_add(&BoxedUint::one()),
-        );
+        // Use a surrogate value of `1` in case a modulus of `0` is passed.
+        // This will be rejected by the `is_odd` check above, which will fail and return `None`.
+        let modulus_nz = NonZero::new(BoxedUint::conditional_select(
+            &modulus,
+            &BoxedUint::one_with_precision(modulus.bits_precision()),
+            modulus.is_zero(),
+        ))
+        .expect("modulus ensured non-zero");
 
-        let r2 = BoxedUint::ct_map(
-            bits_precision,
-            BoxedUint::ct_and_then(bits_precision, r.clone(), |r| {
-                r.square().rem_vartime(&modulus.widen(bits_precision * 2)) // TODO(tarcieri): constant time
-            }),
-            |r2| r2.shorten(bits_precision),
-        );
+        let r = BoxedUint::max(bits_precision)
+            .rem_vartime(&modulus_nz)
+            .wrapping_add(&BoxedUint::one());
+
+        let r2 = r
+            .square()
+            .rem_vartime(&modulus_nz.widen(bits_precision * 2)) // TODO(tarcieri): constant time
+            .shorten(bits_precision);
 
         // Since we are calculating the inverse modulo (Word::MAX+1),
         // we can take the modulo right away and calculate the inverse of the first limb only.
@@ -58,31 +62,17 @@ impl BoxedResidueParams {
         let mod_neg_inv =
             Limb(Word::MIN.wrapping_sub(modulus_lo.inv_mod2k(Word::BITS as usize).limbs[0].0));
 
-        let r3 = BoxedUint::ct_map(bits_precision, r2.clone(), |r2| {
-            montgomery_reduction_boxed(&mut r2.square(), &modulus, mod_neg_inv)
-        });
-
-        // Not quite constant time, but shouldn't be an issue in practice, hopefully.
-        // The branching is just around constructing the return value.
-        let r = Option::<BoxedUint>::from(r);
-        let r2 = Option::<BoxedUint>::from(r2);
-        let r3 = Option::<BoxedUint>::from(r3);
-
-        let params = r.and_then(|r| {
-            r2.and_then(|r2| {
-                r3.map(|r3| Self {
-                    modulus,
-                    r,
-                    r2,
-                    r3,
-                    mod_neg_inv,
-                })
-            })
-        });
-
-        let is_some = Choice::from(params.is_some() as u8);
-        let placeholder = Self::placeholder(bits_precision);
-        CtOption::new(params.unwrap_or(placeholder), is_some & is_odd)
+        let r3 = montgomery_reduction_boxed(&mut r2.square(), &modulus, mod_neg_inv);
+
+        let params = Self {
+            modulus,
+            r,
+            r2,
+            r3,
+            mod_neg_inv,
+        };
+
+        CtOption::new(params, is_odd)
     }
 
     /// Modulus value.
@@ -94,19 +84,6 @@ impl BoxedResidueParams {
     pub fn bits_precision(&self) -> usize {
         self.modulus.bits_precision()
     }
-
-    /// Create a placeholder value with the given precision, used as a default for `CtOption`.
-    fn placeholder(bits_precision: usize) -> Self {
-        let zero = BoxedUint::zero_with_precision(bits_precision);
-
-        Self {
-            modulus: zero.clone(),
-            r: zero.clone(),
-            r2: zero.clone(),
-            r3: zero,
-            mod_neg_inv: Limb::ZERO,
-        }
-    }
 }
 
 /// A residue represented using heap-allocated limbs.

src/non_zero.rs

@@ -22,7 +22,7 @@ use serdect::serde::{
 
 /// Wrapper type for non-zero integers.
 #[derive(Copy, Clone, Debug, Default, Eq, PartialEq, PartialOrd, Ord)]
-pub struct NonZero<T: Zero>(T);
+pub struct NonZero<T: Zero>(pub(crate) T);
 
 impl NonZero<Limb> {
     /// Creates a new non-zero limb in a const context.
@@ -49,6 +49,11 @@ where
         let is_zero = n.is_zero();
         CtOption::new(Self(n), !is_zero)
     }
+
+    /// Returns the inner value.
+    pub fn get(self) -> T {
+        self.0
+    }
 }
 
 impl<T> NonZero<T>

src/uint/add_mod.rs

@@ -77,8 +77,8 @@ mod tests {
                 ];
 
                 for special in &moduli {
-                    let p = &NonZero::new(Uint::ZERO.wrapping_sub(&Uint::from_word(special.0)))
-                        .unwrap();
+                    let p =
+                        &NonZero::new(Uint::ZERO.wrapping_sub(&Uint::from(special.get()))).unwrap();
 
                     let minus_one = p.wrapping_sub(&Uint::ONE);
 

src/uint/boxed.rs

@@ -6,7 +6,6 @@ mod bit_and;
 mod bit_or;
 mod bits;
 mod cmp;
-mod ct;
 mod div;
 pub(crate) mod encoding;
 mod inv_mod;
@@ -16,7 +15,7 @@ mod shr;
 mod sub;
 mod sub_mod;
 
-use crate::{Limb, Uint, Word, Zero, U128, U64};
+use crate::{Limb, NonZero, Uint, Word, Zero, U128, U64};
 use alloc::{boxed::Box, vec, vec::Vec};
 use core::fmt;
 use subtle::{Choice, ConditionallySelectable};
@@ -32,7 +31,7 @@ use zeroize::Zeroize;
 /// Unlike many other heap-allocated big integer libraries, this type is not
 /// arbitrary precision and will wrap at its fixed-precision rather than
 /// automatically growing.
-#[derive(Clone, Default)]
+#[derive(Clone)]
 pub struct BoxedUint {
     /// Boxed slice containing limbs.
     ///
@@ -43,7 +42,9 @@ pub struct BoxedUint {
 impl BoxedUint {
     /// Get the value `0` represented as succinctly as possible.
     pub fn zero() -> Self {
-        Self::default()
+        Self {
+            limbs: vec![Limb::ZERO; 1].into(),
+        }
     }
 
     /// Get the value `0` with the given number of bits of precision.
@@ -247,6 +248,15 @@ impl BoxedUint {
     }
 }
 
+impl NonZero<BoxedUint> {
+    /// Widen this type's precision to the given number of bits.
+    ///
+    /// See [`BoxedUint::widen`] for more information, including panic conditions.
+    pub fn widen(&self, bits_precision: usize) -> Self {
+        NonZero(self.0.widen(bits_precision))
+    }
+}
+
 impl AsRef<[Word]> for BoxedUint {
     fn as_ref(&self) -> &[Word] {
         self.as_words()
@@ -271,15 +281,9 @@ impl AsMut<[Limb]> for BoxedUint {
     }
 }
 
-impl fmt::Debug for BoxedUint {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "BoxedUint(0x{self:X})")
-    }
-}
-
-impl fmt::Display for BoxedUint {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        fmt::UpperHex::fmt(self, f)
+impl Default for BoxedUint {
+    fn default() -> Self {
+        Self::zero()
     }
 }
 
@@ -355,6 +359,25 @@ impl Zero for BoxedUint {
     }
 }
 
+#[cfg(feature = "zeroize")]
+impl Zeroize for BoxedUint {
+    fn zeroize(&mut self) {
+        self.limbs.zeroize();
+    }
+}
+
+impl fmt::Debug for BoxedUint {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "BoxedUint(0x{self:X})")
+    }
+}
+
+impl fmt::Display for BoxedUint {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::UpperHex::fmt(self, f)
+    }
+}
+
 impl fmt::LowerHex for BoxedUint {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if self.limbs.is_empty() {
@@ -381,13 +404,6 @@ impl fmt::UpperHex for BoxedUint {
     }
 }
 
-#[cfg(feature = "zeroize")]
-impl Zeroize for BoxedUint {
-    fn zeroize(&mut self) {
-        self.limbs.zeroize();
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::BoxedUint;

src/uint/boxed/div.rs

@@ -1,16 +1,18 @@
 //! [`BoxedUint`] division operations.
 
-use crate::{BoxedUint, Limb};
-use subtle::{ConstantTimeEq, CtOption};
+use crate::{BoxedUint, Limb, NonZero};
+use subtle::ConstantTimeEq;
 
 impl BoxedUint {
     /// Computes self % rhs, returns the remainder.
     ///
     /// Variable-time with respect to `rhs`.
     ///
+    /// # Panics
+    ///
     /// Panics if `self` and `rhs` have different precisions.
-    // TODO(tarcieri): make infallible by making `rhs` into `NonZero`; don't panic
-    pub fn rem_vartime(&self, rhs: &Self) -> CtOption<Self> {
+    // TODO(tarcieri): handle different precisions without panicking
+    pub fn rem_vartime(&self, rhs: &NonZero<Self>) -> Self {
         debug_assert_eq!(self.nlimbs(), rhs.nlimbs());
         let mb = rhs.bits();
         let mut bd = self.bits_precision() - mb;
@@ -21,24 +23,22 @@ impl BoxedUint {
             let (r, borrow) = rem.sbb(&c, Limb::ZERO);
             rem = Self::conditional_select(&r, &rem, !borrow.ct_eq(&Limb::ZERO));
             if bd == 0 {
-                break;
+                break rem;
             }
             bd -= 1;
             c = c.shr_vartime(1);
         }
-
-        CtOption::new(rem, !(mb as u32).ct_eq(&0))
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use super::BoxedUint;
+    use super::{BoxedUint, NonZero};
 
     #[test]
     fn rem_vartime() {
         let n = BoxedUint::from(0xFFEECCBBAA99887766u128);
-        let p = BoxedUint::from(997u128);
-        assert_eq!(BoxedUint::from(648u128), n.rem_vartime(&p).unwrap());
+        let p = NonZero::new(BoxedUint::from(997u128)).unwrap();
+        assert_eq!(BoxedUint::from(648u128), n.rem_vartime(&p));
     }
 }

src/uint/mul_mod.rs

@@ -106,8 +106,8 @@ mod tests {
                 ];
 
                 for special in &moduli {
-                    let p = &NonZero::new(Uint::ZERO.wrapping_sub(&Uint::from_word(special.0)))
-                        .unwrap();
+                    let p =
+                        &NonZero::new(Uint::ZERO.wrapping_sub(&Uint::from(special.get()))).unwrap();
 
                     let minus_one = p.wrapping_sub(&Uint::ONE);
 

src/uint/sub_mod.rs

@@ -140,8 +140,8 @@ mod tests {
                 ];
 
                 for special in &moduli {
-                    let p = &NonZero::new(Uint::ZERO.wrapping_sub(&Uint::from_word(special.0)))
-                        .unwrap();
+                    let p =
+                        &NonZero::new(Uint::ZERO.wrapping_sub(&Uint::from(special.get()))).unwrap();
 
                     let minus_one = p.wrapping_sub(&Uint::ONE);