Merge remote-tracking branch 'dalek/develop' into develop

Cargo.toml

@@ -22,13 +22,13 @@ travis-ci = { repository = "dalek-cryptography/ed25519-dalek", branch = "master"
 features = ["nightly", "batch"]
 
 [dependencies]
-clear_on_drop = { version = "0.2" }
 curve25519-dalek = { version = "2", default-features = false }
 merlin = { version = "1", default-features = false, optional = true, git = "https://github.com/isislovecruft/merlin", branch = "develop" }
 rand = { version = "0.7", default-features = false, optional = true }
 rand_core = { version = "0.5", default-features = false, optional = true }
 serde = { version = "1.0", optional = true }
 sha2 = { version = "0.8", default-features = false }
+zeroize = { version = "1", default-features = false, features = ["zeroize_derive"] }
 
 [dev-dependencies]
 hex = "^0.4"
@@ -46,8 +46,8 @@ harness = false
 [features]
 default = ["std", "u64_backend"]
 std = ["curve25519-dalek/std", "sha2/std", "rand/std"]
-alloc = ["curve25519-dalek/alloc", "rand/alloc"]
-nightly = ["curve25519-dalek/nightly", "clear_on_drop/nightly", "rand/nightly"]
+alloc = ["curve25519-dalek/alloc", "rand/alloc", "zeroize/alloc"]
+nightly = ["curve25519-dalek/nightly", "rand/nightly"]
 batch = ["merlin", "rand"]
 # This feature enables deterministic batch verification.
 batch_deterministic = ["merlin", "rand", "rand_core"]

src/ed25519.rs

@@ -34,7 +34,7 @@ pub use crate::secret::*;
 pub use crate::signature::*;
 
 /// An ed25519 keypair.
-#[derive(Debug, Default)] // we derive Default in order to use the clear() method in Drop
+#[derive(Debug)]
 pub struct Keypair {
     /// The secret half of this keypair.
     pub secret: SecretKey,
@@ -444,26 +444,3 @@ impl<'d> Deserialize<'d> for Keypair {
         deserializer.deserialize_bytes(KeypairVisitor)
     }
 }
-
-#[cfg(test)]
-mod test {
-    use super::*;
-
-    use clear_on_drop::clear::Clear;
-
-    #[test]
-    fn keypair_clear_on_drop() {
-        let mut keypair: Keypair = Keypair::from_bytes(&[1u8; KEYPAIR_LENGTH][..]).unwrap();
-
-        keypair.clear();
-
-        fn as_bytes<T>(x: &T) -> &[u8] {
-            use std::mem;
-            use std::slice;
-
-            unsafe { slice::from_raw_parts(x as *const T as *const u8, mem::size_of_val(x)) }
-        }
-
-        assert!(!as_bytes(&keypair).contains(&0x15));
-    }
-}

src/lib.rs

@@ -239,7 +239,6 @@ extern crate std;
 
 #[cfg(all(feature = "alloc", not(feature = "std")))]
 extern crate alloc;
-extern crate clear_on_drop;
 extern crate curve25519_dalek;
 #[cfg(all(any(feature = "batch", feature = "batch_deterministic"), any(feature = "std", feature = "alloc")))]
 extern crate merlin;
@@ -248,6 +247,7 @@ extern crate rand;
 #[cfg(feature = "serde")]
 extern crate serde;
 extern crate sha2;
+extern crate zeroize;
 
 #[cfg(all(any(feature = "batch", feature = "batch_deterministic"), any(feature = "std", feature = "alloc")))]
 mod batch;

src/secret.rs

@@ -11,8 +11,6 @@
 
 use core::fmt::Debug;
 
-use clear_on_drop::clear::Clear;
-
 use curve25519_dalek::constants;
 use curve25519_dalek::digest::generic_array::typenum::U64;
 use curve25519_dalek::digest::Digest;
@@ -32,13 +30,19 @@ use serde::{Deserialize, Serialize};
 #[cfg(feature = "serde")]
 use serde::{Deserializer, Serializer};
 
+use zeroize::Zeroize;
+
 use crate::constants::*;
 use crate::errors::*;
 use crate::public::*;
 use crate::signature::*;
 
 /// An EdDSA secret key.
-#[derive(Default)] // we derive Default in order to use the clear() method in Drop
+///
+/// Instances of this secret are automatically overwritten with zeroes when they
+/// fall out of scope.
+#[derive(Zeroize)]
+#[zeroize(drop)] // Overwrite secret key material with null bytes when it goes out of scope.
 pub struct SecretKey(pub(crate) [u8; SECRET_KEY_LENGTH]);
 
 impl Debug for SecretKey {
@@ -47,13 +51,6 @@ 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.clear();
-    }
-}
-
 impl AsRef<[u8]> for SecretKey {
     fn as_ref(&self) -> &[u8] {
         self.as_bytes()
@@ -223,6 +220,9 @@ impl<'d> Deserialize<'d> for SecretKey {
 /// upper half is used a sort of half-baked, ill-designed² pseudo-domain-separation
 /// "nonce"-like thing, which is used during signature production by
 /// concatenating it with the message to be signed before the message is hashed.
+///
+/// Instances of this secret are automatically overwritten with zeroes when they
+/// fall out of scope.
 //
 // ¹ This results in a slight bias towards non-uniformity at one spectrum of
 // the range of valid keys.  Oh well: not my idea; not my problem.
@@ -250,20 +250,13 @@ impl<'d> Deserialize<'d> for SecretKey {
 // same signature scheme, and which both fail in exactly the same way.  For a
 // better-designed, Schnorr-based signature scheme, see Trevor Perrin's work on
 // "generalised EdDSA" and "VXEdDSA".
-#[derive(Default)] // we derive Default in order to use the clear() method in Drop
+#[derive(Zeroize)]
+#[zeroize(drop)] // Overwrite secret key material with null bytes when it goes out of scope.
 pub struct ExpandedSecretKey {
     pub(crate) key: Scalar,
     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.clear();
-        self.nonce.clear();
-    }
-}
-
 impl<'a> From<&'a SecretKey> for ExpandedSecretKey {
     /// Construct an `ExpandedSecretKey` from a `SecretKey`.
     ///
@@ -554,3 +547,23 @@ impl<'d> Deserialize<'d> for ExpandedSecretKey {
         deserializer.deserialize_bytes(ExpandedSecretKeyVisitor)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn secret_key_zeroize_on_drop() {
+        let secret_ptr: *const u8;
+
+        { // scope for the secret to ensure it's been dropped
+            let secret = SecretKey::from_bytes(&[0x15u8; 32][..]).unwrap();
+
+            secret_ptr = secret.0.as_ptr();
+        }
+
+        let memory: &[u8] = unsafe { ::std::slice::from_raw_parts(secret_ptr, 32) };
+
+        assert!(!memory.contains(&0x15));
+    }
+}