From 28484536b675d88b0cf0440f7a17ef3c814adfe6 Mon Sep 17 00:00:00 2001 From: Jakub Domeracki Date: Mon, 30 Mar 2026 12:25:33 +0200 Subject: [PATCH] fix(enterprise/aibridgeproxyd): return 403 for blocked private IP CONNECT attempts (#23360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when a CONNECT tunnel was blocked because the destination resolved to a private/reserved IP range, the proxy returned 502 Bad Gateway — implying an upstream failure rather than a deliberate policy block. Introduce `blockedIPError` as a sentinel type returned by both `checkBlockedIP` and `checkBlockedIPAndDial`. `ConnectionErrHandler` now inspects the error with `errors.As` and returns 403 Forbidden for policy blocks, keeping 502 for genuine dial failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- enterprise/aibridgeproxyd/aibridgeproxyd.go | 31 ++++++++++++++++--- .../aibridgeproxyd/aibridgeproxyd_test.go | 30 +++++++++++++++--- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/enterprise/aibridgeproxyd/aibridgeproxyd.go b/enterprise/aibridgeproxyd/aibridgeproxyd.go index 5d831d13a7..a23aa11105 100644 --- a/enterprise/aibridgeproxyd/aibridgeproxyd.go +++ b/enterprise/aibridgeproxyd/aibridgeproxyd.go @@ -57,6 +57,20 @@ var proxyAuthRequiredMsg = []byte(http.StatusText(http.StatusProxyAuthRequired)) // to GoproxyCa. In production, only one server runs, so this has no impact. var loadMITMOnce sync.Once +// blockedIPError is returned by checkBlockedIP and checkBlockedIPAndDial when +// a connection is blocked because the destination resolves to a private or +// reserved IP range. ConnectionErrHandler uses this type to return 403 +// Forbidden instead of the generic 502 Bad Gateway, since the block is a +// policy decision rather than an upstream failure. +type blockedIPError struct { + host string + ip net.IP +} + +func (e *blockedIPError) Error() string { + return fmt.Sprintf("connection to %s (%s) blocked: destination is in a private/reserved IP range", e.host, e.ip) +} + // blockedIPRanges defines private, reserved, and special-purpose IP ranges // that are blocked by default to prevent connections to internal networks. // Operators can selectively allow specific ranges via AllowedPrivateCIDRs. @@ -371,9 +385,16 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error) // Override goproxy's default CONNECT error handler to avoid leaking // internal error details to clients. Errors are still logged by the caller. - proxy.ConnectionErrHandler = func(w io.Writer, _ *goproxy.ProxyCtx, _ error) { - msg := "Bad Gateway" - _, _ = fmt.Fprintf(w, "HTTP/1.1 502 Bad Gateway\r\nContent-Type: text/plain\r\nContent-Length: %d\r\n\r\n%s", len(msg), msg) + // Policy blocks (private/reserved IP ranges) return 403 Forbidden; all + // other dial failures return 502 Bad Gateway. + proxy.ConnectionErrHandler = func(w io.Writer, _ *goproxy.ProxyCtx, err error) { + status := http.StatusBadGateway + var blocked *blockedIPError + if errors.As(err, &blocked) { + status = http.StatusForbidden + } + statusText := http.StatusText(status) + _, _ = fmt.Fprintf(w, "HTTP/1.1 %d %s\r\nContent-Type: text/plain\r\nContent-Length: %d\r\n\r\n%s", status, statusText, len(statusText), statusText) } // Reject CONNECT requests to non-standard ports. @@ -829,7 +850,7 @@ func (s *Server) checkBlockedIP(ctx context.Context, addr string) error { slog.F("port", port), slog.F("resolved_ip", ip.IP.String()), ) - return xerrors.Errorf("connection to %s (%s) blocked: destination is in a private/reserved IP range", host, ip.IP) + return &blockedIPError{host: host, ip: ip.IP} } } return nil @@ -868,7 +889,7 @@ func (s *Server) checkBlockedIPAndDial(ctx context.Context, network, addr string slog.F("port", port), slog.F("resolved_ip", ip.String()), ) - return xerrors.Errorf("CONNECT to private/reserved IP %s (%s) is blocked", ip, host) + return &blockedIPError{host: host, ip: ip} } return nil }, diff --git a/enterprise/aibridgeproxyd/aibridgeproxyd_test.go b/enterprise/aibridgeproxyd/aibridgeproxyd_test.go index d5fd2e8de5..4bd970d230 100644 --- a/enterprise/aibridgeproxyd/aibridgeproxyd_test.go +++ b/enterprise/aibridgeproxyd/aibridgeproxyd_test.go @@ -2103,6 +2103,7 @@ func TestProxy_PrivateIPBlocking(t *testing.T) { allowedCIDRs []string coderAccessURLFn func(targetHostname, port string) string expectBlocked bool + expectDialFail bool }{ { // Direct IP: by default, all private/reserved IPs are blocked. @@ -2162,6 +2163,14 @@ func TestProxy_PrivateIPBlocking(t *testing.T) { }, expectBlocked: false, }, + { + // A domain reserved by RFC 2606 that never resolves causes a plain dial + // failure (not a blocked IP). The proxy should return 502 Bad Gateway, + // not 403, to confirm the two error paths are distinguished correctly. + name: "DialFailureReturns502", + targetHostname: "host.invalid", + expectDialFail: true, + }, } for _, tt := range tests { @@ -2203,16 +2212,27 @@ func TestProxy_PrivateIPBlocking(t *testing.T) { srv := newTestProxy(t, opts...) - if tt.expectBlocked { - // Use a raw CONNECT to observe the 502 returned when ConnectDial fails. - // Go's HTTP client does not expose the response for non-2xx CONNECT results. + switch { + case tt.expectBlocked: + // Use a raw CONNECT to observe the 403 returned when ConnectDial blocks + // a private/reserved IP. Go's HTTP client does not expose the response + // for non-2xx CONNECT results. + resp := sendConnect(t, srv.Addr(), connectTarget, makeProxyAuthHeader("test-token")) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + require.Equal(t, "Forbidden", string(body), "error details should not be leaked to the client") + case tt.expectDialFail: + // Use a raw CONNECT to observe the 502 returned when ConnectDial fails + // for a reason other than a blocked IP (e.g. unresolvable hostname). resp := sendConnect(t, srv.Addr(), connectTarget, makeProxyAuthHeader("test-token")) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) require.NoError(t, err) require.Equal(t, http.StatusBadGateway, resp.StatusCode) - require.Equal(t, "Bad Gateway", string(body), "error details should not be leaked to the client") - } else { + require.Equal(t, "Bad Gateway", string(body)) + default: certPool := x509.NewCertPool() certPool.AddCert(targetServer.Certificate()) // InsecureSkipVerify is needed for "localhost": by default the cert SAN is 127.0.0.1.