diff --git a/crates/radicle-cli/examples/rad-key-mismatch.md b/crates/radicle-cli/examples/rad-key-mismatch.md new file mode 100644 index 00000000..0366a828 --- /dev/null +++ b/crates/radicle-cli/examples/rad-key-mismatch.md @@ -0,0 +1,6 @@ +This test assumes that one of the two keys in `$RAD_HOME/keys` was swapped so that `$RAD_HOME/keys/radicle{,.pub}` do not match anymore. + +``` (fail) +$ rad issue open --title "flux capacitor underpowered" --description "Flux capacitor power requirements exceed current supply" --no-announce +✗ Error: secret and public keys in '[..]/.radicle/keys' do not match +``` \ No newline at end of file diff --git a/crates/radicle-cli/src/terminal/io.rs b/crates/radicle-cli/src/terminal/io.rs index c1538837..c3d47039 100644 --- a/crates/radicle-cli/src/terminal/io.rs +++ b/crates/radicle-cli/src/terminal/io.rs @@ -45,9 +45,15 @@ impl inquire::validator::StringValidator for PassphraseValidator { /// Get the signer. First we try getting it from ssh-agent, otherwise we prompt the user, /// if we're connected to a TTY. pub fn signer(profile: &Profile) -> anyhow::Result { - if let Ok(signer) = profile.signer() { - return Ok(signer); + match profile.signer() { + Ok(signer) => return Ok(signer), + Err(err) if !err.prompt_for_passphrase() => return Err(anyhow!(err)), + Err(_) => { + // The error returned is potentially recoverable by prompting + // the user for the correct passphrase. + } } + let validator = PassphraseValidator::new(profile.keystore.clone()); let passphrase = match passphrase(validator)? { Some(p) => p, diff --git a/crates/radicle-cli/tests/commands.rs b/crates/radicle-cli/tests/commands.rs index 5a8b8f3a..4a09df05 100644 --- a/crates/radicle-cli/tests/commands.rs +++ b/crates/radicle-cli/tests/commands.rs @@ -1,6 +1,6 @@ use std::path::Path; use std::str::FromStr; -use std::{net, thread, time}; +use std::{fs, net, thread, time}; use radicle::cob; use radicle::git; @@ -55,6 +55,20 @@ fn rad_auth() { test("examples/rad-auth.md", Path::new("."), None, []).unwrap(); } +#[test] +fn rad_key_mismatch() { + let mut environment = Environment::new(); + let alice = environment.profile("alice"); + environment.repository(&alice); + + environment.test("rad-init", &alice).unwrap(); + + // Replace the public key with one that does not match the secret key anymore. + fs::write(alice.home.path().join("keys").join("radicle.pub"), "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIE6Ul/D+P0I/Hl1JVOWGS8Z589us9FqKQXWv8OMOpKCh snakeoil\n").unwrap(); + + environment.test("rad-key-mismatch", &alice).unwrap(); +} + #[test] fn rad_auth_errors() { test("examples/rad-auth-errors.md", Path::new("."), None, []).unwrap(); diff --git a/crates/radicle-crypto/src/ssh/keystore.rs b/crates/radicle-crypto/src/ssh/keystore.rs index 75e26b65..ab5e4868 100644 --- a/crates/radicle-crypto/src/ssh/keystore.rs +++ b/crates/radicle-crypto/src/ssh/keystore.rs @@ -183,6 +183,8 @@ pub enum MemorySignerError { NotFound(PathBuf), #[error("invalid passphrase")] InvalidPassphrase, + #[error("secret and public keys in '{path}' do not match")] + KeyMismatch { path: PathBuf }, } /// An in-memory signer that keeps its secret key internally @@ -272,6 +274,12 @@ impl MemorySigner { })? .ok_or_else(|| MemorySignerError::NotFound(keystore.path().to_path_buf()))?; + secret + .validate_public_key(&public) + .map_err(|_| MemorySignerError::KeyMismatch { + path: keystore.path().to_path_buf(), + })?; + Ok(Self { public, secret }) } diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index c5cc75f7..2d305c07 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -74,6 +74,9 @@ pub enum Error { /// Profile error. #[error(transparent)] Profile(#[from] radicle::profile::Error), + /// Signer error. + #[error(transparent)] + Signer(#[from] radicle::profile::SignerError), /// Parse error for object IDs. #[error(transparent)] ParseObjectId(#[from] ParseObjectId), diff --git a/crates/radicle/src/profile.rs b/crates/radicle/src/profile.rs index 0533b1cd..739a6f42 100644 --- a/crates/radicle/src/profile.rs +++ b/crates/radicle/src/profile.rs @@ -172,14 +172,8 @@ pub enum Error { Routing(#[from] node::routing::Error), #[error(transparent)] Keystore(#[from] keystore::Error), - #[error(transparent)] - MemorySigner(#[from] keystore::MemorySignerError), #[error("no radicle profile found at path '{0}'")] NotFound(PathBuf), - #[error("error connecting to ssh-agent: {0}")] - Agent(#[from] crate::crypto::ssh::agent::Error), - #[error("radicle key `{0}` is not registered; run `rad auth` to register it with ssh-agent")] - KeyNotRegistered(PublicKey), #[error(transparent)] PolicyStore(#[from] node::policy::store::Error), #[error(transparent)] @@ -194,6 +188,37 @@ pub enum Error { Storage(#[from] storage::Error), } +#[derive(Debug, Error)] +pub enum SignerError { + #[error(transparent)] + MemorySigner(#[from] keystore::MemorySignerError), + + #[error(transparent)] + Agent(#[from] crate::crypto::ssh::agent::Error), + + #[error("radicle key `{0}` is not registered; run `rad auth` to register it with ssh-agent")] + KeyNotRegistered(PublicKey), + + #[error(transparent)] + Keystore(#[from] keystore::Error), + + #[error("error connecting to ssh-agent: {source}")] + AgentConnection { + source: crate::crypto::ssh::agent::Error, + }, +} + +impl SignerError { + /// Some signer errors are potentially recoverable by prompting the user + /// for a password. + pub fn prompt_for_passphrase(&self) -> bool { + matches!( + self, + Self::AgentConnection { .. } | Self::KeyNotRegistered(_) + ) + } +} + #[derive(Debug, Clone)] pub struct Profile { pub home: Home, @@ -296,7 +321,7 @@ impl Profile { Did::from(self.public_key) } - pub fn signer(&self) -> Result { + pub fn signer(&self) -> Result { if !self.keystore.is_encrypted()? { let signer = keystore::MemorySigner::load(&self.keystore, None)?; return Ok(Device::from(signer).boxed()); @@ -307,16 +332,12 @@ impl Profile { return Ok(Device::from(signer).boxed()); } - match Agent::connect() { - Ok(agent) => { - let signer = agent.signer(self.public_key); - if signer.is_ready()? { - Ok(Device::from(signer).boxed()) - } else { - Err(Error::KeyNotRegistered(self.public_key)) - } - } - Err(err) => Err(err.into()), + let agent = Agent::connect().map_err(|source| SignerError::AgentConnection { source })?; + let signer = agent.signer(self.public_key); + if signer.is_ready()? { + Ok(Device::from(signer).boxed()) + } else { + Err(SignerError::KeyNotRegistered(self.public_key)) } }