From 5333a30b60cb6cbf785c4dcb695189a5d1c18690 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Tue, 23 May 2023 08:39:53 -0500 Subject: [PATCH 1/4] Enforce odd modulus in `impl_modulus!` --- src/uint/modular/constant_mod/macros.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/uint/modular/constant_mod/macros.rs b/src/uint/modular/constant_mod/macros.rs index 1583ca529..c637d2ce0 100644 --- a/src/uint/modular/constant_mod/macros.rs +++ b/src/uint/modular/constant_mod/macros.rs @@ -2,6 +2,7 @@ #[macro_export] /// Implements a modulus with the given name, type, and value, in that specific order. Please `use crypto_bigint::traits::Encoding` to make this work. /// For example, `impl_modulus!(MyModulus, U256, "73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001");` implements a 256-bit modulus named `MyModulus`. +/// The modulus _must_ be odd, or this will panic. macro_rules! impl_modulus { ($name:ident, $uint_type:ty, $value:expr) => { #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] @@ -14,8 +15,16 @@ macro_rules! impl_modulus { $crate::Concat>, { const LIMBS: usize = { $crate::nlimbs!(<$uint_type>::BITS) }; - const MODULUS: $crate::Uint<{ $crate::nlimbs!(<$uint_type>::BITS) }> = - <$uint_type>::from_be_hex($value); + const MODULUS: $crate::Uint<{ $crate::nlimbs!(<$uint_type>::BITS) }> = { + let res = <$uint_type>::from_be_hex($value); + + // Check that the modulus is odd + if (res.as_limbs()[0].0 & 1) == 0 { + panic!("modulus must be odd"); + } + + res + }; const R: $crate::Uint<{ $crate::nlimbs!(<$uint_type>::BITS) }> = $crate::Uint::MAX .const_rem(&Self::MODULUS) .0 From de8e08839792bfdc24c8626e3fec5e73ee38980d Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Thu, 25 May 2023 11:10:02 -0500 Subject: [PATCH 2/4] Enforce in `Residue` and `const_residue!` --- src/uint/modular.rs | 4 ++-- src/uint/modular/constant_mod.rs | 30 ++++++++++++++++++++++++- src/uint/modular/constant_mod/macros.rs | 6 ++++- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/uint/modular.rs b/src/uint/modular.rs index cd560aa38..1448feb59 100644 --- a/src/uint/modular.rs +++ b/src/uint/modular.rs @@ -146,7 +146,7 @@ mod tests { fn test_new_retrieve() { let x = U256::from_be_hex("44acf6b7e36c1342c2c5897204fe09504e1e2efb1a900377dbc4e7a6a133ec56"); - let x_mod = Residue::::new(&x); + let x_mod = Residue::::new_checked(&x).unwrap(); // Confirm that when creating a Modular and retrieving the value, that it equals the original assert_eq!(x, x_mod.retrieve()); @@ -157,7 +157,7 @@ mod tests { let x = U256::from_be_hex("44acf6b7e36c1342c2c5897204fe09504e1e2efb1a900377dbc4e7a6a133ec56"); assert_eq!( - Residue::::new(&x), + Residue::::new_checked(&x).unwrap(), const_residue!(x, Modulus2) ); } diff --git a/src/uint/modular/constant_mod.rs b/src/uint/modular/constant_mod.rs index 92b6ce761..b246cb98d 100644 --- a/src/uint/modular/constant_mod.rs +++ b/src/uint/modular/constant_mod.rs @@ -1,6 +1,6 @@ use core::{fmt::Debug, marker::PhantomData}; -use subtle::{Choice, ConditionallySelectable, ConstantTimeEq}; +use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption}; use crate::{Limb, Uint, Zero}; @@ -88,7 +88,17 @@ impl, const LIMBS: usize> Residue { }; /// Instantiates a new `Residue` that represents this `integer` mod `MOD`. + /// If the modulus represented by `MOD` is not odd, this function will panic; use [`new_checked`][`Residue::new_checked`] if you want to be able to detect an invalid modulus. + #[deprecated( + since = "0.5.3", + note = "This will return an `Option` in a future version to account for an invalid modulus, but for now will panic if this happens. Consider using `new_checked` until then." + )] pub const fn new(integer: &Uint) -> Self { + // A valid modulus must be odd + if MOD::MODULUS.ct_is_odd().to_u8() == 0 { + panic!("modulus must be odd"); + } + let product = integer.mul_wide(&MOD::R2); let montgomery_form = montgomery_reduction::(&product, &MOD::MODULUS, MOD::MOD_NEG_INV); @@ -99,6 +109,23 @@ impl, const LIMBS: usize> Residue { } } + /// Instantiates a new `Residue` that represents this `integer` mod `MOD` if the modulus is odd. + /// Returns a `CtOption` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`Residue::new`], which can panic. + pub fn new_checked(integer: &Uint) -> CtOption { + let product = integer.mul_wide(&MOD::R2); + let montgomery_form = + montgomery_reduction::(&product, &MOD::MODULUS, MOD::MOD_NEG_INV); + + // A valid modulus must be odd, which we can check in constant time + CtOption::new( + Self { + montgomery_form, + phantom: PhantomData, + }, + MOD::MODULUS.ct_is_odd().into(), + ) + } + /// Retrieves the integer currently encoded in this `Residue`, guaranteed to be reduced. pub const fn retrieve(&self) -> Uint { montgomery_reduction::( @@ -176,6 +203,7 @@ impl, const LIMBS: usize> Zero for Residue } #[cfg(feature = "rand_core")] +#[allow(deprecated)] impl Random for Residue where MOD: ResidueParams, diff --git a/src/uint/modular/constant_mod/macros.rs b/src/uint/modular/constant_mod/macros.rs index c637d2ce0..abe213243 100644 --- a/src/uint/modular/constant_mod/macros.rs +++ b/src/uint/modular/constant_mod/macros.rs @@ -52,8 +52,12 @@ macro_rules! impl_modulus { #[macro_export] /// Creates a `Residue` with the given value for a specific modulus. /// For example, `residue!(U256::from(105u64), MyModulus);` creates a `Residue` for 105 mod `MyModulus`. +/// The modulus _must_ be odd, or this will panic. macro_rules! const_residue { ($variable:ident, $modulus:ident) => { - $crate::modular::constant_mod::Residue::<$modulus, { $modulus::LIMBS }>::new(&$variable) + $crate::modular::constant_mod::Residue::<$modulus, { $modulus::LIMBS }>::new_checked( + &$variable, + ) + .unwrap() }; } From c0b42fa54355521a610de508e204437a922428f2 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Thu, 25 May 2023 22:37:11 -0500 Subject: [PATCH 3/4] Update deprecation strategy and refactor --- src/uint/modular.rs | 4 +-- src/uint/modular/constant_mod.rs | 35 ++++++++++--------------- src/uint/modular/constant_mod/macros.rs | 7 ++--- 3 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/uint/modular.rs b/src/uint/modular.rs index 1448feb59..cd560aa38 100644 --- a/src/uint/modular.rs +++ b/src/uint/modular.rs @@ -146,7 +146,7 @@ mod tests { fn test_new_retrieve() { let x = U256::from_be_hex("44acf6b7e36c1342c2c5897204fe09504e1e2efb1a900377dbc4e7a6a133ec56"); - let x_mod = Residue::::new_checked(&x).unwrap(); + let x_mod = Residue::::new(&x); // Confirm that when creating a Modular and retrieving the value, that it equals the original assert_eq!(x, x_mod.retrieve()); @@ -157,7 +157,7 @@ mod tests { let x = U256::from_be_hex("44acf6b7e36c1342c2c5897204fe09504e1e2efb1a900377dbc4e7a6a133ec56"); assert_eq!( - Residue::::new_checked(&x).unwrap(), + Residue::::new(&x), const_residue!(x, Modulus2) ); } diff --git a/src/uint/modular/constant_mod.rs b/src/uint/modular/constant_mod.rs index b246cb98d..8a73567f4 100644 --- a/src/uint/modular/constant_mod.rs +++ b/src/uint/modular/constant_mod.rs @@ -87,18 +87,8 @@ impl, const LIMBS: usize> Residue { phantom: PhantomData, }; - /// Instantiates a new `Residue` that represents this `integer` mod `MOD`. - /// If the modulus represented by `MOD` is not odd, this function will panic; use [`new_checked`][`Residue::new_checked`] if you want to be able to detect an invalid modulus. - #[deprecated( - since = "0.5.3", - note = "This will return an `Option` in a future version to account for an invalid modulus, but for now will panic if this happens. Consider using `new_checked` until then." - )] - pub const fn new(integer: &Uint) -> Self { - // A valid modulus must be odd - if MOD::MODULUS.ct_is_odd().to_u8() == 0 { - panic!("modulus must be odd"); - } - + // Internal helper function to generate a residue; this lets us wrap the constructors more cleanly + const fn generate_residue(integer: &Uint) -> Self { let product = integer.mul_wide(&MOD::R2); let montgomery_form = montgomery_reduction::(&product, &MOD::MODULUS, MOD::MOD_NEG_INV); @@ -109,19 +99,23 @@ impl, const LIMBS: usize> Residue { } } + /// Instantiates a new `Residue` that represents this `integer` mod `MOD`. + /// If the modulus represented by `MOD` is not odd, this function will panic; use [`new_checked`][`Residue::new_checked`] if you want to be able to detect an invalid modulus. + pub const fn new(integer: &Uint) -> Self { + // A valid modulus must be odd + if MOD::MODULUS.ct_is_odd().to_u8() == 0 { + panic!("modulus must be odd"); + } + + Self::generate_residue(integer) + } + /// Instantiates a new `Residue` that represents this `integer` mod `MOD` if the modulus is odd. /// Returns a `CtOption` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`Residue::new`], which can panic. pub fn new_checked(integer: &Uint) -> CtOption { - let product = integer.mul_wide(&MOD::R2); - let montgomery_form = - montgomery_reduction::(&product, &MOD::MODULUS, MOD::MOD_NEG_INV); - // A valid modulus must be odd, which we can check in constant time CtOption::new( - Self { - montgomery_form, - phantom: PhantomData, - }, + Self::generate_residue(integer), MOD::MODULUS.ct_is_odd().into(), ) } @@ -203,7 +197,6 @@ impl, const LIMBS: usize> Zero for Residue } #[cfg(feature = "rand_core")] -#[allow(deprecated)] impl Random for Residue where MOD: ResidueParams, diff --git a/src/uint/modular/constant_mod/macros.rs b/src/uint/modular/constant_mod/macros.rs index abe213243..70dc4379d 100644 --- a/src/uint/modular/constant_mod/macros.rs +++ b/src/uint/modular/constant_mod/macros.rs @@ -19,7 +19,7 @@ macro_rules! impl_modulus { let res = <$uint_type>::from_be_hex($value); // Check that the modulus is odd - if (res.as_limbs()[0].0 & 1) == 0 { + if res.as_limbs()[0].0 & 1 == 0 { panic!("modulus must be odd"); } @@ -55,9 +55,6 @@ macro_rules! impl_modulus { /// The modulus _must_ be odd, or this will panic. macro_rules! const_residue { ($variable:ident, $modulus:ident) => { - $crate::modular::constant_mod::Residue::<$modulus, { $modulus::LIMBS }>::new_checked( - &$variable, - ) - .unwrap() + $crate::modular::constant_mod::Residue::<$modulus, { $modulus::LIMBS }>::new(&$variable) }; } From 6dee970678770af290b014f79ad822ed2044ef44 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Fri, 26 May 2023 10:14:47 -0500 Subject: [PATCH 4/4] Add a TODO for eventually getting rid of `new_checked` Co-authored-by: Tony Arcieri --- src/uint/modular/constant_mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uint/modular/constant_mod.rs b/src/uint/modular/constant_mod.rs index 8a73567f4..b775af45b 100644 --- a/src/uint/modular/constant_mod.rs +++ b/src/uint/modular/constant_mod.rs @@ -112,6 +112,8 @@ impl, const LIMBS: usize> Residue { /// Instantiates a new `Residue` that represents this `integer` mod `MOD` if the modulus is odd. /// Returns a `CtOption` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`Residue::new`], which can panic. + // TODO: remove this method when we can use `generic_const_exprs.` to ensure the modulus is + // always valid. pub fn new_checked(integer: &Uint) -> CtOption { // A valid modulus must be odd, which we can check in constant time CtOption::new(