From b8208637589ddf840345507add925f7480eed1fb Mon Sep 17 00:00:00 2001
From: Bogdan Opanchuk <bogdan@opanchuk.net>
Date: Sun, 1 Dec 2024 12:12:36 -0800
Subject: [PATCH] Less zeroize uses

---
 synedrion/src/cggmp21/params.rs   | 30 +++++++++++++-----------------
 synedrion/src/curve/arithmetic.rs | 16 +++-------------
 synedrion/src/paillier/rsa.rs     | 15 ++-------------
 synedrion/src/tools/secret.rs     |  6 +++++-
 4 files changed, 23 insertions(+), 44 deletions(-)

diff --git a/synedrion/src/cggmp21/params.rs b/synedrion/src/cggmp21/params.rs
index 4844e6d8..18066614 100644
--- a/synedrion/src/cggmp21/params.rs
+++ b/synedrion/src/cggmp21/params.rs
@@ -7,7 +7,6 @@ use core::fmt::Debug;
 use k256::elliptic_curve::bigint::Uint as K256Uint;
 use secrecy::{ExposeSecret, ExposeSecretMut};
 use serde::{Deserialize, Serialize};
-use zeroize::Zeroize;
 
 use crate::{
     curve::{Curve, Scalar, ORDER},
@@ -226,25 +225,21 @@ pub(crate) fn secret_uint_from_scalar<P: SchemeParams>(
 
     debug_assert!(uint_len >= scalar_len);
     repr.expose_secret_mut().as_mut()[uint_len - scalar_len..].copy_from_slice(scalar_bytes.expose_secret());
-    Secret::init_with(|| {
-        let mut repr = *repr.expose_secret();
-        let result = <P::Paillier as PaillierParams>::Uint::from_be_bytes(repr);
-        repr.zeroize();
-        result
-    })
+    Secret::init_with(|| <P::Paillier as PaillierParams>::Uint::from_be_bytes(*repr.expose_secret()))
 }
 
 pub(crate) fn secret_signed_from_scalar<P: SchemeParams>(
     value: &Secret<Scalar>,
 ) -> Secret<Signed<<P::Paillier as PaillierParams>::Uint>> {
     Secret::init_with(|| {
-        let mut uint = *secret_uint_from_scalar::<P>(value).expose_secret();
-        let result = Signed::new_positive(uint, ORDER.bits_vartime() as u32).expect(concat![
+        Signed::new_positive(
+            *secret_uint_from_scalar::<P>(value).expose_secret(),
+            ORDER.bits_vartime() as u32,
+        )
+        .expect(concat![
             "a curve scalar value is smaller than the curve order, ",
             "and the curve order fits in `PaillierParams::Uint`"
-        ]);
-        uint.zeroize();
-        result
+        ])
     })
 }
 
@@ -252,13 +247,14 @@ pub(crate) fn secret_bounded_from_scalar<P: SchemeParams>(
     value: &Secret<Scalar>,
 ) -> Secret<Bounded<<P::Paillier as PaillierParams>::Uint>> {
     Secret::init_with(|| {
-        let mut uint = *secret_uint_from_scalar::<P>(value).expose_secret();
-        let result = Bounded::new(uint, ORDER.bits_vartime() as u32).expect(concat![
+        Bounded::new(
+            *secret_uint_from_scalar::<P>(value).expose_secret(),
+            ORDER.bits_vartime() as u32,
+        )
+        .expect(concat![
             "a curve scalar value is smaller than the curve order, ",
             "and the curve order fits in `PaillierParams::Uint`"
-        ]);
-        uint.zeroize();
-        result
+        ])
     })
 }
 
diff --git a/synedrion/src/curve/arithmetic.rs b/synedrion/src/curve/arithmetic.rs
index 347f9224..b7cfe2a6 100644
--- a/synedrion/src/curve/arithmetic.rs
+++ b/synedrion/src/curve/arithmetic.rs
@@ -112,12 +112,7 @@ impl Scalar {
     }
 
     pub fn from_signing_key(sk: &SigningKey) -> Secret<Self> {
-        Secret::init_with(|| {
-            let mut scalar = *sk.as_nonzero_scalar().as_ref();
-            let result = Self(scalar);
-            scalar.zeroize();
-            result
-        })
+        Secret::init_with(|| Self(*sk.as_nonzero_scalar().as_ref()))
     }
 
     pub(crate) fn try_from_bytes(bytes: &[u8]) -> Result<Self, String> {
@@ -132,13 +127,8 @@ impl Scalar {
 
 impl Secret<Scalar> {
     pub fn to_signing_key(&self) -> Option<SigningKey> {
-        let nonzero_scalar: Secret<NonZeroScalar<_>> = Secret::try_init_with(|| {
-            let mut scalar = self.expose_secret().0;
-            let nz_scalar = Option::from(NonZeroScalar::new(scalar)).ok_or(());
-            scalar.zeroize();
-            nz_scalar
-        })
-        .ok()?;
+        let nonzero_scalar: Secret<NonZeroScalar<_>> =
+            Secret::maybe_init_with(|| Option::from(NonZeroScalar::new(self.expose_secret().0)))?;
         // SigningKey can be instantiated from NonZeroScalar directly, but that method takes it by value,
         // so it is more likely to leave traces of secret data on the stack. `SecretKey::from()` takes a reference.
         let secret_key = SecretKey::from(nonzero_scalar.expose_secret());
diff --git a/synedrion/src/paillier/rsa.rs b/synedrion/src/paillier/rsa.rs
index 200db5fa..4acd65c3 100644
--- a/synedrion/src/paillier/rsa.rs
+++ b/synedrion/src/paillier/rsa.rs
@@ -3,7 +3,6 @@ use crypto_primes::RandomPrimeWithRng;
 use rand_core::CryptoRngCore;
 use secrecy::ExposeSecret;
 use serde::{Deserialize, Serialize};
-use zeroize::Zeroize;
 
 use super::params::PaillierParams;
 use crate::{
@@ -122,21 +121,11 @@ impl<P: PaillierParams> SecretPrimes<P> {
     }
 
     pub fn p(&self) -> Secret<P::Uint> {
-        Secret::init_with(|| {
-            let mut p = self.primes.p.expose_secret().clone();
-            let p_wide = p.clone().to_wide();
-            p.zeroize();
-            p_wide
-        })
+        Secret::init_with(|| self.primes.p.expose_secret().to_wide())
     }
 
     pub fn q(&self) -> Secret<P::Uint> {
-        Secret::init_with(|| {
-            let mut q = self.primes.q.expose_secret().clone();
-            let q_wide = q.clone().to_wide();
-            q.zeroize();
-            q_wide
-        })
+        Secret::init_with(|| self.primes.q.expose_secret().to_wide())
     }
 
     pub fn p_signed(&self) -> Secret<Signed<P::Uint>> {
diff --git a/synedrion/src/tools/secret.rs b/synedrion/src/tools/secret.rs
index 7678fcfc..038fd9bf 100644
--- a/synedrion/src/tools/secret.rs
+++ b/synedrion/src/tools/secret.rs
@@ -33,6 +33,10 @@ impl<T: Zeroize + Clone> Secret<T> {
     pub fn try_init_with<E>(ctr: impl FnOnce() -> Result<T, E>) -> Result<Self, E> {
         Ok(Self(SecretBox::try_init_with(ctr)?))
     }
+
+    pub fn maybe_init_with(ctr: impl FnOnce() -> Option<T>) -> Option<Self> {
+        Some(Self(SecretBox::try_init_with(|| ctr().ok_or(())).ok()?))
+    }
 }
 
 impl<T: Zeroize + Clone> Clone for Secret<T> {
@@ -395,7 +399,7 @@ impl Secret<Scalar> {
     }
 
     pub fn invert(&self) -> Option<Secret<Scalar>> {
-        Secret::try_init_with(|| Option::from(self.expose_secret().invert()).ok_or(())).ok()
+        Secret::maybe_init_with(|| Option::from(self.expose_secret().invert()))
     }
 }