cli: Detect key mismatch
When run with a secret and public key that do not cryptographically match, `fn radicle_cli::terminal::io::signer` would prompt the user for a password to unlock the secret key, and then carry on with a mismatching key pair. Fix this by verifying that the keys match in `MemorySigner::load` and only swallow errors initializing the signer in cases where prompting for a password makes sense. It does not make sense in the case of mismatched keys.
This commit is contained in:
parent
ed8b086045
commit
fafb3493dc
|
|
@ -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
|
||||
```
|
||||
|
|
@ -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<BoxedDevice> {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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 })
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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<BoxedDevice, Error> {
|
||||
pub fn signer(&self) -> Result<BoxedDevice, SignerError> {
|
||||
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))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue