ssh: Use winpipe for SSH agent on Windows

The trait `radicle_ssh::agent::client::ClientStream` trait was
simplified, because `winpipe::WinStream` is so similar to
`std::os::unix::net::UnixStream` that we can just use the same
implementation for both platforms.

However, obtaining the process identifier of the SSH agent process is
not common on Windows, so we instead track the path that we used to
connect to the agent. This is also helpful on Unix. In some cases, the
path will even contain the process identifier. The path is printed by
`rad self`.

Errors are cleaned up, and error messages are improved.

crates/radicle-cli/src/commands/self.rs

@@ -146,7 +146,9 @@ fn all(profile: &Profile) -> anyhow::Result<()> {
     let ssh_agent = match ssh::agent::Agent::connect() {
         Ok(c) => term::format::positive(format!(
             "running ({})",
-            c.pid().map(|p| p.to_string()).unwrap_or(String::from("?"))
+            c.path()
+                .map(|p| p.display().to_string())
+                .unwrap_or(String::from("?"))
         )),
         Err(e) if e.is_not_running() => term::format::yellow(String::from("not running")),
         Err(e) => term::format::negative(format!("error: {e}")),

crates/radicle-crypto/src/ssh.rs

@@ -261,13 +261,6 @@ mod test {
     }
 
     impl ClientStream for DummyStream {
-        fn connect<P>(_path: P) -> Result<AgentClient<Self>, Error>
-        where
-            P: AsRef<std::path::Path> + Send,
-        {
-            panic!("This function should never be called!")
-        }
-
         fn request(&mut self, buf: &[u8]) -> Result<Buffer, Error> {
             *self.incoming.lock().unwrap() = buf.to_vec();
 
@@ -304,7 +297,7 @@ mod test {
         ];
 
         let stream = DummyStream::default();
-        let mut agent = AgentClient::connect(stream.clone());
+        let mut agent = AgentClient::new(None, stream.clone());
 
         agent.remove_identity(&pk).unwrap();
 
@@ -334,7 +327,7 @@ mod test {
         ];
 
         let stream = DummyStream::default();
-        let mut agent = AgentClient::connect(stream.clone());
+        let mut agent = AgentClient::new(None, stream.clone());
         let data: Vec<u8> = vec![1, 2, 3, 4, 5, 6, 7, 8, 9];
 
         agent.sign(&pk, &data).ok();

crates/radicle-crypto/src/ssh/agent.rs

@@ -1,26 +1,23 @@
 use std::cell::RefCell;
+use std::path::Path;
 
-pub use radicle_ssh::agent::client::AgentClient;
+pub use radicle_ssh as ssh;
-pub use radicle_ssh::agent::client::Error;
+pub use ssh::agent::client::{AgentClient, Error};
-pub use radicle_ssh::{self as ssh, agent::client::ClientStream};
 
 use crate::{PublicKey, SecretKey, Signature, Signer, SignerError};
 
-#[cfg(not(unix))]
-pub use std::net::TcpStream as Stream;
-#[cfg(unix)]
-pub use std::os::unix::net::UnixStream as Stream;
-
 use super::ExtendedSignature;
 
 pub struct Agent {
-    client: AgentClient<Stream>,
+    client: AgentClient,
 }
 
 impl Agent {
     /// Connect to a running SSH agent.
-    pub fn connect() -> Result<Self, ssh::agent::client::Error> {
+    pub fn connect() -> Result<Self, Error> {
-        Stream::connect_env().map(|client| Self { client })
+        Ok(Self {
+            client: AgentClient::connect_env()?,
+        })
     }
 
     /// Register a key with the agent.
@@ -45,8 +42,8 @@ impl Agent {
         AgentSigner::new(self, key)
     }
 
-    pub fn pid(&self) -> Option<u32> {
+    pub fn path(&self) -> Option<&Path> {
-        self.client.pid()
+        self.client.path()
     }
 
     pub fn request_identities(&mut self) -> Result<Vec<PublicKey>, ssh::agent::client::Error> {

crates/radicle-ssh/Cargo.toml

@@ -18,3 +18,6 @@ byteorder = "1.4"
 log = { workspace = true }
 thiserror = { workspace = true }
 zeroize = { workspace = true }
+
+[target.'cfg(windows)'.dependencies]
+winpipe = { workspace = true }

crates/radicle-ssh/src/agent/client.rs

@@ -1,11 +1,15 @@
 use std::fmt;
 use std::io::{Read, Write};
 use std::ops::DerefMut;
-use std::os::unix::net::UnixStream;
+use std::path::{Path, PathBuf};
-use std::path::Path;
-use std::str::FromStr;
 
-use byteorder::{BigEndian, ByteOrder, WriteBytesExt};
+#[cfg(unix)]
+pub use std::os::unix::net::UnixStream as Stream;
+
+#[cfg(windows)]
+pub use winpipe::WinStream as Stream;
+
+use byteorder::{BigEndian, ByteOrder as _, WriteBytesExt};
 use log::*;
 use thiserror::Error;
 use zeroize::Zeroize as _;
@@ -21,74 +25,103 @@ pub type Signature = [u8; 64];
 #[derive(Debug, Error)]
 pub enum Error {
     /// Agent protocol error.
-    #[error("Agent protocol error")]
+    #[error("SSH agent replied with unexpected data, violating the SSH agent protocol.")]
     AgentProtocolError,
-    #[error("Agent failure")]
+    #[error(
+        "SSH agent replied with failure (protocol message number 5), which could not be handled."
+    )]
     AgentFailure,
-    #[error("Unable to connect to ssh-agent. The environment variable `SSH_AUTH_SOCK` was set, but it points to a nonexistent file or directory.")]
+    #[error("Unable to connect to SSH agent because '{path}' was not found: {source}")]
-    BadAuthSock,
+    BadAuthSock {
-    #[error(transparent)]
+        path: String,
+        source: std::io::Error,
+    },
+    #[error("Encoding error while communicating with SSH agent: {0}")]
     Encoding(#[from] encoding::Error),
-    #[error("Environment variable `{0}` not found")]
+    #[error("Unable to read environment variable '{var}': {source}")]
-    EnvVar(&'static str),
+    EnvVar {
-    #[error(transparent)]
+        var: String,
+        source: std::env::VarError,
+    },
+    #[error("I/O error while communicating with SSH agent: {0}")]
     Io(#[from] std::io::Error),
-    #[error(transparent)]
-    Private(Box<dyn std::error::Error + Send + Sync + 'static>),
-    #[error(transparent)]
-    Public(Box<dyn std::error::Error + Send + Sync + 'static>),
-    #[error(transparent)]
-    Signature(Box<dyn std::error::Error + Send + Sync + 'static>),
 }
 
 impl Error {
     pub fn is_not_running(&self) -> bool {
-        matches!(self, Self::EnvVar("SSH_AUTH_SOCK"))
+        matches!(self, Self::EnvVar { .. } | Self::BadAuthSock { .. })
     }
 }
 
 /// SSH agent client.
-pub struct AgentClient<S> {
+pub struct AgentClient<S = Stream> {
+    /// The path that was originally used to connect to the agent.
+    path: Option<PathBuf>,
+
+    /// The underlying stream to the SSH agent.
     stream: S,
 }
 
 impl<S> AgentClient<S> {
-    /// Connect to an SSH agent via the provided stream (on Unix, usually a Unix-domain socket).
+    pub fn path(&self) -> Option<&Path> {
-    pub fn connect(stream: S) -> Self {
+        self.path.as_deref()
-        AgentClient { stream }
-    }
-
-    /// Get the agent PID.
-    pub fn pid(&self) -> Option<u32> {
-        std::env::var("SSH_AGENT_PID")
-            .ok()
-            .and_then(|v| u32::from_str(&v).ok())
     }
 }
 
-pub trait ClientStream: Sized + Send + Sync {
+impl AgentClient<Stream> {
-    /// Send an agent request through the stream and read the response.
+    /// Connect to an SSH agent at the provided path.
-    fn request(&mut self, req: &[u8]) -> Result<Buffer, Error>;
+    pub fn connect<P>(path: P) -> Result<Self, Error>
-
-    /// How to connect the streaming socket
-    fn connect<P>(path: P) -> Result<AgentClient<Self>, Error>
     where
-        P: AsRef<Path> + Send;
+        P: AsRef<Path>,
+    {
+        let path = path.as_ref().to_owned();
 
-    fn connect_env() -> Result<AgentClient<Self>, Error> {
+        let stream = match Stream::connect(&path) {
-        let Ok(var) = std::env::var("SSH_AUTH_SOCK") else {
+            Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
-            return Err(Error::EnvVar("SSH_AUTH_SOCK"));
+                return Err(Error::BadAuthSock {
-        };
+                    path: path.display().to_string(),
-        match Self::connect(var) {
+                    source: err,
-            Err(Error::Io(io_err)) if io_err.kind() == std::io::ErrorKind::NotFound => {
+                })
-                Err(Error::BadAuthSock)
             }
-            other => other,
+            Err(err) => return Err(Error::Io(err)),
-        }
+            Ok(stream) => stream,
+        };
+
+        Ok(Self {
+            path: Some(path),
+            stream,
+        })
+    }
+
+    pub fn connect_env() -> Result<Self, Error> {
+        const SSH_AUTH_SOCK: &str = "SSH_AUTH_SOCK";
+
+        let path = match std::env::var(SSH_AUTH_SOCK) {
+            Ok(var) => var,
+            Err(err) => {
+                if cfg!(windows) {
+                    // Windows uses a named pipe for the SSH agent, which
+                    // we fall back to in case reading the environment
+                    // variable fails.
+                    "\\\\.\\pipe\\openssh-ssh-agent".to_string()
+                } else {
+                    return Err(Error::EnvVar {
+                        var: SSH_AUTH_SOCK.to_string(),
+                        source: err,
+                    });
+                }
+            }
+        };
+
+        Self::connect(path)
     }
 }
 
-impl<S: ClientStream> AgentClient<S> {
+impl<Stream: ClientStream> AgentClient<Stream> {
+    pub fn new(path: Option<PathBuf>, stream: Stream) -> Self {
+        Self { path, stream }
+    }
+
     /// Send a key to the agent, with a (possibly empty) slice of constraints
     /// to apply when using the key to sign.
     pub fn add_identity<K>(&mut self, key: &K, constraints: &[Constraint]) -> Result<(), Error>
@@ -372,35 +405,11 @@ impl<S: ClientStream> AgentClient<S> {
     }
 }
 
-#[cfg(not(unix))]
+pub trait ClientStream: Sized + Send + Sync {
-impl ClientStream for TcpStream {
+    fn request(&mut self, msg: &[u8]) -> Result<Buffer, Error>;
-    fn connect_uds<P>(_: P) -> Result<AgentClient<Self>, Error>
-    where
-        P: AsRef<Path> + Send,
-    {
-        Err(Error::AgentFailure)
-    }
-
-    fn read_response(&mut self, _: &mut Buffer) -> Result<(), Error> {
-        Err(Error::AgentFailure)
-    }
-
-    fn connect_env() -> Result<AgentClient<Self>, Error> {
-        Err(Error::AgentFailure)
-    }
 }
 
-#[cfg(unix)]
+impl<S: Read + Write + Sized + Send + Sync> ClientStream for S {
-impl ClientStream for UnixStream {
-    fn connect<P>(path: P) -> Result<AgentClient<Self>, Error>
-    where
-        P: AsRef<Path> + Send,
-    {
-        let stream = UnixStream::connect(path)?;
-
-        Ok(AgentClient { stream })
-    }
-
     fn request(&mut self, msg: &[u8]) -> Result<Buffer, Error> {
         let mut resp = Buffer::default();