From 756b220143c19c029f5d7215c92c497825b48d3b Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 29 Oct 2026 22:06:56 -0400 Subject: [PATCH 1/4] Fix: Bridge a CONNECT whose origin is dead so a redirect can still apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session redirected away from an inference server (#1595) never got its redirect when the original server was down, because two gates killed the connection before the pipeline could run: - handleConnect (and the transparent listener) pre-dial the CONNECT target and answered 502 dial_failed when the dial failed, before the bridge verdict was ever consulted. With the TLS bridge enabled, a failed pre-dial now continues with no upstream instead: the bridge can still take the connection, and a plugin redirect sends the decrypted request to a live server. The plain-tunnel path re-dials (tunnelOrDial) so it always has a real upstream; without the bridge the 502 stands. - bridgeServe's pre-forge origin probe fell back to a plain opaque tunnel on any failure, and an opaque tunnel is the one shape a redirect cannot reach. The probe is unchanged for origins that dialed — its passthrough fallback is right where the client's own e2e TLS still works — but is skipped when the pre-dial already failed, where it would only re-confirm a dead origin at up to 10s more. Failed pre-dials are memoized per target for 30s: a client that opens a CONNECT per request (undici) must not pay a dead destination's dial timeout — 10s on a filtered destination — on every one. The memo never misroutes: a skipped dial is re-attempted wherever its result would have been used. Fixes #1595 Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../forwardproxy/bridge_program_test.go | 16 +- core/listener/forwardproxy/redirect_test.go | 124 +++++++++++ core/listener/forwardproxy/server.go | 198 +++++++++++++----- core/listener/forwardproxy/transparent.go | 56 +++-- .../listener/forwardproxy/tunnelclose_test.go | 2 +- .../tunnelreason_integration_test.go | 16 +- 6 files changed, 325 insertions(+), 87 deletions(-) diff --git a/core/listener/forwardproxy/bridge_program_test.go b/core/listener/forwardproxy/bridge_program_test.go index cab37c3b4..ca11ae821 100644 --- a/core/listener/forwardproxy/bridge_program_test.go +++ b/core/listener/forwardproxy/bridge_program_test.go @@ -239,7 +239,7 @@ func TestBridgeProgram_AStaleProcessDoesNotStopItsProgram(t *testing.T) { for range 2 { tl := discardTunnel() tl.program = &staleProg - sc.server.bridgeServe(rejectingClient(t), sc.target, hostOnly(sc.target), tl) + sc.server.bridgeServe(rejectingClient(t), sc.target, hostOnly(sc.target), tl, false) } if sc.engine.Programs.Contains(claude) { t.Fatal("the stale process's refusals were recorded against its program, which a fresh claude shares") @@ -267,7 +267,7 @@ func TestBridgeServe_RecordsANamedProgramsRefusalAgainstTheProgram(t *testing.T) tl := discardTunnel() tl.program = &prog - s.bridgeServe(rejectingClient(t), authority, host, tl) + s.bridgeServe(rejectingClient(t), authority, host, tl, false) if !s.TLSBridge.Programs.Contains(prog.Key()) { t.Error("the refusal was not recorded against the program") } @@ -287,7 +287,7 @@ func TestBridgeServe_RecordsAStaleProcesssRefusalAgainstTheProcess(t *testing.T) tl := discardTunnel() tl.program = &prog - s.bridgeServe(rejectingClient(t), authority, host, tl) + s.bridgeServe(rejectingClient(t), authority, host, tl, false) if !s.TLSBridge.Programs.Contains(prog.ProcessKey()) { t.Error("a process older than the CA did not have its refusal recorded against the process") } @@ -321,7 +321,7 @@ func TestBridgeServe_WithoutEvidenceOfStalenessRecordsAgainstTheProgram(t *testi tl := discardTunnel() tl.program = &prog - s.bridgeServe(rejectingClient(t), authority, hostOnly(authority), tl) + s.bridgeServe(rejectingClient(t), authority, hostOnly(authority), tl, false) if !s.TLSBridge.Programs.Contains(prog.Key()) { t.Error("the refusal was not recorded against the program") } @@ -346,7 +346,7 @@ func TestBridgeServe_RejectionLogNamesTheAgent(t *testing.T) { logbuf.Reset() tl := discardTunnel() tl.program = &prog - s.bridgeServe(rejectingClient(t), authority, hostOnly(authority), tl) + s.bridgeServe(rejectingClient(t), authority, hostOnly(authority), tl, false) got := logbuf.String() if !strings.Contains(got, "program="+exePython) { t.Fatalf("the rejection line does not name the program:\n%s", got) @@ -367,7 +367,7 @@ func TestBridgeServe_UnnamedTunnelRecordsAgainstTheHost(t *testing.T) { s.TLSBridge.Programs = tlsbridge.NewProgramSkipSet() host := hostOnly(authority) - s.bridgeServe(rejectingClient(t), authority, host, discardTunnel()) + s.bridgeServe(rejectingClient(t), authority, host, discardTunnel(), false) if !s.TLSBridge.Skip.Contains(host) { t.Error("an unnamed tunnel's refusal was not recorded against the host") } @@ -381,7 +381,7 @@ func TestBridgeServe_ProgramWithoutProgramMemoryFallsBackToTheHost(t *testing.T) tl := discardTunnel() tl.program = &tlsbridge.Program{Exe: exeHelm} - s.bridgeServe(rejectingClient(t), authority, host, tl) + s.bridgeServe(rejectingClient(t), authority, host, tl, false) if !s.TLSBridge.Skip.Contains(host) { t.Error("with no program memory the refusal was not recorded against the host") } @@ -400,7 +400,7 @@ func TestBridgeServe_ASuccessClearsTheProgramsEntry(t *testing.T) { tl.program = &prog // bridgeServe blocks serving the decrypted connection, so run it and wait. - go s.bridgeServe(trustingClient(t, s.TLSBridge.CAPEM), authority, host, tl) + go s.bridgeServe(trustingClient(t, s.TLSBridge.CAPEM), authority, host, tl, false) eventually(t, func() bool { return !s.TLSBridge.Programs.Contains(prog.Key()) && !s.TLSBridge.Programs.Contains(prog.ProcessKey()) }, "a completed handshake to clear the program's entry and the process's") diff --git a/core/listener/forwardproxy/redirect_test.go b/core/listener/forwardproxy/redirect_test.go index 324ae8e65..48d6668b8 100644 --- a/core/listener/forwardproxy/redirect_test.go +++ b/core/listener/forwardproxy/redirect_test.go @@ -517,3 +517,127 @@ func TestForwardProxy_ADeniedRedirectRecordsBothHosts(t *testing.T) { t.Errorf("origin b served a denied request: %v", hosts) } } + +// #1595's headline: a session redirected away from a server that has since died. +// The pre-forge probe of the dead origin must not fall back to a plain tunnel — a +// plain tunnel dials the same dead origin, and an opaque tunnel is the one shape a +// redirect cannot reach. The bridge proceeds, and the redirect sends the request to +// the live server. +func TestConnectBridge_RedirectWorksWhenTheNamedOriginIsDead(t *testing.T) { + a := newOrigin(t, "FROM-A", (*httptest.Server).StartTLS) + b := newOrigin(t, "FROM-B", (*httptest.Server).StartTLS) + engine := redirectBridge(t, portOf(a.authority()), certPEM(a.Server)) + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + proxy := newRedirectProxy(t, store, engine, &redirectPlugin{target: b.URL}) + + a.Close() // the server the client still names is gone + + status, body := bridgedGet(t, proxy, engine.CAPEM, a.authority(), "/secret") + if status != http.StatusOK || body != "FROM-B" { + t.Fatalf("response = %d %q, want 200 FROM-B — the redirect never applied", status, body) + } + if hosts, _ := b.gets(); len(hosts) != 1 || hosts[0] != b.authority() { + t.Errorf("origin b saw GETs %v, want one for %s", hosts, b.authority()) + } +} + +// The passthrough fallback keeps its reason to exist: an origin that is reachable +// but whose certificate the bridge cannot verify. Bridging would re-originate into +// the same verification failure, so the connection is tunnelled untouched and the +// agent's own e2e TLS decides. +func TestConnectBridge_UntrustedOriginStillFallsOpenToTunnel(t *testing.T) { + a := newOrigin(t, "FROM-A", (*httptest.Server).StartTLS) + foreign, err := tlsbridge.NewEphemeralSource() + if err != nil { + t.Fatalf("NewEphemeralSource: %v", err) + } + // The bridge's upstream client trusts a CA that is not the origin's. + engine := redirectBridge(t, portOf(a.authority()), foreign.CACertPEM()) + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + proxy := newRedirectProxy(t, store, engine, &redirectPlugin{target: a.URL}) + + rawConn, err := net.Dial("tcp", mustParseURL(proxy.URL).Host) + if err != nil { + t.Fatalf("dial proxy: %v", err) + } + defer func() { _ = rawConn.Close() }() + _ = rawConn.SetDeadline(time.Now().Add(10 * time.Second)) + connectReq, err := http.NewRequest(http.MethodConnect, "//"+a.authority(), nil) + if err != nil { + t.Fatalf("new CONNECT request: %v", err) + } + connectReq.Host = a.authority() + connectReq.URL.Host = a.authority() + if err := connectReq.Write(rawConn); err != nil { + t.Fatalf("write CONNECT: %v", err) + } + br := bufio.NewReader(rawConn) + connectResp, err := http.ReadResponse(br, connectReq) + if err != nil { + t.Fatalf("read CONNECT response: %v", err) + } + _ = connectResp.Body.Close() + if connectResp.StatusCode != http.StatusOK { + t.Fatalf("CONNECT status = %d, want 200", connectResp.StatusCode) + } + pool := x509.NewCertPool() + if !pool.AppendCertsFromPEM(engine.CAPEM) { + t.Fatal("bridge CA PEM did not parse") + } + tconn := tls.Client(&bufferedConn{Conn: rawConn, r: br}, &tls.Config{ + ServerName: hostOnly(a.authority()), + RootCAs: pool, + NextProtos: []string{"http/1.1"}, + }) + if err := tconn.Handshake(); err == nil { + _ = tconn.Close() + t.Fatal("handshake against a leaf the bridge did not forge succeeded; the connection should have been tunnelled to the real origin, whose cert this client does not trust") + } + eventually(t, func() bool { + return len(tunnelReasons(store, session.DefaultSessionID)) > 0 + }, "the passthrough tunnel's open row to be recorded") + for _, reason := range tunnelReasons(store, session.DefaultSessionID) { + if reason != pipeline.TunnelOriginUnverified { + t.Errorf("tunnel reason = %q, want %q", reason, pipeline.TunnelOriginUnverified) + } + } +} + +// A failed pre-dial is memoized: a client that opens a CONNECT per request +// (undici) must not pay a dead destination's dial cost — on a filtered +// destination, its full connectDialTimeout — on every one of them. +func TestConnectBridge_DeadOriginDialIsMemoized(t *testing.T) { + a := newOrigin(t, "FROM-A", (*httptest.Server).StartTLS) + b := newOrigin(t, "FROM-B", (*httptest.Server).StartTLS) + engine := redirectBridge(t, portOf(a.authority()), certPEM(a.Server)) + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + proxy := newRedirectProxy(t, store, engine, &redirectPlugin{target: b.URL}) + + var dials struct { + mu sync.Mutex + n int + } + inner := dialTCP + dialTCP = func(network, addr string, d time.Duration) (net.Conn, error) { + dials.mu.Lock() + dials.n++ + dials.mu.Unlock() + return inner(network, addr, d) + } + t.Cleanup(func() { dialTCP = inner }) + + a.Close() // the server the client still names is gone + for i := 0; i < 2; i++ { + if status, body := bridgedGet(t, proxy, engine.CAPEM, a.authority(), "/secret"); status != http.StatusOK || body != "FROM-B" { + t.Fatalf("request %d: response = %d %q, want 200 FROM-B", i, status, body) + } + } + dials.mu.Lock() + defer dials.mu.Unlock() + if dials.n != 1 { + t.Errorf("pre-dial ran %d times for 2 connections, want 1 (failures are memoized)", dials.n) + } +} diff --git a/core/listener/forwardproxy/server.go b/core/listener/forwardproxy/server.go index 80c124324..4de175af3 100644 --- a/core/listener/forwardproxy/server.go +++ b/core/listener/forwardproxy/server.go @@ -208,6 +208,11 @@ type Server struct { caFingerprintStr string bridgeWarnOnce sync.Once bridgeWarned atomic.Bool + + // dialMu guards dialFails, the memo of failed CONNECT pre-dials (dialUpstream). + // Lazy: the Server can be built as a literal in tests. + dialMu sync.Mutex + dialFails map[string]dialFailMemo } // MTLSOptions configures outbound mTLS for the forward proxy. When @@ -1047,29 +1052,40 @@ func (s *Server) bridgeMemory(tl *tunnelLog, host string) (*tlsbridge.SkipSet, s // bridgeServe records its close too. A bridged tunnel is settled by whichever of // finish and the last handler's release comes last, which records what the tunnel // still owes — its open and a close — only if no request recorded a row. On the -// false path the caller tunnels and records the close itself. -func (s *Server) bridgeServe(client net.Conn, authority, host string, tl *tunnelLog) bool { +// false path the caller tunnels and records the close itself. originDead says the +// CONNECT's own pre-dial failed, in which case the origin probe is skipped: see +// step 1. +func (s *Server) bridgeServe(client net.Conn, authority, host string, tl *tunnelLog, originDead bool) bool { // 1) Verify upstream reachability + cert via the dedicated client, BEFORE forging. // HEAD avoids GET side-effects; a non-2xx status still returns err==nil (cert // verified), which is all we need. Only a transport/TLS error fails here. The // verify is bounded by its own context timeout so a slow/stalled origin can't // pin the bridging goroutine — the timeout is on this probe ONLY, not on the // relay (which must stay unbounded for streaming responses). - ctx, cancel := context.WithTimeout(context.Background(), upstreamVerifyTimeout) - defer cancel() - req, err := http.NewRequestWithContext(ctx, http.MethodHead, "https://"+authority, nil) - if err != nil { - slog.Info("tls-bridge passthrough", "host", host, "reason", "upstream-verify", "error", err) - tl.open(pipeline.TunnelOriginUnverified) - return false - } - resp, err := s.TLSBridge.Upstream.Do(req) - if err != nil { - slog.Info("tls-bridge passthrough", "host", host, "reason", "upstream-verify", "error", err) - tl.open(pipeline.TunnelOriginUnverified) - return false // fall back to plain tunnel — agent's own e2e TLS still reaches origin + // + // Skipped when the pre-dial already failed (originDead): the probe would only + // re-confirm a dead origin at up to upstreamVerifyTimeout more, and its + // passthrough fallback dials the same dead origin. Bridging proceeds so a + // plugin redirect (inference-router moving a pinned session) can still send + // the request to a live server (#1595); the re-originated request is verified + // against whatever host it actually goes to. + if !originDead { + ctx, cancel := context.WithTimeout(context.Background(), upstreamVerifyTimeout) + defer cancel() + req, err := http.NewRequestWithContext(ctx, http.MethodHead, "https://"+authority, nil) + if err != nil { + slog.Info("tls-bridge passthrough", "host", host, "reason", "upstream-verify", "error", err) + tl.open(pipeline.TunnelOriginUnverified) + return false + } + resp, err := s.TLSBridge.Upstream.Do(req) + if err != nil { + slog.Info("tls-bridge passthrough", "host", host, "reason", "upstream-verify", "error", err) + tl.open(pipeline.TunnelOriginUnverified) + return false // fall back to plain tunnel — agent's own e2e TLS still reaches origin + } + _ = resp.Body.Close() } - _ = resp.Body.Close() // 2) Forge + terminate downstream. tconn, err := s.TLSBridge.Term.Terminate(client, hostOnly(authority)) @@ -1186,6 +1202,69 @@ func (s *Server) bridgeServe(client net.Conn, authority, host string, tl *tunnel return true } +// dialFailMemo is a memoized failed CONNECT pre-dial: the error and when it was +// taken. dialFailMemoTTL bounds how long it is believed. +type dialFailMemo struct { + err error + at time.Time +} + +// dialFailMemoTTL is how long a failed pre-dial is remembered, so a destination +// that drops packets — where a dial burns the whole connectDialTimeout — is not +// re-dialed per CONNECT. Short because the memo never misroutes: a skipped dial is +// re-attempted wherever its result would have been used (the plain-tunnel path +// dials fresh in tunnelOrDial; the bridge path re-originates to a different host +// and dials that), so the staleness it can buy is one skipped dial, never a +// misrouted request. +const dialFailMemoTTL = 30 * time.Second + +// dialTCP is net.DialTimeout; a variable so tests can count dials. +var dialTCP = net.DialTimeout + +// dialUpstream is a CONNECT's pre-dial, memoizing failures per target for +// dialFailMemoTTL (#1595: a client that opens a CONNECT per request must not pay +// a dead destination's dial timeout every time). +func (s *Server) dialUpstream(dialTarget string) (net.Conn, error) { + s.dialMu.Lock() + if s.dialFails == nil { + s.dialFails = make(map[string]dialFailMemo) + } + if m, ok := s.dialFails[dialTarget]; ok { + if time.Since(m.at) < dialFailMemoTTL { + s.dialMu.Unlock() + return nil, m.err + } + delete(s.dialFails, dialTarget) + } + s.dialMu.Unlock() + + conn, err := dialTCP("tcp", dialTarget, connectDialTimeout) + if err == nil { + return conn, nil + } + s.dialMu.Lock() + s.dialFails[dialTarget] = dialFailMemo{err: err, at: time.Now()} + s.dialMu.Unlock() + return nil, err +} + +// tunnelOrDial serves a tunnel whose pre-dialed upstream is gone — the bridge +// declined the connection, or fell open after the origin probe failed — by dialing +// fresh: a working dial tunnels normally, a failing one closes the client with the +// dial error on the close row. The client already has its 200, so that stays the +// status and the error says why the tunnel died. +func (s *Server) tunnelOrDial(clientConn net.Conn, dialTarget string, tl *tunnelLog) { + up2, derr := net.DialTimeout("tcp", dialTarget, connectDialTimeout) + if derr != nil { + _ = clientConn.Close() + tl.close(http.StatusOK, dialError(derr), 0, 0) + return + } + sent, received := tunnel(clientConn, up2) + _ = up2.Close() + tl.close(http.StatusOK, nil, sent, received) +} + // misdirectedBody is the 421's body. Static: the Host it refuses is the client's own // input, and echoing it back would make the proxy a reflector. const misdirectedBody = "misdirected request: the Host header does not name this tunnel's destination\n" @@ -2575,31 +2654,48 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { return } // Not a refusal: the name would not resolve. Same shape as a dial - // failure — nothing was dialed, and the client gets the 502. + // failure — nothing was dialed, and the client gets the 502. With the + // bridge enabled, fall through instead: the dial below fails the same + // way and takes the bridge-first branch, where a redirect can still + // move the request to a name that resolves (#1595). // Both values sanitized: the name is client-controlled, and the error // embeds it (net.DNSError quotes the name it failed to resolve). - slog.Warn("forward-proxy: CONNECT destination resolution failed", - "host", pipeline.SanitizeControlRunes(r.Host), - "error", pipeline.SanitizeControlRunes(cerr.Error())) - http.Error(w, `{"error":"bad gateway"}`, http.StatusBadGateway) - tl.open(pipeline.TunnelDialFailed) - tl.close(http.StatusBadGateway, dialError(cerr), 0, 0) - return + if s.TLSBridge == nil { + slog.Warn("forward-proxy: CONNECT destination resolution failed", + "host", pipeline.SanitizeControlRunes(r.Host), + "error", pipeline.SanitizeControlRunes(cerr.Error())) + http.Error(w, `{"error":"bad gateway"}`, http.StatusBadGateway) + tl.open(pipeline.TunnelDialFailed) + tl.close(http.StatusBadGateway, dialError(cerr), 0, 0) + return + } } dialTarget = checked // The audit trail against rebinding: "asked for the name, dialed this // address" — moca's egress conformance reads it off the tunnel open row. pctx.DialedAddr = checked } - upstream, err := net.DialTimeout("tcp", dialTarget, connectDialTimeout) + upstream, err := s.dialUpstream(dialTarget) + // dialFail remembers a pre-dial that failed while the TLS bridge is enabled: + // the connection continues with no upstream, because the bridge can still take + // it — a plugin redirect (inference-router moving a pinned session) sends the + // decrypted request to a live server, and an opaque tunnel to a dead origin + // helps nobody (#1595). Every path below that would have used the upstream + // handles a nil one by closing with this error or re-dialing (tunnelOrDial). + var dialFail error if err != nil { - slog.Warn("forward-proxy: CONNECT upstream dial failed", "host", r.Host, "error", err) - http.Error(w, `{"error":"bad gateway"}`, http.StatusBadGateway) - // Recorded rather than dropped: an unreachable destination used to leave no - // trace in the timeline at all. - tl.open(pipeline.TunnelDialFailed) - tl.close(http.StatusBadGateway, dialError(err), 0, 0) - return + if s.TLSBridge == nil { + slog.Warn("forward-proxy: CONNECT upstream dial failed", "host", r.Host, "error", err) + http.Error(w, `{"error":"bad gateway"}`, http.StatusBadGateway) + // Recorded rather than dropped: an unreachable destination used to leave no + // trace in the timeline at all. + tl.open(pipeline.TunnelDialFailed) + tl.close(http.StatusBadGateway, dialError(err), 0, 0) + return + } + slog.Warn("forward-proxy: CONNECT upstream dial failed; trying the TLS bridge", + "host", r.Host, "error", err) + dialFail = err } clientConn, _, err := w.(http.Hijacker).Hijack() @@ -2615,7 +2711,9 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { // io.Copy goroutines until the OS finally times the socket out. // 30s is loose enough to not perturb idle traffic and tight enough // that operators get prompt cleanup on dead connections. - enableKeepalive(upstream) + if upstream != nil { + enableKeepalive(upstream) + } enableKeepalive(clientConn) // Tell the agent the tunnel is up. Per RFC 7231 §4.3.6 a 200 to @@ -2640,27 +2738,17 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { reason, bridge = s.bridgeVerdict(r, tl, key, first) if bridge { s.noteBridgeAttempt() - _ = upstream.Close() // bridgeServe dials its own verified upstream - if s.bridgeServe(clientConn, authority, key, tl) { - return + if upstream != nil { + _ = upstream.Close() // bridgeServe dials its own verified upstream } - // fell open → re-dial for the tunnel. That tunnel is opaque, so it closes - // like any other opaque one. dialTarget, not r.Host: the address was - // already checked once, and re-resolving here would reopen the rebinding - // window the check closed. - up2, derr := net.DialTimeout("tcp", dialTarget, connectDialTimeout) - if derr != nil { - // The client already has its 200, so that stays the status, and the - // error says why the tunnel died. Closing the client is what makes - // that close row true: the hijacked conn is this handler's to close, - // and it used to be left open for the client to time out on. - _ = clientConn.Close() - tl.close(http.StatusOK, dialError(derr), 0, 0) + if s.bridgeServe(clientConn, authority, key, tl, dialFail != nil) { return } - sent, received := tunnel(clientConn, up2) - _ = up2.Close() - tl.close(http.StatusOK, nil, sent, received) + // fell open → re-dial for the tunnel. That tunnel is opaque, so it + // closes like any other opaque one. dialTarget, not r.Host: the address + // was already checked once, and re-resolving here would reopen the + // rebinding window the check closed. + s.tunnelOrDial(clientConn, dialTarget, tl) return } } @@ -2669,6 +2757,14 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { // bridgeServe, so this is where their reason gets recorded. tl.open(reason) + // The pre-dial failed (or was skipped as memoized-dead) and the bridge + // declined the connection: dial now — the memo's optimism was for the bridge + // path, and a plain tunnel still needs a real upstream. + if upstream == nil { + s.tunnelOrDial(clientConn, dialTarget, tl) + return + } + // Bidirectional copy until either side closes. sent, received := tunnel(clientConn, upstream) tl.close(http.StatusOK, nil, sent, received) diff --git a/core/listener/forwardproxy/transparent.go b/core/listener/forwardproxy/transparent.go index 5cb8070ee..4f4bb3990 100644 --- a/core/listener/forwardproxy/transparent.go +++ b/core/listener/forwardproxy/transparent.go @@ -183,18 +183,34 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) { dialTarget = checked pctx.DialedAddr = checked } - upstream, err := net.DialTimeout("tcp", dialTarget, connectDialTimeout) + upstream, err := s.dialUpstream(dialTarget) + // dialFail remembers a pre-dial that failed while the TLS bridge is enabled: + // the connection continues with no upstream, because the bridge can still take + // it and a plugin redirect can send the decrypted request to a live server + // (#1595) — the same rule as handleConnect's CONNECT path. + var dialFail error if err != nil { - slog.Warn("transparent-proxy: upstream dial failed", "host", host, "dst", dst, "error", err) - // The 502 is synthetic — this path sends the client no status at all — but the - // failure is recorded as it is on CONNECT rather than leaving no trace. - tl.open(pipeline.TunnelDialFailed) - tl.close(http.StatusBadGateway, dialError(err), 0, 0) - return + if s.TLSBridge == nil { + slog.Warn("transparent-proxy: upstream dial failed", "host", host, "dst", dst, "error", err) + // The 502 is synthetic — this path sends the client no status at all — but the + // failure is recorded as it is on CONNECT rather than leaving no trace. + tl.open(pipeline.TunnelDialFailed) + tl.close(http.StatusBadGateway, dialError(err), 0, 0) + return + } + slog.Warn("transparent-proxy: upstream dial failed; trying the TLS bridge", + "host", host, "dst", dst, "error", err) + dialFail = err } - defer func() { _ = upstream.Close() }() + defer func() { + if upstream != nil { + _ = upstream.Close() + } + }() - enableKeepalive(upstream) + if upstream != nil { + enableKeepalive(upstream) + } // The transparent path records before its own bridge decision for now, so it // reports only that the tunnel opened. Threading the reason through here too @@ -215,28 +231,30 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) { if !s.TLSBridge.Skip.Contains(key) { v, reason := s.TLSBridge.Decision.Classify(key, portOf(dst), first) if v == tlsbridge.Terminate { - _ = upstream.Close() // bridgeServe dials its own verified upstream; drop the pre-dial + if upstream != nil { + _ = upstream.Close() // bridgeServe dials its own verified upstream; drop the pre-dial + } // The open is already recorded above, so bridgeServe's own open calls are // no-ops here; what it still records through tl is the close. - if s.bridgeServe(clientConn, host, key, tl) { + if s.bridgeServe(clientConn, host, key, tl, dialFail != nil) { return } // bridgeServe fell open (upstream-verify failed) → re-dial for the tunnel. // dialTarget: the checked address, never re-resolved — same rule as // handleConnect's fall-open re-dial. - up2, derr := net.DialTimeout("tcp", dialTarget, connectDialTimeout) - if derr != nil { - tl.close(http.StatusOK, dialError(derr), 0, 0) - return - } - sent, received := tunnel(clientConn, up2) - _ = up2.Close() - tl.close(http.StatusOK, nil, sent, received) + s.tunnelOrDial(clientConn, dialTarget, tl) return } slog.Info("tls-bridge passthrough", "host", key, "reason", reason) } } + // The pre-dial failed (or was skipped as memoized-dead) and the bridge + // declined the connection: dial now — a plain tunnel still needs a real + // upstream, the same rule as handleConnect's. + if upstream == nil { + s.tunnelOrDial(clientConn, dialTarget, tl) + return + } sent, received := tunnel(clientConn, upstream) tl.close(http.StatusOK, nil, sent, received) } diff --git a/core/listener/forwardproxy/tunnelclose_test.go b/core/listener/forwardproxy/tunnelclose_test.go index 97d0f0d6e..90803fadb 100644 --- a/core/listener/forwardproxy/tunnelclose_test.go +++ b/core/listener/forwardproxy/tunnelclose_test.go @@ -655,7 +655,7 @@ func TestBridgeServe_HandshakeFailureRecordsItsClose(t *testing.T) { s, store, authority := bridgeForRejectTest(t) tl := s.newTunnelLog(&pipeline.Context{Direction: pipeline.Outbound, Host: authority}, false) - if !s.bridgeServe(rejectingClient(t), authority, hostOnly(authority), tl) { + if !s.bridgeServe(rejectingClient(t), authority, hostOnly(authority), tl, false) { t.Fatal("bridgeServe returned false; the connection is dead post-forge") } diff --git a/core/listener/forwardproxy/tunnelreason_integration_test.go b/core/listener/forwardproxy/tunnelreason_integration_test.go index 42c464249..91264fb90 100644 --- a/core/listener/forwardproxy/tunnelreason_integration_test.go +++ b/core/listener/forwardproxy/tunnelreason_integration_test.go @@ -142,7 +142,7 @@ func TestClientRejectedCA_WarnsEvenAfterOtherTrafficBridged(t *testing.T) { client := rejectingClient(t) pctx := &pipeline.Context{Direction: pipeline.Outbound, Host: authority} - if handled := s.bridgeServe(client, authority, hostOnly(authority), s.newTunnelLog(pctx, false)); !handled { + if handled := s.bridgeServe(client, authority, hostOnly(authority), s.newTunnelLog(pctx, false), false); !handled { t.Fatal("bridgeServe returned false; the connection is dead post-forge and must be reported handled") } @@ -188,7 +188,7 @@ func TestClientRejectedCA_SkipsHostAfterwards(t *testing.T) { if s.TLSBridge.Skip.Contains(host) { t.Fatal("host skipped before any failure") } - s.bridgeServe(rejectingClient(t), authority, host, discardTunnel()) + s.bridgeServe(rejectingClient(t), authority, host, discardTunnel(), false) if !s.TLSBridge.Skip.Contains(host) { t.Error("host not skipped after a rejected forge; the client's retry would fail again") } @@ -207,7 +207,7 @@ func TestClientHungUp_GetsNoRestartAdvice(t *testing.T) { t.Cleanup(func() { slog.SetDefault(prev) }) pctx := &pipeline.Context{Direction: pipeline.Outbound, Host: authority} - s.bridgeServe(hangUpClient(t), authority, hostOnly(authority), s.newTunnelLog(pctx, false)) + s.bridgeServe(hangUpClient(t), authority, hostOnly(authority), s.newTunnelLog(pctx, false), false) got := logbuf.String() if !strings.Contains(got, string(pipeline.TunnelClientHungUp)) { @@ -397,7 +397,7 @@ func TestHangUpAlsoSeedsTheSkip(t *testing.T) { host := hostOnly(authority) pctx := &pipeline.Context{Direction: pipeline.Outbound, Host: authority} - s.bridgeServe(hangUpClient(t), authority, host, s.newTunnelLog(pctx, false)) + s.bridgeServe(hangUpClient(t), authority, host, s.newTunnelLog(pctx, false), false) // The first failure reports what actually happened, not a CA rejection. opens, _ := tunnelRows(store, session.DefaultSessionID) @@ -474,14 +474,14 @@ func TestOneStaleClientDoesNotSuppressAHealthyOne(t *testing.T) { host := hostOnly(authority) // 1. The stale client rejects our leaf; the host is skipped for everyone. - s.bridgeServe(rejectingClient(t), authority, host, discardTunnel()) + s.bridgeServe(rejectingClient(t), authority, host, discardTunnel(), false) if !s.TLSBridge.Skip.Contains(host) { t.Fatal("a rejected forge did not skip the host") } // 2. The healthy client bridges. bridgeServe blocks serving the decrypted // connection, so run it and wait for the skip to clear. - go s.bridgeServe(trustingClient(t, s.TLSBridge.CAPEM), authority, host, discardTunnel()) + go s.bridgeServe(trustingClient(t, s.TLSBridge.CAPEM), authority, host, discardTunnel(), false) deadline := time.Now().Add(3 * time.Second) for time.Now().Before(deadline) { @@ -521,7 +521,7 @@ func TestClientRejectedCA_NamesTheCAItself(t *testing.T) { slog.SetDefault(slog.New(slog.NewTextHandler(&logbuf, &slog.HandlerOptions{Level: slog.LevelWarn}))) t.Cleanup(func() { slog.SetDefault(prev) }) - s.bridgeServe(rejectingClient(t), authority, hostOnly(authority), discardTunnel()) + s.bridgeServe(rejectingClient(t), authority, hostOnly(authority), discardTunnel(), false) got := logbuf.String() if !strings.Contains(got, "ca_fingerprint=") { @@ -596,7 +596,7 @@ func TestClientHungUp_GetsNoCAIdentity(t *testing.T) { slog.SetDefault(slog.New(slog.NewTextHandler(&logbuf, &slog.HandlerOptions{Level: slog.LevelWarn}))) t.Cleanup(func() { slog.SetDefault(prev) }) - s.bridgeServe(hangUpClient(t), authority, hostOnly(authority), discardTunnel()) + s.bridgeServe(hangUpClient(t), authority, hostOnly(authority), discardTunnel(), false) if got := logbuf.String(); strings.Contains(got, "ca_fingerprint=") || strings.Contains(got, "ca_file=") { t.Errorf("a hang-up was given CA identity it cannot justify:\n%s", got) From e23b01517656fdf9ee2318c4cd9fb0e3f7e52355 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 29 Oct 2026 22:25:32 -0400 Subject: [PATCH 2/4] Fix: Guard the dead-predial cleanups and keep the dial-failure memo bridge-only Fixes review: - a nil upstream conn panicked in the hijack-failure and 200-write-failure branches of handleConnect, reached when the pre-dial failed with the bridge enabled - the dial-failure memo was consulted without the bridge too, where nothing re-dials, so a recovered origin kept answering 502 for the memo's TTL Files: core/listener/forwardproxy/redirect_test.go, core/listener/forwardproxy/server.go Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- core/listener/forwardproxy/redirect_test.go | 108 ++++++++++++++++++++ core/listener/forwardproxy/server.go | 43 +++++--- 2 files changed, 134 insertions(+), 17 deletions(-) diff --git a/core/listener/forwardproxy/redirect_test.go b/core/listener/forwardproxy/redirect_test.go index 48d6668b8..62b036458 100644 --- a/core/listener/forwardproxy/redirect_test.go +++ b/core/listener/forwardproxy/redirect_test.go @@ -6,6 +6,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/pem" + "errors" "io" "net" "net/http" @@ -641,3 +642,110 @@ func TestConnectBridge_DeadOriginDialIsMemoized(t *testing.T) { t.Errorf("pre-dial ran %d times for 2 connections, want 1 (failures are memoized)", dials.n) } } + +// The two cleanups after a failed pre-dial — the hijack failing, and the 200 +// write failing because the client is already gone — both closed the upstream, +// which is nil exactly when the pre-dial failed with the bridge enabled. A nil +// net.Conn has no method to call, so each cleanup panicked instead. +func TestHandleConnect_DeadPredialCleanupsDoNotPanic(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + target := l.Addr().String() + _ = l.Close() // the pre-dial target: refused fast on loopback + + d, err := tlsbridge.NewDecision(tlsbridge.DecisionOpts{Ports: map[int]bool{1: true}}) + if err != nil { + t.Fatalf("NewDecision: %v", err) + } + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + s := connectServer(t, store, &tlsbridge.Engine{Decision: d, Skip: tlsbridge.NewSkipSet()}) + + connect := func() *http.Request { + return &http.Request{ + Method: http.MethodConnect, + Host: target, + URL: &url.URL{Host: target}, + Header: http.Header{}, + } + } + + t.Run("hijack fails", func(t *testing.T) { + s.handleConnect(&hijackStubRW{hijackErr: errors.New("not now")}, connect()) + }) + t.Run("client gone before the 200", func(t *testing.T) { + s.handleConnect(&hijackStubRW{}, connect()) + }) +} + +// hijackStubRW hands handleConnect a hijacked connection it controls: +// hijackErr makes Hijack itself fail; otherwise the conn's Write fails, standing +// in for a client that hung up before the proxy could answer 200. +type hijackStubRW struct { + hijackErr error +} + +func (w *hijackStubRW) Header() http.Header { return http.Header{} } +func (w *hijackStubRW) Write([]byte) (int, error) { return 0, nil } +func (w *hijackStubRW) WriteHeader(int) {} + +func (w *hijackStubRW) Hijack() (net.Conn, *bufio.ReadWriter, error) { + if w.hijackErr != nil { + return nil, nil, w.hijackErr + } + return &failingWriteConn{}, bufio.NewReadWriter(bufio.NewReader(strings.NewReader("")), bufio.NewWriter(io.Discard)), nil +} + +// failingWriteConn is a hijacked client conn whose Write always fails and whose +// Close is a no-op; only those two are called on it before handleConnect returns. +type failingWriteConn struct{ net.Conn } + +func (c *failingWriteConn) Write([]byte) (int, error) { return 0, errors.New("client gone") } +func (c *failingWriteConn) Close() error { return nil } + +// Without the bridge, a failed pre-dial must not be memoized: the 502 is the +// last word on that path — nothing re-dials where the dial's result would have +// been used — so a memo hit would keep answering 502 for an origin that came +// back within the memo's TTL. +func TestHandleConnect_NoBridgeDialsFreshAfterAFailedDial(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + target := l.Addr().String() + _ = l.Close() // dead for the first CONNECT + + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + s := connectServer(t, store, nil) + proxy := httptest.NewServer(http.HandlerFunc(s.handleRequest)) + defer proxy.Close() + proxyAddr := strings.TrimPrefix(proxy.URL, "http://") + + if _, _, resp := sendConnect(t, proxyAddr, target); resp.StatusCode != http.StatusBadGateway { + t.Fatalf("first CONNECT status = %d, want 502 (the destination is dead)", resp.StatusCode) + } + + // The origin comes back on the same port. + origin, err := net.Listen("tcp", target) + if err != nil { + t.Fatalf("re-listen: %v", err) + } + defer func() { _ = origin.Close() }() + go func() { + for { + c, err := origin.Accept() + if err != nil { + return + } + _ = c.Close() + } + }() + + _, _, resp := sendConnect(t, proxyAddr, target) + if resp.StatusCode != http.StatusOK { + t.Fatalf("second CONNECT status = %d, want 200 — the origin is back, so the dial must be attempted again", resp.StatusCode) + } +} diff --git a/core/listener/forwardproxy/server.go b/core/listener/forwardproxy/server.go index 4de175af3..ee5e92ab3 100644 --- a/core/listener/forwardproxy/server.go +++ b/core/listener/forwardproxy/server.go @@ -1211,11 +1211,12 @@ type dialFailMemo struct { // dialFailMemoTTL is how long a failed pre-dial is remembered, so a destination // that drops packets — where a dial burns the whole connectDialTimeout — is not -// re-dialed per CONNECT. Short because the memo never misroutes: a skipped dial is +// re-dialed per CONNECT. Bridge-enabled CONNECTs only: there a skipped dial is // re-attempted wherever its result would have been used (the plain-tunnel path // dials fresh in tunnelOrDial; the bridge path re-originates to a different host // and dials that), so the staleness it can buy is one skipped dial, never a -// misrouted request. +// misrouted request. Without the bridge the 502 is the last word, so a memo hit +// would keep answering 502 for an origin that came back within the TTL. const dialFailMemoTTL = 30 * time.Second // dialTCP is net.DialTimeout; a variable so tests can count dials. @@ -1225,26 +1226,30 @@ var dialTCP = net.DialTimeout // dialFailMemoTTL (#1595: a client that opens a CONNECT per request must not pay // a dead destination's dial timeout every time). func (s *Server) dialUpstream(dialTarget string) (net.Conn, error) { - s.dialMu.Lock() - if s.dialFails == nil { - s.dialFails = make(map[string]dialFailMemo) - } - if m, ok := s.dialFails[dialTarget]; ok { - if time.Since(m.at) < dialFailMemoTTL { - s.dialMu.Unlock() - return nil, m.err + if s.TLSBridge != nil { + s.dialMu.Lock() + if s.dialFails == nil { + s.dialFails = make(map[string]dialFailMemo) + } + if m, ok := s.dialFails[dialTarget]; ok { + if time.Since(m.at) < dialFailMemoTTL { + s.dialMu.Unlock() + return nil, m.err + } + delete(s.dialFails, dialTarget) } - delete(s.dialFails, dialTarget) + s.dialMu.Unlock() } - s.dialMu.Unlock() conn, err := dialTCP("tcp", dialTarget, connectDialTimeout) if err == nil { return conn, nil } - s.dialMu.Lock() - s.dialFails[dialTarget] = dialFailMemo{err: err, at: time.Now()} - s.dialMu.Unlock() + if s.TLSBridge != nil { + s.dialMu.Lock() + s.dialFails[dialTarget] = dialFailMemo{err: err, at: time.Now()} + s.dialMu.Unlock() + } return nil, err } @@ -2700,7 +2705,9 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { clientConn, _, err := w.(http.Hijacker).Hijack() if err != nil { - _ = upstream.Close() + if upstream != nil { + _ = upstream.Close() + } slog.Error("forward-proxy: CONNECT hijack failed", "host", r.Host, "error", err) return } @@ -2721,7 +2728,9 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { // subsequent bytes from either side are application data. if _, err := clientConn.Write([]byte("HTTP/1.1 200 Connection Established\r\n\r\n")); err != nil { _ = clientConn.Close() - _ = upstream.Close() + if upstream != nil { + _ = upstream.Close() + } slog.Debug("forward-proxy: CONNECT 200 write failed", "host", r.Host, "error", err) return } From 4480707e5a05b3875b1ad53a68082a523e282c10 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 30 Oct 2026 07:43:06 -0400 Subject: [PATCH 3/4] Fix: Gate the dead-origin fall-through on the bridge's ports and stop re-dialing a fresh failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bridge-enabled fall-through now applies only where the bridge could actually take the connection (its decision's port set), so a dead destination on any other port keeps the immediate 502 the pre-PR proxy answered. A resolution failure is carried as the pre-dial's failure instead of dialing the empty checked address, which masked the DNS error and pooled every unresolvable name under one memo key. A pre-dial failure that came from a real dial is not re-dialed on a bridge decline or fall-open — the client already has its 200, and a second dial only repeats the first one's timeout — and such a decline records the dial-failed open reason. Recording a memo failure sweeps entries the TTL has expired, so distinct dead targets cannot grow the map without bound, and tunnelOrDial dials through the dialTCP seam so tests can count its re-dials. Fixes review: - coderabbit: dialFails entries were deleted only on a repeat CONNECT past the TTL, so distinct dead targets grew the map unbounded - coderabbit: a non-refusal CheckAddr error fell through with an empty dialTarget, dialing "" (missing-address error masking the DNS error, shared "" memo key) - clawgenti: the fall-through was gated on bridge existence, not on the bridge's ports, so non-bridged CONNECT ports lost their 502 - clawgenti: a declined bridge re-dialed a target the pre-dial had just failed, costing a second connectDialTimeout behind the client's 200, and the open row lost the dial-failed reason - clawgenti: the dialFailMemoTTL comment said a stale entry buys one skipped dial; it also skips the origin probe Files: core/listener/forwardproxy/redirect_test.go, core/listener/forwardproxy/server.go, core/listener/forwardproxy/transparent.go Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- core/listener/forwardproxy/redirect_test.go | 210 +++++++++++++++++++- core/listener/forwardproxy/server.go | 124 +++++++++--- core/listener/forwardproxy/transparent.go | 31 ++- 3 files changed, 320 insertions(+), 45 deletions(-) diff --git a/core/listener/forwardproxy/redirect_test.go b/core/listener/forwardproxy/redirect_test.go index 62b036458..eaf1a7468 100644 --- a/core/listener/forwardproxy/redirect_test.go +++ b/core/listener/forwardproxy/redirect_test.go @@ -17,6 +17,7 @@ import ( "testing" "time" + "github.com/rossoctl/cortex/core/listener/egressguard" "github.com/rossoctl/cortex/core/pipeline" "github.com/rossoctl/cortex/core/session" "github.com/rossoctl/cortex/core/tlsbridge" @@ -655,7 +656,10 @@ func TestHandleConnect_DeadPredialCleanupsDoNotPanic(t *testing.T) { target := l.Addr().String() _ = l.Close() // the pre-dial target: refused fast on loopback - d, err := tlsbridge.NewDecision(tlsbridge.DecisionOpts{Ports: map[int]bool{1: true}}) + // The bridge must handle the target's port: the fall-through is gated on + // that, so a Decision for a port nothing uses would take the 502 path and + // never reach the nil-upstream cleanups this test exists to drive. + d, err := tlsbridge.NewDecision(tlsbridge.DecisionOpts{Ports: map[int]bool{portOf(target): true}}) if err != nil { t.Fatalf("NewDecision: %v", err) } @@ -749,3 +753,207 @@ func TestHandleConnect_NoBridgeDialsFreshAfterAFailedDial(t *testing.T) { t.Fatalf("second CONNECT status = %d, want 200 — the origin is back, so the dial must be attempted again", resp.StatusCode) } } + +// A dead destination on a port the bridge never terminates keeps its immediate +// 502: the fall-through that lets the bridge take a dead origin exists so a +// redirect can apply, and a redirect can only apply on a port the bridge +// intercepts. Everywhere else the old answer stands. +func TestConnectBridge_UnbridgedPortKeepsItsDialFailure502(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + target := l.Addr().String() + _ = l.Close() // dead for the pre-dial + + d, err := tlsbridge.NewDecision(tlsbridge.DecisionOpts{Ports: map[int]bool{1: true}}) + if err != nil { + t.Fatalf("NewDecision: %v", err) + } + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + s := connectServer(t, store, &tlsbridge.Engine{Decision: d, Skip: tlsbridge.NewSkipSet()}) + proxy := httptest.NewServer(http.HandlerFunc(s.handleRequest)) + defer proxy.Close() + + _, _, resp := sendConnect(t, strings.TrimPrefix(proxy.URL, "http://"), target) + if resp.StatusCode != http.StatusBadGateway { + t.Fatalf("CONNECT status = %d, want 502 — a dead destination on a port the bridge cannot take is answered, not fallen through", resp.StatusCode) + } +} + +// declineAndCountDials serves a proxy whose bridge intercepts target's port but +// declines the connection — the client's post-CONNECT bytes are not a TLS +// record — over a dead target, counting every dial the proxy makes. It returns +// the proxy's address and a snapshot of the dial count. +func declineAndCountDials(t *testing.T, target string) (proxyAddr string, dials func() int) { + t.Helper() + d, err := tlsbridge.NewDecision(tlsbridge.DecisionOpts{Ports: map[int]bool{portOf(target): true}}) + if err != nil { + t.Fatalf("NewDecision: %v", err) + } + store := session.New(5*time.Minute, 100, 0) + t.Cleanup(store.Close) + s := connectServer(t, store, &tlsbridge.Engine{Decision: d, Skip: tlsbridge.NewSkipSet()}) + + var mu sync.Mutex + n := 0 + inner := dialTCP + dialTCP = func(network, addr string, d time.Duration) (net.Conn, error) { + mu.Lock() + n++ + mu.Unlock() + return inner(network, addr, d) + } + t.Cleanup(func() { dialTCP = inner }) + + proxy := httptest.NewServer(http.HandlerFunc(s.handleRequest)) + t.Cleanup(proxy.Close) + return strings.TrimPrefix(proxy.URL, "http://"), func() int { + mu.Lock() + defer mu.Unlock() + return n + } +} + +// connectAndDecline sends a CONNECT to a dead target and follows it with five +// plaintext bytes, so the bridge's peek classifies the connection as non-TLS +// and declines it. It fails the test unless the CONNECT is answered 200. +func connectAndDecline(t *testing.T, proxyAddr, target string) { + t.Helper() + raw, br, resp := sendConnect(t, proxyAddr, target) + if resp.StatusCode != http.StatusOK { + t.Fatalf("CONNECT status = %d, want 200", resp.StatusCode) + } + if _, err := raw.Write([]byte("HELLO")); err != nil { // not a TLS record: the bridge declines + t.Fatalf("write through tunnel: %v", err) + } + _, _ = io.ReadAll(br) // the proxy closes the tunnel; wait for it +} + +// The bridge declined and the pre-dial already failed on a real dial: the +// client has its 200, and a second dial would only repeat the first one's +// failure — on a filtered destination, another full connectDialTimeout — +// before the close. One dial, then the close, with the dial-failed row the +// docs and dashboards key on. +func TestConnectBridge_DeclinedBridgeDoesNotRedialAFreshFailure(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + target := l.Addr().String() + _ = l.Close() // dead for the pre-dial + + proxyAddr, dials := declineAndCountDials(t, target) + connectAndDecline(t, proxyAddr, target) + + if n := dials(); n != 1 { + t.Errorf("dials = %d, want 1 — the pre-dial already failed; re-dialing repeats it behind the client's 200", n) + } +} + +// A memo hit is the one decline where the re-dial is wanted: nothing was dialed +// for this connection, and the origin may have come back within the memo's TTL. +// Two CONNECTs therefore cost two dials — the first one's failed pre-dial, and +// the second one's fresh attempt — not three. +func TestConnectBridge_AMemoizedFailureRedialsWhenTheBridgeDeclines(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + target := l.Addr().String() + _ = l.Close() // dead for every dial + + proxyAddr, dials := declineAndCountDials(t, target) + connectAndDecline(t, proxyAddr, target) + connectAndDecline(t, proxyAddr, target) + + if n := dials(); n != 2 { + t.Errorf("dials = %d, want 2 — the first CONNECT's failed pre-dial, and the memo-hit CONNECT's fresh re-dial", n) + } +} + +// A resolution failure is the answer, not a dial of the empty checked address: +// with the bridge able to take the connection nothing is dialed (the bridge +// re-originates the redirected request itself), so no "missing address" error +// can mask the resolver's, and no empty memo key can pool every unresolvable +// name. +func TestConnectBridge_ResolutionFailureIsNotDialed(t *testing.T) { + b := newOrigin(t, "FROM-B", (*httptest.Server).StartTLS) + engine := redirectBridge(t, 443, certPEM(b.Server)) + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + p, err := pipeline.New([]pipeline.Plugin{&redirectPlugin{target: b.URL}}) + if err != nil { + t.Fatalf("pipeline.New: %v", err) + } + s := &Server{ + OutboundPipeline: pipeline.NewHolder(p), + Sessions: store, + Client: http.DefaultClient, + TLSBridge: engine, + // The zero Guard has no resolver, so checking the name fails with a + // non-refusal error — the shape a real guard produces for a name that + // will not resolve. + EgressGuard: &egressguard.Guard{}, + } + proxy := httptest.NewServer(s.Handler()) + defer proxy.Close() + + var mu sync.Mutex + n := 0 + inner := dialTCP + dialTCP = func(network, addr string, d time.Duration) (net.Conn, error) { + mu.Lock() + n++ + mu.Unlock() + return inner(network, addr, d) + } + t.Cleanup(func() { dialTCP = inner }) + + if status, body := bridgedGet(t, proxy, engine.CAPEM, "no-such-host.test:443", "/secret"); status != http.StatusOK || body != "FROM-B" { + t.Fatalf("response = %d %q, want 200 FROM-B — the redirect never applied", status, body) + } + mu.Lock() + defer mu.Unlock() + if n != 0 { + t.Errorf("dials = %d, want 0 — a resolution failure must not be dialed, least of all as an empty address", n) + } +} + +// The memo is keyed by client-named targets, and a scanner CONNECTing to many +// distinct dead names must not grow it without bound: recording a failure +// sweeps the entries the TTL has already expired. +func TestDialUpstream_SweepsExpiredMemoEntries(t *testing.T) { + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + s := connectServer(t, store, nil) + + s.dialMu.Lock() + s.dialFails = map[string]dialFailMemo{ + "expired:1": {err: errors.New("old"), at: time.Now().Add(-dialFailMemoTTL - time.Second)}, + "fresh:1": {err: errors.New("recent"), at: time.Now()}, + } + s.dialMu.Unlock() + + inner := dialTCP + dialTCP = func(network, addr string, d time.Duration) (net.Conn, error) { + return nil, errors.New("dead") + } + t.Cleanup(func() { dialTCP = inner }) + + if _, err, _ := s.dialUpstream("other:1", true); err == nil { + t.Fatal("dialUpstream succeeded against a failing dial") + } + + s.dialMu.Lock() + defer s.dialMu.Unlock() + if _, ok := s.dialFails["expired:1"]; ok { + t.Error("the expired memo entry survived recording a new failure") + } + for _, key := range []string{"fresh:1", "other:1"} { + if _, ok := s.dialFails[key]; !ok { + t.Errorf("memo entry %q missing; want only the expired one swept", key) + } + } +} diff --git a/core/listener/forwardproxy/server.go b/core/listener/forwardproxy/server.go index ee5e92ab3..59bce0e2e 100644 --- a/core/listener/forwardproxy/server.go +++ b/core/listener/forwardproxy/server.go @@ -1216,17 +1216,35 @@ type dialFailMemo struct { // dials fresh in tunnelOrDial; the bridge path re-originates to a different host // and dials that), so the staleness it can buy is one skipped dial, never a // misrouted request. Without the bridge the 502 is the last word, so a memo hit -// would keep answering 502 for an origin that came back within the TTL. +// would keep answering 502 for an origin that came back within the TTL. A stale +// hit also skips the origin probe (bridgeServe), not just the dial: an origin +// that comes back within the TTL with a certificate the bridge cannot verify is +// bridged rather than passed through, and the request fails at re-origination — +// bounded by the TTL, and the failure is loud rather than silent. const dialFailMemoTTL = 30 * time.Second // dialTCP is net.DialTimeout; a variable so tests can count dials. var dialTCP = net.DialTimeout +// bridgeable reports whether the TLS bridge could take a connection to +// authority's port. The dead-origin fall-through and its memo are for those +// connections only: anywhere else the bridge would decline by port anyway, so +// a failed pre-dial keeps its immediate 502 instead of a 200 the bridge cannot +// use. +func (s *Server) bridgeable(authority string) bool { + return s.TLSBridge != nil && s.TLSBridge.Decision.HandlesPort(portOf(authority)) +} + // dialUpstream is a CONNECT's pre-dial, memoizing failures per target for // dialFailMemoTTL (#1595: a client that opens a CONNECT per request must not pay -// a dead destination's dial timeout every time). -func (s *Server) dialUpstream(dialTarget string) (net.Conn, error) { - if s.TLSBridge != nil { +// a dead destination's dial timeout every time). bridgeable gates the memo: where +// the bridge cannot take the connection the failure is answered with a 502, not +// skipped, so remembering it would only pin that 502 past an origin that came +// back. The final return says whether the failure came from the memo rather than +// a dial this call made — a caller that would re-dial (tunnelOrDial) should not, +// because a failure from its own dial is already the answer. +func (s *Server) dialUpstream(dialTarget string, bridgeable bool) (net.Conn, error, bool) { + if bridgeable { s.dialMu.Lock() if s.dialFails == nil { s.dialFails = make(map[string]dialFailMemo) @@ -1234,7 +1252,7 @@ func (s *Server) dialUpstream(dialTarget string) (net.Conn, error) { if m, ok := s.dialFails[dialTarget]; ok { if time.Since(m.at) < dialFailMemoTTL { s.dialMu.Unlock() - return nil, m.err + return nil, m.err, true } delete(s.dialFails, dialTarget) } @@ -1243,23 +1261,39 @@ func (s *Server) dialUpstream(dialTarget string) (net.Conn, error) { conn, err := dialTCP("tcp", dialTarget, connectDialTimeout) if err == nil { - return conn, nil + return conn, nil, false } - if s.TLSBridge != nil { + if bridgeable { s.dialMu.Lock() - s.dialFails[dialTarget] = dialFailMemo{err: err, at: time.Now()} + // Sweep while inserting: an entry is otherwise deleted only when its own + // target is CONNECTed again after the TTL, so distinct dead targets — a + // scanner's stock in trade — would grow the map without bound. + now := time.Now() + for k, m := range s.dialFails { + if now.Sub(m.at) >= dialFailMemoTTL { + delete(s.dialFails, k) + } + } + s.dialFails[dialTarget] = dialFailMemo{err: err, at: now} s.dialMu.Unlock() } - return nil, err + return nil, err, false } // tunnelOrDial serves a tunnel whose pre-dialed upstream is gone — the bridge // declined the connection, or fell open after the origin probe failed — by dialing // fresh: a working dial tunnels normally, a failing one closes the client with the // dial error on the close row. The client already has its 200, so that stays the -// status and the error says why the tunnel died. -func (s *Server) tunnelOrDial(clientConn net.Conn, dialTarget string, tl *tunnelLog) { - up2, derr := net.DialTimeout("tcp", dialTarget, connectDialTimeout) +// status and the error says why the tunnel died. fail, when non-nil, is a failure +// the pre-dial produced on a real dial: re-dialing would only repeat it, so the +// tunnel closes with it instead. +func (s *Server) tunnelOrDial(clientConn net.Conn, dialTarget string, tl *tunnelLog, fail error) { + if fail != nil { + _ = clientConn.Close() + tl.close(http.StatusOK, dialError(fail), 0, 0) + return + } + up2, derr := dialTCP("tcp", dialTarget, connectDialTimeout) if derr != nil { _ = clientConn.Close() tl.close(http.StatusOK, dialError(derr), 0, 0) @@ -2634,6 +2668,8 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { // verbatim by the bridge-fall-open re-dial below, so no second resolution // can ever re-answer between check and connect (the DNS-rebinding window). dialTarget := r.Host + bridgeable := s.bridgeable(r.Host) + var resolveErr error // a non-refusal CheckAddr failure: the name would not resolve if s.EgressGuard != nil { checked, cerr := s.EgressGuard.CheckAddr(r.Context(), "tcp", r.Host) if cerr != nil { @@ -2659,13 +2695,14 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { return } // Not a refusal: the name would not resolve. Same shape as a dial - // failure — nothing was dialed, and the client gets the 502. With the - // bridge enabled, fall through instead: the dial below fails the same - // way and takes the bridge-first branch, where a redirect can still - // move the request to a name that resolves (#1595). + // failure — nothing was dialed, and the client gets the 502. Where the + // bridge could take the connection, fall through instead, carrying the + // resolution error as the pre-dial's failure: the bridge can still + // bridge it, and a redirect can move the request to a name that + // resolves (#1595). The empty checked address is never dialed. // Both values sanitized: the name is client-controlled, and the error // embeds it (net.DNSError quotes the name it failed to resolve). - if s.TLSBridge == nil { + if !bridgeable { slog.Warn("forward-proxy: CONNECT destination resolution failed", "host", pipeline.SanitizeControlRunes(r.Host), "error", pipeline.SanitizeControlRunes(cerr.Error())) @@ -2674,22 +2711,31 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { tl.close(http.StatusBadGateway, dialError(cerr), 0, 0) return } + resolveErr = cerr } dialTarget = checked // The audit trail against rebinding: "asked for the name, dialed this // address" — moca's egress conformance reads it off the tunnel open row. pctx.DialedAddr = checked } - upstream, err := s.dialUpstream(dialTarget) - // dialFail remembers a pre-dial that failed while the TLS bridge is enabled: - // the connection continues with no upstream, because the bridge can still take - // it — a plugin redirect (inference-router moving a pinned session) sends the - // decrypted request to a live server, and an opaque tunnel to a dead origin - // helps nobody (#1595). Every path below that would have used the upstream - // handles a nil one by closing with this error or re-dialing (tunnelOrDial). - var dialFail error + var upstream net.Conn + var memoized bool + err := resolveErr + if err == nil { + upstream, err, memoized = s.dialUpstream(dialTarget, bridgeable) + } + // dialFail remembers a pre-dial that failed where the bridge could take the + // connection: the connection continues with no upstream, because the bridge + // can still take it — a plugin redirect (inference-router moving a pinned + // session) sends the decrypted request to a live server, and an opaque tunnel + // to a dead origin helps nobody (#1595). Every path below that would have used + // the upstream handles a nil one by closing with this error or re-dialing + // (tunnelOrDial). freshDialFail is the failure when it came from a dial this + // connection made itself, not a memo hit: re-dialing would only repeat it + // behind the 200 the client already has, so tunnelOrDial closes with it. + var dialFail, freshDialFail error if err != nil { - if s.TLSBridge == nil { + if !bridgeable { slog.Warn("forward-proxy: CONNECT upstream dial failed", "host", r.Host, "error", err) http.Error(w, `{"error":"bad gateway"}`, http.StatusBadGateway) // Recorded rather than dropped: an unreachable destination used to leave no @@ -2701,6 +2747,9 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { slog.Warn("forward-proxy: CONNECT upstream dial failed; trying the TLS bridge", "host", r.Host, "error", err) dialFail = err + if !memoized { + freshDialFail = err + } } clientConn, _, err := w.(http.Hijacker).Hijack() @@ -2756,23 +2805,32 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) { // fell open → re-dial for the tunnel. That tunnel is opaque, so it // closes like any other opaque one. dialTarget, not r.Host: the address // was already checked once, and re-resolving here would reopen the - // rebinding window the check closed. - s.tunnelOrDial(clientConn, dialTarget, tl) + // rebinding window the check closed. freshDialFail, when set, skips the + // re-dial: the pre-dial already failed on a real dial of this target. + s.tunnelOrDial(clientConn, dialTarget, tl, freshDialFail) return } } // Every path that reaches here left the bytes opaque without entering - // bridgeServe, so this is where their reason gets recorded. - tl.open(reason) - + // bridgeServe, so this is where their reason gets recorded — except a + // certainly-dead origin, which keeps the dial-failed reason the docs and + // dashboards key on, whatever made the bridge decline. // The pre-dial failed (or was skipped as memoized-dead) and the bridge // declined the connection: dial now — the memo's optimism was for the bridge - // path, and a plain tunnel still needs a real upstream. + // path, and a plain tunnel still needs a real upstream. A fresh failure needs + // no second dial; a memo hit is up to dialFailMemoTTL old, and the origin may + // have come back. if upstream == nil { - s.tunnelOrDial(clientConn, dialTarget, tl) + if freshDialFail != nil { + tl.open(pipeline.TunnelDialFailed) + } else { + tl.open(reason) + } + s.tunnelOrDial(clientConn, dialTarget, tl, freshDialFail) return } + tl.open(reason) // Bidirectional copy until either side closes. sent, received := tunnel(clientConn, upstream) diff --git a/core/listener/forwardproxy/transparent.go b/core/listener/forwardproxy/transparent.go index 4f4bb3990..151a242a3 100644 --- a/core/listener/forwardproxy/transparent.go +++ b/core/listener/forwardproxy/transparent.go @@ -183,14 +183,17 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) { dialTarget = checked pctx.DialedAddr = checked } - upstream, err := s.dialUpstream(dialTarget) - // dialFail remembers a pre-dial that failed while the TLS bridge is enabled: - // the connection continues with no upstream, because the bridge can still take - // it and a plugin redirect can send the decrypted request to a live server - // (#1595) — the same rule as handleConnect's CONNECT path. - var dialFail error + upstream, err, memoized := s.dialUpstream(dialTarget, s.bridgeable(dst)) + // dialFail remembers a pre-dial that failed where the bridge could take the + // connection: the connection continues with no upstream, because the bridge + // can still take it and a plugin redirect can send the decrypted request to a + // live server (#1595) — the same rule as handleConnect's CONNECT path. + // freshDialFail is the failure when it came from a dial this connection made + // itself, not a memo hit: a re-dial would only repeat it, so tunnelOrDial + // closes with it instead. + var dialFail, freshDialFail error if err != nil { - if s.TLSBridge == nil { + if !s.bridgeable(dst) { slog.Warn("transparent-proxy: upstream dial failed", "host", host, "dst", dst, "error", err) // The 502 is synthetic — this path sends the client no status at all — but the // failure is recorded as it is on CONNECT rather than leaving no trace. @@ -201,6 +204,9 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) { slog.Warn("transparent-proxy: upstream dial failed; trying the TLS bridge", "host", host, "dst", dst, "error", err) dialFail = err + if !memoized { + freshDialFail = err + } } defer func() { if upstream != nil { @@ -241,8 +247,9 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) { } // bridgeServe fell open (upstream-verify failed) → re-dial for the tunnel. // dialTarget: the checked address, never re-resolved — same rule as - // handleConnect's fall-open re-dial. - s.tunnelOrDial(clientConn, dialTarget, tl) + // handleConnect's fall-open re-dial. freshDialFail, when set, skips + // the re-dial: the pre-dial already failed on a real dial. + s.tunnelOrDial(clientConn, dialTarget, tl, freshDialFail) return } slog.Info("tls-bridge passthrough", "host", key, "reason", reason) @@ -250,9 +257,11 @@ func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) { } // The pre-dial failed (or was skipped as memoized-dead) and the bridge // declined the connection: dial now — a plain tunnel still needs a real - // upstream, the same rule as handleConnect's. + // upstream, the same rule as handleConnect's. A fresh failure needs no second + // dial; a memo hit is up to dialFailMemoTTL old, and the origin may have + // come back. if upstream == nil { - s.tunnelOrDial(clientConn, dialTarget, tl) + s.tunnelOrDial(clientConn, dialTarget, tl, freshDialFail) return } sent, received := tunnel(clientConn, upstream) From e75ff1f07c790d4d494615befe5441e959ca4d66 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 30 Oct 2026 07:51:53 -0400 Subject: [PATCH 4/4] Docs: Say what a dead-origin CONNECT records with the TLS bridge on The dial-failed row said no tunnel opened and the close is a 502. Where the bridge is enabled for the connection's port that has been false since this PR's first commit, and this round stamping the open row's reason made it actively misleading: the CONNECT is answered 200 first (because a redirect can still apply), and a bridge that then declines closes with that 200 and the dial error. Fixes review: - both reviewers: docs/laptop-service.md's dial-failed row, CLAUDE.md's tunnel close-status sentence, and TunnelDialFailed's doc comment all described only the no-bridge shape Files: CLAUDE.md, core/pipeline/session.go, docs/laptop-service.md Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- CLAUDE.md | 2 +- core/pipeline/session.go | 6 +++++- docs/laptop-service.md | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6fc149803..226beb1f3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -676,7 +676,7 @@ Every event on `/v1/sessions/{id}` and `/v1/events` carries: - `requestedHost` — present only when a plugin redirected the request (`pctx.Redirect`): the host the client asked for. `host` is where the request actually went, and usage, the cost ledger and pricing all follow `host`. agentop's detail pane shows both on a `redirected:` line. - `inference.requestedModel` — present only when a plugin changed the model (`pctx.SetRequestModel`): the model the client asked for. `inference.model` is the model the request was sent for, which settlement prices; agentop's detail pane shows both on a `model:` line. - `bytesUp`, `bytesDown` — **body** bytes, one direction per row: a request row carries `bytesUp`, the body **as forwarded** (so a `tool-prune`-style rewrite is reflected, since the count is taken after the pipeline ran), and a response row carries `bytesDown`, the body **as it came back from the destination** — not as delivered, because extproc records the row before it emits a response mutator's replacement. Headers are not counted. A tunnel close is the one row carrying both. **Zero means the listener counted nothing, not that the body was empty**, and there is no third value for "unknown": every listener buffers bodies only when a plugin asks, so an unbuffered request reports zero. A response relayed straight through is counted, not zero — both proxies record the row after the copy (#1251's fix), so the bytes they relayed through their own process are on it. The one arm that still reports ↓0 is extproc with a body-free pipeline: it asked Envoy for body mode NONE, so the bytes it never saw are bytes it asked not to see — `core/listener/parity`'s `TestDivergence_InboundResponseBodyCountWhenNoPluginReadsIt` pins that disagreement with its reason. agentop renders a zero side blank rather than as `0B`, and that is why its BYTES column can be trusted as a size when it shows one. Everywhere else the three listeners count the same thing for the same response, which took some doing: extproc sums the chunks Envoy hands it, while the proxies count off the upstream body at one choke point each, so every arm — buffered, block-by-block SSE, or chunk copy — reports the same figure. `core/listener/internal/bodycount` and `core/listener/parity`'s bytes fixtures hold that line. -- `tunnel`, `tunnelReason` — an opaque CONNECT (or transparent-redirect) tunnel records two rows sharing a `requestId`: the open (`phase: "request"`, `tunnelReason` saying why the bytes stayed opaque) and, when the tunnel ends, the close (`phase: "response"`). The close carries the CONNECT's own `statusCode` — 200, or 502 with the dial error in `error` when the destination could not be reached (`tunnelReason: "dial-failed"`) — plus `durationMs` for how long the tunnel stayed open and, in `bytesUp`/`bytesDown`, the bytes it carried each way (up = client to destination; opaque bytes have no body/header split, so these are everything that crossed). It is not the destination's status: that travels inside the client's end-to-end TLS. A bridged tunnel's open is recorded with its first decrypted request — in that request's session, directly before it, stamped with its time — and records no close, because the request carries its own response; a bridged tunnel that recorded no request gets its open and a close when it ends. Tunnel rows are kept out of `/v1/usage`: a tunnel's lifetime is not a request latency. `tunnelReason: "dest-private"` with a 403 close and `error.kind: "destination_private"` is the egress guard refusing the destination before any dial — never confused with `dial-failed`, which attempted. +- `tunnel`, `tunnelReason` — an opaque CONNECT (or transparent-redirect) tunnel records two rows sharing a `requestId`: the open (`phase: "request"`, `tunnelReason` saying why the bytes stayed opaque) and, when the tunnel ends, the close (`phase: "response"`). The close carries the CONNECT's own `statusCode` — 200, or 502 with the dial error in `error` when the destination could not be reached and no TLS bridge could take the connection (`tunnelReason: "dial-failed"`; where the bridge is enabled for the port the CONNECT is answered 200 first, because a redirect can still apply, and a bridge that then declines closes with that 200 and the dial error) — plus `durationMs` for how long the tunnel stayed open and, in `bytesUp`/`bytesDown`, the bytes it carried each way (up = client to destination; opaque bytes have no body/header split, so these are everything that crossed). It is not the destination's status: that travels inside the client's end-to-end TLS. A bridged tunnel's open is recorded with its first decrypted request — in that request's session, directly before it, stamped with its time — and records no close, because the request carries its own response; a bridged tunnel that recorded no request gets its open and a close when it ends. Tunnel rows are kept out of `/v1/usage`: a tunnel's lifetime is not a request latency. `tunnelReason: "dest-private"` with a 403 close and `error.kind: "destination_private"` is the egress guard refusing the destination before any dial — never confused with `dial-failed`, which attempted. - `dialAddr` — the address the egress guard checked and the proxy dialed, `"ip:port"`, on a tunnel's open row when `listener.egress_guard` is on. With `host` it is the audit trail against DNS rebinding: under the guard the proxy resolves once, checks that answer, and dials exactly it, so the two fields together say "asked for the name, reached this address". Absent when the guard is off and on rows that dialed nothing. - `httpMethod`, `httpPath` — the HTTP verb and path, so a request no parser recognized is still identifiable rather than showing only a host. Distinct from the `method` inside `a2a` / `mcp`, which is a protocol method name. On an opaque tunnel `httpMethod` is `CONNECT` and `httpPath` is absent — opaque bytes carry no request line. The path is query-stripped and percent-decoded, so query-borne credentials never reach the timeline, but a secret in a path *segment* (a bot token, a webhook path) does survive on this unauthenticated surface — worth knowing before exporting events off-box. - `subagent` — present on rows from a Claude Code subagent or agent-team teammate: the `X-Claude-Code-Agent-Id` the request carried. For a subagent that is also the `` in `~/.claude/projects///subagents/agent-.meta.json`; for a teammate the join is unverified. Resolved by `Context.Subagent`, pinned with the User-Agent by `ResolveClient` so no plugin can re-file a row, and **refused rather than sanitised** when unusable (control runes, over 128 bytes), because it is a join key and a cleaned-up id joins nothing. It does not move the row: a subagent's requests stay in their parent's session, so the header must not be added to `session.id_headers`. **It names an agent, it does not classify the row:** `inference.agentRole` stays the field that says whether a row is a subagent's, and a teammate's row is `agentRole: main` with `subagent` set — Claude Code writes `cc_is_subagent=true` only for `agentType: subagent`. diff --git a/core/pipeline/session.go b/core/pipeline/session.go index 58e101429..051eb3f45 100644 --- a/core/pipeline/session.go +++ b/core/pipeline/session.go @@ -469,7 +469,11 @@ const ( TunnelPassthroughHost TunnelReason = "passthrough-host" // TunnelDialFailed — the proxy could not reach the destination at all, so no // tunnel ever opened. Recorded with a 502 close row carrying the dial error, so - // an unreachable destination shows in the timeline instead of leaving no trace. + // an unreachable destination shows in the timeline instead of leaving no trace — + // except where the TLS bridge is enabled for the connection's port: there the + // CONNECT is answered 200 first (a redirect can still send the request to a + // live server, #1595), and a bridge that then declines or falls open closes + // with that 200 and the dial error. TunnelDialFailed TunnelReason = "dial-failed" // TunnelPassthroughUnknown is the fallback when a lower layer declines to // intercept for a reason this vocabulary does not yet name. It exists so that diff --git a/docs/laptop-service.md b/docs/laptop-service.md index 3399c31e6..364adc620 100644 --- a/docs/laptop-service.md +++ b/docs/laptop-service.md @@ -814,7 +814,7 @@ The other reasons you may see, and what each one asks of you: | `client-hung-up` | The client vanished mid-handshake. Often a cancelled request; not evidence about trust, which is why it carries no advice. OpenCode hangs up rather than rejecting the CA; see [its page](agents/opencode.md#ca-trust). | usually no | | `handshake-failed` | Some other handshake failure — a version, cipher or ALPN mismatch, or Cortex failing to mint a certificate. | check `error=` in the log | | `origin-unverified` | **Cortex** could not verify the destination's certificate, so it declined to vouch for it. Bridging would have meant terminating TLS for a server we could not authenticate. | investigate the destination | -| `dial-failed` | Cortex could not reach the destination at all — a DNS failure, a refused connection or a timeout — so no tunnel opened. Its response row is a `502` whose `error` carries the dial error. The TCP dial is bounded at 10s, so a destination that silently drops the SYN (off the VPN that reaches it, a dead NAT) fails within that rather than hanging the agent for longer. | check the destination and the network path to it | +| `dial-failed` | Cortex could not reach the destination at all — a DNS failure, a refused connection or a timeout. Without the TLS bridge the CONNECT is answered `502` and no tunnel opened. With the bridge on (this install's default) the CONNECT is answered `200` first, because a redirect can still send the request to a live server; when nothing rescued it, the close row carries that `200` with the dial error in `error`. The TCP dial is bounded at 10s, so a destination that silently drops the SYN (off the VPN that reaches it, a dead NAT) fails within that rather than hanging the agent for longer. | check the destination and the network path to it | Only a CA rejection asks you to restart anything: `client-rejected-ca` itself, and `program-refused` when the earlier failure it remembers was one. The others are either