From 6b8802f0fabf596697200b71fb988dfd5b4d2657 Mon Sep 17 00:00:00 2001 From: ado11231 Date: Sat, 17 Oct 2026 22:39:43 -0400 Subject: [PATCH 1/2] feat: share the ssh connection on every route Port forwarding adds and removes forwards on a running connection with ssh -O forward, which needs a shared connection. Only iroh had one. Each account, address, and port now gets its own socket, so a connection opened over one route never carries calls meant for another. --- crates/ally-cli/src/ssh.rs | 97 +++++++++++++++++++++++++++++++---- crates/ally-cli/src/tunnel.rs | 49 +----------------- 2 files changed, 89 insertions(+), 57 deletions(-) diff --git a/crates/ally-cli/src/ssh.rs b/crates/ally-cli/src/ssh.rs index 2f1de58..ef2dc51 100644 --- a/crates/ally-cli/src/ssh.rs +++ b/crates/ally-cli/src/ssh.rs @@ -4,6 +4,7 @@ use crate::route::{self, Route}; use crate::tunnel; use ally_core::config; use ally_core::presentation::Style; +use ally_core::source; use anyhow::Context; use shell_words::join; use std::path::PathBuf; @@ -24,7 +25,34 @@ const SSH_OPTIONS: &[&str] = &[ "LogLevel=ERROR", ]; -/// How long a call through the shared iroh connection may take before the connection is +/// How long a shared connection outlives its last ssh call. Long enough to carry one +/// command's control, sync, and run calls, short enough that an Agent that went away is +/// noticed on the next command rather than minutes later. +const SHARED_FOR_SECONDS: u32 = 30; + +/// Where ssh keeps the connection it shares between calls, so one command pays for the +/// connection once and ports can be forwarded through it while it runs. Kept in a private +/// folder and named by `route`, so each Agent and each way of reaching it gets its own. +/// `None` when no folder fits or none can be made private, which only means no sharing. +fn shared_socket(route: &str) -> Option { + let user = std::env::var("USER").unwrap_or_else(|_| "user".to_string()); + let name = format!("{}.sock", &source::digest(route.as_bytes())[..12]); + [std::env::temp_dir(), PathBuf::from("/tmp")] + .into_iter() + .map(|base| base.join(format!("ally-{user}"))) + .find(|dir| { + dir.join(&name).as_os_str().len() + SOCKET_SUFFIX < SOCKET_LIMIT + && ally_core::storage::private_dir(dir).is_ok() + }) + .map(|dir| dir.join(name)) +} + +/// The longest Unix socket path macOS accepts, and what ssh appends to the name while it +/// creates the socket. +const SOCKET_LIMIT: usize = 104; +const SOCKET_SUFFIX: usize = 17; + +/// How long a call through the shared connection may take before the connection is /// treated as dead. A live one answers in well under a second. const SHARE_CHECK: Duration = Duration::from_secs(3); @@ -67,7 +95,7 @@ pub struct RemoteCommand { pub host_key_alias: Option, /// How ssh reaches the Agent when no address answers directly. pub proxy: Option, - /// The socket ssh shares one connection through across calls, used over iroh. + /// The socket ssh shares one connection through across calls. pub shared: Option, /// Whether to ask for a terminal on the Agent. See `wants_terminal`. pub tty: bool, @@ -85,18 +113,18 @@ impl RemoteCommand { /// Everything ssh needs comes from the config, so nobody has to keep an entry in /// `~/.ssh/config` in step with this. pub fn to(agent: &config::Agent, program: String, args: Vec) -> RemoteCommand { - let (host, proxy, shared) = match route::resolve(agent) { - Route::Direct { host, .. } => (host, None, None), + let (host, proxy, through) = match route::resolve(agent) { + Route::Direct { host, .. } => (host.clone(), None, host), Route::Iroh { key } => ( agent.host.clone(), Some(tunnel::proxy_command(&key)), - tunnel::shared_socket(&key), + format!("iroh {key}"), ), }; let command = RemoteCommand { host, proxy, - shared, + shared: shared_socket(&share_route(&agent.user, &through, agent.port)), user: Some(agent.user.clone()), port: agent.port, identity_file: agent.identity_file.clone(), @@ -114,8 +142,9 @@ impl RemoteCommand { command } - /// A shared iroh connection to an Agent that crashed stays open until iroh gives up on - /// it, about 30 seconds, and every call through it waits that long. So before the first + /// A shared connection to an Agent that crashed stays open until ssh or iroh gives up + /// on it, which takes from 30 seconds to a few minutes, and every call through it waits + /// that long. So before the first /// reuse, one quick call goes through it, and a connection that does not answer is /// closed so ssh opens a new one. The rsync helper skips this, since its parent checked. fn drop_stale_share(&self) { @@ -226,7 +255,7 @@ impl RemoteCommand { argv.push("-o".to_string()); argv.push(format!("ControlPath=\"{}\"", socket.display())); argv.push("-o".to_string()); - argv.push(format!("ControlPersist={}", tunnel::SHARED_FOR_SECONDS)); + argv.push(format!("ControlPersist={SHARED_FOR_SECONDS}")); } if let Some(alias) = &self.host_key_alias { @@ -323,6 +352,15 @@ impl RemoteCommand { } } +/// What names a shared connection: who logs in, through which address or iroh key, and on +/// which port. +fn share_route(user: &str, through: &str, port: Option) -> String { + match port { + Some(port) => format!("{user}@{through}:{port}"), + None => format!("{user}@{through}"), + } +} + /// One line of framed output. A progress bar redraws its line with carriage returns, so /// only what it drew last is kept. fn framed_line(line: &str) -> String { @@ -732,6 +770,47 @@ mod tests { assert!(argv.contains(&"ControlPersist=30".to_string()), "{argv:?}"); } + /// Unix sockets fail to bind past roughly 104 bytes of path on macOS. + #[test] + fn the_shared_socket_path_is_short_and_private() { + let path = shared_socket(&share_route("me", "iroh abcdef", None)).unwrap(); + + assert!( + path.as_os_str().len() + SOCKET_SUFFIX < SOCKET_LIMIT, + "{}", + path.display() + ); + let mode = std::fs::metadata(path.parent().unwrap()) + .unwrap() + .permissions(); + assert_eq!( + std::os::unix::fs::PermissionsExt::mode(&mode) & 0o777, + 0o700 + ); + } + + /// A connection opened over one address must never carry calls meant for another, or + /// a run would say "via tailnet" while its bytes take the local network. + #[test] + fn each_account_address_and_port_gets_its_own_shared_connection() { + let routes = [ + share_route("ado", "10.0.0.193", None), + share_route("ado", "100.67.90.119", None), + share_route("ado", "iroh abcdef", None), + share_route("other", "10.0.0.193", None), + share_route("ado", "10.0.0.193", Some(2222)), + ]; + let sockets: std::collections::HashSet<_> = routes + .iter() + .map(|route| shared_socket(route).unwrap()) + .collect(); + assert_eq!(sockets.len(), routes.len()); + assert_eq!( + shared_socket(&routes[0]), + shared_socket(&share_route("ado", "10.0.0.193", None)) + ); + } + #[test] fn no_key_configured_means_no_i_flag() { let argv = remote(&["hi"]).to_ssh_args(); diff --git a/crates/ally-cli/src/tunnel.rs b/crates/ally-cli/src/tunnel.rs index 614b9eb..fc4104c 100644 --- a/crates/ally-cli/src/tunnel.rs +++ b/crates/ally-cli/src/tunnel.rs @@ -7,15 +7,9 @@ use ally_core::tunnel::{self, ALPN}; use anyhow::Context; use iroh::endpoint::{Connection, presets}; use iroh::{Endpoint, EndpointAddr}; -use std::path::PathBuf; use std::time::Duration; use tokio::io::{AsyncRead, AsyncWrite}; -/// How long a shared iroh connection outlives its last ssh call. Long enough to carry one -/// command's control, sync, and run calls, short enough that an Agent that went away is -/// noticed on the next command rather than minutes later. -pub const SHARED_FOR_SECONDS: u32 = 30; - /// The ProxyCommand line for reaching the Agent with iroh key `key`, which the route has /// already checked. ssh runs it through a shell after expanding its own `%` tokens, so /// every part is quoted and every `%` doubled. @@ -27,28 +21,6 @@ pub fn proxy_command(key: &str) -> String { proxy_line(&exe, key) } -/// Where ssh keeps the connection it shares between calls over iroh, so one command pays -/// for the iroh handshake once. Kept in a private folder and named by the Agent's key, so -/// each Agent gets its own. `None` when no folder fits or none can be made private, which -/// only means no sharing. -pub fn shared_socket(key: &str) -> Option { - let user = std::env::var("USER").unwrap_or_else(|_| "user".to_string()); - let name = format!("{}.sock", &key[..key.len().min(12)]); - [std::env::temp_dir(), PathBuf::from("/tmp")] - .into_iter() - .map(|base| base.join(format!("ally-{user}"))) - .find(|dir| { - dir.join(&name).as_os_str().len() + SOCKET_SUFFIX < SOCKET_LIMIT - && ally_core::storage::private_dir(dir).is_ok() - }) - .map(|dir| dir.join(name)) -} - -/// The longest Unix socket path macOS accepts, and what ssh appends to the name while it -/// creates the socket. -const SOCKET_LIMIT: usize = 104; -const SOCKET_SUFFIX: usize = 17; - fn proxy_line(exe: &str, key: &str) -> String { shell_words::join([exe, "internal-tunnel", key]).replace('%', "%%") } @@ -158,6 +130,7 @@ mod tests { use super::*; use iroh::RelayMode; use iroh::SecretKey; + use std::path::PathBuf; use tokio::io::{AsyncReadExt, AsyncWriteExt}; #[test] @@ -179,26 +152,6 @@ mod tests { ); } - /// Unix sockets fail to bind past roughly 104 bytes of path on macOS. - #[test] - fn the_shared_socket_path_is_short_and_private() { - let key = SecretKey::generate().public().to_string(); - let path = shared_socket(&key).unwrap(); - - assert!( - path.as_os_str().len() + SOCKET_SUFFIX < SOCKET_LIMIT, - "{}", - path.display() - ); - let mode = std::fs::metadata(path.parent().unwrap()) - .unwrap() - .permissions(); - assert_eq!( - std::os::unix::fs::PermissionsExt::mode(&mode) & 0o777, - 0o700 - ); - } - async fn endpoint(alpns: Vec>) -> Endpoint { Endpoint::builder(presets::Minimal) .relay_mode(RelayMode::Disabled) From 9ccaa079dfcad17fe5166391227ece6d8c7cf6c2 Mon Sep 17 00:00:00 2001 From: ado11231 Date: Sat, 17 Oct 2026 22:39:44 -0400 Subject: [PATCH 2/2] docs: ssh shares its connection on every route --- docs/ARCHITECTURE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e2f2235..dd1441c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -322,7 +322,7 @@ ally/ | `build.rs` | Puts the build's commit into `--version`, using `git` when present. | `git` | `lib` | | `client.rs` | The pairing connection, and the `Control` connection over `ssh`. | `route`, `ssh`, `tunnel` | `transfer`, `watch`, most commands | | `route.rs` | Chooses the path: local network, tailnet, or iroh. Prints the `▶` location line. | none | `client`, `ssh`, `transfer`, `watch`, `run`, `attach`, `tools` | -| `ssh.rs` | Builds every `ssh` call, with safe quoting and a shared connection. | `route`, `tunnel` | `client`, `run`, `attach`, `unlink` | +| `ssh.rs` | Builds every `ssh` call, with safe quoting and a connection shared on every route. | `route`, `tunnel`, `source` | `client`, `run`, `attach`, `unlink` | | `tunnel.rs` | `ally internal-tunnel`: the Client's end of an iroh connection. | `project` | `client`, `ssh` | | `transfer.rs` | Performs a sync: lists files, plans, calls `rsync`, and reports. | `client`, `project`, `route` | `run`, `attach`, `sync` | | `project.rs` | Finds the current project and its ID on each Agent. | none | `transfer`, `tunnel`, most commands |