Skip to content

Commit 6646b57

Browse files
committed
fix,docs: Loopback self-redirect guard + document IP-level gating
Address follow-up review on the transparent listener: - transparentproxy.dispatch: drop connections whose SO_ORIGINAL_DST is a loopback address. A genuinely REDIRECTed connection never has a loopback original destination (the enforce-redirect iptables RETURN loopback before the REDIRECT), so a loopback dst means a direct dial to the listener port where SO_ORIGINAL_DST reports the listener's own address — tunnelling to it would spiral into ever more connections/goroutines. Defense-in-depth. - HandleTransparentConn: document the gating-key difference. CONNECT gates on the hostname (r.Host); captured traffic gates on the SO_ORIGINAL_DST IP:port, so domain-based egress policy does not match captured bypass traffic the way it matches explicit-proxy traffic. Acceptable while opt-in; the always-on operator PR must first add SNI-peek for parity or consciously accept IP-level policy. Tracked as a prerequisite for the default flip. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
1 parent cae8cfe commit 6646b57

2 files changed

Lines changed: 26 additions & 0 deletions

File tree

authbridge/authlib/listener/forwardproxy/transparent.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,18 @@ import (
2626
// gates, dials dst, and copies bytes both ways. dst is "host:port" recovered
2727
// from SO_ORIGINAL_DST by the transparent listener.
2828
//
29+
// GATING LIMITATION (important before enforce-redirect goes always-on):
30+
// CONNECT carries a hostname in r.Host, so the outbound pipeline gates captured
31+
// CONNECT traffic on the domain (e.g. "api.openai.com:443"). SO_ORIGINAL_DST
32+
// yields only an IP:port, so here pctx.Host is an IP literal. Any host/domain-
33+
// based egress policy (allowlist-by-domain, host->audience routing) therefore
34+
// does NOT match captured bypass traffic the way it matches explicit-proxy
35+
// traffic — the gate runs, but sees an IP. This is acceptable while the operator
36+
// flag keeps enforce-redirect opt-in, but the always-on operator PR MUST first
37+
// either (a) peek the TLS ClientHello SNI here and set pctx.Host to the SNI
38+
// hostname for parity with CONNECT, or (b) consciously accept IP-level egress
39+
// policy for captured traffic. Tracked as a prerequisite for the default flip.
40+
//
2941
// HandleTransparentConn owns clientConn's lifecycle and always closes it.
3042
func (s *Server) HandleTransparentConn(clientConn net.Conn, dst string) {
3143
defer func() { _ = clientConn.Close() }()

authbridge/authlib/listener/transparentproxy/server.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,19 @@ func (s *Server) dispatch(conn *net.TCPConn) {
7676
_ = conn.Close()
7777
return
7878
}
79+
// Defense-in-depth against a self-redirect loop: a genuinely REDIRECTed
80+
// connection never has a loopback original destination (the enforce-redirect
81+
// iptables rules RETURN loopback before the REDIRECT). So a loopback dst here
82+
// means a direct dial to the listener port, where SO_ORIGINAL_DST reports the
83+
// listener's own address — tunnelling to it would spiral into ever more
84+
// connections/goroutines. Drop it.
85+
if host, _, splitErr := net.SplitHostPort(dst); splitErr == nil {
86+
if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() {
87+
slog.Warn("transparent-proxy: dropping connection whose original destination is loopback (not a redirect; would self-loop)",
88+
"remote", conn.RemoteAddr().String(), "dst", dst)
89+
_ = conn.Close()
90+
return
91+
}
92+
}
7993
s.handle(conn, dst)
8094
}

0 commit comments

Comments
 (0)