From c62c6d90c72530b405b4eddd2f0006e173384986 Mon Sep 17 00:00:00 2001 From: JR Date: Mon, 6 Apr 2026 16:32:39 +0200 Subject: [PATCH 1/3] Fix SSH relay failure when config has LocalForward/DynamicForward directives The reverse relay SSH fallback (used when ControlMaster dynamic forwarding is unavailable) spawns a standalone SSH connection with -S none. This connection reads ~/.ssh/config and attempts to establish ALL configured forwards (LocalForward, DynamicForward, RemoteForward), which conflict with ports already bound by the primary SSH connection. With ExitOnForwardFailure=yes, the relay process exits before the RemoteForward can be established. Fix: Use -F /dev/null to bypass ~/.ssh/config entirely for the standalone relay connection. Transport-critical options (ProxyJump, ProxyCommand, IdentityFile, HostKeyAlgorithms, KexAlgorithms, Ciphers, etc.) are resolved via ssh -G and passed explicitly, preserving connectivity without config-file forwards. ProxyJump hosts are also resolved via ssh -G so that Host aliases work even though -F /dev/null is propagated to jump host SSH processes. Additionally, add controlpath to the backgroundSSHOptions filter (both static and instance versions). Previously, -o ControlPath=... could appear after -S none in the relay command, which on macOS can re-enable mux attachment despite the explicit -S none. Fixes #2596 --- Sources/Workspace.swift | 175 ++++++++++++++++++++++++++++++++++++- docs/remote-daemon-spec.md | 2 +- 2 files changed, 174 insertions(+), 3 deletions(-) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index e94abe72c..6a3ac6a95 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -1073,6 +1073,7 @@ enum WorkspaceRemoteSSHBatchCommandBuilder { private static let batchSSHControlOptionKeys: Set = [ "controlmaster", "controlpersist", + "controlpath", ] static func daemonTransportArguments( @@ -4009,9 +4010,20 @@ final class WorkspaceRemoteSessionController { private func reverseRelayArguments(relayPort: Int, localRelayPort: Int) -> [String] { // Fallback standalone transport when dynamic forwarding through an existing // control master is unavailable. - var args: [String] = ["-N", "-T", "-S", "none"] - args += sshCommonArguments(batchMode: true) + // + // Uses -F /dev/null to bypass ~/.ssh/config entirely, preventing config-file + // LocalForward/DynamicForward/RemoteForward directives from conflicting with + // ports already bound by the primary SSH connection. Transport-critical options + // (ProxyJump, ProxyCommand, HostKeyAlgorithms, etc.) are resolved via ssh -G + // and passed explicitly so connectivity is preserved without config-file forwards. + var args: [String] = ["-N", "-T", "-S", "none", "-F", "/dev/null"] + args += resolvedRelayTransportOptions() args += [ + "-o", "ConnectTimeout=6", + "-o", "ServerAliveInterval=20", + "-o", "ServerAliveCountMax=2", + "-o", "StrictHostKeyChecking=accept-new", + "-o", "BatchMode=yes", "-o", "ExitOnForwardFailure=yes", "-o", "RequestTTY=no", "-R", "127.0.0.1:\(relayPort):127.0.0.1:\(localRelayPort)", @@ -4020,6 +4032,164 @@ final class WorkspaceRemoteSessionController { return args } + /// Resolve transport-critical SSH options for the relay by running `ssh -G`. + /// This extracts options like ProxyJump, ProxyCommand, IdentityFile, HostKeyAlgorithms, + /// etc. from the user's SSH config without carrying over forwarding directives. + private func resolvedRelayTransportOptions() -> [String] { + // Build the ssh -G probe command with the same port/identity/options as the + // primary connection so Host/Match blocks resolve identically. + var probeArgs: [String] = ["-G"] + if let port = configuration.port { + probeArgs += ["-p", String(port)] + } + if let identityFile = configuration.identityFile, + !identityFile.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { + probeArgs += ["-i", identityFile] + } + for option in backgroundSSHOptions(configuration.sshOptions) { + probeArgs += ["-o", option] + } + probeArgs.append(configuration.destination) + + guard let result = try? sshExec(arguments: probeArgs, timeout: 5), + result.status == 0 else { + debugLog("remote.relay.sshG.failed falling back to explicit config only") + // Fallback: use just the explicitly configured options without ssh -G resolution. + return explicitRelayTransportOptions() + } + + return parseRelayTransportOptions(from: result.stdout) + } + + /// Keys from `ssh -G` output that are needed for transport but NOT forwarding. + private static let relayTransportKeys: Set = [ + "hostname", + "user", + "port", + "proxyjump", + "proxycommand", + "identityfile", + "identitiesonly", + "hostkeyalgorithms", + "kexalgorithms", + "ciphers", + "macs", + "hostkeyalias", + "userknownhostsfile", + "globalknownhostsfile", + "pubkeyacceptedalgorithms", + "preferredauthentications", + ] + + /// Parse `ssh -G` output and extract transport-critical options as `-o Key=Value` pairs. + private func parseRelayTransportOptions(from sshGOutput: String) -> [String] { + var args: [String] = [] + + for line in sshGOutput.split(separator: "\n") { + let parts = line.split(maxSplits: 1, whereSeparator: \.isWhitespace) + guard parts.count == 2 else { continue } + let key = parts[0].lowercased() + let value = parts[1].trimmingCharacters(in: .whitespacesAndNewlines) + + guard Self.relayTransportKeys.contains(key), !value.isEmpty else { continue } + + switch key { + case "hostname": + args += ["-o", "Hostname=\(value)"] + case "user": + args += ["-l", value] + case "port": + // Only add if not already set via configuration.port + if configuration.port == nil, let portNum = Int(value), portNum != 22 { + args += ["-p", value] + } + case "identityfile": + // ssh -G may emit multiple identity files; include all that exist on disk. + let expanded = (value as NSString).expandingTildeInPath + if FileManager.default.fileExists(atPath: expanded) { + args += ["-i", value] + } + case "proxyjump": + if value.lowercased() != "none" { + // Resolve the ProxyJump chain so jump hosts don't need config-file + // resolution (since -F /dev/null is propagated to jump connections). + let resolved = resolveProxyJumpChain(value) + args += ["-J", resolved] + } + case "proxycommand": + if value.lowercased() != "none" { + args += ["-o", "ProxyCommand=\(value)"] + } + default: + args += ["-o", "\(parts[0])=\(value)"] + } + } + + return args + } + + /// Resolve a ProxyJump chain by running `ssh -G` on each hop to get its + /// actual hostname, user, and port. This is needed because `-F /dev/null` + /// is propagated to jump host SSH processes, preventing config-file Host + /// alias resolution. + /// + /// Input: "jumphost,bastion" (comma-separated hops, may be config aliases) + /// Output: "admin@jump.example.com:22,user@bastion.example.com:22" (resolved) + private func resolveProxyJumpChain(_ chain: String) -> String { + let hops = chain.split(separator: ",").map { + $0.trimmingCharacters(in: .whitespacesAndNewlines) + } + + let resolved = hops.map { hop -> String in + // If the hop already contains @ or : it's likely already explicit + if hop.contains("@") && hop.contains(":") { + return hop + } + // Try to resolve via ssh -G + guard let result = try? sshExec(arguments: ["-G", hop], timeout: 5), + result.status == 0 else { + return hop // Can't resolve; pass through as-is + } + var hostname: String? + var user: String? + var port: String? + for line in result.stdout.split(separator: "\n") { + let parts = line.split(maxSplits: 1, whereSeparator: \.isWhitespace) + guard parts.count == 2 else { continue } + let key = parts[0].lowercased() + let val = parts[1].trimmingCharacters(in: .whitespacesAndNewlines) + switch key { + case "hostname": hostname = val + case "user": user = val + case "port": port = val + default: break + } + } + guard let resolvedHost = hostname else { return hop } + var resolved = "" + if let user { resolved += "\(user)@" } + resolved += resolvedHost + if let port, port != "22" { resolved += ":\(port)" } + return resolved + } + + return resolved.joined(separator: ",") + } + + /// Fallback transport options when ssh -G is unavailable. Uses only the + /// explicitly configured port and identity file. + private func explicitRelayTransportOptions() -> [String] { + var args: [String] = [] + if let port = configuration.port { + args += ["-p", String(port)] + } + if let identityFile = configuration.identityFile, + !identityFile.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { + args += ["-i", identityFile] + } + return args + } + private func startReverseRelayViaControlMasterLocked(forwardSpec: String) -> Bool { guard let arguments = WorkspaceRemoteSSHBatchCommandBuilder.reverseRelayControlMasterArguments( configuration: configuration, @@ -4119,6 +4289,7 @@ final class WorkspaceRemoteSessionController { let batchSSHControlOptionKeys: Set = [ "controlmaster", "controlpersist", + "controlpath", ] return normalizedSSHOptions(options).filter { option in guard let key = sshOptionKey(option) else { return false } diff --git a/docs/remote-daemon-spec.md b/docs/remote-daemon-spec.md index 3c8bb0c8a..6fc35e148 100644 --- a/docs/remote-daemon-spec.md +++ b/docs/remote-daemon-spec.md @@ -45,7 +45,7 @@ This is a **living implementation spec** (also called an **execution spec**): a - `DONE` `cmuxd-remote` includes a table-driven CLI relay (`cli` subcommand) that maps CLI args to v1 text or v2 JSON-RPC messages. - `DONE` busybox-style argv[0] detection: when invoked as `cmux` via wrapper/symlink, auto-dispatches to CLI relay. - `DONE` background `ssh -N -R 127.0.0.1:PORT:127.0.0.1:LOCAL_RELAY_PORT` process reverse-forwards a TCP port to a dedicated authenticated local relay server. Uses TCP instead of Unix socket forwarding because many servers have `AllowStreamLocalForwarding` disabled. -- `DONE` relay process uses `-S none` / standalone SSH transport (avoids ControlMaster multiplexing and inherited `RemoteForward` directives) and `ExitOnForwardFailure=yes` so dead reverse binds fail fast instead of publishing bad relay metadata. +- `DONE` relay process uses `-S none -F /dev/null` / standalone SSH transport with no config file (avoids ControlMaster multiplexing and prevents config-file `LocalForward`/`DynamicForward`/`RemoteForward` directives from conflicting with the primary connection). Transport-critical options (ProxyJump, IdentityFile, etc.) are resolved via `ssh -G` and passed explicitly. `ExitOnForwardFailure=yes` ensures dead reverse binds fail fast instead of publishing bad relay metadata. - `DONE` relay address written to `~/.cmux/socket_addr` on the remote only after the reverse forward survives startup validation. - `DONE` Go CLI no longer polls for relay readiness. It dials the published relay once and only refreshes `~/.cmux/socket_addr` a single time to recover from a stale shared address rewrite. - `DONE` `cmux ssh` startup exports session-local `CMUX_SOCKET_PATH=127.0.0.1:` so parallel sessions pin to their own relay instead of racing on shared socket_addr. From 886b0ca3d90bcda0a3e088762c9cda29cc57e1ce Mon Sep 17 00:00:00 2001 From: JR Date: Mon, 6 Apr 2026 17:00:03 +0200 Subject: [PATCH 2/3] Address review feedback: port regression, StrictHostKeyChecking, IPv6 Fix P1 port regression: when configuration.port is set, the ssh -G resolved port was skipped but never re-added. Now uses configuration.port as the effective port with ssh -G value as fallback. Fix P2 StrictHostKeyChecking TOFU downgrade: no longer hard-codes accept-new when the user's SSH config has an explicit policy. The resolved value from ssh -G is propagated; accept-new is only applied as a default when no policy was configured. Fix IPv6 ProxyJump formatting: bracket IPv6 literals when a non-default port is appended (user@[2001:db8::1]:2222 instead of the malformed user@2001:db8::1:2222). Add missing transport keys: certificatefile, identityagent, and stricthostkeychecking are now included in the relayTransportKeys whitelist so they pass through from ssh -G output. --- Sources/Workspace.swift | 56 +++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 6a3ac6a95..1b744eef1 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -4007,6 +4007,12 @@ final class WorkspaceRemoteSessionController { } } + /// Resolved SSH transport options with metadata about what was set. + private struct ResolvedTransportOptions { + let args: [String] + let hasStrictHostKeyChecking: Bool + } + private func reverseRelayArguments(relayPort: Int, localRelayPort: Int) -> [String] { // Fallback standalone transport when dynamic forwarding through an existing // control master is unavailable. @@ -4016,16 +4022,22 @@ final class WorkspaceRemoteSessionController { // ports already bound by the primary SSH connection. Transport-critical options // (ProxyJump, ProxyCommand, HostKeyAlgorithms, etc.) are resolved via ssh -G // and passed explicitly so connectivity is preserved without config-file forwards. + let resolved = resolvedRelayTransportOptions() var args: [String] = ["-N", "-T", "-S", "none", "-F", "/dev/null"] - args += resolvedRelayTransportOptions() + args += resolved.args args += [ "-o", "ConnectTimeout=6", "-o", "ServerAliveInterval=20", "-o", "ServerAliveCountMax=2", - "-o", "StrictHostKeyChecking=accept-new", "-o", "BatchMode=yes", "-o", "ExitOnForwardFailure=yes", "-o", "RequestTTY=no", + ] + // Only add StrictHostKeyChecking default if not resolved from user config. + if !resolved.hasStrictHostKeyChecking { + args += ["-o", "StrictHostKeyChecking=accept-new"] + } + args += [ "-R", "127.0.0.1:\(relayPort):127.0.0.1:\(localRelayPort)", configuration.destination, ] @@ -4035,7 +4047,7 @@ final class WorkspaceRemoteSessionController { /// Resolve transport-critical SSH options for the relay by running `ssh -G`. /// This extracts options like ProxyJump, ProxyCommand, IdentityFile, HostKeyAlgorithms, /// etc. from the user's SSH config without carrying over forwarding directives. - private func resolvedRelayTransportOptions() -> [String] { + private func resolvedRelayTransportOptions() -> ResolvedTransportOptions { // Build the ssh -G probe command with the same port/identity/options as the // primary connection so Host/Match blocks resolve identically. var probeArgs: [String] = ["-G"] @@ -4055,7 +4067,10 @@ final class WorkspaceRemoteSessionController { result.status == 0 else { debugLog("remote.relay.sshG.failed falling back to explicit config only") // Fallback: use just the explicitly configured options without ssh -G resolution. - return explicitRelayTransportOptions() + return ResolvedTransportOptions( + args: explicitRelayTransportOptions(), + hasStrictHostKeyChecking: false + ) } return parseRelayTransportOptions(from: result.stdout) @@ -4070,6 +4085,8 @@ final class WorkspaceRemoteSessionController { "proxycommand", "identityfile", "identitiesonly", + "certificatefile", + "identityagent", "hostkeyalgorithms", "kexalgorithms", "ciphers", @@ -4079,11 +4096,13 @@ final class WorkspaceRemoteSessionController { "globalknownhostsfile", "pubkeyacceptedalgorithms", "preferredauthentications", + "stricthostkeychecking", ] /// Parse `ssh -G` output and extract transport-critical options as `-o Key=Value` pairs. - private func parseRelayTransportOptions(from sshGOutput: String) -> [String] { + private func parseRelayTransportOptions(from sshGOutput: String) -> ResolvedTransportOptions { var args: [String] = [] + var hasStrictHostKeyChecking = false for line in sshGOutput.split(separator: "\n") { let parts = line.split(maxSplits: 1, whereSeparator: \.isWhitespace) @@ -4099,9 +4118,10 @@ final class WorkspaceRemoteSessionController { case "user": args += ["-l", value] case "port": - // Only add if not already set via configuration.port - if configuration.port == nil, let portNum = Int(value), portNum != 22 { - args += ["-p", value] + // Prefer explicitly configured port; fall back to ssh -G resolved value. + let effectivePort = configuration.port.map(String.init) ?? value + if let portNum = Int(effectivePort), portNum != 22 { + args += ["-p", effectivePort] } case "identityfile": // ssh -G may emit multiple identity files; include all that exist on disk. @@ -4120,12 +4140,15 @@ final class WorkspaceRemoteSessionController { if value.lowercased() != "none" { args += ["-o", "ProxyCommand=\(value)"] } + case "stricthostkeychecking": + hasStrictHostKeyChecking = true + args += ["-o", "StrictHostKeyChecking=\(value)"] default: args += ["-o", "\(parts[0])=\(value)"] } } - return args + return ResolvedTransportOptions(args: args, hasStrictHostKeyChecking: hasStrictHostKeyChecking) } /// Resolve a ProxyJump chain by running `ssh -G` on each hop to get its @@ -4141,8 +4164,8 @@ final class WorkspaceRemoteSessionController { } let resolved = hops.map { hop -> String in - // If the hop already contains @ or : it's likely already explicit - if hop.contains("@") && hop.contains(":") { + // If the hop already looks fully qualified (user@host:port), pass through. + if hop.contains("@") && (hop.contains("]:") || (!hop.contains("[") && hop.last?.isNumber == true && hop.split(separator: ":").count == 2)) { return hop } // Try to resolve via ssh -G @@ -4168,8 +4191,15 @@ final class WorkspaceRemoteSessionController { guard let resolvedHost = hostname else { return hop } var resolved = "" if let user { resolved += "\(user)@" } - resolved += resolvedHost - if let port, port != "22" { resolved += ":\(port)" } + // Bracket IPv6 literals when a non-default port is appended. + let needsPort = port != nil && port != "22" + let isIPv6 = resolvedHost.contains(":") + if isIPv6 && needsPort { + resolved += "[\(resolvedHost)]:\(port!)" + } else { + resolved += resolvedHost + if needsPort { resolved += ":\(port!)" } + } return resolved } From 2d0298bbc5bb6a035de77e1fd87c491c375ecc28 Mon Sep 17 00:00:00 2001 From: JR Date: Mon, 6 Apr 2026 17:16:30 +0200 Subject: [PATCH 3/3] Fix critical: revert controlpath from shared filter, harden StrictHostKeyChecking fallback Revert controlpath from batchSSHControlOptionKeys (both static and instance). Adding it there broke the preferred ControlMaster path and batch SSH/SCP helpers that need ControlPath to attach to the existing control socket. The original secondary bug (ControlPath leaking into the standalone relay command) is now moot since the relay uses -F /dev/null and builds args from ssh -G output, not sshCommonArguments. Harden StrictHostKeyChecking fallback: when ssh -G fails and we fall back to explicit options, also check configuration.sshOptions for an explicit StrictHostKeyChecking value before defaulting to accept-new. This prevents silently downgrading a user's host-key policy on the conservative fallback path. Document ProxyJump per-hop config limitation: the -J string format cannot carry hop-specific IdentityFile, CertificateFile, or HostKeyAlias. The preferred ControlMaster path does not have this limitation. The SSH agent handles authentication for most cases. --- Sources/Workspace.swift | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 1b744eef1..df4c4b3b0 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -1073,7 +1073,6 @@ enum WorkspaceRemoteSSHBatchCommandBuilder { private static let batchSSHControlOptionKeys: Set = [ "controlmaster", "controlpersist", - "controlpath", ] static func daemonTransportArguments( @@ -4033,8 +4032,10 @@ final class WorkspaceRemoteSessionController { "-o", "ExitOnForwardFailure=yes", "-o", "RequestTTY=no", ] - // Only add StrictHostKeyChecking default if not resolved from user config. - if !resolved.hasStrictHostKeyChecking { + // Only add StrictHostKeyChecking default if not resolved from ssh -G and not + // explicitly set in configuration.sshOptions. + if !resolved.hasStrictHostKeyChecking + && !hasSSHOptionKey(configuration.sshOptions, key: "StrictHostKeyChecking") { args += ["-o", "StrictHostKeyChecking=accept-new"] } args += [ @@ -4158,6 +4159,15 @@ final class WorkspaceRemoteSessionController { /// /// Input: "jumphost,bastion" (comma-separated hops, may be config aliases) /// Output: "admin@jump.example.com:22,user@bastion.example.com:22" (resolved) + /// + /// Limitation: per-hop config beyond hostname/user/port (e.g. hop-specific + /// IdentityFile, CertificateFile, HostKeyAlias) cannot be represented in + /// the `-J` string and is lost when `-F /dev/null` propagates to jump + /// processes. This affects bastion chains where the jump host requires a + /// dedicated key or host-key alias. The preferred ControlMaster path does + /// not have this limitation since it reuses the primary connection. The SSH + /// agent will handle authentication for most cases; users with non-agent + /// hop-specific keys should ensure the ControlMaster path succeeds. private func resolveProxyJumpChain(_ chain: String) -> String { let hops = chain.split(separator: ",").map { $0.trimmingCharacters(in: .whitespacesAndNewlines) @@ -4319,7 +4329,6 @@ final class WorkspaceRemoteSessionController { let batchSSHControlOptionKeys: Set = [ "controlmaster", "controlpersist", - "controlpath", ] return normalizedSSHOptions(options).filter { option in guard let key = sshOptionKey(option) else { return false }