parity-crypto: use mlock for Secret

parity-crypto/Cargo.toml

@@ -31,6 +31,8 @@ sha2 = "0.8.0"
 subtle = "2.2.1"
 tiny-keccak = { version = "2.0", features = ["keccak"] }
 zeroize = { version = "1.0.0", default-features = false }
+region = { version = "2.2.0", optional = true }
+log = { version = "0.4.11", optional = true }
 
 [dev-dependencies]
 criterion = "0.3.0"
@@ -40,4 +42,4 @@ hex-literal = "0.2.1"
 default = []
 # public key crypto utils
 # moved from ethkey module in parity ethereum repository
-publickey = ["secp256k1", "lazy_static", "ethereum-types", "rustc-hex"]
+publickey = ["ethereum-types", "lazy_static", "log", "region", "rustc-hex", "secp256k1"]

parity-crypto/src/publickey/secret_key.rs

@@ -21,14 +21,29 @@ use zeroize::Zeroize;
 use crate::publickey::Error;
 
 /// Represents secret key
-#[derive(Clone, PartialEq, Eq)]
 pub struct Secret {
+	// We're using `region::lock` to avoid swapping secret data to disk.
+	//
+	// The potential error is ignored as this security measure is optional
+	// and the syscall may fail due to multiple reasons, e.g. on windows
+	// there is a small amount of maximal pages a process can lock by default.
+	// Also it's not guaranteed that a memory page won't be written to disk,
+	// e.g. in case of hibernation or memory starvation.
+	//
+	// Since we don't propagate the error if a syscall fails,
+	// the guard returned by `region::lock` is optional.
+	mlock_guard: Option<region::LockGuard>,
 	inner: Box<H256>,
 }
 
 impl Drop for Secret {
 	fn drop(&mut self) {
-		self.inner.0.zeroize()
+		self.inner.0.zeroize();
+		// `region::LockGuard::drop` will panic only if
+		// `region::unlock()` has failed for some reason
+		// AND `debug_assertions` are enabled.
+		// https://docs.rs/region/2.2.0/src/region/lock.rs.html#86-91
+		let _ = self.mlock_guard.take();
 	}
 }
 
@@ -57,23 +72,36 @@ impl Secret {
 		if key.len() != 32 {
 			return None;
 		}
-		let mut h = H256::zero();
-		h.as_bytes_mut().copy_from_slice(&key[0..32]);
-		Some(Secret { inner: Box::new(h) })
+		let mut me = Self::zero();
+		me.inner.as_bytes_mut().copy_from_slice(&key[0..32]);
+		Some(me)
 	}
 
 	/// Creates a `Secret` from the given `str` representation,
 	/// returning an error for hex big endian representation of
 	/// the secret.
 	/// Caller is responsible to zeroize input slice.
 	pub fn copy_from_str(s: &str) -> Result<Self, Error> {
-		let h = H256::from_str(s).map_err(|e| Error::Custom(format!("{:?}", e)))?;
-		Ok(Secret { inner: Box::new(h) })
+		let mut h = H256::from_str(s).map_err(|e| Error::Custom(format!("{:?}", e)))?;
+		let mut me = Self::zero();
+		me.inner.as_bytes_mut().copy_from_slice(h.as_bytes());
+		h.0.zeroize();
+		Ok(me)
 	}
 
 	/// Creates zero key, which is invalid for crypto operations, but valid for math operation.
 	pub fn zero() -> Self {
-		Secret { inner: Box::new(H256::zero()) }
+		let inner = Box::new(H256::zero());
+		let bytes = inner.as_bytes();
+		// Try to lock the memory page and convert the result to `Option`.
+		let mlock_guard = match region::lock(bytes.as_ptr(), bytes.len()) {
+			Ok(guard) => Some(guard),
+			Err(e) => {
+				log::warn!("Failed to lock memory page with a Secret: {}", e);
+				None
+			}
+		};
+		Self { mlock_guard, inner }
 	}
 
 	/// Imports and validates the key.
@@ -98,7 +126,7 @@ impl Secret {
 		match (self.is_zero(), other.is_zero()) {
 			(true, true) | (false, true) => Ok(()),
 			(true, false) => {
-				*self = other.clone();
+				self.clone_from(other);
 				Ok(())
 			}
 			(false, false) => {
@@ -118,7 +146,7 @@ impl Secret {
 		match (self.is_zero(), other.is_zero()) {
 			(true, true) | (false, true) => Ok(()),
 			(true, false) => {
-				*self = other.clone();
+				self.clone_from(other);
 				self.neg()
 			}
 			(false, false) => {
@@ -213,6 +241,25 @@ impl Secret {
 	}
 }
 
+impl Clone for Secret {
+	fn clone(&self) -> Self {
+		let mut copy = Self::zero();
+		copy.clone_from(self);
+		copy
+	}
+	fn clone_from(&mut self, other: &Self) {
+		self.inner.as_bytes_mut().copy_from_slice(other.inner.as_bytes());
+	}
+}
+
+impl PartialEq for Secret {
+	fn eq(&self, other: &Self) -> bool {
+		self.inner == other.inner
+	}
+}
+
+impl Eq for Secret {}
+
 #[deprecated(since = "0.6.2", note = "please use `copy_from_str` instead, input is not zeroized")]
 impl FromStr for Secret {
 	type Err = Error;
@@ -224,18 +271,22 @@ impl FromStr for Secret {
 impl From<[u8; 32]> for Secret {
 	#[inline(always)]
 	fn from(mut k: [u8; 32]) -> Self {
-		let result = Secret { inner: Box::new(H256(k)) };
+		let mut me = Self::zero();
+		let mut secret = H256(k);
+		me.inner.as_bytes_mut().copy_from_slice(secret.as_bytes());
+		secret.0.zeroize();
 		k.zeroize();
-		result
+		me
 	}
 }
 
 impl From<H256> for Secret {
 	#[inline(always)]
 	fn from(mut s: H256) -> Self {
-		let result = s.0.into();
+		let mut me = Self::zero();
+		me.inner.as_bytes_mut().copy_from_slice(s.as_bytes());
 		s.0.zeroize();
-		result
+		me
 	}
 }
 
@@ -256,7 +307,7 @@ impl TryFrom<&[u8]> for Secret {
 		if b.len() != SECP256K1_SECRET_KEY_SIZE {
 			return Err(Error::InvalidSecretKey);
 		}
-		Ok(Self { inner: Box::new(H256::from_slice(b)) })
+		Ok(Self::copy_from_slice(b).expect("checked len above; qed"))
 	}
 }