Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
crypto/ssh/keystore: Explicit paths
Lorenz Leutgeb committed 7 months ago
commit d0cba05edba71896ccda517469903f55a9712f1d
parent bc4a13902ca80b1c496ab65670c0526141663e9e
5 files changed +132 -58
modified crates/radicle-cli/src/commands/auth.rs
@@ -202,7 +202,7 @@ pub fn authenticate(options: Options, profile: &Profile) -> anyhow::Result<()> {

    // Try RAD_PASSPHRASE fallback.
    if let Some(passphrase) = profile::env::passphrase() {
-
        ssh::keystore::MemorySigner::load(&profile.keystore, Some(passphrase))
+
        ssh::keystore::MemorySigner::load(&profile.keystore, Some(&passphrase))
            .map_err(|_| anyhow!("`{}` is invalid", env::RAD_PASSPHRASE))?;
        return Ok(());
    }
@@ -225,7 +225,7 @@ pub fn register(
) -> anyhow::Result<()> {
    let secret = profile
        .keystore
-
        .secret_key(Some(passphrase))
+
        .secret_key(Some(&passphrase))
        .map_err(|e| {
            if e.is_crypto_err() {
                anyhow!("could not decrypt secret key: invalid passphrase")
@@ -233,7 +233,7 @@ pub fn register(
                e.into()
            }
        })?
-
        .ok_or_else(|| anyhow!("Key not found in {:?}", profile.keystore.path()))?;
+
        .ok_or_else(|| anyhow!("Key not found in {:?}", profile.keystore.secret_key_path()))?;

    agent.register(&secret)?;

modified crates/radicle-cli/src/terminal/io.rs
@@ -59,7 +59,7 @@ pub fn signer(profile: &Profile) -> anyhow::Result<BoxedDevice> {
        Err(e) => return Err(e.into()),
    };
    let spinner = spinner("Unsealing key...");
-
    let signer = MemorySigner::load(&profile.keystore, Some(passphrase))?;
+
    let signer = MemorySigner::load(&profile.keystore, Some(&passphrase))?;

    spinner.finish();

modified crates/radicle-cli/tests/util/environment.rs
@@ -170,7 +170,7 @@ impl Environment {
        radicle::cob::cache::Store::open(cobs_db).unwrap();

        transport::local::register(storage.clone());
-
        let keystore = Keystore::new(&home.keys());
+
        let keystore = Keystore::new(home.keys());
        keystore.store(keypair.clone(), "radicle", None).unwrap();

        // Ensures that each user has a unique but deterministic public key.
modified crates/radicle-crypto/src/ssh/keystore.rs
@@ -22,8 +22,8 @@ pub enum Error {
    Ssh(#[from] ssh_key::Error),
    #[error("invalid key type, expected ed25519 key")]
    InvalidKeyType,
-
    #[error("keystore already initialized")]
-
    AlreadyInitialized,
+
    #[error("keystore already initialized, '{0}' exists")]
+
    AlreadyInitialized(PathBuf),
    #[error("keystore is encrypted; a passphrase is required")]
    PassphraseMissing,
}
@@ -38,28 +38,52 @@ impl Error {
/// Stores keys on disk, in OpenSSH format.
#[derive(Debug, Clone)]
pub struct Keystore {
-
    path: PathBuf,
+
    path_secret: PathBuf,
+
    path_public: Option<PathBuf>,
}

impl Keystore {
-
    /// Create a new keystore pointing to the given path. Use [`Keystore::init`] to initialize.
-
    pub fn new<P: AsRef<Path>>(path: &P) -> Self {
+
    /// Create a new keystore pointing to the given path.
+
    ///
+
    /// Use [`Keystore::init`] to initialize.
+
    pub fn new<P: AsRef<Path>>(path: P) -> Self {
+
        const DEFAULT_SECRET_KEY_FILE_NAME: &str = "radicle";
+
        const DEFAULT_PUBLIC_KEY_FILE_NAME: &str = "radicle.pub";
+

+
        let keys = path.as_ref().to_path_buf();
+

+
        Self {
+
            path_secret: keys.join(DEFAULT_SECRET_KEY_FILE_NAME),
+
            path_public: Some(keys.join(DEFAULT_PUBLIC_KEY_FILE_NAME)),
+
        }
+
    }
+

+
    /// Create a new keystore pointing to the given paths.
+
    ///
+
    /// Use [`Keystore::init`] to initialize.
+
    pub fn from_secret_path<P: AsRef<Path>>(secret: &P) -> Self {
        Self {
-
            path: path.as_ref().to_path_buf(),
+
            path_secret: secret.as_ref().to_path_buf(),
+
            path_public: None,
        }
    }

-
    /// Get the path to the keystore.
-
    pub fn path(&self) -> &Path {
-
        self.path.as_path()
+
    /// Get the path to the secret key backing the keystore.
+
    pub fn secret_key_path(&self) -> &Path {
+
        self.path_secret.as_path()
+
    }
+

+
    /// Get the path to the public key backing the keystore, if present.
+
    pub fn public_key_path(&self) -> Option<&Path> {
+
        self.path_public.as_deref()
    }

-
    /// Initialize a keystore by generating a key pair and storing the secret and public key
-
    /// at the given path.
+
    /// Initialize a keystore by generating a key pair and storing the secret
+
    /// and public key at the given path.
    ///
-
    /// The `comment` is associated with the private key.
-
    /// The `passphrase` is used to encrypt the private key.
-
    /// The `seed` is used to derive the private key and should almost always be generated.
+
    /// The `comment` is associated with the private key. The `passphrase` is
+
    /// used to encrypt the private key. The `seed` is used to derive the
+
    /// private key and should almost always be generated.
    ///
    /// If `passphrase` is `None`, the key is not encrypted.
    pub fn init(
@@ -71,7 +95,7 @@ impl Keystore {
        self.store(KeyPair::from_seed(seed), comment, passphrase)
    }

-
    /// Store a keypair on disk. Returns an error if the key already exists.
+
    /// Store a keypair on disk. Returns an error if any of the two key files already exist.
    pub fn store(
        &self,
        keypair: KeyPair,
@@ -87,13 +111,21 @@ impl Keystore {
            secret
        };
        let public = secret.public_key();
-
        let path = self.path.join("radicle");

-
        if path.exists() {
-
            return Err(Error::AlreadyInitialized);
+
        if self.path_secret.exists() {
+
            return Err(Error::AlreadyInitialized(self.path_secret.to_path_buf()));
        }

-
        {
+
        if let Some(path_public) = &self.path_public {
+
            if path_public.exists() {
+
                return Err(Error::AlreadyInitialized(path_public.to_path_buf()));
+
            }
+
        }
+

+
        // NOTE: If [`PathBuf::parent`] returns `None`,
+
        // then the path is at root or empty, so don't
+
        // attempt to create any parents.
+
        self.path_secret.parent().map_or(Ok(()), |parent| {
            let mut builder = fs::DirBuilder::new();
            builder.recursive(true);

@@ -103,41 +135,75 @@ impl Keystore {
                builder.mode(0o700);
            }

-
            builder.create(&self.path)?;
-
        }
+
            builder.create(parent)
+
        })?;
+
        secret.write_openssh_file(&self.path_secret, ssh_key::LineEnding::default())?;
+

+
        if let Some(path_public) = &self.path_public {
+
            path_public.parent().map_or(Ok(()), |parent| {
+
                let mut builder = fs::DirBuilder::new();
+
                builder.recursive(true);

-
        secret.write_openssh_file(&path, ssh_key::LineEnding::default())?;
-
        public.write_openssh_file(&path.with_extension("pub"))?;
+
                #[cfg(unix)]
+
                {
+
                    use std::os::unix::fs::DirBuilderExt as _;
+
                    builder.mode(0o700);
+
                }
+

+
                builder.create(parent)
+
            })?;
+
            public.write_openssh_file(path_public)?;
+
        }

        Ok(keypair.pk.into())
    }

    /// Load the public key from the store. Returns `None` if it wasn't found.
    pub fn public_key(&self) -> Result<Option<PublicKey>, Error> {
-
        let path = self.path.join("radicle.pub");
-
        if !path.exists() {
+
        let Some(path_public) = &self.path_public else {
+
            return Ok(None);
+
        };
+

+
        if !path_public.exists() {
            return Ok(None);
        }

-
        let public = ssh_key::PublicKey::read_openssh_file(&path)?;
-
        match public.try_into() {
-
            Ok(public) => Ok(Some(public)),
-
            _ => Err(Error::InvalidKeyType),
+
        let public = ssh_key::PublicKey::read_openssh_file(path_public)?;
+
        PublicKey::try_from(public)
+
            .map(Some)
+
            .map_err(|_| Error::InvalidKeyType)
+
    }
+

+
    /// Load the public key from the store. Attempts to compute it from the
+
    /// private key if no public key is set up.
+
    ///
+
    /// Returns `None` if it wasn't found.
+
    pub fn public_key_derived(
+
        &self,
+
        passphrase: Option<&Passphrase>,
+
    ) -> Result<Option<PublicKey>, Error> {
+
        if let Ok(Some(public_key)) = self.public_key() {
+
            return Ok(Some(public_key));
        }
+

+
        let Some(secret_key) = self.secret_key(passphrase)? else {
+
            return Ok(None);
+
        };
+

+
        Ok(Some(PublicKey(secret_key.public_key())))
    }

    /// 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: Option<Passphrase>,
+
        passphrase: Option<&Passphrase>,
    ) -> Result<Option<Zeroizing<SecretKey>>, Error> {
-
        let path = self.path.join("radicle");
-
        if !path.exists() {
+
        if !self.path_secret.exists() {
            return Ok(None);
        }

-
        let secret = ssh_key::PrivateKey::read_openssh_file(&path)?;
+
        let secret = ssh_key::PrivateKey::read_openssh_file(&self.path_secret)?;
        let secret = if let Some(p) = passphrase {
            secret.decrypt(p)?
        } else if secret.is_encrypted() {
@@ -155,12 +221,11 @@ impl Keystore {

    /// Check that the passphrase is valid.
    pub fn is_valid_passphrase(&self, passphrase: &Passphrase) -> Result<bool, Error> {
-
        let path = self.path.join("radicle");
-
        if !path.exists() {
+
        if !self.path_secret.exists() {
            return Err(Error::Io(io::ErrorKind::NotFound.into()));
        }

-
        let secret = ssh_key::PrivateKey::read_openssh_file(&path)?;
+
        let secret = ssh_key::PrivateKey::read_openssh_file(&self.path_secret)?;
        let valid = secret.decrypt(passphrase).is_ok();

        Ok(valid)
@@ -168,8 +233,7 @@ impl Keystore {

    /// Check whether the secret key is encrypted.
    pub fn is_encrypted(&self) -> Result<bool, Error> {
-
        let path = self.path.join("radicle");
-
        let secret = ssh_key::PrivateKey::read_openssh_file(&path)?;
+
        let secret = ssh_key::PrivateKey::read_openssh_file(&self.path_secret)?;

        Ok(secret.is_encrypted())
    }
@@ -183,6 +247,8 @@ pub enum MemorySignerError {
    NotFound(PathBuf),
    #[error("invalid passphrase")]
    InvalidPassphrase,
+
    #[error("secret key does not match given public key")]
+
    KeyMismatch,
}

/// An in-memory signer that keeps its secret key internally
@@ -256,11 +322,8 @@ impl MemorySigner {
    /// Load this signer from a keystore, given a secret key passphrase.
    pub fn load(
        keystore: &Keystore,
-
        passphrase: Option<Passphrase>,
+
        passphrase: Option<&Passphrase>,
    ) -> Result<Self, MemorySignerError> {
-
        let public = keystore
-
            .public_key()?
-
            .ok_or_else(|| MemorySignerError::NotFound(keystore.path().to_path_buf()))?;
        let secret = keystore
            .secret_key(passphrase)
            .map_err(|e| {
@@ -270,9 +333,20 @@ impl MemorySigner {
                    e.into()
                }
            })?
-
            .ok_or_else(|| MemorySignerError::NotFound(keystore.path().to_path_buf()))?;
+
            .ok_or_else(|| MemorySignerError::NotFound(keystore.secret_key_path().to_path_buf()))?;
+

+
        let public = secret.public_key();

-
        Ok(Self { public, secret })
+
        if let Some(expected_public) = keystore.public_key()? {
+
            if public != *expected_public {
+
                return Err(MemorySignerError::KeyMismatch);
+
            }
+
        }
+

+
        Ok(Self {
+
            public: crate::PublicKey(public),
+
            secret,
+
        })
    }

    /// Box this signer into a trait object.
@@ -312,7 +386,7 @@ mod tests {
    #[test]
    fn test_init_passphrase() {
        let tmp = tempfile::tempdir().unwrap();
-
        let store = Keystore::new(&tmp.path());
+
        let store = Keystore::new(tmp.path());

        let public = store
            .init(
@@ -325,20 +399,20 @@ mod tests {
        assert!(store.is_encrypted().unwrap());

        let secret = store
-
            .secret_key(Some("hunter".to_owned().into()))
+
            .secret_key(Some(&"hunter".to_owned().into()))
            .unwrap()
            .unwrap();
        assert_eq!(PublicKey::from(secret.public_key()), public);

        store
-
            .secret_key(Some("blunder".to_owned().into()))
+
            .secret_key(Some(&"blunder".to_owned().into()))
            .unwrap_err(); // Wrong passphrase.
    }

    #[test]
    fn test_init_no_passphrase() {
        let tmp = tempfile::tempdir().unwrap();
-
        let store = Keystore::new(&tmp.path());
+
        let store = Keystore::new(tmp.path());

        let public = store.init("test", None, ec25519::Seed::default()).unwrap();
        assert_eq!(public, store.public_key().unwrap().unwrap());
@@ -351,7 +425,7 @@ mod tests {
    #[test]
    fn test_signer() {
        let tmp = tempfile::tempdir().unwrap();
-
        let store = Keystore::new(&tmp.path());
+
        let store = Keystore::new(tmp.path());

        let public = store
            .init(
@@ -360,7 +434,7 @@ mod tests {
                ec25519::Seed::default(),
            )
            .unwrap();
-
        let signer = MemorySigner::load(&store, Some("hunter".to_owned().into())).unwrap();
+
        let signer = MemorySigner::load(&store, Some(&"hunter".to_owned().into())).unwrap();

        assert_eq!(public, *signer.public_key());
    }
modified crates/radicle/src/profile.rs
@@ -210,7 +210,7 @@ impl Profile {
        passphrase: Option<Passphrase>,
        seed: crypto::Seed,
    ) -> Result<Self, Error> {
-
        let keystore = Keystore::new(&home.keys());
+
        let keystore = Keystore::new(home.keys());
        let public_key = keystore.init("radicle", passphrase, seed)?;
        let config = Config::init(alias.clone(), home.config().as_path())?;
        let storage = Storage::open(
@@ -251,7 +251,7 @@ impl Profile {

    pub fn load() -> Result<Self, Error> {
        let home = self::home()?;
-
        let keystore = Keystore::new(&home.keys());
+
        let keystore = Keystore::new(home.keys());
        let public_key = keystore
            .public_key()?
            .ok_or_else(|| Error::NotFound(home.path().to_path_buf()))?;
@@ -303,7 +303,7 @@ impl Profile {
        }

        if let Some(passphrase) = env::passphrase() {
-
            let signer = keystore::MemorySigner::load(&self.keystore, Some(passphrase))?;
+
            let signer = keystore::MemorySigner::load(&self.keystore, Some(&passphrase))?;
            return Ok(Device::from(signer).boxed());
        }