implement insecure-erase feature

Author: kwantam

Cargo.toml

@@ -35,6 +35,7 @@ global-context = ["std"]
 # if you are doing a no-std build, then this feature does nothing
 # and is not necessary.)
 global-context-less-secure = ["global-context"]
+insecure-erase = ["secp256k1-sys/insecure-erase"]
 
 [dependencies]
 secp256k1-sys = { version = "0.8.0", default-features = false, path = "./secp256k1-sys" }

README.md

@@ -38,6 +38,10 @@ Alternatively add symlinks in your `.git/hooks` directory to any of the githooks
 We use a custom Rust compiler configuration conditional to guard the bench mark code. To run the
 bench marks use: `RUSTFLAGS='--cfg=bench' cargo +nightly bench --features=recovery`.
 
+### A note on the `insecure-erase` feature
+
+The `insecure-erase` feature is provided to assist other libraries in building secure secret erasure. When the `insecure-erase` feature is enabled, secret types (`SecretKey`, `KeyPair`, `SharedSecret`, `Scalar`, and `DisplaySecret`) have a method called `insecure_erase` that *attempts* to overwrite the contained secret. This library makes no guarantees about the security of using `insecure_erase`. In particular, since all of these types are `Copy`, the compiler is free to move and copy them, thereby making additional copies in memory. For more information, consult the [`zeroize`](https://docs.rs/zeroize) documentation.
+
 ## Fuzzing
 
 If you want to fuzz this library, or any library which depends on it, you will

secp256k1-sys/Cargo.toml

@@ -32,4 +32,4 @@ recovery = []
 lowmemory = []
 std = ["alloc"]
 alloc = []
-
+insecure-erase = []

secp256k1-sys/src/lib.rs

@@ -454,6 +454,33 @@ impl KeyPair {
             pk
         }
     }
+
+    /// This function attempts to erase the contents of the underlying array.
+    /// Note, however, that since this type is `Copy`, ensuring that the contents
+    /// of this array don't end up elsewhere in memory is very subtle. For more
+    /// discussion on this, please see the documentation of the [`zeroize`] crate.
+    /// [`zeroize`]: https://docs.rs/zeroize
+    #[cfg(feature = "insecure-erase")]
+    #[inline]
+    pub fn insecure_erase(&mut self) {
+        // DUMMY is a valid key pair with secret key `[1u8; 32]`
+        const DUMMY: [c_uchar; 96] = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 143, 7, 221, 213, 233, 245, 23, 156, 255, 25, 72, 96, 52, 24, 30, 215, 101, 5, 186, 170, 213, 62, 93, 153, 64, 100, 18, 123, 86, 197, 132, 27, 209, 232, 168, 105, 122, 212, 34, 81, 222, 57, 246, 167, 32, 129, 223, 223, 66, 171, 197, 66, 166, 214, 254, 7, 21, 84, 139, 88, 143, 175, 190, 112];
+        insecure_erase_impl(&mut self.0, DUMMY);
+    }
+}
+
+/// This function implements a best attempt at secure erasure using Rust intrinsics.
+/// The implementation is based on the approach used by the [`zeroize`] crate.
+/// [`zeroize`]: https://docs.rs/zeroize
+#[cfg(feature = "insecure-erase")]
+#[inline(always)]
+pub fn insecure_erase_impl<T>(dst: &mut T, src: T) {
+    use core::sync::atomic;
+    // overwrite using volatile value
+    unsafe { ptr::write_volatile(dst, src); }
+
+    // prevent future accesses from being reordered to before erasure
+    atomic::compiler_fence(atomic::Ordering::SeqCst);
 }
 
 #[cfg(not(fuzzing))]

src/ecdh.rs

@@ -46,6 +46,7 @@ const SHARED_SECRET_SIZE: usize = constants::SECRET_KEY_SIZE;
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct SharedSecret([u8; SHARED_SECRET_SIZE]);
 impl_display_secret!(SharedSecret);
+impl_insecure_erase!(SharedSecret, 0, [0u8; SHARED_SECRET_SIZE]);
 
 impl SharedSecret {
     /// Creates a new shared secret from a pubkey and secret key.

src/key.rs

@@ -66,6 +66,7 @@ use crate::{hashes, ThirtyTwoByteHash};
 #[derive(Copy, Clone)]
 pub struct SecretKey([u8; constants::SECRET_KEY_SIZE]);
 impl_display_secret!(SecretKey);
+impl_insecure_erase!(SecretKey, 0, [1u8; constants::SECRET_KEY_SIZE]);
 
 impl PartialEq for SecretKey {
     /// This implementation is designed to be constant time to help prevent side channel attacks.
@@ -992,6 +993,15 @@ impl KeyPair {
     pub fn sign_schnorr(&self, msg: Message) -> schnorr::Signature {
         SECP256K1.sign_schnorr(&msg, self)
     }
+
+    /// This function attempts to erase the contents of the underlying array.
+    /// Note, however, that since this type is `Copy`, ensuring that the contents
+    /// of this array don't end up elsewhere in memory is very subtle. For more
+    /// discussion on this, please see the documentation of the [`zeroize`] crate.
+    /// [`zeroize`]: https://docs.rs/zeroize
+    #[inline]
+    #[cfg(feature = "insecure-erase")]
+    pub fn insecure_erase(&mut self) { self.0.insecure_erase(); }
 }
 
 impl From<KeyPair> for SecretKey {
@@ -1603,6 +1613,17 @@ mod test {
         assert_eq!(PublicKey::from_slice(&pk1.serialize_uncompressed()[..]), Ok(pk1));
     }
 
+    #[test]
+    #[cfg(feature = "insecure-erase")]
+    fn erased_keypair_is_valid() {
+        let s = Secp256k1::new();
+        let kp = KeyPair::from_seckey_slice(&s, &[1u8; constants::SECRET_KEY_SIZE])
+            .expect("valid secret key");
+        let mut kp2 = kp;
+        kp2.insecure_erase();
+        assert!(kp.eq_fast_unstable(&kp2));
+    }
+
     #[test]
     #[rustfmt::skip]
     fn invalid_secret_key() {

src/macros.rs

@@ -91,6 +91,23 @@ macro_rules! impl_pretty_debug {
     };
 }
 
+macro_rules! impl_insecure_erase {
+    ($thing:ident, $target:tt, $value:expr) => {
+        #[cfg(feature = "insecure-erase")]
+        impl $thing {
+            /// This function attempts to erase the contents of the underlying array.
+            /// Note, however, that since this type is `Copy`, ensuring that the contents
+            /// of this array don't end up elsewhere in memory is very subtle. For more
+            /// discussion on this, please see the documentation of the [`zeroize`] crate.
+            /// [`zeroize`]: https://docs.rs/zeroize
+            #[inline]
+            pub fn insecure_erase(&mut self) {
+                secp256k1_sys::insecure_erase_impl(&mut self.$target, $value);
+            }
+        }
+    };
+}
+
 /// Formats error. If `std` feature is OFF appends error source (delimited by `: `). We do this
 /// because `e.source()` is only available in std builds, without this macro the error source is
 /// lost for no-std builds.

src/scalar.rs

@@ -23,6 +23,7 @@ use crate::constants;
 #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
 pub struct Scalar([u8; 32]);
 impl_pretty_debug!(Scalar);
+impl_insecure_erase!(Scalar, 0, [0u8; 32]);
 
 const MAX_RAW: [u8; 32] = [
     0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFE,

src/secret.rs

@@ -86,6 +86,7 @@ macro_rules! impl_display_secret {
 pub struct DisplaySecret {
     secret: [u8; SECRET_KEY_SIZE],
 }
+impl_insecure_erase!(DisplaySecret, secret, [0u8; SECRET_KEY_SIZE]);
 
 impl fmt::Debug for DisplaySecret {
     #[inline]