Improving safety of handling secret keys in different forms

Cargo.toml

@@ -27,6 +27,7 @@ recovery = ["secp256k1-sys/recovery"]
 lowmemory = ["secp256k1-sys/lowmemory"]
 global-context = ["std", "rand-std", "global-context-less-secure"]
 global-context-less-secure = []
+serde-secrets = ["serde", "std"]
 
 [dependencies]
 secp256k1-sys = { version = "0.4.1", default-features = false, path = "./secp256k1-sys" }

contrib/test.sh

@@ -1,6 +1,6 @@
 #!/bin/sh -ex
 
-FEATURES="bitcoin_hashes global-context lowmemory rand rand-std recovery serde"
+FEATURES="bitcoin_hashes global-context lowmemory rand rand-std recovery serde serde-secrets"
 
 # Use toolchain if explicitly specified
 if [ -n "$TOOLCHAIN" ]

secp256k1-sys/src/lib.rs

@@ -99,6 +99,7 @@ pub type SchnorrNonceFn = Option<unsafe extern "C" fn(
 
 /// Library-internal representation of a Secp256k1 public key
 #[repr(C)]
+#[derive(Copy)]
 pub struct PublicKey([c_uchar; 64]);
 impl_array_newtype!(PublicKey, c_uchar, 64);
 impl_raw_debug!(PublicKey);
@@ -141,6 +142,7 @@ impl hash::Hash for PublicKey {
 
 /// Library-internal representation of a Secp256k1 signature
 #[repr(C)]
+#[derive(Copy)]
 pub struct Signature([c_uchar; 64]);
 impl_array_newtype!(Signature, c_uchar, 64);
 impl_raw_debug!(Signature);
@@ -176,6 +178,7 @@ impl Signature {
 }
 
 #[repr(C)]
+#[derive(Copy)]
 pub struct XOnlyPublicKey([c_uchar; 64]);
 impl_array_newtype!(XOnlyPublicKey, c_uchar, 64);
 impl_raw_debug!(XOnlyPublicKey);
@@ -221,6 +224,12 @@ pub struct KeyPair([c_uchar; 96]);
 impl_array_newtype!(KeyPair, c_uchar, 96);
 impl_raw_debug!(KeyPair);
 
+impl Drop for KeyPair {
+    fn drop(&mut self) {
+        self.0.copy_from_slice(&[0u8; 96]);
+    }
+}
+
 impl KeyPair {
     /// Creates an "uninitialized" FFI keypair which is zeroed out
     ///

secp256k1-sys/src/macros.rs

@@ -13,27 +13,52 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
-// This is a macro that routinely comes in handy
+/// Implements newtype wrapping methods
 #[macro_export]
-macro_rules! impl_array_newtype {
-    ($thing:ident, $ty:ty, $len:expr) => {
-        impl Copy for $thing {}
-
+macro_rules! impl_ptr_newtype {
+    ($thing:ident, $ty:ty) => {
         impl $thing {
             #[inline]
+            #[allow(unused)]
             /// Converts the object to a raw pointer for FFI interfacing
-            pub fn as_ptr(&self) -> *const $ty {
+            pub(crate) fn as_ptr(&self) -> *const $ty {
                 let &$thing(ref dat) = self;
                 dat.as_ptr()
             }
 
             #[inline]
+            #[allow(unused)]
             /// Converts the object to a mutable raw pointer for FFI interfacing
-            pub fn as_mut_ptr(&mut self) -> *mut $ty {
+            pub(crate) fn as_mut_ptr(&mut self) -> *mut $ty {
+                let &mut $thing(ref mut dat) = self;
+                dat.as_mut_ptr()
+            }
+        }
+
+        impl $crate::CPtr for $thing {
+            type Target = $ty;
+
+            fn as_c_ptr(&self) -> *const Self::Target {
+                let &$thing(ref dat) = self;
+                dat.as_ptr()
+            }
+
+            fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
                 let &mut $thing(ref mut dat) = self;
                 dat.as_mut_ptr()
             }
+        }
+    }
+}
 
+/// Generates implementation of main array accessing methods for a newtype wrapping some inner array
+/// type
+#[macro_export]
+macro_rules! impl_array_newtype {
+    ($thing:ident, $ty:ty, $len:expr) => {
+        impl_ptr_newtype!($thing, $ty);
+
+        impl $thing {
             #[inline]
             /// Returns the length of the object as an array
             pub fn len(&self) -> usize { $len }
@@ -43,6 +68,14 @@ macro_rules! impl_array_newtype {
             pub fn is_empty(&self) -> bool { false }
         }
 
+        impl Clone for $thing {
+            #[inline]
+            fn clone(&self) -> $thing {
+                let &$thing(ref dat) = self;
+                $thing(dat.clone())
+            }
+        }
+
         impl AsRef<[$ty; $len]> for $thing {
             #[inline]
             /// Gets a reference to the underlying array
@@ -55,7 +88,7 @@ macro_rules! impl_array_newtype {
         impl PartialEq for $thing {
             #[inline]
             fn eq(&self, other: &$thing) -> bool {
-                &self[..] == &other[..]
+                &self.0[..] == &other[..]
             }
         }
 
@@ -75,14 +108,6 @@ macro_rules! impl_array_newtype {
             }
         }
 
-        impl Clone for $thing {
-            #[inline]
-            fn clone(&self) -> $thing {
-                let &$thing(ref dat) = self;
-                $thing(dat.clone())
-            }
-        }
-
         impl ::core::ops::Index<usize> for $thing {
             type Output = $ty;
 
@@ -132,24 +157,6 @@ macro_rules! impl_array_newtype {
                 &dat[..]
             }
         }
-        impl $crate::CPtr for $thing {
-            type Target = $ty;
-            fn as_c_ptr(&self) -> *const Self::Target {
-                if self.is_empty() {
-                    ::core::ptr::null()
-                } else {
-                    self.as_ptr()
-                }
-            }
-
-            fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
-                if self.is_empty() {
-                    ::core::ptr::null::<Self::Target>() as *mut _
-                } else {
-                    self.as_mut_ptr()
-                }
-            }
-        }
     }
 }
 
@@ -158,7 +165,7 @@ macro_rules! impl_raw_debug {
     ($thing:ident) => {
         impl ::core::fmt::Debug for $thing {
             fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
-                for i in self[..].iter().cloned() {
+                for i in &self[..] {
                     write!(f, "{:02x}", i)?;
                 }
                 Ok(())

secp256k1-sys/src/recovery.rs

@@ -20,6 +20,7 @@ use {Context, Signature, NonceFn, PublicKey};
 
 /// Library-internal representation of a Secp256k1 signature + recovery ID
 #[repr(C)]
+#[derive(Copy)]
 pub struct RecoverableSignature([c_uchar; 65]);
 impl_array_newtype!(RecoverableSignature, c_uchar, 65);
 impl_raw_debug!(RecoverableSignature);

src/context.rs

@@ -1,5 +1,5 @@
 use core::marker::PhantomData;
-use core::mem::{self, ManuallyDrop};
+use core::mem::ManuallyDrop;
 use ffi::{self, CPtr, types::AlignedType};
 use ffi::types::{c_uint, c_void};
 use Error;
@@ -105,7 +105,7 @@ mod alloc_only {
     impl private::Sealed for VerifyOnly {}
 
     use super::*;
-    const ALIGN_TO: usize = mem::align_of::<AlignedType>();
+    const ALIGN_TO: usize = ::core::mem::align_of::<AlignedType>();
 
     /// Represents the set of capabilities needed for signing.
     pub enum SignOnly {}