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 75841c4dfecdfbc1cab67275fec4132d7182fe5d
parent ed8b086045ee5d7bd1327f579de7861a1cf49e3b
2 files changed +96 -44
modified crates/radicle-cli/src/commands/auth.rs
@@ -240,7 +240,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-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.
+
    /// 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()
    }

-
    /// Initialize a keystore by generating a key pair and storing the secret and public key
-
    /// at the given 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.
    ///
-
    /// 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,27 +135,43 @@ impl Keystore {
                builder.mode(0o700);
            }

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

-
        secret.write_openssh_file(&path, ssh_key::LineEnding::default())?;
-
        public.write_openssh_file(&path.with_extension("pub"))?;
+
        if let Some(path_public) = &self.path_public {
+
            path_public.parent().map_or(Ok(()), |parent| {
+
                let mut builder = fs::DirBuilder::new();
+
                builder.recursive(true);
+

+
                #[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);
-
        }
+
        };

-
        let public = ssh_key::PublicKey::read_openssh_file(&path)?;
-
        match public.try_into() {
-
            Ok(public) => Ok(Some(public)),
-
            _ => Err(Error::InvalidKeyType),
+
        if !path_public.exists() {
+
            return Ok(None);
        }
+

+
        let public = ssh_key::PublicKey::read_openssh_file(path_public)?;
+
        PublicKey::try_from(public)
+
            .map(Some)
+
            .map_err(|_| Error::InvalidKeyType)
    }

    /// Load the secret key from the store, decrypting it with the given passphrase.
@@ -132,12 +180,13 @@ impl Keystore {
        &self,
        passphrase: Option<Passphrase>,
    ) -> Result<Option<Zeroizing<SecretKey>>, Error> {
-
        let path = self.path.join("radicle");
+
        let path = &self.path_secret;
        if !path.exists() {
            return Ok(None);
        }

-
        let secret = ssh_key::PrivateKey::read_openssh_file(&path)?;
+
        let secret = ssh_key::PrivateKey::read_openssh_file(path)?;
+

        let secret = if let Some(p) = passphrase {
            secret.decrypt(p)?
        } else if secret.is_encrypted() {
@@ -155,12 +204,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 +216,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())
    }
@@ -258,9 +305,6 @@ impl MemorySigner {
        keystore: &Keystore,
        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,7 +314,15 @@ 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 = {
+
            if let Ok(Some(public)) = keystore.public_key() {
+
                public
+
            } else {
+
                PublicKey(secret.public_key())
+
            }
+
        };

        Ok(Self { public, secret })
    }
@@ -312,7 +364,7 @@ mod tests {
    #[test]
    fn test_init_passphrase() {
        let tmp = tempfile::tempdir().unwrap();
-
        let store = Keystore::new(&tmp.path());
+
        let store = Keystore::new(&tmp);

        let public = store
            .init(
@@ -338,7 +390,7 @@ mod tests {
    #[test]
    fn test_init_no_passphrase() {
        let tmp = tempfile::tempdir().unwrap();
-
        let store = Keystore::new(&tmp.path());
+
        let store = Keystore::new(&tmp);

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

        let public = store
            .init(