diff --git a/Cargo.lock b/Cargo.lock index 8c4e0abd..cfa8c163 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1401,6 +1401,7 @@ dependencies = [ "ssh-key", "tempfile", "thiserror", + "zeroize", ] [[package]] diff --git a/radicle-crypto/Cargo.toml b/radicle-crypto/Cargo.toml index ff5a9ab3..4ebb8736 100644 --- a/radicle-crypto/Cargo.toml +++ b/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" diff --git a/radicle-crypto/src/lib.rs b/radicle-crypto/src/lib.rs index 26e51f70..f9a6b392 100644 --- a/radicle-crypto/src/lib.rs +++ b/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 { + 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 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 { diff --git a/radicle-crypto/src/ssh.rs b/radicle-crypto/src/ssh.rs index 9b969e4f..d0d8c778 100644 --- a/radicle-crypto/src/ssh.rs +++ b/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 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 { @@ -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] diff --git a/radicle-crypto/src/ssh/agent.rs b/radicle-crypto/src/ssh/agent.rs index 02e8e087..6f3d9854 100644 --- a/radicle-crypto/src/ssh/agent.rs +++ b/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. diff --git a/radicle-crypto/src/ssh/keystore.rs b/radicle-crypto/src/ssh/keystore.rs index 5d6ddb94..7bbb3e51 100644 --- a/radicle-crypto/src/ssh/keystore.rs +++ b/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, Error> { + pub fn secret_key(&self, passphrase: &str) -> Result>, 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, } impl Signer for MemorySigner { diff --git a/radicle-crypto/src/test/arbitrary.rs b/radicle-crypto/src/test/arbitrary.rs index 25443de7..1610166c 100644 --- a/radicle-crypto/src/test/arbitrary.rs +++ b/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)) } } diff --git a/radicle-crypto/src/test/signer.rs b/radicle-crypto/src/test/signer.rs index cc8fa277..f28e83ad 100644 --- a/radicle-crypto/src/test/signer.rs +++ b/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(), } } }