Fix: zeroize Point and BigInt on drop, and other zeroize changes

src/arithmetic/big_gmp.rs

@@ -15,13 +15,11 @@
 */
 
 use std::convert::{TryFrom, TryInto};
-use std::sync::atomic;
-use std::{fmt, ops, ptr};
+use std::{fmt, ops};
 
 use gmp::mpz::Mpz;
 use gmp::sign::Sign;
 use num_traits::{One, Zero};
-use zeroize::Zeroize;
 
 use super::errors::*;
 use super::traits::*;
@@ -51,23 +49,6 @@ impl BigInt {
     }
 }
 
-#[allow(deprecated)]
-impl ZeroizeBN for BigInt {
-    fn zeroize_bn(&mut self) {
-        unsafe { ptr::write_volatile(&mut self.gmp, Mpz::zero()) };
-        atomic::fence(atomic::Ordering::SeqCst);
-        atomic::compiler_fence(atomic::Ordering::SeqCst);
-    }
-}
-
-impl Zeroize for BigInt {
-    fn zeroize(&mut self) {
-        unsafe { ptr::write_volatile(&mut self.gmp, Mpz::zero()) };
-        atomic::fence(atomic::Ordering::SeqCst);
-        atomic::compiler_fence(atomic::Ordering::SeqCst);
-    }
-}
-
 impl Converter for BigInt {
     fn to_bytes(&self) -> Vec<u8> {
         (&self.gmp).into()

src/arithmetic/big_native.rs

@@ -8,6 +8,7 @@ use super::traits::*;
 
 use num_bigint::BigInt as BN;
 use num_bigint::Sign;
+use zeroize::Zeroize;
 
 mod primes;
 mod ring_algorithms;
@@ -31,7 +32,7 @@ impl BigInt {
         &mut self.num
     }
     fn into_inner(self) -> BN {
-        self.num
+        self.num.clone()
     }
 }
 
@@ -42,12 +43,27 @@ impl ZeroizeBN for BigInt {
     }
 }
 
-impl zeroize::Zeroize for BigInt {
+impl Zeroize for BigInt {
     fn zeroize(&mut self) {
-        use std::{ptr, sync::atomic};
-        unsafe { ptr::write_volatile(&mut self.num, Zero::zero()) };
-        atomic::fence(atomic::Ordering::SeqCst);
+        use core::{ptr, sync::atomic};
+        // Copy the inner so we can read the data inside
+        let original = unsafe { ptr::read(&mut self.num) };
+        // Replace self with a zeroed integer.
+        unsafe { ptr::write_volatile(self, Self::zero()) };
+        let (mut sign, uint) = original.into_parts();
+        // Zero out the temp sign in case it's a secret somehow
+        unsafe { ptr::write_volatile(&mut sign, Sign::NoSign) };
+        // zero out the bigint's data itself.
+        // This is semi-UB because it's a repr(Rust) type, but because it's a single field we can assume it matches the wrapper.
+        let mut data: Vec<usize> = unsafe { core::mem::transmute(uint) };
         atomic::compiler_fence(atomic::Ordering::SeqCst);
+        data.zeroize();
+    }
+}
+
+impl Drop for BigInt {
+    fn drop(&mut self) {
+        self.zeroize();
     }
 }
 
@@ -351,7 +367,7 @@ crate::__bigint_impl_assigns! {
 impl ops::Neg for BigInt {
     type Output = BigInt;
     fn neg(self) -> Self::Output {
-        self.num.neg().wrap()
+        (&self.num).neg().wrap()
     }
 }
 impl ops::Neg for &BigInt {

src/arithmetic/mod.rs

@@ -377,10 +377,9 @@ mod test {
         T: Converter + BasicOps + Modulo + Samplable + NumberTests + EGCD + BitManipulation,
         T: Primes,
         // Deprecated but not deleted yet traits from self::traits module
-        T: ZeroizeBN,
         u64: ConvertFrom<BigInt>,
         // Foreign traits implementations
-        T: zeroize::Zeroize + num_traits::One + num_traits::Zero,
+        T: num_traits::One + num_traits::Zero,
         T: num_traits::Num + num_integer::Integer + num_integer::Roots,
         // Conversion traits
         for<'a> u64: std::convert::TryFrom<&'a BigInt>,

src/cryptographic_primitives/commitments/pedersen_commitment.rs

@@ -6,6 +6,7 @@
 */
 
 use std::marker::PhantomData;
+use zeroize::Zeroizing;
 
 use super::traits::Commitment;
 use super::SECURITY_BITS;
@@ -29,9 +30,9 @@ impl<E: Curve> Commitment<Point<E>> for PedersenCommitment<E> {
         let h = Point::base_point2();
         let message_scalar: Scalar<E> = Scalar::from(message);
         let blinding_scalar: Scalar<E> = Scalar::from(blinding_factor);
-        let mg = g * message_scalar;
-        let rh = h * blinding_scalar;
-        mg + rh
+        let mg = Zeroizing::new(g * message_scalar);
+        let rh = Zeroizing::new(h * blinding_scalar);
+        &*mg + &*rh
     }
 
     fn create_commitment(message: &BigInt) -> (Point<E>, BigInt) {

src/cryptographic_primitives/proofs/sigma_correct_homomorphic_elgamal_enc.rs

@@ -9,6 +9,8 @@
 use digest::Digest;
 use serde::{Deserialize, Serialize};
 
+use zeroize::Zeroizing;
+
 use crate::cryptographic_primitives::hashing::DigestExt;
 use crate::elliptic::curves::{Curve, Point, Scalar};
 use crate::marker::HashChoice;
@@ -56,10 +58,10 @@ impl<E: Curve, H: Digest + Clone> HomoELGamalProof<E, H> {
     ) -> HomoELGamalProof<E, H> {
         let s1: Scalar<E> = Scalar::random();
         let s2: Scalar<E> = Scalar::random();
-        let A1 = &delta.H * &s1;
-        let A2 = &delta.Y * &s2;
+        let A1 = Zeroizing::new(&delta.H * &s1);
+        let A2 = Zeroizing::new(&delta.Y * &s2);
         let A3 = &delta.G * &s2;
-        let T = A1 + A2;
+        let T = &*A1 + &*A2;
         let e = H::new()
             .chain_point(&T)
             .chain_point(&A3)

src/cryptographic_primitives/secret_sharing/feldman_vss.rs

@@ -24,8 +24,8 @@ pub struct ShamirSecretSharing {
 /// Feldman VSS, based on  Paul Feldman. 1987. A practical scheme for non-interactive verifiable secret sharing.
 /// In Foundations of Computer Science, 1987., 28th Annual Symposium on.IEEE, 427–43
 ///
-/// implementation details: The code is using FE and GE. Each party is given an index from 1,..,n and a secret share of type FE.
-/// The index of the party is also the point on the polynomial where we treat this number as u32 but converting it to FE internally.
+/// Implementation details: The code is generic over the curve. Each party is given an index from `1,..,n` and a secret share of type `Scalar<E>`.
+/// The index of the party is also the point on the polynomial where we treat this number as `u32` but converting it to `Scalar<E>` internally.
 #[derive(Clone, PartialEq, Debug, Serialize, Deserialize)]
 #[serde(bound = "")]
 pub struct VerifiableSS<E: Curve> {

src/cryptographic_primitives/twoparty/coin_flip_optimal_rounds.rs

@@ -15,7 +15,7 @@ use crate::cryptographic_primitives::proofs::sigma_valid_pedersen_blind::Pederse
 use crate::elliptic::curves::{Curve, Point, Scalar};
 
 /// based on How To Simulate It – A Tutorial on the Simulation
-/// Proof Technique. protocol 7.3: Multiple coin tossing. which provide simulatble constant round
+/// Proof Technique. protocol 7.3: Multiple coin tossing. which provide simulatable constant round
 /// coin toss
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 #[serde(bound = "")]

src/elliptic/curves/test.rs

@@ -4,6 +4,8 @@ use std::iter;
 
 use rand::{rngs::OsRng, Rng};
 
+use zeroize::Zeroize;
+
 use crate::arithmetic::*;
 use crate::test_for_all_curves;
 
@@ -358,3 +360,14 @@ fn scalar_assign_negation<E: Curve>() {
     };
     assert_eq!(s_neg_1, s_neg_2);
 }
+
+test_for_all_curves!(point_zeroize);
+fn point_zeroize<E: Curve>() {
+    let mut first_point = E::Point::generator().scalar_mul(&random_nonzero_scalar());
+    let first_copy = first_point.clone();
+    first_point.zeroize();
+    assert_ne!(first_point, first_copy);
+    let mut second_point = E::Point::generator().scalar_mul(&random_nonzero_scalar());
+    second_point.zeroize();
+    assert_eq!(first_point, second_point);
+}

src/elliptic/curves/wrappers/point.rs

@@ -1,5 +1,7 @@
 use std::{fmt, iter};
 
+use zeroize::Zeroize;
+
 use crate::elliptic::curves::traits::*;
 use crate::BigInt;
 
@@ -307,3 +309,9 @@ impl<'p, E: Curve> iter::Sum<&'p Point<E>> for Point<E> {
         iter.fold(Point::zero(), |acc, p| acc + p)
     }
 }
+
+impl<E: Curve> Zeroize for Point<E> {
+    fn zeroize(&mut self) {
+        self.raw_point.zeroize();
+    }
+}