From d896886691c4c63f22c959ebfe630fd9b9604541 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Fri, 20 Jul 2018 20:20:40 +0000 Subject: [PATCH 1/4] Overwrite secret key material with zeroes on drop. --- src/ed25519.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/ed25519.rs b/src/ed25519.rs index 919ecb4..e25003d 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -173,6 +173,13 @@ impl Debug for SecretKey { } } +/// Overwrite secret key material with null bytes when it goes out of scope. +impl Drop for SecretKey { + fn drop(&mut self) { + self.0 = [0u8; SECRET_KEY_LENGTH]; + } +} + impl SecretKey { /// Expand this `SecretKey` into an `ExpandedSecretKey`. pub fn expand(&self) -> ExpandedSecretKey where D: Digest + Default { @@ -375,6 +382,14 @@ pub struct ExpandedSecretKey { pub (crate) nonce: [u8; 32], } +/// Overwrite secret key material with null bytes when it goes out of scope. +impl Drop for ExpandedSecretKey { + fn drop(&mut self) { + self.key = Scalar::zero(); + self.nonce = [0u8; 32]; + } +} + #[cfg(feature = "sha2")] impl<'a> From<&'a SecretKey> for ExpandedSecretKey { /// Construct an `ExpandedSecretKey` from a `SecretKey`. From 6513d4980acbb4a25322d18927a4a734111279f9 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Fri, 20 Jul 2018 22:28:39 +0000 Subject: [PATCH 2/4] Implement Drop for secret key material using clear_on_drop. --- Cargo.toml | 5 ++++- src/ed25519.rs | 33 ++++++++++++++++++++++++++++----- src/lib.rs | 1 + 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e37ea8d..dccd33d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,9 @@ optional = true version = "^0.1.1" default-features = false +[dependencies.clear_on_drop] +version = "0.2" + [dev-dependencies] hex = "^0.3" sha2 = "^0.7" @@ -56,7 +59,7 @@ harness = false default = ["std", "u64_backend"] # We don't add "rand/std" here because it would enable a bunch of Fuchsia dependencies. std = ["curve25519-dalek/std"] -nightly = ["curve25519-dalek/nightly", "rand/nightly"] +nightly = ["curve25519-dalek/nightly", "rand/nightly", "clear_on_drop/nightly"] asm = ["sha2/asm"] yolocrypto = ["curve25519-dalek/yolocrypto"] u64_backend = ["curve25519-dalek/u64_backend"] diff --git a/src/ed25519.rs b/src/ed25519.rs index e25003d..aaec874 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -10,6 +10,7 @@ //! A Rust implementation of ed25519 EdDSA key generation, signing, and //! verification. +use core::default::Default; use core::fmt::{Debug}; use rand::CryptoRng; @@ -27,6 +28,8 @@ use serde::de::Visitor; #[cfg(feature = "sha2")] use sha2::Sha512; +use clear_on_drop::clear::Clear; + use digest::Digest; use generic_array::typenum::U64; @@ -165,6 +168,7 @@ impl<'d> Deserialize<'d> for Signature { /// An EdDSA secret key. #[repr(C)] +#[derive(Default)] pub struct SecretKey(pub (crate) [u8; SECRET_KEY_LENGTH]); impl Debug for SecretKey { @@ -176,7 +180,7 @@ impl Debug for SecretKey { /// Overwrite secret key material with null bytes when it goes out of scope. impl Drop for SecretKey { fn drop(&mut self) { - self.0 = [0u8; SECRET_KEY_LENGTH]; + self.0.clear(); } } @@ -377,6 +381,7 @@ impl<'d> Deserialize<'d> for SecretKey { // better-designed, Schnorr-based signature scheme, see Trevor Perrin's work on // "generalised EdDSA" and "VXEdDSA". #[repr(C)] +#[derive(Default)] pub struct ExpandedSecretKey { pub (crate) key: Scalar, pub (crate) nonce: [u8; 32], @@ -385,8 +390,8 @@ pub struct ExpandedSecretKey { /// Overwrite secret key material with null bytes when it goes out of scope. impl Drop for ExpandedSecretKey { fn drop(&mut self) { - self.key = Scalar::zero(); - self.nonce = [0u8; 32]; + self.key.clear(); + self.nonce.clear(); } } @@ -698,7 +703,7 @@ impl<'d> Deserialize<'d> for ExpandedSecretKey { } /// An ed25519 public key. -#[derive(Copy, Clone, Eq, PartialEq)] +#[derive(Copy, Clone, Default, Eq, PartialEq)] #[repr(C)] pub struct PublicKey(pub (crate) CompressedEdwardsY); @@ -909,7 +914,7 @@ impl<'d> Deserialize<'d> for PublicKey { } /// An ed25519 keypair. -#[derive(Debug)] +#[derive(Debug, Default)] #[repr(C)] pub struct Keypair { /// The secret half of this keypair. @@ -1431,6 +1436,24 @@ mod test { 175, 002, 026, 104, 247, 007, 081, 026, ])))) } + #[test] + fn keypair_clear_on_drop() { + let mut keypair: Keypair = Keypair::from_bytes(&[15u8; KEYPAIR_LENGTH][..]).unwrap(); + + keypair.clear(); + + fn as_bytes(x: &T) -> &[u8] { + use core::mem; + use core::slice; + + unsafe { + slice::from_raw_parts(x as *const T as *const u8, mem::size_of_val(x)) + } + } + + assert!(!as_bytes(&keypair).contains(&0x15)); + } + #[cfg(all(test, feature = "serde"))] use bincode::{serialize, deserialize, Infinite}; diff --git a/src/lib.rs b/src/lib.rs index b74999a..2e599bc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -264,6 +264,7 @@ extern crate generic_array; extern crate digest; extern crate failure; extern crate rand; +extern crate clear_on_drop; #[cfg(any(feature = "std", test))] #[macro_use] From 81a3d3298b4cc16eabb7dc965d7b3f3a5e8e87c3 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Thu, 26 Jul 2018 20:15:13 +0000 Subject: [PATCH 3/4] Leave a comment explaining why we derive Default. --- src/ed25519.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ed25519.rs b/src/ed25519.rs index aaec874..8f62f8e 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -168,7 +168,7 @@ impl<'d> Deserialize<'d> for Signature { /// An EdDSA secret key. #[repr(C)] -#[derive(Default)] +#[derive(Default)] // we derive Default in order to use the clear() method in Drop pub struct SecretKey(pub (crate) [u8; SECRET_KEY_LENGTH]); impl Debug for SecretKey { @@ -381,7 +381,7 @@ impl<'d> Deserialize<'d> for SecretKey { // better-designed, Schnorr-based signature scheme, see Trevor Perrin's work on // "generalised EdDSA" and "VXEdDSA". #[repr(C)] -#[derive(Default)] +#[derive(Default)] // we derive Default in order to use the clear() method in Drop pub struct ExpandedSecretKey { pub (crate) key: Scalar, pub (crate) nonce: [u8; 32], @@ -914,7 +914,7 @@ impl<'d> Deserialize<'d> for PublicKey { } /// An ed25519 keypair. -#[derive(Debug, Default)] +#[derive(Debug, Default)] // we derive Default in order to use the clear() method in Drop #[repr(C)] pub struct Keypair { /// The secret half of this keypair. From 050d2a01e5e0ed543b61eb0358179868740025d0 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Fri, 27 Jul 2018 03:38:34 +0000 Subject: [PATCH 4/4] Bump curve25519-dalek dependency to 0.19. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index dccd33d..107c46e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ exclude = [ ".gitignore", "TESTVECTORS", "res/*" ] travis-ci = { repository = "dalek-cryptography/ed25519-dalek", branch = "master"} [dependencies.curve25519-dalek] -version = "0.18" +version = "0.19" default-features = false [dependencies.rand]