Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 9 commits into from
21 changes: 1 addition & 20 deletions src/arithmetic/big_gmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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()
Expand Down
17 changes: 12 additions & 5 deletions src/arithmetic/big_native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,7 +32,7 @@ impl BigInt {
&mut self.num
}
fn into_inner(self) -> BN {
self.num
self.num.clone()
}
}

Expand All @@ -42,14 +43,14 @@ impl ZeroizeBN for BigInt {
}
}

impl zeroize::Zeroize for BigInt {
impl Zeroize for BigInt {
nskybytskyi marked this conversation as resolved.
Show resolved Hide resolved
fn zeroize(&mut self) {
survived marked this conversation as resolved.
Show resolved Hide resolved
use core::{ptr, sync::atomic};
// Copy the inner so we can read the data inside
let original = unsafe { ptr::read(self) };
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.num.into_parts();
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.
Expand All @@ -60,6 +61,12 @@ impl zeroize::Zeroize for BigInt {
}
}

impl Drop for BigInt {
fn drop(&mut self) {
self.zeroize();
}
}

impl Converter for BigInt {
fn to_bytes(&self) -> Vec<u8> {
let (_sign, bytes) = self.num.to_bytes_be();
Expand Down Expand Up @@ -360,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 {
Expand Down
3 changes: 1 addition & 2 deletions src/arithmetic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down