diff --git a/CHANGELOG.md b/CHANGELOG.md index 817e41fb7..c3e9c2196 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Implement `Zeroize` for `EncodedUint` ([#1342]) +### Fixed +- `u32` overflow of the safegcd iteration count at precisions of 93558 bits and above, which made + `invert_mod`/`invert_odd_mod` spuriously report no inverse and `gcd` return wrong results ([#1346]) + [#1342]: https://github.com/RustCrypto/crypto-bigint/pull/1342 +[#1346]: https://github.com/RustCrypto/crypto-bigint/pull/1346 ## 0.7.5 (2026-06-22) ### Added diff --git a/src/modular/safegcd.rs b/src/modular/safegcd.rs index f7b23e175..b279a40f1 100644 --- a/src/modular/safegcd.rs +++ b/src/modular/safegcd.rs @@ -343,12 +343,23 @@ const fn shr_in_place_wide( /// Calculate the maximum number of iterations required according to /// safegcd-bounds: +/// +/// Computed in `u64` because `45907 * bits + 30179` exceeds `u32::MAX` once `bits >= 93558`. +/// The count itself (about `2.3 * bits`) only exceeds `u32::MAX` for `bits` above `1864517463`, +/// which panics rather than wrapping. // NOTE: the division is non-constant-time, but this is used to compute the number of iterations we // perform which is leaked in timing information #[inline] #[allow(clippy::integer_division_remainder_used, reason = "public parameter")] +#[allow(clippy::cast_lossless, reason = "`const fn`")] +#[allow(clippy::cast_possible_truncation, reason = "checked by the assertion")] const fn iterations(bits: u32) -> u32 { - (45907 * bits + 30179) / 19929 + let iterations = (45907 * bits as u64 + 30179) / 19929; + assert!( + iterations <= u32::MAX as u64, + "precision too large for safegcd" + ); + iterations as u32 } /// A `Uint` which carries a separate sign in order to maintain the same range. @@ -530,7 +541,7 @@ impl PartialEq for SignedInt { #[cfg(test)] mod tests { - use super::SafeGcdInverter; + use super::{SafeGcdInverter, iterations}; use crate::{U128, U256, modular::safegcd::shr_in_place_wide}; #[test] @@ -568,4 +579,23 @@ mod tests { ); assert_eq!(b_hi, U128::from_u128(0x111111112222222)); } + + #[test] + fn iterations_do_not_overflow() { + // Values from the `safegcd-bounds` formula evaluated in arbitrary precision. + assert_eq!(iterations(256), 591); + assert_eq!(iterations(93_504), 215_390); + // `45907 * bits + 30179` first exceeds `u32::MAX` here. + assert_eq!(iterations(93_558), 215_514); + assert_eq!(iterations(93_568), 215_537); + assert_eq!(iterations(98_304), 226_447); + // The largest precision whose count still fits in `u32`. + assert_eq!(iterations(1_864_517_463), 4_294_967_294); + } + + #[test] + #[should_panic(expected = "precision too large for safegcd")] + fn iterations_panics_beyond_u32() { + iterations(1_864_517_464); + } } diff --git a/src/modular/safegcd/boxed.rs b/src/modular/safegcd/boxed.rs index 09f4695a5..35d730abe 100644 --- a/src/modular/safegcd/boxed.rs +++ b/src/modular/safegcd/boxed.rs @@ -441,8 +441,62 @@ impl fmt::Debug for SignedBoxedInt { #[cfg(test)] mod tests { - use super::BoxedSafeGcdInverter; - use crate::BoxedUint; + use super::{BoxedSafeGcdInverter, gcd_odd, invert_odd_mod}; + use crate::{BoxedUint, Odd, Resize}; + use num_bigint::BigUint; + use num_integer::Integer; + + /// The smallest precision at which `45907 * bits + 30179` no longer fits in `u32`. + const LARGE_BITS: u32 = 93_568; + + /// A deterministic, odd, full-width `BoxedUint` of `bits` bits; different seeds give + /// unrelated values. + fn large_odd(bits: u32, seed: u32) -> BoxedUint { + let mut bytes: alloc::vec::Vec = (0..bits >> 3) + .map(|i| { + let h = (i ^ seed.wrapping_mul(0x9E37_79B9)).wrapping_mul(0x85EB_CA6B); + let h = (h ^ (h >> 13)).wrapping_mul(0xC2B2_AE35); + (h ^ (h >> 16)).to_le_bytes()[0] + }) + .collect(); + bytes[0] |= 0x80; + *bytes.last_mut().unwrap() |= 1; + BoxedUint::from_be_slice(&bytes, bits).unwrap() + } + + fn to_big(x: &BoxedUint) -> BigUint { + BigUint::from_bytes_be(&x.to_be_bytes()) + } + + #[test] + fn invert_odd_mod_large_precision() { + let m = large_odd(LARGE_BITS, 1); + let a = BoxedUint::from(65537u32).resize(LARGE_BITS); + assert_eq!(to_big(&a).gcd(&to_big(&m)), BigUint::from(1u32)); + let m = Odd::new(m).unwrap(); + + for inv in [ + invert_odd_mod::(&a, &m).unwrap(), + invert_odd_mod::(&a, &m).unwrap(), + ] { + assert_eq!( + (to_big(&a) * to_big(&inv)) % to_big(m.as_ref()), + BigUint::from(1u32) + ); + } + } + + #[test] + fn gcd_odd_large_precision() { + // Two unrelated full-width values: the reduction needs the whole iteration budget, + // unlike a pair with a large common factor and small cofactors. + let f = Odd::new(large_odd(LARGE_BITS, 2)).unwrap(); + let g = large_odd(LARGE_BITS, 4); + let expected = to_big(f.as_ref()).gcd(&to_big(&g)); + + assert_eq!(to_big(gcd_odd::(&f, &g).as_ref()), expected); + assert_eq!(to_big(gcd_odd::(&f, &g).as_ref()), expected); + } #[test] fn invert() { diff --git a/src/uint/boxed/invert_mod.rs b/src/uint/boxed/invert_mod.rs index db56af268..537c5eafd 100644 --- a/src/uint/boxed/invert_mod.rs +++ b/src/uint/boxed/invert_mod.rs @@ -184,10 +184,40 @@ impl InvertMod for BoxedUint { #[cfg(test)] mod tests { - use crate::{Limb, Odd, Resize, U256}; + use crate::{Limb, NonZero, Odd, Resize, U256}; use super::BoxedUint; use hex_literal::hex; + use num_bigint::BigUint; + use num_integer::Integer; + + /// `invert_mod` at a precision where the safegcd iteration count overflowed `u32` + /// (`45907 * bits + 30179 > u32::MAX` from 93558 bits), with an even modulus so that + /// both the odd-modulus and the `2^k` paths are exercised. + #[test] + fn invert_mod_large_precision_even_modulus() { + const BITS: u32 = 93_568; + let mut bytes: alloc::vec::Vec = (0..BITS >> 3) + .map(|i| { + let h = (i ^ 0xDA94_2042).wrapping_mul(0x85EB_CA6B); + let h = (h ^ (h >> 13)).wrapping_mul(0xC2B2_AE35); + (h ^ (h >> 16)).to_le_bytes()[0] + }) + .collect(); + bytes[0] |= 0x80; + *bytes.last_mut().unwrap() &= !1; + let m = BoxedUint::from_be_slice(&bytes, BITS).unwrap(); + let a = BoxedUint::from(65537u32).resize(BITS); + + let to_big = |x: &BoxedUint| BigUint::from_bytes_be(&x.to_be_bytes()); + assert_eq!(to_big(&a).gcd(&to_big(&m)), BigUint::from(1u32)); + + let inv = a.invert_mod(&NonZero::new(m.clone()).unwrap()).unwrap(); + assert_eq!( + (to_big(&a) * to_big(&inv)) % to_big(&m), + BigUint::from(1u32) + ); + } #[test] fn invert_mod2k() {