Fix divergent behavior in integer backends

CHANGELOG.md

@@ -1,3 +1,33 @@
+## v0.4.0
+* Fix divergent behavior on different integer backends. [#49] \
+  Each of the following change is **possibly breaking** even if it doesn't change the API:
+  * `Integer::from_str_radix(s, radix)` requires that `s` is composed of optional leading single
+    sign (`+` or `-`) and followed by ASCII alphanum characters
+    * `num-bigint` backend used to allow underscore characters `_` in integer encoding, which now
+      lead to a parsing error
+    * `rug` backend used to allow whitspace characters (` `, `\t`, `\n`) in integer encoding,
+      which is no longer accepted as well.
+  * `num-bigint` backend used to panic when called `invert`, `pow_mod`, or `combine` with
+    `modulo=0`. Now it returns `None` in these cases (which is aligned with `rug` backend)
+  * `Integer::random_below`, `random_below_ref`, and `assign_random_below` return an error if upper
+    bound is `<= 0` (there are no number to sample the integer from). `rug` used to panicked in
+    those cases, while `num-bigint` ignored this condition, and simply treated the input as if it
+    was positive.
+  * `jacobi` returns an error when invalid `n` is provided. Both backend behaved differently: `rug`
+    used to return some integer, while `num-bigint` used to return `0` in release and panic in debug
+    build.
+  * `num_bigint` integer used to panic in `x << shift` (or `>>`) when the `shift` was negative. Now
+    it reverses the shift direction, like `rug` does: e.g. `x << -3` becomes `x >> 3`
+  * Safe prime generation returns an error if asked to generate a prime of bit length `< 100`, as
+    sampling small primes is not supported by the underlying algorithm. Note that cryptographic
+    primes need to be way larger. E.g. RSA requires primes to be at least 1536-bits.
+  * Prime generation returns an error if asked to generate a prime of bit length `< 2` (there are no
+    primes of that bitsize). Previously, function would panic or hang in infinite loop.
+  * `sample[_pm]_in_mult_group_of` returns an error if provided `n < 2`. Previously, `n=0` would
+    panic or hang (depending on the backend), and `n=1` would hang in infinite loop.
+
+[#49]: https://github.com/LFDT-Lockness/fast-paillier/pull/49
+
 ## v0.3.3
 * Unify encoding of integer=0 across backends [#45]
 * Deserialize returns an error on invalid radix, instead of panicking [#45]

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "fast-paillier"
-version = "0.3.3"
+version = "0.4.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "Optimized Paillier encryption scheme"

benches/comparison.rs

@@ -30,8 +30,8 @@ fn encryption(c: &mut criterion::Criterion) {
     let mut group = c.benchmark_group("Encrypt");
 
     let mut generate_inputs = || {
-        let x = ek.n().random_below_ref(&mut rng) - ek.half_n();
-        let nonce = Integer::sample_in_mult_group_of(&mut rng, ek.n());
+        let x = ek.n().random_below_ref(&mut rng).unwrap() - ek.half_n();
+        let nonce = Integer::sample_in_mult_group_of(&mut rng, ek.n()).unwrap();
         (x, nonce)
     };
 
@@ -83,7 +83,7 @@ fn decryption(c: &mut criterion::Criterion) {
 
     let mut group = c.benchmark_group("Decrypt");
 
-    let mut generate_inputs = || Integer::sample_in_mult_group_of(&mut rng, ek.nn());
+    let mut generate_inputs = || Integer::sample_in_mult_group_of(&mut rng, ek.nn()).unwrap();
 
     group.bench_function("Decrypt with CRT", |b| {
         b.iter_batched(
@@ -123,8 +123,8 @@ fn omul(c: &mut criterion::Criterion) {
     let mut group = c.benchmark_group("OMul");
 
     let mut generate_inputs = || {
-        let scalar = Integer::sample_pm_in_mult_group_of(&mut rng, ek.n());
-        let enc_x = Integer::sample_in_mult_group_of(&mut rng, ek.nn());
+        let scalar = Integer::sample_pm_in_mult_group_of(&mut rng, ek.n()).unwrap();
+        let enc_x = Integer::sample_in_mult_group_of(&mut rng, ek.nn()).unwrap();
         (scalar, enc_x)
     };
 
@@ -148,7 +148,7 @@ fn omul(c: &mut criterion::Criterion) {
 pub fn naive_safe_prime(rng: &mut impl rand_core::RngCore, bits: u32) -> Integer {
     use fast_paillier::backend::IsPrime;
     loop {
-        let mut x = Integer::generate_prime(rng, bits - 1);
+        let mut x = Integer::generate_prime(rng, bits - 1).unwrap();
         x <<= 1;
         x += 1;
 

src/backend.rs

@@ -73,34 +73,56 @@ impl Integer {
     }
 
     /// Samples `x` in Z*_n
-    pub fn sample_in_mult_group_of(rng: &mut impl rand_core::RngCore, n: &Self) -> Self {
+    ///
+    /// Returns an error if `n <= 1`.
+    pub fn sample_in_mult_group_of(
+        rng: &mut impl rand_core::RngCore,
+        n: &Self,
+    ) -> Result<Self, InvalidInput> {
+        if *n <= Integer::one() {
+            return Err(InvalidInputReason::SampleInMultGroupSize.into());
+        }
         let mut x = Integer::zero();
         loop {
-            x.assign_random_below(n, rng);
+            x.assign_random_below(n, rng)?;
             if x.in_mult_group_of(n) {
-                return x;
+                return Ok(x);
             }
         }
     }
 
     /// Samples `x` such that abs(x) is in `Z*_n`
-    pub fn sample_pm_in_mult_group_of(rng: &mut impl rand_core::RngCore, n: &Self) -> Self {
+    ///
+    /// Returns an error if `n <= 1`.
+    pub fn sample_pm_in_mult_group_of(
+        rng: &mut impl rand_core::RngCore,
+        n: &Self,
+    ) -> Result<Self, InvalidInput> {
+        if *n <= Integer::one() {
+            return Err(InvalidInputReason::SampleInMultGroupSize.into());
+        }
         let mut x = Integer::zero();
         let mut sign_buf = [0u8; 1];
         loop {
-            x.assign_random_below(n, rng);
+            x.assign_random_below(n, rng)?;
             rng.fill_bytes(&mut sign_buf);
             if sign_buf[0] & 1 == 1 {
                 x = -x;
             }
             if x.abs_in_mult_group_of(n) {
-                return x;
+                return Ok(x);
             }
         }
     }
 
     /// Generates a random safe prime
-    pub fn generate_safe_prime(rng: &mut impl rand_core::RngCore, bits: u32) -> Self {
+    ///
+    /// Returns an error if `bits < 100`. Note that cryptographic primes need to be way larger.
+    /// E.g. RSA requires primes to be at least 1536-bits.
+    pub fn generate_safe_prime(
+        rng: &mut impl rand_core::RngCore,
+        bits: u32,
+    ) -> Result<Self, InvalidInput> {
         sieve_generate_safe_primes(rng, bits, 135)
     }
 }
@@ -112,14 +134,24 @@ impl Integer {
 /// The best way to select the parameter is by trial. The one used by
 /// [`generate_safe_prime`] is indistinguishable from optimal for 500-1700 bit
 /// lengths.
+///
+/// Returns an error if `bits < 100`. Note that cryptographic primes need to be way larger.
+/// E.g. RSA requires primes to be at least 1536-bits.
 pub fn sieve_generate_safe_primes(
     rng: &mut impl rand_core::RngCore,
     bits: u32,
     amount: usize,
-) -> Integer {
+) -> Result<Integer, InvalidInput> {
     use crate::backend::IsPrime;
     use crate::utils::small_primes;
 
+    // sieve optimization, the way it is written, rejects valid safe primes of small bitsize.
+    // The fix would be to use subset of `small_primes` for small `bits`, but we never want
+    // to generate small safe primes, so no point in complicating the code.
+    if bits < 100 {
+        return Err(InvalidInputReason::SafePrimeBitSize.into());
+    }
+
     let amount = amount.min(small_primes::SMALL_PRIMES.len());
     let mut x = Integer::zero();
 
@@ -143,7 +175,7 @@ pub fn sieve_generate_safe_primes(
             x <<= 1;
             x += 1;
             if let IsPrime::Yes | IsPrime::Probably = x.is_probably_prime_without_sieve() {
-                return x;
+                return Ok(x);
             }
         }
     }
@@ -320,6 +352,29 @@ mod serialize {
     }
 }
 
+/// Error returned by methods on integer when provided inputs are not acceptable
+#[derive(Debug, thiserror::Error)]
+#[error("invalid input: {0}")]
+pub struct InvalidInput(#[from] InvalidInputReason);
+
+#[derive(Debug, thiserror::Error)]
+enum InvalidInputReason {
+    #[error("random_below: upper bound must be positive")]
+    RandomBelowUpperBound,
+    #[error("jacobi: `n` must be an odd number larger than 2")]
+    Jacobi,
+    #[error(
+        "generate_safe_prime algorithm can only generate primes from 100 bits; note that \
+        cryptographic primes need to be way larger. E.g. RSA requires primes to be at least \
+        1536-bits."
+    )]
+    SafePrimeBitSize,
+    #[error("generate_prime was called with `bit_size < 2`, there are no primes of this bit size")]
+    PrimeBitSize,
+    #[error("sample_in_[pm_]mult_group: `n` must be at least 2")]
+    SampleInMultGroupSize,
+}
+
 #[cfg(test)]
 mod test {
     use super::Integer;
@@ -328,10 +383,8 @@ mod test {
     fn safe_prime_size() {
         let mut rng = rand_dev::DevRng::new();
         for size in [500, 512, 513, 514] {
-            let mut prime = Integer::generate_safe_prime(&mut rng, size);
-            // rug doesn't have bit length operations, so
-            prime >>= size - 1;
-            assert_eq!(prime, Integer::one());
+            let prime = Integer::generate_safe_prime(&mut rng, size).unwrap();
+            assert_eq!(prime.significant_bits(), size as u64);
         }
     }
 

src/backend/macro_defs.rs

@@ -165,10 +165,8 @@ macro_rules! make_all_ops {
         $crate::backend::macro_defs::make_assign!(Integer, RemAssign, rem_assign);
 
         $crate::backend::macro_defs::make_ops_for_primitive!(Integer, Shl, shl, u32 $(, $complete)?);
-        $crate::backend::macro_defs::make_ops_for_primitive!(Integer, Shl, shl, i32 $(, $complete)?);
         $crate::backend::macro_defs::make_ops_for_primitive!(Integer, Shl, shl, usize $(, $complete)?);
         $crate::backend::macro_defs::make_ops_for_primitive!(Integer, Shr, shr, u32 $(, $complete)?);
-        $crate::backend::macro_defs::make_ops_for_primitive!(Integer, Shr, shr, i32 $(, $complete)?);
         $crate::backend::macro_defs::make_ops_for_primitive!(Integer, Shr, shr, usize $(, $complete)?);
 
         impl core::ops::Neg for $type {