mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: always verify TLS on aibridgeproxyd upstream transport (#26131)
## Problem aibridgeproxyd's HTTP transport (`proxy.Tr`) was configured with secure TLS defaults only when an upstream proxy was set. Without one, it fell back to [goproxy's default transport](https://github.com/elazarl/goproxy/blob/v1.8.0/proxy.go#L152), which has `InsecureSkipVerify: true`, leaving the connection between the proxy and aibridge vulnerable to MITM on HTTPS deployments. This PR moves the secure transport assignment outside the upstream proxy branch so it applies unconditionally. ## Changes * Apply secure TLS defaults to `proxy.Tr` unconditionally (verified `RootCAs`, `MinVersion: TLS 1.2`). * Add `TestProxy_AIBridgeTLSVerification` to cover the verification path between the proxy and aibridge. ## Notes * **Behavior change for `HTTPS_PROXY` env var**: previously, when `UpstreamProxy` was unset, `proxy.Tr` honored `HTTP_PROXY` and `HTTPS_PROXY` env vars. After this PR it does not, since MITM'd requests now always go directly to aibridge. This matches the behavior when `UpstreamProxy` is configured, which already ignored env vars. * **HTTPS deployments with a private CA**: when `CoderAccessURL` is HTTPS and its TLS certificate (or the load balancer's certificate fronting it) is signed by a CA not in the system trust store, the proxy will now fail with `x509: certificate signed by unknown authority`. Closes https://linear.app/codercom/issue/AIGOV-386/ai-bridge-proxy-uses-goproxy-default-with-tls-verification-disabled > [!NOTE] > Initially generated by Claude Opus 4.7, modified and reviewed by @ssncferreira
This commit is contained in:
@@ -303,14 +303,21 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
|
||||
proxy.CertStore = NewCertCache()
|
||||
}
|
||||
|
||||
// Always set secure TLS defaults, overriding goproxy's default.
|
||||
// This ensures secure TLS connections for:
|
||||
// - HTTPS upstream proxy connections
|
||||
// - MITM'd requests if aibridge uses HTTPS
|
||||
// Override goproxy's default transport, which has InsecureSkipVerify: true.
|
||||
// This applies to all proxy.Tr traffic: MITM'd requests forwarded to aibridge,
|
||||
// passthrough requests, and HTTPS upstream proxy connections. Proxy is
|
||||
// intentionally unset so MITM'd requests go directly to aibridge, never
|
||||
// through an upstream proxy or HTTPS_PROXY env var.
|
||||
rootCAs, err := x509.SystemCertPool()
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("failed to load system certificate pool: %w", err)
|
||||
}
|
||||
proxy.Tr = &http.Transport{
|
||||
TLSClientConfig: &tls.Config{
|
||||
MinVersion: tls.VersionTLS12,
|
||||
RootCAs: rootCAs,
|
||||
},
|
||||
}
|
||||
|
||||
srv := &Server{
|
||||
ctx: ctx,
|
||||
@@ -353,15 +360,6 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
|
||||
}
|
||||
}
|
||||
|
||||
// Set transport without Proxy to ensure MITM'd requests go directly to aibridge,
|
||||
// not through any upstream proxy.
|
||||
proxy.Tr = &http.Transport{
|
||||
TLSClientConfig: &tls.Config{
|
||||
MinVersion: tls.VersionTLS12,
|
||||
RootCAs: rootCAs,
|
||||
},
|
||||
}
|
||||
|
||||
// Add custom CA certificate if provided (for corporate proxies with private CAs).
|
||||
// If no CA certificate is provided, the system certificate pool is used.
|
||||
if opts.UpstreamProxyCA != "" {
|
||||
|
||||
@@ -1624,6 +1624,37 @@ func TestListenerTLS(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestProxy_AIBridgeTLSVerification verifies the proxy refuses to forward
|
||||
// MITM'd requests to an aibridge endpoint whose TLS certificate is not trusted.
|
||||
func TestProxy_AIBridgeTLSVerification(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// HTTPS server with a self-signed cert untrusted by the system pool,
|
||||
// standing in for aibridge.
|
||||
aibridgeServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
t.Cleanup(aibridgeServer.Close)
|
||||
|
||||
srv := newTestProxy(t,
|
||||
withCoderAccessURL(aibridgeServer.URL),
|
||||
withProviderHosts(aibridgeproxyd.HostAnthropic),
|
||||
)
|
||||
|
||||
client := newProxyClient(t, srv, makeProxyAuthHeader("test-token"), getProxyCertPool(t), false)
|
||||
|
||||
req, err := http.NewRequestWithContext(t.Context(), http.MethodPost,
|
||||
"https://"+aibridgeproxyd.HostAnthropic+"/v1/messages",
|
||||
strings.NewReader(`{}`))
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err := client.Do(req)
|
||||
if resp != nil {
|
||||
defer resp.Body.Close()
|
||||
}
|
||||
require.Error(t, err, "proxy must refuse to forward MITM'd requests to an untrusted aibridge cert")
|
||||
}
|
||||
|
||||
// TestServeCACert validates that a configured certificate file can be served correctly by the API.
|
||||
//
|
||||
// Note: Tests for certificate file errors (missing file, invalid PEM) are
|
||||
|
||||
Reference in New Issue
Block a user