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

Replace clear_on_drop with zeroize #17

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ serde = { version = "1", default-features = false, features = ["alloc"] }
serde_derive = { version = "1", default-features = false }
thiserror = { version = "1", optional = true }
merlin = { version = "3", default-features = false }
clear_on_drop = { version = "0.2", default-features = false }
zeroize = { version = "1.1.0", default-features = false, features = ["alloc"], optional = true }

[dev-dependencies]
hex = "0.3"
Expand All @@ -37,11 +37,11 @@ bincode = "1"
rand_chacha = "0.3"

[features]
default = ["std"]
default = ["std", "zeroize"]
avx2_backend = ["curve25519-dalek/avx2_backend"]
yoloproofs = []
std = ["rand", "rand/std", "rand/std_rng", "thiserror", "curve25519-dalek/std"]
nightly = ["curve25519-dalek/nightly", "curve25519-dalek/alloc", "subtle/nightly", "clear_on_drop/nightly"]
nightly = ["curve25519-dalek/nightly", "curve25519-dalek/alloc", "subtle/nightly"]
docs = ["nightly"]


Expand Down
43 changes: 15 additions & 28 deletions src/r1cs/prover.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![allow(non_snake_case)]

use clear_on_drop::clear::Clear;
use core::borrow::BorrowMut;
use core::mem;
use curve25519_dalek::ristretto::{CompressedRistretto, RistrettoPoint};
Expand All @@ -18,6 +17,8 @@ use crate::generators::{BulletproofGens, PedersenGens};
use crate::inner_product_proof::InnerProductProof;
use crate::r1cs::Metrics;
use crate::transcript::TranscriptProtocol;
#[cfg(feature = "zeroize")]
use zeroize::Zeroize;

/// A [`ConstraintSystem`] implementation for use by the prover.
///
Expand Down Expand Up @@ -74,24 +75,13 @@ pub struct RandomizingProver<'g, T: BorrowMut<Transcript>> {
/// Overwrite secrets with null bytes when they go out of scope.
impl Drop for Secrets {
fn drop(&mut self) {
self.v.clear();
self.v_blinding.clear();

// Important: due to how ClearOnDrop auto-implements InitializableFromZeroed
// for T: Default, calling .clear() on Vec compiles, but does not
// clear the content. Instead, it only clears the Vec's header.
// Clearing the underlying buffer item-by-item will do the job, but will
// keep the header as-is, which is fine since the header does not contain secrets.
for e in self.a_L.iter_mut() {
e.clear();
}
for e in self.a_R.iter_mut() {
e.clear();
}
for e in self.a_O.iter_mut() {
e.clear();
}
// XXX use ClearOnDrop instead of doing the above
self.v.zeroize();
self.v_blinding.zeroize();

self.a_L.zeroize();

self.a_R.zeroize();
self.a_O.zeroize();
}
}

Expand Down Expand Up @@ -697,17 +687,14 @@ impl<'g, T: BorrowMut<Transcript>> Prover<'g, T> {
r_vec,
);

// We do not yet have a ClearOnDrop wrapper for Vec<Scalar>.
// When PR 202 [1] is merged, we can simply wrap s_L and s_R at the point of creation.
// [1] https://github.com/dalek-cryptography/curve25519-dalek/pull/202
for scalar in s_L1
.iter_mut()
.chain(s_L2.iter_mut())
.chain(s_R1.iter_mut())
.chain(s_R2.iter_mut())
#[cfg(feature = "zeroize")]
{
scalar.clear();
s_L1.clear();
s_L2.clear();
s_R1.clear();
s_R2.clear();
}

let proof = R1CSProof {
A_I1,
A_O1,
Expand Down
45 changes: 21 additions & 24 deletions src/range_proof/party.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@
extern crate alloc;

use alloc::vec::Vec;
use clear_on_drop::clear::Clear;
use core::iter;
use curve25519_dalek::ristretto::{CompressedRistretto, RistrettoPoint};
use curve25519_dalek::scalar::Scalar;
use curve25519_dalek::traits::MultiscalarMul;
use rand_core::{CryptoRng, RngCore};
#[cfg(feature = "zeroize")]
use zeroize::Zeroize;

use crate::errors::MPCError;
use crate::generators::{BulletproofGens, PedersenGens};
Expand Down Expand Up @@ -144,11 +145,13 @@ impl<'a> PartyAwaitingPosition<'a> {
}
}


/// Overwrite secrets with null bytes when they go out of scope.
#[cfg(feature = "zeroize")]
impl<'a> Drop for PartyAwaitingPosition<'a> {
fn drop(&mut self) {
self.v.clear();
self.v_blinding.clear();
self.v.zeroize();
self.v_blinding.zeroize();
}
}

Expand Down Expand Up @@ -238,24 +241,17 @@ impl<'a> PartyAwaitingBitChallenge<'a> {
}

/// Overwrite secrets with null bytes when they go out of scope.
#[cfg(feature = "zeroize")]
impl<'a> Drop for PartyAwaitingBitChallenge<'a> {
fn drop(&mut self) {
self.v.clear();
self.v_blinding.clear();
self.a_blinding.clear();
self.s_blinding.clear();

// Important: due to how ClearOnDrop auto-implements InitializableFromZeroed
// for T: Default, calling .clear() on Vec compiles, but does not
// clear the content. Instead, it only clears the Vec's header.
// Clearing the underlying buffer item-by-item will do the job, but will
// keep the header as-is, which is fine since the header does not contain secrets.
for e in self.s_L.iter_mut() {
e.clear();
}
for e in self.s_R.iter_mut() {
e.clear();
}
self.v.zeroize();
self.v_blinding.zeroize();
self.a_blinding.zeroize();
self.s_blinding.zeroize();

self.s_L.zeroize();

self.s_R.zeroize();
}
}

Expand Down Expand Up @@ -306,13 +302,14 @@ impl PartyAwaitingPolyChallenge {
}

/// Overwrite secrets with null bytes when they go out of scope.
#[cfg(feature = "zeroize")]
impl Drop for PartyAwaitingPolyChallenge {
fn drop(&mut self) {
self.v_blinding.clear();
self.a_blinding.clear();
self.s_blinding.clear();
self.t_1_blinding.clear();
self.t_2_blinding.clear();
self.v_blinding.zeroize();
self.a_blinding.zeroize();
self.s_blinding.zeroize();
self.t_1_blinding.zeroize();
self.t_2_blinding.zeroize();

// Note: polynomials r_poly, l_poly and t_poly
// are cleared within their own Drop impls.
Expand Down
70 changes: 14 additions & 56 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,20 @@ extern crate alloc;

use alloc::vec;
use alloc::vec::Vec;
use clear_on_drop::clear::Clear;
use curve25519_dalek::scalar::Scalar;
#[cfg(feature = "zeroize")]
use zeroize::Zeroize;

use crate::inner_product_proof::inner_product;

/// Represents a degree-1 vector polynomial \\(\mathbf{a} + \mathbf{b} \cdot x\\).
#[cfg_attr(feature = "zeroize", derive(Zeroize), zeroize(drop))]
pub struct VecPoly1(pub Vec<Scalar>, pub Vec<Scalar>);

/// Represents a degree-3 vector polynomial
/// \\(\mathbf{a} + \mathbf{b} \cdot x + \mathbf{c} \cdot x^2 + \mathbf{d} \cdot x^3 \\).
#[cfg(feature = "yoloproofs")]
#[cfg_attr(feature = "zeroize", derive(Zeroize), zeroize(drop))]
pub struct VecPoly3(
pub Vec<Scalar>,
pub Vec<Scalar>,
Expand All @@ -24,11 +27,13 @@ pub struct VecPoly3(
);

/// Represents a degree-2 scalar polynomial \\(a + b \cdot x + c \cdot x^2\\)
#[cfg_attr(feature = "zeroize", derive(Zeroize), zeroize(drop))]
pub struct Poly2(pub Scalar, pub Scalar, pub Scalar);

/// Represents a degree-6 scalar polynomial, without the zeroth degree
/// \\(a \cdot x + b \cdot x^2 + c \cdot x^3 + d \cdot x^4 + e \cdot x^5 + f \cdot x^6\\)
#[cfg(feature = "yoloproofs")]
#[cfg_attr(feature = "zeroize", derive(Zeroize), zeroize(drop))]
pub struct Poly6 {
pub t1: Scalar,
pub t2: Scalar,
Expand Down Expand Up @@ -167,55 +172,6 @@ impl Poly6 {
}
}

impl Drop for VecPoly1 {
fn drop(&mut self) {
for e in self.0.iter_mut() {
e.clear();
}
for e in self.1.iter_mut() {
e.clear();
}
}
}

impl Drop for Poly2 {
fn drop(&mut self) {
self.0.clear();
self.1.clear();
self.2.clear();
}
}

#[cfg(feature = "yoloproofs")]
impl Drop for VecPoly3 {
fn drop(&mut self) {
for e in self.0.iter_mut() {
e.clear();
}
for e in self.1.iter_mut() {
e.clear();
}
for e in self.2.iter_mut() {
e.clear();
}
for e in self.3.iter_mut() {
e.clear();
}
}
}

#[cfg(feature = "yoloproofs")]
impl Drop for Poly6 {
fn drop(&mut self) {
self.t1.clear();
self.t2.clear();
self.t3.clear();
self.t4.clear();
self.t5.clear();
self.t6.clear();
}
}

/// Raises `x` to the power `n` using binary exponentiation,
/// with (1 to 2)*lg(n) scalar multiplications.
/// TODO: a consttime version of this would be awfully similar to a Montgomery ladder.
Expand Down Expand Up @@ -350,12 +306,13 @@ mod tests {
assert_eq!(sum_of_powers_slow(&x, 6), Scalar::from(111111u64));
}

#[cfg(feature = "zeroize")]
#[test]
fn vec_of_scalars_clear_on_drop() {
fn vec_of_scalars_zeroize() {
let mut v = vec![Scalar::from(24u64), Scalar::from(42u64)];

for e in v.iter_mut() {
e.clear();
e.zeroize();
}

fn flat_slice<T>(x: &[T]) -> &[u8] {
Expand All @@ -370,17 +327,18 @@ mod tests {
assert_eq!(v[1], Scalar::zero());
}

#[cfg(feature = "zeroize")]
#[test]
fn tuple_of_scalars_clear_on_drop() {
fn tuple_of_scalars_zeroize() {
let mut v = Poly2(
Scalar::from(24u64),
Scalar::from(42u64),
Scalar::from(255u64),
);

v.0.clear();
v.1.clear();
v.2.clear();
v.0.zeroize();
v.1.zeroize();
v.2.zeroize();

fn as_bytes<T>(x: &T) -> &[u8] {
use core::mem;
Expand Down