a73x

38deccfb

fix: the -A preflight asks the agent, it does not just dial it

a73x   2026-08-21 10:20

Commit message
fix: the -A preflight asks the agent, it does not just dial it

`connect()` + `close()` proves a socket exists, not that an agent is behind
it — and the one place `-A` is easiest to get wrong is exactly where that
distinction matters. Inside a mux session `SSH_AUTH_SOCK` names the daemon's
own per-session socket, which accepts every connection and only afterwards
looks for an offerer to route it to. So the dial succeeded, the preflight
passed, and the nested client attached as an offerer that could answer
nothing: precisely the silent agentless offerer the check exists to refuse,
and one that can out-rank a working `-A` client because `agentAnswerer`
picks the latest-active OFFERER and offering is a declaration.

The probe is now a round trip — SSH_AGENTC_REQUEST_IDENTITIES, the five
bytes `ssh-add -l` sends, and a wait for a reply header. That is also what
keeps the legitimate nesting working, which no environment-based check
could: with an `-A` client on the outer session the request is forwarded
out, the real agent answers, and the nested `-A` is allowed.

Fails open on silence, closed on a hangup: the refusal is immediate, so
slowness is not the discriminator, and a hardware token or a cold-starting
gpg-agent must not be refused by a check that only meant to catch an
absence.

The daemon still pumps blind; proxy, protocol and the QUIC modules still
carry opaque bytes. What learned five bytes of ssh-agent framing is the
client, which is the agent's own peer.

Patch: 3e1a136a
README.md
Old New
@@ -178,20 +178,23 @@ and every client would share one identity — but it does mean local sessions
178 need `mux -A --sock PATH` (or `mux -A` on the auto-started daemon) where 178 need `mux -A --sock PATH` (or `mux -A` on the auto-started daemon) where
179 they previously needed nothing. 179 they previously needed nothing.
180 180
181 `-A` with no agent running is a usage error, not a silent no-op: `mux` 181 `-A` with no agent running is a usage error, not a silent no-op: `mux` asks
182 dials `$SSH_AUTH_SOCK` before it attaches and exits 2 if nothing answers, 182 `$SSH_AUTH_SOCK` to list its identities before it attaches — the request
183 including the common case of a variable left behind by an agent that has 183 `ssh-add -l` sends — and exits 2 if nothing answers, including the common
184 died. Without that check the flag was kept silently — the offer is a 184 case of a variable left behind by an agent that has died. Without that
185 declaration, not a capability — and the first sign was `permission denied 185 check the flag was kept silently — the offer is a declaration, not a
186 (publickey)` from a git remote inside the session. 186 capability — and the first sign was `permission denied (publickey)` from a
187 187 git remote inside the session.
188 One case the preflight cannot catch: run `mux -A` from *inside* a mux 188
189 session and `SSH_AUTH_SOCK` is the daemon's own socket, which accepts every 189 It is a request and not a bare connect because of nesting. Run `mux -A`
190 connection and only refuses afterwards — so the dial succeeds and the inner 190 from *inside* a mux session and `SSH_AUTH_SOCK` is the daemon's own socket,
191 client attaches. If an `-A` client is attached to the outer session the 191 which accepts every connection and only afterwards looks for a client to
192 chain works end to end; if not, the inner one is the silent agentless 192 route it to; a dial succeeds there whether or not anyone can answer. Asking
193 offerer this check exists to catch. Nesting is the one place to check by 193 tells the two apart: with an `-A` client on the outer session the request is
194 hand, with `ssh-add -l`. 194 forwarded out and the real agent replies, so the nested `-A` is allowed;
195 with nobody offering, the daemon hangs up and the nested `-A` is refused.
196 An agent that is merely slow still passes — the refusal is immediate, so
197 being slow is not what separates them.
195 198
196 `-A` goes on the attach form (`mux -A HOST`, `mux -A quic://HOST`, 199 `-A` goes on the attach form (`mux -A HOST`, `mux -A quic://HOST`,
197 `mux -A --sock PATH`), not on `mux wall`, which refuses a flag where a 200 `mux -A --sock PATH`), not on `mux wall`, which refuses a flag where a
docs/decisions.md
Old New
@@ -5149,3 +5149,52 @@ to ship if it disagrees with `build.zig` — a stale stage directory under a
5149 bumped number is the failure a hand-run recipe actually has. The binaries are 5149 bumped number is the failure a hand-run recipe actually has. The binaries are
5150 reproducible (a rebuild matched the published sha256s byte for byte); the 5150 reproducible (a rebuild matched the published sha256s byte for byte); the
5151 gzip wrapper is not, since it embeds an mtime. 5151 gzip wrapper is not, since it embeds an mtime.
5152
5153 ## 2026-08-21 (the `-A` preflight asks instead of dialling)
5154
5155 **A connect proves a socket exists, not that an agent is behind it.** The
5156 preflight shipped as `connect()` + `close()`, which is the right question
5157 everywhere except the one place `-A` is easiest to get wrong: inside a mux
5158 session, where `SSH_AUTH_SOCK` names the daemon's own per-session socket.
5159 The daemon accepts every connection and only afterwards looks for an offerer
5160 to route it to — deliberately, so that a session nobody has offered an agent
5161 to refuses fast instead of making ssh wait out a timeout. The dial therefore
5162 succeeded, the preflight passed, and the nested client attached as an
5163 offerer that could answer nothing. It could also out-rank a working `-A`
5164 client on the inner session, since `agentAnswerer` picks the latest-active
5165 OFFERER and offering is a declaration, not a capability.
5166
5167 So the probe became a round trip: `SSH_AGENTC_REQUEST_IDENTITIES` — the
5168 five bytes `ssh-add -l` sends — and a wait for a reply header. This also
5169 keeps the legitimate nesting working, which no environment-based check
5170 could: with an `-A` client on the outer session the request is forwarded
5171 out, the real agent answers, and the nested `-A` is allowed. The alternative
5172 considered and rejected was noticing `proto.session_env` is set and
5173 `SSH_AUTH_SOCK` is that session's socket, the signal the self-attach refusal
5174 reads. It cannot distinguish the two cases at all; it would refuse the chain
5175 that works.
5176
5177 **Fail open on silence, closed on a hangup.** The refusal is a close on an
5178 already-accepted connection and arrives in microseconds, so slowness is not
5179 the discriminator: a poll timeout PASSES. That keeps a hardware token or a
5180 cold-starting gpg-agent — both of which can take a while to answer a first
5181 request — from being refused by a check that only meant to catch an absence.
5182 The 500ms bound is not asked to separate refused from slow; it only has to
5183 outlast a real agent's round trip, including one forwarded back out of an
5184 outer session over a link with an RTT.
5185
5186 **The layering.** The daemon still pumps blind and `proxy.zig`,
5187 `protocol.zig` and the QUIC modules still carry opaque bytes. What knows
5188 five bytes of ssh-agent framing is the CLIENT, which is the agent's own peer
5189 and the process that made the `-A` promise — a different thing from a
5190 transport that would be parsing someone else's exchange. `send` with
5191 `MSG_NOSIGNAL`, not `write`: the peer may already be gone and the preflight
5192 runs before the client installs any signal handling.
5193
5194 The unit test's stand-in compares against a spelled-out `{0,0,0,1,11}`
5195 rather than against the constant under test — mutating the constant survived
5196 until it did, which is the "checks that fail green" hazard in its smallest
5197 form. e2e scenario 58 runs the nested case for real: inside a session nobody
5198 offered an agent to, a nested `mux -A` must exit 2 AND say so with the
5199 preflight's message, because the self-attach refusal standing behind it also
5200 exits 2 and the code alone cannot tell which one spoke.
src/mux_main.zig
Old New
@@ -78,15 +78,59 @@ const ParseResult = union(enum) {
78 usage_error, 78 usage_error,
79 }; 79 };
80 80
81 /// Whether an ssh-agent is actually there to forward. A dial and a close: 81 /// `SSH_AGENTC_REQUEST_IDENTITIES` in the ssh-agent framing: a 4-byte
82 /// the agent is a unix socket on this box, so the question has a cheap 82 /// big-endian length, then the message type. `ssh-add -l` sends exactly
83 /// definite answer and nothing downstream has to guess from a variable 83 /// this, which is why every agent implementation answers it — with an
84 /// being merely set — a stale `SSH_AUTH_SOCK` left by a dead agent is the 84 /// identities list, or a failure if it holds no keys. Either is proof of
85 /// ordinary case, not an exotic one. 85 /// an agent; the preflight never reads past the type.
86 ///
87 /// This is the one place mux knows any ssh-agent bytes, and it belongs
88 /// here rather than in `protocol`, `proxy` or the QUIC modules: the
89 /// forwarding path stays opaque end to end (the daemon pumps blind, by
90 /// design), and what speaks here is the client, which is the agent's own
91 /// peer and the process that made the `-A` promise.
92 const agent_request_identities = [_]u8{ 0, 0, 0, 1, 11 };
93
94 /// How long a probe waits for an answer before deciding it cannot tell.
95 /// The asymmetry below is what sets it: a refusal is a hangup on an
96 /// already-accepted connection and arrives in microseconds, so this bound
97 /// is not asked to separate refused from slow — it only has to outlast a
98 /// real agent's round trip, including one forwarded back out of an outer
99 /// session over a link with an RTT.
100 const agent_probe_ms = 500;
101
102 /// Whether an ssh-agent is actually there to forward. A request and a
103 /// reply, not a dial: inside a mux session `SSH_AUTH_SOCK` names the
104 /// DAEMON's per-session socket, which accepts every connection and only
105 /// then looks for a client to route it to. A bare connect passes there
106 /// even when nobody is offering — and the client it waves through is
107 /// precisely the silent offerer this check exists to refuse, one that can
108 /// out-rank a working `-A` client because offering is a declaration and
109 /// not a capability. The nested case where the outer session DOES have an
110 /// answerer is the one that must keep working, and it does: the request is
111 /// forwarded out and the real agent's reply comes back.
112 ///
113 /// Fails open on silence, closed on a hangup. A refusal is immediate, so
114 /// taking too long is not the discriminator; a hardware token or a
115 /// cold-started gpg-agent may be slow and is still an agent, and refusing
116 /// one would break a setup that works.
86 fn agentReachable(path: []const u8) bool { 117 fn agentReachable(path: []const u8) bool {
87 const fd = client.connectAgent(path) orelse return false; 118 const fd = client.connectAgent(path) orelse return false;
88 std.posix.close(fd); 119 defer std.posix.close(fd);
89 return true; 120
121 // MSG_NOSIGNAL rather than a `write`: the peer may already be gone, and
122 // the preflight runs before the client installs any signal handling, so
123 // an EPIPE has to arrive as an error and not as a fatal signal.
124 _ = std.posix.send(fd, &agent_request_identities, std.posix.MSG.NOSIGNAL) catch return false;
125
126 var pfd = [_]std.posix.pollfd{.{ .fd = fd, .events = std.posix.POLL.IN, .revents = 0 }};
127 const ready = std.posix.poll(&pfd, agent_probe_ms) catch return true;
128 if (ready == 0) return true;
129 var reply: [1]u8 = undefined;
130 // Zero bytes is EOF: accepted, then hung up without answering. That is
131 // the daemon with no offerer behind it, and the only shape refused here.
132 const n = std.posix.read(fd, &reply) catch return false;
133 return n != 0;
90 } 134 }
91 135
92 /// The environment variable consulted when `--key` is absent. Named rather 136 /// The environment variable consulted when `--key` is absent. Named rather
@@ -855,29 +899,95 @@ test {
855 std.testing.refAllDeclsRecursive(@This()); 899 std.testing.refAllDeclsRecursive(@This());
856 } 900 }
857 901
858 test "agentReachable: a live socket answers, a stale or unset path does not" { 902 /// A socket that answers the way a real ssh-agent does, or one that hangs
859 // The three states a user is actually in: an agent running, a variable 903 /// up the way the daemon does for a session nobody has offered an agent to.
904 /// A thread because the probe is a round trip — it writes before it reads,
905 /// so a listener nobody is accepting on cannot play either part.
906 const AgentStub = struct {
907 listener: *std.net.Server,
908 answer: bool,
909 /// Set only when the exact bytes `ssh-add -l` sends arrived. Asserted
910 /// by the test, because every other expectation here is also satisfied
911 /// by a probe that asks nothing and times out.
912 asked: bool = false,
913
914 fn run(self: *AgentStub) void {
915 const conn = self.listener.accept() catch return;
916 defer std.posix.close(conn.stream.handle);
917 if (!self.answer) return;
918 var buf: [64]u8 = undefined;
919 const n = std.posix.read(conn.stream.handle, &buf) catch return;
920 // Spelled out rather than compared against `agent_request_identities`:
921 // the constant IS what is under test, and a test that reads it back
922 // would accept any bytes the client decided to send.
923 self.asked = std.mem.eql(u8, buf[0..n], &[_]u8{ 0, 0, 0, 1, 11 });
924 if (!self.asked) return;
925 // SSH_AGENT_IDENTITIES_ANSWER carrying zero keys. An agent holding
926 // nothing still proves an agent is there, which is the whole
927 // question — the preflight never looks at the key list.
928 const reply = [_]u8{ 0, 0, 0, 5, 12, 0, 0, 0, 0 };
929 _ = std.posix.write(conn.stream.handle, &reply) catch {};
930 }
931 };
932
933 test "agentReachable: an agent answers; a socket that hangs up is not one" {
934 // The states a user is actually in: an agent running, the daemon's own
935 // per-session socket with nobody offering behind it, a variable
860 // pointing at an agent that has died, and no variable at all. Only the 936 // pointing at an agent that has died, and no variable at all. Only the
861 // first may attach with `-A`. 937 // first may attach with `-A`.
862 var tmp = try TmpDir.make(); 938 var tmp = try TmpDir.make();
863 defer tmp.cleanup(); 939 defer tmp.cleanup();
864 var buf: [128]u8 = undefined; 940 var buf: [128]u8 = undefined;
865 const sock = try std.fmt.bufPrintZ(&buf, "{s}/agent.sock", .{tmp.path()}); 941 const sock = try std.fmt.bufPrintZ(&buf, "{s}/agent.sock", .{tmp.path()});
866 const addr = try std.net.Address.initUnix(sock);
867 var listener = try addr.listen(.{});
868 942
869 try std.testing.expect(agentReachable(sock)); 943 {
944 const addr = try std.net.Address.initUnix(sock);
945 var listener = try addr.listen(.{});
946 defer listener.deinit();
947 var stub = AgentStub{ .listener = &listener, .answer = true };
948 const th = try std.Thread.spawn(.{}, AgentStub.run, .{&stub});
949 const reachable = agentReachable(sock);
950 th.join();
951 try std.testing.expect(reachable);
952 try std.testing.expect(stub.asked);
953 }
954 std.fs.deleteFileAbsolute(sock) catch {};
955
956 // The case a bare connect cannot see, and the reason this is a request
957 // and not a dial: inside a session `SSH_AUTH_SOCK` names the DAEMON,
958 // which accepts every connection and only then decides it has no
959 // client to route it to. The connect succeeds; the exchange does not.
960 {
961 const addr = try std.net.Address.initUnix(sock);
962 var listener = try addr.listen(.{});
963 defer listener.deinit();
964 var stub = AgentStub{ .listener = &listener, .answer = false };
965 const th = try std.Thread.spawn(.{}, AgentStub.run, .{&stub});
966 defer th.join();
967 try std.testing.expect(!agentReachable(sock));
968 }
969 std.fs.deleteFileAbsolute(sock) catch {};
970
971 // Fail OPEN on silence, closed only on a hangup. A listener nobody is
972 // accepting on is the shape a slow or wedged agent presents, and a
973 // slow agent is still an agent; the daemon's refusal is immediate, so
974 // taking too long is not what separates the two.
975 {
976 const addr = try std.net.Address.initUnix(sock);
977 var listener = try addr.listen(.{});
978 defer listener.deinit();
979 try std.testing.expect(agentReachable(sock));
980 }
870 981
871 // The stale case, and the reason this dials rather than reading the 982 // The stale case, and the reason this dials at all rather than reading
872 // variable: the agent is gone but its socket FILE is still there, so the 983 // the variable: the agent is gone but its socket FILE is still there,
873 // path stats fine and the connect is refused. `deinit` closes the 984 // so the path stats fine and the connect is refused. `deinit` closed
874 // listener without unlinking, which is exactly what a killed agent 985 // the listener above without unlinking, which is exactly what a killed
875 // leaves behind. 986 // agent leaves behind.
876 listener.deinit();
877 try std.fs.accessAbsolute(sock, .{}); 987 try std.fs.accessAbsolute(sock, .{});
878 try std.testing.expect(!agentReachable(sock)); 988 try std.testing.expect(!agentReachable(sock));
879 989
880 // And the two cheaper absences, so all three of a user's states are 990 // And the two cheaper absences, so every one of a user's states is
881 // covered by the one probe. 991 // covered by the one probe.
882 std.fs.deleteFileAbsolute(sock) catch {}; 992 std.fs.deleteFileAbsolute(sock) catch {};
883 try std.testing.expect(!agentReachable(sock)); 993 try std.testing.expect(!agentReachable(sock));
test/e2e.sh
Old New
@@ -7179,6 +7179,54 @@ grep -qE "agtrc=[1-9]" "$OUT.agtn" || {
7179 echo "e2e FAIL: agent-refusal: the leg took ${AMS}ms — a dial with nobody to" 7179 echo "e2e FAIL: agent-refusal: the leg took ${AMS}ms — a dial with nobody to"
7180 echo " answer it must be refused, not retried" 7180 echo " answer it must be refused, not retried"
7181 exit 1; } 7181 exit 1; }
7182
7183 # --- ...and a nested `-A` inside such a session is refused, not passed -----
7184 #
7185 # The preflight's whole job is to refuse a client that cannot answer a
7186 # challenge. Inside a session `SSH_AUTH_SOCK` names the daemon, which
7187 # accepts every dial and only afterwards looks for an offerer to route it
7188 # to — so the connect the preflight used to do succeeded there, and the
7189 # nested client attached as an offerer that can answer nothing. Worse, it
7190 # out-ranks a working `-A` client, because the answerer is the latest-active
7191 # OFFERER and offering is a declaration.
7192 #
7193 # `--session noagent` is the session this shell is already in, deliberately:
7194 # the preflight runs before any transport work, so it is what must answer
7195 # here, and the self-attach refusal standing behind it means a regression
7196 # fails this leg instead of spawning a nested attach into the pty. Which of
7197 # the two spoke is what the message grep decides; the exit code alone
7198 # cannot, since both refusals use 2.
7199 #
7200 # Unquoted heredoc, unlike the leg above: $MUX and $SOCK48 have to be
7201 # expanded by THIS shell, so `$?` is escaped to reach the session's.
7202 set +e
7203 timeout 40 "$PTYCLIENT" --cols 100 --rows 30 --out "$OUT.agtnest" --err "$OUT.agtnest.err" \
7204 -- "$MUX" --sock "$SOCK48" --session noagent > "$OUT.agtnest.log" 2>&1 <<EOF
7205 expect \x1b[?1049h 15000
7206 settle 400 15000
7207 send $MUX -A --sock $SOCK48 --session noagent; echo nest=\$?\n
7208 settle 1500 15000
7209 send exit\n
7210 waitexit 10000
7211 EOF
7212 RC=$?
7213 set -e
7214 [ "$RC" -eq 0 ] || {
7215 echo "e2e FAIL: agent-nested: the leg exited $RC:"
7216 cat "$OUT.agtnest.log"; exit 1; }
7217 # Both needles are echo-proof: the typed line spells `nest=$?` and says
7218 # nothing about ssh-agents, so neither `nest=2` nor the message below can
7219 # come from the shell echoing what it was sent.
7220 grep -q "nest=2" "$OUT.agtnest" || {
7221 echo "e2e FAIL: agent-nested: a nested \`mux -A\` did not exit 2 — the"
7222 echo " preflight let through a client with no agent behind it:"
7223 cat -v "$OUT.agtnest"; exit 1; }
7224 grep -q "no ssh-agent answering at" "$OUT.agtnest" || {
7225 echo "e2e FAIL: agent-nested: the nested \`mux -A\` was refused, but not by"
7226 echo " the agent preflight — something else answered first:"
7227 cat -v "$OUT.agtnest"; exit 1; }
7228 ok "agent forwarding: a nested -A with nobody offering is refused"
7229
7182 assert_stopped "$SOCK48" "$D42PID" "agent forwarding" "$OUT.agtstop" 7230 assert_stopped "$SOCK48" "$D42PID" "agent forwarding" "$OUT.agtstop"
7183 D42PID="" 7231 D42PID=""
7184 # The agent has no more work; ended here rather than in the trap so a green 7232 # The agent has no more work; ended here rather than in the trap so a green
@@ -7454,8 +7502,8 @@ DPID=""
7454 # 59th is the `-A` preflight, and no convergence point because it never 7502 # 59th is the `-A` preflight, and no convergence point because it never
7455 # attaches: what it asserts on is an exit code and a message, before any 7503 # attaches: what it asserts on is an exit code and a message, before any
7456 # transport exists to converge. 7504 # transport exists to converge.
7457 [ "$OK_COUNT" = "59" ] || { 7505 [ "$OK_COUNT" = "60" ] || {
7458 echo "e2e FAIL: $OK_COUNT scenario checkpoints ran, the pin says 59 —" 7506 echo "e2e FAIL: $OK_COUNT scenario checkpoints ran, the pin says 60 —"
7459 echo " a scenario was added (update the pin) or silently lost" 7507 echo " a scenario was added (update the pin) or silently lost"
7460 exit 1 7508 exit 1
7461 } 7509 }
@@ -7463,4 +7511,4 @@ DPID=""
7463 echo "e2e FAIL: $CONV_COUNT convergence points ran, the pin says 35" 7511 echo "e2e FAIL: $CONV_COUNT convergence points ran, the pin says 35"
7464 exit 1 7512 exit 1
7465 } 7513 }
7466 echo "e2e OK (59 scenarios, 35 convergence points)" 7514 echo "e2e OK (60 scenarios, 35 convergence points)"