From 737c5be5019eaebde133d0b320a41faec9971d8b Mon Sep 17 00:00:00 2001 From: zi0Black <13380579+zi0Black@users.noreply.github.com> Date: Mon, 15 Jan 2024 19:37:40 +0100 Subject: [PATCH 1/5] Replace clear_on_drop with zeroize --- Cargo.toml | 4 ++-- src/r1cs/prover.rs | 43 ++++++++++++++------------------------ src/range_proof/party.rs | 45 +++++++++++++++++++--------------------- src/util.rs | 38 +++++++++++---------------------- 4 files changed, 50 insertions(+), 80 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 67d79ae3..bafdaca5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" @@ -41,7 +41,7 @@ default = ["std"] 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"] diff --git a/src/r1cs/prover.rs b/src/r1cs/prover.rs index 21b652b3..ad84761c 100644 --- a/src/r1cs/prover.rs +++ b/src/r1cs/prover.rs @@ -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}; @@ -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. /// @@ -74,24 +75,13 @@ pub struct RandomizingProver<'g, T: BorrowMut> { /// 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(); } } @@ -697,17 +687,14 @@ impl<'g, T: BorrowMut> Prover<'g, T> { r_vec, ); - // We do not yet have a ClearOnDrop wrapper for Vec. - // 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, diff --git a/src/range_proof/party.rs b/src/range_proof/party.rs index ebb232cc..136a05b6 100644 --- a/src/range_proof/party.rs +++ b/src/range_proof/party.rs @@ -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}; @@ -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(); } } @@ -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(); } } @@ -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. diff --git a/src/util.rs b/src/util.rs index dd7ce2fe..42f63bcd 100644 --- a/src/util.rs +++ b/src/util.rs @@ -5,12 +5,14 @@ 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, pub Vec); /// Represents a degree-3 vector polynomial @@ -24,6 +26,7 @@ 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 @@ -167,25 +170,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) { @@ -350,12 +334,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(x: &[T]) -> &[u8] { @@ -370,17 +355,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(x: &T) -> &[u8] { use core::mem; From 1fbe5ffa6b4610796741132ebfc72c0234e62cbd Mon Sep 17 00:00:00 2001 From: zi0Black <13380579+zi0Black@users.noreply.github.com> Date: Tue, 16 Jan 2024 14:48:54 +0100 Subject: [PATCH 2/5] Upgrade zeroize create --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index bafdaca5..7c2db4aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 } -zeroize = { version = "1.1.0", default-features = false, features = ["alloc"], optional = true } +zeroize = { version = "1.7.0", default-features = false, features = ["alloc"], optional = true } [dev-dependencies] hex = "0.3" From bb81c9a15c7248a82b21aa69f1577a594efb5e97 Mon Sep 17 00:00:00 2001 From: zi0Black <13380579+zi0Black@users.noreply.github.com> Date: Tue, 16 Jan 2024 18:15:02 +0100 Subject: [PATCH 3/5] Missing default feature --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 7c2db4aa..4d303bc6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,7 @@ 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"] From dd5de30e3d0cc818c76955d6931623f6ef8697cd Mon Sep 17 00:00:00 2001 From: zi0Black <13380579+zi0Black@users.noreply.github.com> Date: Wed, 17 Jan 2024 11:16:48 +0100 Subject: [PATCH 4/5] Removed clear_on_drop leftovers from yoloproofs code --- src/util.rs | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/src/util.rs b/src/util.rs index 42f63bcd..86383fad 100644 --- a/src/util.rs +++ b/src/util.rs @@ -18,6 +18,7 @@ pub struct VecPoly1(pub Vec, pub Vec); /// 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, pub Vec, @@ -32,6 +33,7 @@ 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, @@ -170,36 +172,6 @@ impl Poly6 { } } -#[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. From f77ddf5f09a8242d5df32d0b9fec3c5a49c29f38 Mon Sep 17 00:00:00 2001 From: zi0Black <13380579+zi0Black@users.noreply.github.com> Date: Wed, 17 Jan 2024 13:22:25 +0100 Subject: [PATCH 5/5] Relaxed zeroize version --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 4d303bc6..044fa14d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 } -zeroize = { version = "1.7.0", default-features = false, features = ["alloc"], optional = true } +zeroize = { version = "1.1.0", default-features = false, features = ["alloc"], optional = true } [dev-dependencies] hex = "0.3"