mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix(enterprise/aibridgeproxyd): return 403 for blocked private IP CONNECT attempts (#23360)
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 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
7a5fd4c790
commit
28484536b6
@@ -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
|
||||
},
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user