From 9b90216b6324d0d8285d59765a84b972fc265fed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Strug?= Date: Fri, 12 Apr 2024 14:44:45 +0200 Subject: [PATCH 1/6] Added Zeroize annotation to Ed25519PrivateKey --- Cargo.toml | 1 + radix-common/Cargo.toml | 1 + radix-common/src/crypto/ed25519/private_key.rs | 2 ++ 3 files changed, 4 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index bcc6250e1ee..95d71c824d1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -125,6 +125,7 @@ wasmer = { version = "2.2.1" } wasmer-compiler-singlepass = { version = "2.2.1" } wasmparser = { version = "0.107.0", default-features = false } extend = { version = "1.2.0" } +zeroize = { version = "1.7.0" } # Both the release and test profiles use `panic = "unwind"` to allow certain parts of the Radix # Engine to be able to catch panics. As an example, the native-vm has a `catch_unwind` to catch diff --git a/radix-common/Cargo.toml b/radix-common/Cargo.toml index 65a5979f94b..e2de46b403a 100644 --- a/radix-common/Cargo.toml +++ b/radix-common/Cargo.toml @@ -31,6 +31,7 @@ ed25519-dalek = { workspace = true, features = ["u64_backend"] } secp256k1 = { workspace = true, features = ["recovery"], optional = true } blst = { workspace = true, optional = false } sha3 = { workspace = true, optional = false } +zeroize = { workspace = true, optional = false } [dev-dependencies] serde_json = { workspace = true } diff --git a/radix-common/src/crypto/ed25519/private_key.rs b/radix-common/src/crypto/ed25519/private_key.rs index 5ac75758719..af49e1c58c1 100644 --- a/radix-common/src/crypto/ed25519/private_key.rs +++ b/radix-common/src/crypto/ed25519/private_key.rs @@ -1,7 +1,9 @@ use super::Ed25519Signature; use crate::internal_prelude::*; use ed25519_dalek::{Keypair, PublicKey, SecretKey, Signer}; +use zeroize::{Zeroize, ZeroizeOnDrop}; +#[derive(Zeroize, ZeroizeOnDrop)] pub struct Ed25519PrivateKey(SecretKey); impl Ed25519PrivateKey { From bde72a2e3c89ff7997ddf1f25d90d1894a8c4674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Strug?= Date: Fri, 12 Apr 2024 17:09:38 +0200 Subject: [PATCH 2/6] Updated Cargo.lock --- examples/everything/Cargo.lock | 5 +++-- radix-clis/Cargo.lock | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/examples/everything/Cargo.lock b/examples/everything/Cargo.lock index 3c2e1d3f77d..70f7b887a11 100644 --- a/examples/everything/Cargo.lock +++ b/examples/everything/Cargo.lock @@ -817,6 +817,7 @@ dependencies = [ "serde", "sha3", "strum", + "zeroize", ] [[package]] @@ -1885,9 +1886,9 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.3.0" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4756f7db3f7b5574938c3eb1c117038b8e07f95ee6718c0efad4ac21508f1efd" +checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" dependencies = [ "zeroize_derive", ] diff --git a/radix-clis/Cargo.lock b/radix-clis/Cargo.lock index edf9932d980..ddfd922583d 100644 --- a/radix-clis/Cargo.lock +++ b/radix-clis/Cargo.lock @@ -1145,6 +1145,7 @@ dependencies = [ "secp256k1", "sha3", "strum", + "zeroize", ] [[package]] @@ -2303,9 +2304,9 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.3.0" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4756f7db3f7b5574938c3eb1c117038b8e07f95ee6718c0efad4ac21508f1efd" +checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" dependencies = [ "zeroize_derive", ] From 791dd3be0dda11150807eb8aeaeaff48487e062a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Strug?= Date: Fri, 12 Apr 2024 17:54:26 +0200 Subject: [PATCH 3/6] Updated zeroize crate version --- Cargo.toml | 2 +- examples/everything/Cargo.lock | 4 ++-- radix-clis/Cargo.lock | 4 ++-- radix-common/src/crypto/ed25519/private_key.rs | 5 +++-- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 95d71c824d1..51c26a16831 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -125,7 +125,7 @@ wasmer = { version = "2.2.1" } wasmer-compiler-singlepass = { version = "2.2.1" } wasmparser = { version = "0.107.0", default-features = false } extend = { version = "1.2.0" } -zeroize = { version = "1.7.0" } +zeroize = { version = "1.3.0" } # Both the release and test profiles use `panic = "unwind"` to allow certain parts of the Radix # Engine to be able to catch panics. As an example, the native-vm has a `catch_unwind` to catch diff --git a/examples/everything/Cargo.lock b/examples/everything/Cargo.lock index 70f7b887a11..cbd9d68982c 100644 --- a/examples/everything/Cargo.lock +++ b/examples/everything/Cargo.lock @@ -1886,9 +1886,9 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.7.0" +version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" +checksum = "4756f7db3f7b5574938c3eb1c117038b8e07f95ee6718c0efad4ac21508f1efd" dependencies = [ "zeroize_derive", ] diff --git a/radix-clis/Cargo.lock b/radix-clis/Cargo.lock index ddfd922583d..571046f1672 100644 --- a/radix-clis/Cargo.lock +++ b/radix-clis/Cargo.lock @@ -2304,9 +2304,9 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.7.0" +version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" +checksum = "4756f7db3f7b5574938c3eb1c117038b8e07f95ee6718c0efad4ac21508f1efd" dependencies = [ "zeroize_derive", ] diff --git a/radix-common/src/crypto/ed25519/private_key.rs b/radix-common/src/crypto/ed25519/private_key.rs index af49e1c58c1..74f4a16edf1 100644 --- a/radix-common/src/crypto/ed25519/private_key.rs +++ b/radix-common/src/crypto/ed25519/private_key.rs @@ -1,9 +1,10 @@ use super::Ed25519Signature; use crate::internal_prelude::*; use ed25519_dalek::{Keypair, PublicKey, SecretKey, Signer}; -use zeroize::{Zeroize, ZeroizeOnDrop}; +use zeroize::Zeroize; -#[derive(Zeroize, ZeroizeOnDrop)] +#[derive(Zeroize)] +#[zeroize(drop)] pub struct Ed25519PrivateKey(SecretKey); impl Ed25519PrivateKey { From 34ef710c2b323ce0bd56a59bd1324cd4a83c5dc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Strug?= Date: Mon, 15 Apr 2024 13:12:11 +0200 Subject: [PATCH 4/6] Added Zeroize to Secp256k1PrivateKey --- .../src/crypto/secp256k1/private_key.rs | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/radix-common/src/crypto/secp256k1/private_key.rs b/radix-common/src/crypto/secp256k1/private_key.rs index e854d31c4ec..1e2169360ed 100644 --- a/radix-common/src/crypto/secp256k1/private_key.rs +++ b/radix-common/src/crypto/secp256k1/private_key.rs @@ -1,35 +1,49 @@ +use super::Secp256k1Signature; use crate::internal_prelude::*; use ::secp256k1::{All, Message, PublicKey, Secp256k1, SecretKey}; - -use super::Secp256k1Signature; +use zeroize::{DefaultIsZeroes, Zeroize}; lazy_static::lazy_static! { pub(crate) static ref SECP256K1_CTX: Secp256k1 = secp256k1::Secp256k1::new(); } -pub struct Secp256k1PrivateKey(SecretKey); +#[derive(Copy, Clone)] +pub struct SecretKeyWrapper(SecretKey); +impl Default for SecretKeyWrapper { + fn default() -> Self { + let mut key = + SecretKey::from_slice(&[0xaau8; secp256k1::constants::SECRET_KEY_SIZE]).unwrap(); + key.non_secure_erase(); + Self(key) + } +} +impl DefaultIsZeroes for SecretKeyWrapper {} + +#[derive(Zeroize)] +#[zeroize(drop)] +pub struct Secp256k1PrivateKey(SecretKeyWrapper); impl Secp256k1PrivateKey { - pub const LENGTH: usize = 32; + pub const LENGTH: usize = secp256k1::constants::SECRET_KEY_SIZE; pub fn public_key(&self) -> Secp256k1PublicKey { - Secp256k1PublicKey(PublicKey::from_secret_key(&SECP256K1_CTX, &self.0).serialize()) + Secp256k1PublicKey(PublicKey::from_secret_key(&SECP256K1_CTX, &self.0 .0).serialize()) } pub fn sign(&self, msg_hash: &impl IsHash) -> Secp256k1Signature { let m = Message::from_digest_slice(msg_hash.as_ref()).expect("Hash is always a valid message"); - let signature = SECP256K1_CTX.sign_ecdsa_recoverable(&m, &self.0); + let signature = SECP256K1_CTX.sign_ecdsa_recoverable(&m, &self.0 .0); let (recovery_id, signature_data) = signature.serialize_compact(); - let mut buf = [0u8; 65]; + let mut buf = [0u8; Secp256k1Signature::LENGTH]; buf[0] = recovery_id.to_i32() as u8; buf[1..].copy_from_slice(&signature_data); Secp256k1Signature(buf) } pub fn to_bytes(&self) -> Vec { - self.0.secret_bytes().to_vec() + self.0 .0.secret_bytes().to_vec() } pub fn to_hex(&self) -> String { @@ -46,7 +60,9 @@ impl Secp256k1PrivateKey { if slice.len() != Secp256k1PrivateKey::LENGTH { return Err(()); } - Ok(Self(SecretKey::from_slice(slice).map_err(|_| ())?)) + Ok(Self(SecretKeyWrapper( + SecretKey::from_slice(slice).map_err(|_| ())?, + ))) } pub fn from_u64(n: u64) -> Result { @@ -54,7 +70,9 @@ impl Secp256k1PrivateKey { (&mut bytes[Secp256k1PrivateKey::LENGTH - 8..Secp256k1PrivateKey::LENGTH]) .copy_from_slice(&n.to_be_bytes()); - Ok(Self(SecretKey::from_slice(&bytes).map_err(|_| ())?)) + Ok(Self(SecretKeyWrapper( + SecretKey::from_slice(&bytes).map_err(|_| ())?, + ))) } } @@ -77,4 +95,10 @@ mod tests { assert_eq!(sk.sign(&test_message_hash), sig); assert!(verify_secp256k1(&test_message_hash, &pk, &sig)); } + + #[test] + fn default_value() { + let key: SecretKeyWrapper = SecretKeyWrapper::default(); + assert_eq!(key.0.secret_bytes(), [1u8; 32]); + } } From 56a7d4970a5f83be8ea26a7ad531b55658ed2e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Strug?= Date: Mon, 15 Apr 2024 14:33:20 +0200 Subject: [PATCH 5/6] Changed dfault falue of secp256k1 --- radix-common/src/crypto/secp256k1/private_key.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/radix-common/src/crypto/secp256k1/private_key.rs b/radix-common/src/crypto/secp256k1/private_key.rs index 1e2169360ed..b27485680b3 100644 --- a/radix-common/src/crypto/secp256k1/private_key.rs +++ b/radix-common/src/crypto/secp256k1/private_key.rs @@ -11,10 +11,9 @@ lazy_static::lazy_static! { pub struct SecretKeyWrapper(SecretKey); impl Default for SecretKeyWrapper { fn default() -> Self { - let mut key = - SecretKey::from_slice(&[0xaau8; secp256k1::constants::SECRET_KEY_SIZE]).unwrap(); - key.non_secure_erase(); - Self(key) + let mut data = [0u8; secp256k1::constants::SECRET_KEY_SIZE]; + data[secp256k1::constants::SECRET_KEY_SIZE - 1] = 1; + Self(SecretKey::from_slice(&data).unwrap()) } } impl DefaultIsZeroes for SecretKeyWrapper {} @@ -99,6 +98,12 @@ mod tests { #[test] fn default_value() { let key: SecretKeyWrapper = SecretKeyWrapper::default(); - assert_eq!(key.0.secret_bytes(), [1u8; 32]); + assert_eq!( + key.0.secret_bytes(), + [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 1 + ] + ); } } From 003f9a6066d8a99a903d822cd8d986b98caa76ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Strug?= Date: Tue, 16 Apr 2024 15:53:21 +0200 Subject: [PATCH 6/6] Added test --- radix-common/src/crypto/secp256k1/private_key.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/radix-common/src/crypto/secp256k1/private_key.rs b/radix-common/src/crypto/secp256k1/private_key.rs index b27485680b3..9072037faf2 100644 --- a/radix-common/src/crypto/secp256k1/private_key.rs +++ b/radix-common/src/crypto/secp256k1/private_key.rs @@ -106,4 +106,16 @@ mod tests { ] ); } + + #[test] + fn verify_zeroize() { + let bytes = "4fd3fb62d6b7a4749f75d56d06b0aea1ec2c2a6986d2bfa975d7891585590fea"; + let mut key = Secp256k1PrivateKey::from_bytes(&hex::decode(bytes).unwrap()).unwrap(); + key.zeroize(); + + assert_eq!( + key.0 .0.secret_bytes(), + SecretKeyWrapper::default().0.secret_bytes() + ); + } }