From 4d66e26c5d7700ffffa310e6c5a649745d641753 Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Tue, 23 Jun 2026 14:56:35 -0600 Subject: [PATCH] don't use optimized binary GCD for Jacobi symbol calculation Signed-off-by: Andrew Whitehead --- src/modular/bingcd/gcd.rs | 30 +++++++++--------------------- src/modular/bingcd/xgcd.rs | 29 ++++++++++++----------------- src/uint/mod_symbol.rs | 35 ++++++++++++++++++----------------- 3 files changed, 39 insertions(+), 55 deletions(-) diff --git a/src/modular/bingcd/gcd.rs b/src/modular/bingcd/gcd.rs index 31ed2110a..6ca28d47f 100644 --- a/src/modular/bingcd/gcd.rs +++ b/src/modular/bingcd/gcd.rs @@ -124,7 +124,6 @@ impl Odd> { #[inline(always)] pub(crate) const fn optimized_bingcd(&self, rhs: &Uint) -> Self { self.optimized_bingcd_::<{ U64::BITS }, { U64::LIMBS }, { U128::LIMBS }>(rhs, U64::BITS - 1) - .0 } /// Computes `gcd(self, rhs)`, leveraging the optimized Binary GCD algorithm. @@ -154,9 +153,8 @@ impl Odd> { &self, rhs: &Uint, batch_max: u32, - ) -> (Self, Word) { + ) -> Self { let (mut a, mut b) = (*self.as_ref(), *rhs); - let mut jacobi_neg = 0; let mut iterations = Self::MINIMAL_BINGCD_ITERATIONS; while iterations > 0 { @@ -169,11 +167,10 @@ impl Odd> { let b_ = b.compact::(n); // Compute the batch update matrix from a_ and b_. - let (.., matrix, j_neg) = a_ + let (.., matrix) = a_ .to_odd() .expect_copied("a_ is always odd") .partial_binxgcd::(&b_, batch, Choice::FALSE); - jacobi_neg ^= j_neg; // Update `a` and `b` using the update matrix. // Safe to use vartime: the number of doublings is the same as the batch size. @@ -182,11 +179,8 @@ impl Odd> { (b, _) = updated_b.dropped_abs_sign(); } - ( - a.to_odd() - .expect_copied("gcd of an odd value is always odd"), - jacobi_neg, - ) + a.to_odd() + .expect_copied("gcd of an odd value is always odd") } /// Variable time equivalent of [`Self::optimized_bingcd`]. @@ -196,7 +190,6 @@ impl Odd> { rhs, U64::BITS - 1, ) - .0 } /// Variable time equivalent of [`Self::optimized_bingcd_`]. @@ -209,9 +202,8 @@ impl Odd> { &self, rhs: &Uint, batch_max: u32, - ) -> (Self, Word) { + ) -> Self { let (mut a, mut b) = (*self.as_ref(), *rhs); - let mut jacobi_neg = 0; while !b.is_zero_vartime() { // Construct a_ and b_ as the summary of a and b, respectively. @@ -221,7 +213,7 @@ impl Odd> { if n <= Uint::::BITS { loop { - jacobi_neg ^= bingcd_step(&mut b_, &mut a_).2; + bingcd_step(&mut b_, &mut a_); if b_.is_zero_vartime() { break; } @@ -231,11 +223,10 @@ impl Odd> { } // Compute the batch update matrix from a_ and b_. - let (.., matrix, j_neg) = a_ + let (.., matrix) = a_ .to_odd() .expect_copied("a_ is always odd") .partial_binxgcd::(&b_, batch_max, Choice::FALSE); - jacobi_neg ^= j_neg; // Update `a` and `b` using the update matrix. let (updated_a, updated_b) = matrix.extended_apply_to_vartime((a, b)); @@ -243,11 +234,8 @@ impl Odd> { (b, _) = updated_b.dropped_abs_sign(); } - ( - a.to_odd() - .expect_copied("gcd of an odd value with something else is always odd"), - jacobi_neg, - ) + a.to_odd() + .expect_copied("gcd of an odd value with something else is always odd") } } diff --git a/src/modular/bingcd/xgcd.rs b/src/modular/bingcd/xgcd.rs index 080ada77f..ef3b17661 100644 --- a/src/modular/bingcd/xgcd.rs +++ b/src/modular/bingcd/xgcd.rs @@ -2,7 +2,7 @@ use super::gcd::bingcd_step; use crate::modular::bingcd::matrix::{DividedIntMatrix, DividedPatternMatrix, PatternMatrix, Unit}; use crate::primitives::u32_max; -use crate::{Choice, Int, Limb, NonZeroUint, Odd, OddUint, U64, U128, Uint, Word}; +use crate::{Choice, Int, Limb, NonZeroUint, Odd, OddUint, U64, U128, Uint}; /// Binary XGCD update step. /// @@ -34,8 +34,8 @@ const fn binxgcd_step( b: &mut Uint, matrix: &mut DividedPatternMatrix, halt_at_zero: Choice, -) -> Word { - let (a_odd, swap, j) = bingcd_step(a, b); +) { + let (a_odd, swap, _) = bingcd_step(a, b); // swap if a odd and a < b matrix.conditional_swap_rows(swap); @@ -45,8 +45,6 @@ const fn binxgcd_step( // Double the bottom row of the matrix when a was ≠ 0 and when not halting. matrix.conditional_double_bottom_row(a.is_nonzero().or(halt_at_zero.not())); - - j } /// Container for the raw output of the Binary XGCD algorithm. @@ -209,7 +207,7 @@ impl OddUint { /// Ref: Pornin, Optimized Binary GCD for Modular Inversion, Algorithm 1. /// . pub(crate) const fn classic_binxgcd(&self, rhs: &Self) -> DividedPatternXgcdOutput { - let (gcd, _, matrix, _) = + let (gcd, _, matrix) = self.partial_binxgcd::(rhs.as_ref(), Self::MIN_BINXGCD_ITERATIONS, Choice::TRUE); DividedPatternXgcdOutput { gcd, matrix } } @@ -278,7 +276,7 @@ impl OddUint { Choice::from_u32_le(b_bits, K - 1).or(Choice::from_u32_eq(n, 2 * K)); // Compute the K-1 iteration update matrix from a_ and b_ - let (.., update_matrix, _) = compact_a + let (.., update_matrix) = compact_a .to_odd() .expect_copied("a is always odd") .partial_binxgcd::(&compact_b, K - 1, b_eq_compact_b); @@ -322,7 +320,7 @@ impl OddUint { rhs: &Uint, iterations: u32, halt_at_zero: Choice, - ) -> (Self, Uint, DividedPatternMatrix, Word) { + ) -> (Self, Uint, DividedPatternMatrix) { let (mut a, mut b) = (*self.as_ref(), *rhs); // This matrix corresponds with (f0, g0, f1, g1) in the paper. let mut matrix = DividedPatternMatrix::UNIT; @@ -334,12 +332,10 @@ impl OddUint { Uint::swap(&mut a, &mut b); matrix.swap_rows(); - let mut jacobi_neg = 0; let mut i = 0; while i < iterations { - jacobi_neg ^= - binxgcd_step::(&mut a, &mut b, &mut matrix, halt_at_zero); + binxgcd_step::(&mut a, &mut b, &mut matrix, halt_at_zero); i += 1; } @@ -348,7 +344,7 @@ impl OddUint { matrix.swap_rows(); let a = a.to_odd().expect_copied("a is always odd"); - (a, b, matrix, jacobi_neg) + (a, b, matrix) } } @@ -514,7 +510,7 @@ mod tests { #[test] fn test_partial_binxgcd() { - let (.., matrix, _) = A.partial_binxgcd::<{ U64::LIMBS }>(&B, 5, Choice::TRUE); + let (.., matrix) = A.partial_binxgcd::<{ U64::LIMBS }>(&B, 5, Choice::TRUE); assert_eq!(matrix.k, 5); assert_eq!( matrix, @@ -527,8 +523,7 @@ mod tests { let target_a = U64::from_be_hex("1CB3FB3FA1218FDB").to_odd().unwrap(); let target_b = U64::from_be_hex("0EA028AF0F8966B6"); - let (new_a, new_b, matrix, _) = - A.partial_binxgcd::<{ U64::LIMBS }>(&B, 5, Choice::TRUE); + let (new_a, new_b, matrix) = A.partial_binxgcd::<{ U64::LIMBS }>(&B, 5, Choice::TRUE); assert_eq!(new_a, target_a); assert_eq!(new_b, target_b); @@ -547,7 +542,7 @@ mod tests { #[test] fn test_partial_binxgcd_halts() { - let (gcd, _, matrix, _) = + let (gcd, _, matrix) = SMALL_A.partial_binxgcd::<{ U64::LIMBS }>(&SMALL_B, 60, Choice::TRUE); assert_eq!(matrix.k, 35); assert_eq!(matrix.k_upper_bound, 60); @@ -556,7 +551,7 @@ mod tests { #[test] fn test_partial_binxgcd_does_not_halt() { - let (gcd, .., matrix, _) = + let (gcd, .., matrix) = SMALL_A.partial_binxgcd::<{ U64::LIMBS }>(&SMALL_B, 60, Choice::FALSE); assert_eq!(matrix.k, 60); assert_eq!(matrix.k_upper_bound, 60); diff --git a/src/uint/mod_symbol.rs b/src/uint/mod_symbol.rs index 489c55ef1..02fc75d1b 100644 --- a/src/uint/mod_symbol.rs +++ b/src/uint/mod_symbol.rs @@ -1,6 +1,6 @@ //! Support for computing modular symbols. -use crate::{JacobiSymbol, Odd, U64, U128, Uint}; +use crate::{JacobiSymbol, Odd, Uint}; impl Uint { /// Compute the Jacobi symbol `(self|rhs)`. @@ -19,14 +19,8 @@ impl Uint { return a.jacobi_symbol(rhs); } - let (gcd, jacobi_neg) = if LIMBS < 4 { - rhs.classic_bingcd_(&self.resize()) - } else { - rhs.optimized_bingcd_::<{ U64::BITS }, { U64::LIMBS }, { U128::LIMBS }>( - &self.resize(), - U64::BITS - 2, - ) - }; + let (gcd, jacobi_neg) = rhs.classic_bingcd_(&self.resize()); + // The sign of the Jacobi symbol is represented by jacobi_neg. We select 0 as the // symbol when the GCD is not one, otherwise 1 or -1. let jacobi = (jacobi_neg as i8 * -2 + 1) as i64; @@ -87,14 +81,7 @@ impl Uint { } } - let (gcd, jacobi_neg) = if LIMBS < 4 { - rhs.classic_bingcd_vartime_(&self.resize()) - } else { - rhs.optimized_bingcd_vartime_::<{ U64::BITS }, { U64::LIMBS }, { U128::LIMBS }>( - &self.resize(), - U64::BITS - 2, - ) - }; + let (gcd, jacobi_neg) = rhs.classic_bingcd_vartime_(&self.resize()); JacobiSymbol::from_i8(if gcd.as_ref().cmp_vartime(&Uint::ONE).is_eq() { jacobi_neg as i8 * -2 + 1 } else { @@ -200,4 +187,18 @@ mod tests { assert_eq!(res, res_vartime_small); assert_eq!(res, res_vartime_large); } + + #[test] + // test from issue #1295 - variations in only the middle bits can trip up optimized binary GCD method + fn jacobi_edge() { + use crate::{Odd, U256}; + + assert_eq!( + U256::from_be_hex("0000000000000002108DEAFCB180F023912BEED0186CEEAD593A8507B7DA4E9B") + .jacobi_symbol(&Odd::::from_be_hex( + "0000000000000002108DEAFCB180F023912BEED0186CEEED593A8507B7DA4E9B", + )), + JacobiSymbol::MinusOne + ); + } }