Zeroize secret key in signer

Signed-off-by: Alexis Sellier <alexis@radicle.xyz>

Cargo.lock

@@ -1401,6 +1401,7 @@ dependencies = [
  "ssh-key",
  "tempfile",
  "thiserror",
+ "zeroize",
 ]
 
 [[package]]

radicle-crypto/Cargo.toml

@@ -18,6 +18,7 @@ multibase = { version = "0.9.1" }
 serde = { version = "1", features = ["derive"] }
 sqlite = { version = "0.27.0", optional = true }
 thiserror = { version = "1" }
+zeroize = { version = "1.5.7" }
 
 [dependencies.fastrand]
 version = "1.8.0"

radicle-crypto/src/lib.rs

@@ -138,7 +138,48 @@ impl PublicKey {
 }
 
 /// The private/signing key.
-pub type SecretKey = ed25519::SecretKey;
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
+pub struct SecretKey(ed25519::SecretKey);
+
+impl zeroize::Zeroize for SecretKey {
+    fn zeroize(&mut self) {
+        self.0.zeroize();
+    }
+}
+
+impl TryFrom<&[u8]> for SecretKey {
+    type Error = ed25519::Error;
+
+    fn try_from(bytes: &[u8]) -> Result<Self, ed25519::Error> {
+        ed25519::SecretKey::from_slice(bytes).map(Self)
+    }
+}
+
+impl AsRef<[u8]> for SecretKey {
+    fn as_ref(&self) -> &[u8] {
+        &*self.0
+    }
+}
+
+impl From<[u8; 64]> for SecretKey {
+    fn from(bytes: [u8; 64]) -> Self {
+        Self(ed25519::SecretKey::new(bytes))
+    }
+}
+
+impl From<ed25519::SecretKey> for SecretKey {
+    fn from(other: ed25519::SecretKey) -> Self {
+        Self(other)
+    }
+}
+
+impl Deref for SecretKey {
+    type Target = ed25519::SecretKey;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
 
 #[derive(Error, Debug)]
 pub enum PublicKeyError {

radicle-crypto/src/ssh.rs

@@ -146,16 +146,6 @@ impl Encodable for PublicKey {
     }
 }
 
-// FIXME: Should zeroize, or we should be creating our own type
-// in `crypto`.
-struct SecretKey(crypto::SecretKey);
-
-impl From<crypto::SecretKey> for SecretKey {
-    fn from(other: crypto::SecretKey) -> Self {
-        Self(other)
-    }
-}
-
 #[derive(Debug, Error)]
 pub enum SecretKeyError {
     #[error(transparent)]
@@ -170,7 +160,7 @@ pub enum SecretKeyError {
     Mismatch,
 }
 
-impl Encodable for SecretKey {
+impl Encodable for crypto::SecretKey {
     type Error = SecretKeyError;
 
     fn read(r: &mut encoding::Cursor) -> Result<Self, Self::Error> {
@@ -179,12 +169,12 @@ impl Encodable for SecretKey {
                 let public = r.read_string()?;
                 let pair = r.read_string()?;
                 let _comment = r.read_string()?;
-                let key = crypto::SecretKey::from_slice(pair)?;
+                let key = crypto::SecretKey::try_from(pair)?;
 
                 if public != key.public_key().as_ref() {
                     return Err(SecretKeyError::Mismatch);
                 }
-                Ok(SecretKey(key))
+                Ok(key)
             }
             s => Err(SecretKeyError::UnknownAlgorithm(
                 String::from_utf8_lossy(s).to_string(),
@@ -197,7 +187,7 @@ impl Encodable for SecretKey {
 
         buf.extend_ssh_string(b"ssh-ed25519");
         buf.extend_ssh_string(public.as_ref());
-        buf.extend_ssh_string(&*self.0);
+        buf.extend_ssh_string(self.0.as_ref());
         buf.extend_ssh_string(b"radicle");
     }
 }
@@ -317,10 +307,10 @@ mod test {
 
     use quickcheck_macros::quickcheck;
 
-    use super::{fmt, ExtendedSignature, SecretKey};
+    use super::{fmt, ExtendedSignature};
     use crate as crypto;
     use crate::test::arbitrary::ByteArray;
-    use crate::PublicKey;
+    use crate::{PublicKey, SecretKey};
     use radicle_ssh::agent::client::{AgentClient, ClientStream, Error};
     use radicle_ssh::encoding::*;
 
@@ -347,13 +337,13 @@ mod test {
     #[quickcheck]
     fn prop_encode_decode_sk(input: ByteArray<64>) {
         let mut buf = Buffer::default();
-        let sk = crypto::SecretKey::new(input.into_inner());
-        SecretKey(sk).write(&mut buf);
+        let sk = crypto::SecretKey::from(input.into_inner());
+        sk.write(&mut buf);
 
         let mut cursor = buf.reader(0);
         let output = SecretKey::read(&mut cursor).unwrap();
 
-        assert_eq!(sk, output.0);
+        assert_eq!(sk, output);
     }
 
     #[test]

radicle-crypto/src/ssh/agent.rs

@@ -5,9 +5,7 @@ pub use radicle_ssh::agent::client::AgentClient;
 pub use radicle_ssh::agent::client::Error;
 pub use radicle_ssh::{self as ssh, agent::client::ClientStream};
 
-use crate::ssh::SecretKey;
-use crate::{self as crypto};
-use crate::{PublicKey, Signature, Signer, SignerError};
+use crate::{PublicKey, SecretKey, Signature, Signer, SignerError};
 
 #[cfg(not(unix))]
 pub use std::net::TcpStream as Stream;
@@ -25,8 +23,8 @@ impl Agent {
     }
 
     /// Register a key with the agent.
-    pub fn register(&mut self, key: &crypto::SecretKey) -> Result<(), ssh::Error> {
-        self.client.add_identity(&SecretKey::from(*key), &[])
+    pub fn register(&mut self, key: &SecretKey) -> Result<(), ssh::Error> {
+        self.client.add_identity(key, &[])
     }
 
     /// Get a signer from this agent, given the public key.

radicle-crypto/src/ssh/keystore.rs

@@ -3,6 +3,7 @@ use std::path::{Path, PathBuf};
 use std::{fs, io};
 
 use thiserror::Error;
+use zeroize::Zeroizing;
 
 use crate::{KeyPair, PublicKey, SecretKey, Signature, Signer, SignerError};
 
@@ -82,7 +83,7 @@ impl Keystore {
 
     /// Load the secret key from the store, decrypting it with the given passphrase.
     /// Returns `None` if it wasn't found.
-    pub fn secret_key(&self, passphrase: &str) -> Result<Option<SecretKey>, Error> {
+    pub fn secret_key(&self, passphrase: &str) -> Result<Option<Zeroizing<SecretKey>>, Error> {
         let path = self.path.join("radicle");
         if !path.exists() {
             return Ok(None);
@@ -93,7 +94,7 @@ impl Keystore {
 
         match secret.key_data() {
             ssh_key::private::KeypairData::Ed25519(pair) => {
-                Ok(Some(SecretKey::new(pair.to_bytes())))
+                Ok(Some(SecretKey::from(pair.to_bytes()).into()))
             }
             _ => Err(Error::InvalidKeyType),
         }
@@ -115,7 +116,7 @@ pub enum MemorySignerError {
 #[derive(Debug)]
 pub struct MemorySigner {
     public: PublicKey,
-    secret: SecretKey,
+    secret: Zeroizing<SecretKey>,
 }
 
 impl Signer for MemorySigner {

radicle-crypto/src/test/arbitrary.rs

@@ -1,6 +1,6 @@
 use quickcheck::Arbitrary;
 
-use crate::{test::signer::MockSigner, KeyPair, PublicKey, Seed};
+use crate::{test::signer::MockSigner, KeyPair, PublicKey, SecretKey, Seed};
 
 impl Arbitrary for MockSigner {
     fn arbitrary(g: &mut quickcheck::Gen) -> Self {
@@ -8,7 +8,7 @@ impl Arbitrary for MockSigner {
         let seed = Seed::new(bytes.into_inner());
         let sk = KeyPair::from_seed(seed).sk;
 
-        MockSigner::from(sk)
+        MockSigner::from(SecretKey::from(sk))
     }
 }
 

radicle-crypto/src/test/signer.rs

@@ -16,7 +16,7 @@ impl MockSigner {
         let seed = Seed::new(bytes);
         let keypair = KeyPair::from_seed(seed);
 
-        Self::from(keypair.sk)
+        Self::from(SecretKey::from(keypair.sk))
     }
 }
 
@@ -35,7 +35,7 @@ impl Default for MockSigner {
 
         Self {
             pk: sk.public_key().into(),
-            sk,
+            sk: sk.into(),
         }
     }
 }