Skip to content

Commit 3671534

Browse files
authored
fix(proxy): clear SNI peek deadline before relay, fix socket leaks (#15)
The 10-second read deadline set for SNI peeking in the SNI-deferred path was cleared via defer, which only runs when handleConnect returns. Since handleConnect blocks in relayData for the connection lifetime, the deadline persisted and killed every SNI-deferred connection after 10 seconds. This caused streaming API responses to be truncated (manifesting as OpenAI "terminated" errors), tool call fetches to fail, and periodic TLS handshake failures on chatgpt.com. Fix: clear the deadline explicitly after SNI peek completes, before the relay phase begins. Also fixes two secondary issues: - relayData socket leak: when the first relay direction completed, the function could block indefinitely waiting for the second goroutine if goproxy held the MITM connection open. Close writer and set a read deadline on target to force cleanup. Eliminates CLOSE_WAIT socket accumulation (75 leaked sockets observed). - goproxy Transport stale connections: the MITM proxy Transport had no IdleConnTimeout, causing dead pooled connections to persist indefinitely. Add IdleConnTimeout (90s) and MaxIdleConnsPerHost (4). - goproxy log noise: suppress expected broken pipe and handshake EOF warnings via a filtered logger. These are normal for short-lived polling connections (Telegram getUpdates).
1 parent b643af1 commit 3671534

2 files changed

Lines changed: 60 additions & 6 deletions

File tree

internal/proxy/inject.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,25 @@ func sniAwareTLSConfig(ca *tls.Certificate) func(host string, ctx *goproxy.Proxy
7878
}
7979
}
8080

81+
// filteredWriter wraps an io.Writer and silently drops lines containing any
82+
// of the configured substrings. Used to suppress expected goproxy warnings
83+
// (broken pipe, handshake EOF) that are harmless noise in a proxy with
84+
// short-lived polling connections.
85+
type filteredWriter struct {
86+
inner io.Writer
87+
drop []string
88+
}
89+
90+
func (fw *filteredWriter) Write(p []byte) (int, error) {
91+
s := string(p)
92+
for _, d := range fw.drop {
93+
if strings.Contains(s, d) {
94+
return len(p), nil
95+
}
96+
}
97+
return fw.inner.Write(p)
98+
}
99+
81100
// Injector is an HTTP/HTTPS MITM proxy that intercepts requests and injects
82101
// credentials from the vault. It resolves bindings by destination, decrypts
83102
// credentials, and performs byte-level replacement of phantom tokens in
@@ -177,6 +196,13 @@ func NewInjector(provider vault.Provider, resolver *atomic.Pointer[vault.Binding
177196

178197
proxy := goproxy.NewProxyHttpServer()
179198
proxy.Verbose = false
199+
// Suppress noisy goproxy warnings for expected conditions:
200+
// - "broken pipe" from client closing before response delivery (Telegram polling)
201+
// - "Cannot handshake" from client aborting during TLS setup (timeouts, retries)
202+
proxy.Logger = log.New(&filteredWriter{
203+
inner: log.Writer(),
204+
drop: []string{"broken pipe", "Cannot handshake"},
205+
}, "", 0)
180206

181207
// Build a root CA pool for the outbound transport. Start with system
182208
// roots and add the sluice MITM CA cert. Adding the MITM CA is

internal/proxy/server.go

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,7 +1111,6 @@ func (s *Server) handleConnect(ctx context.Context, writer io.Writer, request *s
11111111
// client is slow to send the TLS ClientHello.
11121112
if conn, ok := writer.(net.Conn); ok {
11131113
conn.SetReadDeadline(time.Now().Add(10 * time.Second)) //nolint:errcheck
1114-
defer conn.SetReadDeadline(time.Time{}) //nolint:errcheck
11151114
}
11161115

11171116
var allow bool
@@ -1120,6 +1119,14 @@ func (s *Server) handleConnect(ctx context.Context, writer io.Writer, request *s
11201119
return nil
11211120
}
11221121

1122+
// Clear the read deadline before the relay phase. The deadline was
1123+
// only needed for the SNI peek. Leaving it active would kill the
1124+
// relay after 10 seconds, terminating long-running connections
1125+
// (streaming API responses, tool calls, etc.).
1126+
if conn, ok := writer.(net.Conn); ok {
1127+
conn.SetReadDeadline(time.Time{}) //nolint:errcheck
1128+
}
1129+
11231130
// Dial with the updated context (FQDN now contains the SNI hostname).
11241131
target, err := s.dial(ctx, "tcp", request.DestAddr.String())
11251132
if err != nil {
@@ -1155,6 +1162,14 @@ func (s *Server) handleConnect(ctx context.Context, writer io.Writer, request *s
11551162
}
11561163

11571164
// relayData bidirectionally copies data between the client and target.
1165+
//
1166+
// When the first direction finishes (either client or target closes), the
1167+
// writer (SOCKS5 connection) is closed to unblock the second goroutine.
1168+
// target is NOT closed here to avoid triggering broken pipe warnings in
1169+
// goproxy. The caller's deferred target.Close() handles final cleanup.
1170+
// If the second goroutine is blocked reading from target (e.g. pending
1171+
// long-poll), a short deadline forces it to return instead of blocking
1172+
// indefinitely and leaking the SOCKS5 connection in CLOSE_WAIT state.
11581173
func (s *Server) relayData(clientReader io.Reader, writer io.Writer, target net.Conn) error {
11591174
errCh := make(chan error, 2)
11601175
go func() {
@@ -1172,12 +1187,25 @@ func (s *Server) relayData(clientReader io.Reader, writer io.Writer, target net.
11721187
errCh <- cpErr
11731188
}()
11741189

1175-
for i := 0; i < 2; i++ {
1176-
if e := <-errCh; e != nil {
1177-
return e
1178-
}
1190+
// Wait for the first direction to complete.
1191+
e1 := <-errCh
1192+
1193+
// Close writer to unblock goroutine 2 if it's stuck writing. Set a
1194+
// read deadline on target to unblock goroutine 2 if it's stuck reading
1195+
// (e.g. goproxy waiting for a long-poll response). This avoids closing
1196+
// target directly, which would trigger broken pipe warnings in goproxy.
1197+
if cl, ok := writer.(io.Closer); ok {
1198+
cl.Close() //nolint:errcheck
11791199
}
1180-
return nil
1200+
target.SetReadDeadline(time.Now().Add(3 * time.Second)) //nolint:errcheck
1201+
1202+
// Drain the second result so the goroutine is not leaked.
1203+
e2 := <-errCh
1204+
1205+
if e1 != nil {
1206+
return e1
1207+
}
1208+
return e2
11811209
}
11821210

11831211
// sniPolicyCheckBeforeDial peeks the first bytes from the client to extract

0 commit comments

Comments
 (0)