Skip to content

Commit 1ceecf0

Browse files
authored
safegcd: compute the iteration count in u64 (#1346)
1 parent 1ab28f6 commit 1ceecf0

4 files changed

Lines changed: 124 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
88
### Added
99
- Implement `Zeroize` for `EncodedUint` ([#1342])
1010

11+
### Fixed
12+
- `u32` overflow of the safegcd iteration count at precisions of 93558 bits and above, which made
13+
`invert_mod`/`invert_odd_mod` spuriously report no inverse and `gcd` return wrong results ([#1346])
14+
1115
[#1342]: https://github.com/RustCrypto/crypto-bigint/pull/1342
16+
[#1346]: https://github.com/RustCrypto/crypto-bigint/pull/1346
1217

1318
## 0.7.5 (2026-06-22)
1419
### Added

‎src/modular/safegcd.rs‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,12 +343,23 @@ const fn shr_in_place_wide<const L: usize, const H: usize>(
343343

344344
/// Calculate the maximum number of iterations required according to
345345
/// safegcd-bounds: <https://github.com/sipa/safegcd-bounds>
346+
///
347+
/// Computed in `u64` because `45907 * bits + 30179` exceeds `u32::MAX` once `bits >= 93558`.
348+
/// The count itself (about `2.3 * bits`) only exceeds `u32::MAX` for `bits` above `1864517463`,
349+
/// which panics rather than wrapping.
346350
// NOTE: the division is non-constant-time, but this is used to compute the number of iterations we
347351
// perform which is leaked in timing information
348352
#[inline]
349353
#[allow(clippy::integer_division_remainder_used, reason = "public parameter")]
354+
#[allow(clippy::cast_lossless, reason = "`const fn`")]
355+
#[allow(clippy::cast_possible_truncation, reason = "checked by the assertion")]
350356
const fn iterations(bits: u32) -> u32 {
351-
(45907 * bits + 30179) / 19929
357+
let iterations = (45907 * bits as u64 + 30179) / 19929;
358+
assert!(
359+
iterations <= u32::MAX as u64,
360+
"precision too large for safegcd"
361+
);
362+
iterations as u32
352363
}
353364

354365
/// A `Uint` which carries a separate sign in order to maintain the same range.
@@ -526,7 +537,7 @@ impl<const LIMBS: usize> PartialEq for SignedInt<LIMBS> {
526537

527538
#[cfg(test)]
528539
mod tests {
529-
use super::SafeGcdInverter;
540+
use super::{SafeGcdInverter, iterations};
530541
use crate::{U128, U256, modular::safegcd::shr_in_place_wide};
531542

532543
#[test]
@@ -564,4 +575,23 @@ mod tests {
564575
);
565576
assert_eq!(b_hi, U128::from_u128(0x111111112222222));
566577
}
578+
579+
#[test]
580+
fn iterations_do_not_overflow() {
581+
// Values from the `safegcd-bounds` formula evaluated in arbitrary precision.
582+
assert_eq!(iterations(256), 591);
583+
assert_eq!(iterations(93_504), 215_390);
584+
// `45907 * bits + 30179` first exceeds `u32::MAX` here.
585+
assert_eq!(iterations(93_558), 215_514);
586+
assert_eq!(iterations(93_568), 215_537);
587+
assert_eq!(iterations(98_304), 226_447);
588+
// The largest precision whose count still fits in `u32`.
589+
assert_eq!(iterations(1_864_517_463), 4_294_967_294);
590+
}
591+
592+
#[test]
593+
#[should_panic(expected = "precision too large for safegcd")]
594+
fn iterations_panics_beyond_u32() {
595+
iterations(1_864_517_464);
596+
}
567597
}

‎src/modular/safegcd/boxed.rs‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,62 @@ impl fmt::Debug for SignedBoxedInt {
437437

438438
#[cfg(test)]
439439
mod tests {
440-
use super::BoxedSafeGcdInverter;
441-
use crate::BoxedUint;
440+
use super::{BoxedSafeGcdInverter, gcd_odd, invert_odd_mod};
441+
use crate::{BoxedUint, Odd, Resize};
442+
use num_bigint::BigUint;
443+
use num_integer::Integer;
444+
445+
/// The smallest precision at which `45907 * bits + 30179` no longer fits in `u32`.
446+
const LARGE_BITS: u32 = 93_568;
447+
448+
/// A deterministic, odd, full-width `BoxedUint` of `bits` bits; different seeds give
449+
/// unrelated values.
450+
fn large_odd(bits: u32, seed: u32) -> BoxedUint {
451+
let mut bytes: alloc::vec::Vec<u8> = (0..bits >> 3)
452+
.map(|i| {
453+
let h = (i ^ seed.wrapping_mul(0x9E37_79B9)).wrapping_mul(0x85EB_CA6B);
454+
let h = (h ^ (h >> 13)).wrapping_mul(0xC2B2_AE35);
455+
(h ^ (h >> 16)).to_le_bytes()[0]
456+
})
457+
.collect();
458+
bytes[0] |= 0x80;
459+
*bytes.last_mut().unwrap() |= 1;
460+
BoxedUint::from_be_slice(&bytes, bits).unwrap()
461+
}
462+
463+
fn to_big(x: &BoxedUint) -> BigUint {
464+
BigUint::from_bytes_be(&x.to_be_bytes())
465+
}
466+
467+
#[test]
468+
fn invert_odd_mod_large_precision() {
469+
let m = large_odd(LARGE_BITS, 1);
470+
let a = BoxedUint::from(65537u32).resize(LARGE_BITS);
471+
assert_eq!(to_big(&a).gcd(&to_big(&m)), BigUint::from(1u32));
472+
let m = Odd::new(m).unwrap();
473+
474+
for inv in [
475+
invert_odd_mod::<false>(&a, &m).unwrap(),
476+
invert_odd_mod::<true>(&a, &m).unwrap(),
477+
] {
478+
assert_eq!(
479+
(to_big(&a) * to_big(&inv)) % to_big(m.as_ref()),
480+
BigUint::from(1u32)
481+
);
482+
}
483+
}
484+
485+
#[test]
486+
fn gcd_odd_large_precision() {
487+
// Two unrelated full-width values: the reduction needs the whole iteration budget,
488+
// unlike a pair with a large common factor and small cofactors.
489+
let f = Odd::new(large_odd(LARGE_BITS, 2)).unwrap();
490+
let g = large_odd(LARGE_BITS, 4);
491+
let expected = to_big(f.as_ref()).gcd(&to_big(&g));
492+
493+
assert_eq!(to_big(gcd_odd::<false>(&f, &g).as_ref()), expected);
494+
assert_eq!(to_big(gcd_odd::<true>(&f, &g).as_ref()), expected);
495+
}
442496

443497
#[test]
444498
fn invert() {

‎src/uint/boxed/invert_mod.rs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,40 @@ impl InvertMod for BoxedUint {
184184

185185
#[cfg(test)]
186186
mod tests {
187-
use crate::{Limb, Odd, Resize, U256};
187+
use crate::{Limb, NonZero, Odd, Resize, U256};
188188

189189
use super::BoxedUint;
190190
use hex_literal::hex;
191+
use num_bigint::BigUint;
192+
use num_integer::Integer;
193+
194+
/// `invert_mod` at a precision where the safegcd iteration count overflowed `u32`
195+
/// (`45907 * bits + 30179 > u32::MAX` from 93558 bits), with an even modulus so that
196+
/// both the odd-modulus and the `2^k` paths are exercised.
197+
#[test]
198+
fn invert_mod_large_precision_even_modulus() {
199+
const BITS: u32 = 93_568;
200+
let mut bytes: alloc::vec::Vec<u8> = (0..BITS >> 3)
201+
.map(|i| {
202+
let h = (i ^ 0xDA94_2042).wrapping_mul(0x85EB_CA6B);
203+
let h = (h ^ (h >> 13)).wrapping_mul(0xC2B2_AE35);
204+
(h ^ (h >> 16)).to_le_bytes()[0]
205+
})
206+
.collect();
207+
bytes[0] |= 0x80;
208+
*bytes.last_mut().unwrap() &= !1;
209+
let m = BoxedUint::from_be_slice(&bytes, BITS).unwrap();
210+
let a = BoxedUint::from(65537u32).resize(BITS);
211+
212+
let to_big = |x: &BoxedUint| BigUint::from_bytes_be(&x.to_be_bytes());
213+
assert_eq!(to_big(&a).gcd(&to_big(&m)), BigUint::from(1u32));
214+
215+
let inv = a.invert_mod(&NonZero::new(m.clone()).unwrap()).unwrap();
216+
assert_eq!(
217+
(to_big(&a) * to_big(&inv)) % to_big(&m),
218+
BigUint::from(1u32)
219+
);
220+
}
191221

192222
#[test]
193223
fn invert_mod2k() {

0 commit comments

Comments
 (0)