From d2b853464d0ecb428ee56704244a22ec77288480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl?= Date: Tue, 22 Sep 2026 17:12:43 +0200 Subject: [PATCH] fix(client): route side-channel handshake through the configured proxy ConnectViaProxy's data path honors HTTP(S)_PROXY, but the transport credentials' side-channel handshake dialed the endpoint directly with a plain net.Dialer to obtain the TLS AuthInfo. Behind an egress proxy that direct dial hangs, so gRPC calls (e.g. roxctl image scan) failed with "authentication handshake failed: dial tcp :443: i/o timeout" even though a proxy was configured (ROX-37149). Add a proxy-aware dialer (HTTP/HTTPS CONNECT with basic-auth + SOCKS5, direct fallback) and use it for the side channel. The WebSocket data transport gets the same Proxy: http.ProxyFromEnvironment treatment, which it was also missing. httpproxy.FromEnvironment is used instead of http.ProxyFromEnvironment to avoid the latter's process-wide env caching (also makes it testable). Prompt: "Address ROX-37149 ... Fix HTTP here. Fix grpc-http1 in ../go-grpc-http1". Partially generated by AI. Co-Authored-By: Claude Opus 4.8 --- client/proxy_dialer.go | 181 ++++++++++++++++++++++++++ client/proxy_dialer_test.go | 238 +++++++++++++++++++++++++++++++++++ client/side_channel_creds.go | 5 +- client/ws_proxy.go | 3 + 4 files changed, 426 insertions(+), 1 deletion(-) create mode 100644 client/proxy_dialer.go create mode 100644 client/proxy_dialer_test.go diff --git a/client/proxy_dialer.go b/client/proxy_dialer.go new file mode 100644 index 0000000..a218733 --- /dev/null +++ b/client/proxy_dialer.go @@ -0,0 +1,181 @@ +// Copyright (c) 2020 StackRox Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License + +package client + +import ( + "bufio" + "bytes" + "context" + "crypto/tls" + "encoding/base64" + "io" + "net" + "net/http" + "net/url" + + "github.com/pkg/errors" + "golang.org/x/net/http/httpproxy" + "golang.org/x/net/proxy" +) + +// dialProxyAwareContext establishes a raw TCP connection to targetAddr, honoring the +// HTTP(S)_PROXY/NO_PROXY environment variables the same way net/http does. This is used +// for the side-channel TLS handshake, which would otherwise dial the target directly and +// hang when direct egress is blocked but a proxy is available (ROX-37149). +// +// The scheme is fixed to "https" because the side channel always performs a TLS handshake +// with the target; this mirrors the reverse-proxy data path, which resolves its proxy for +// the same "https" scheme. httpproxy.FromEnvironment is used rather than +// http.ProxyFromEnvironment because the latter caches the environment process-wide. +func dialProxyAwareContext(ctx context.Context, targetAddr string) (net.Conn, error) { + reqURL := &url.URL{Scheme: "https", Host: targetAddr} + proxyURL, err := httpproxy.FromEnvironment().ProxyFunc()(reqURL) + if err != nil { + return nil, errors.Wrap(err, "determining proxy from environment") + } + if proxyURL == nil { + return (&net.Dialer{}).DialContext(ctx, "tcp", targetAddr) + } + + switch proxyURL.Scheme { + case "socks5", "socks5h": + return dialViaSOCKS5(ctx, proxyURL, targetAddr) + case "http", "https", "": + return dialViaHTTPConnect(ctx, proxyURL, targetAddr) + default: + return nil, errors.Errorf("unsupported proxy scheme %q for proxy %q", proxyURL.Scheme, proxyURL.Redacted()) + } +} + +// dialViaSOCKS5 dials targetAddr through a SOCKS5 proxy. +func dialViaSOCKS5(ctx context.Context, proxyURL *url.URL, targetAddr string) (net.Conn, error) { + var auth *proxy.Auth + if u := proxyURL.User; u != nil { + password, _ := u.Password() + auth = &proxy.Auth{User: u.Username(), Password: password} + } + dialer, err := proxy.SOCKS5("tcp", proxyURL.Host, auth, proxy.Direct) + if err != nil { + return nil, errors.Wrap(err, "creating SOCKS5 dialer") + } + if ctxDialer, ok := dialer.(proxy.ContextDialer); ok { + return ctxDialer.DialContext(ctx, "tcp", targetAddr) + } + return dialer.Dial("tcp", targetAddr) +} + +// dialViaHTTPConnect establishes a tunnel to targetAddr through an HTTP(S) proxy using the +// HTTP CONNECT method, mirroring what net/http.Transport does internally. +func dialViaHTTPConnect(ctx context.Context, proxyURL *url.URL, targetAddr string) (net.Conn, error) { + conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", proxyHostPort(proxyURL)) + if err != nil { + return nil, errors.Wrapf(err, "dialing proxy %q", proxyURL.Redacted()) + } + // Close the connection unless the CONNECT succeeds, so a failed attempt does not leak it. + success := false + defer func() { + if !success { + _ = conn.Close() + } + }() + + if proxyURL.Scheme == "https" { + tlsConn := tls.Client(conn, &tls.Config{ServerName: proxyURL.Hostname()}) + if hsErr := tlsConn.HandshakeContext(ctx); hsErr != nil { + return nil, errors.Wrapf(hsErr, "TLS handshake with proxy %q", proxyURL.Redacted()) + } + conn = tlsConn + } + + // Abort the CONNECT exchange when the context is done. net.Conn has no context-aware + // read/write, so closing the connection is how we unblock it. + if done := ctx.Done(); done != nil { + stop := make(chan struct{}) + defer close(stop) + go func() { + select { + case <-done: + _ = conn.Close() + case <-stop: + } + }() + } + + connectReq := &http.Request{ + Method: http.MethodConnect, + URL: &url.URL{Opaque: targetAddr}, + Host: targetAddr, + Header: make(http.Header), + } + if u := proxyURL.User; u != nil { + password, _ := u.Password() + connectReq.Header.Set("Proxy-Authorization", proxyBasicAuth(u.Username(), password)) + } + if err = connectReq.Write(conn); err != nil { + return nil, errors.Wrapf(err, "sending CONNECT to proxy %q", proxyURL.Redacted()) + } + + br := bufio.NewReader(conn) + resp, err := http.ReadResponse(br, connectReq) + if err != nil { + return nil, errors.Wrapf(err, "reading CONNECT response from proxy %q", proxyURL.Redacted()) + } + // The tunnel is now open; the body carries no meaningful content. + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, errors.Errorf("proxy %q refused CONNECT to %q: %s", proxyURL.Redacted(), targetAddr, resp.Status) + } + + // A well-behaved proxy sends nothing before the tunneled peer does, but preserve any + // bytes the reader buffered past the response headers rather than dropping them. + tunnel := conn + if n := br.Buffered(); n > 0 { + buffered := make([]byte, n) + if _, err := io.ReadFull(br, buffered); err != nil { + return nil, errors.Wrap(err, "reading buffered bytes after CONNECT") + } + tunnel = &prefixConn{Conn: conn, prefix: io.MultiReader(bytes.NewReader(buffered), conn)} + } + + success = true + return tunnel, nil +} + +// prefixConn is a net.Conn whose reads are served from prefix first (used to replay bytes +// that were buffered past the CONNECT response headers). +type prefixConn struct { + net.Conn + prefix io.Reader +} + +func (c *prefixConn) Read(p []byte) (int, error) { + return c.prefix.Read(p) +} + +// proxyHostPort returns the proxy address with a scheme-appropriate default port applied. +func proxyHostPort(proxyURL *url.URL) string { + if proxyURL.Port() != "" { + return proxyURL.Host + } + port := "80" + if proxyURL.Scheme == "https" { + port = "443" + } + return net.JoinHostPort(proxyURL.Hostname(), port) +} + +func proxyBasicAuth(username, password string) string { + return "Basic " + base64.StdEncoding.EncodeToString([]byte(username+":"+password)) +} diff --git a/client/proxy_dialer_test.go b/client/proxy_dialer_test.go new file mode 100644 index 0000000..a8fa878 --- /dev/null +++ b/client/proxy_dialer_test.go @@ -0,0 +1,238 @@ +// Copyright (c) 2020 StackRox Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License + +package client + +import ( + "bufio" + "context" + "io" + "net" + "net/http" + "net/url" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// clearProxyEnv resets all proxy-related environment variables so tests are not affected by +// the ambient environment. t.Setenv restores the previous values when the test finishes. +func clearProxyEnv(t *testing.T) { + for _, v := range []string{ + "HTTP_PROXY", "http_proxy", + "HTTPS_PROXY", "https_proxy", + "NO_PROXY", "no_proxy", + } { + t.Setenv(v, "") + } +} + +// startEchoServer starts a TCP server that echoes back everything it receives. +func startEchoServer(t *testing.T) string { + t.Helper() + lis, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = lis.Close() }) + go func() { + for { + conn, err := lis.Accept() + if err != nil { + return + } + go func() { + defer func() { _ = conn.Close() }() + _, _ = io.Copy(conn, conn) + }() + } + }() + return lis.Addr().String() +} + +type fakeConnectProxy struct { + addr string + + mu sync.Mutex + connectHits int + lastHost string + lastAuth string +} + +// startFakeConnectProxy starts a minimal HTTP proxy that answers CONNECT with the given +// status code. On a 200 response it tunnels the connection to itself (echo), so callers can +// verify that bytes flow through the established tunnel. +func startFakeConnectProxy(t *testing.T, statusLine string, tunnel bool) *fakeConnectProxy { + t.Helper() + lis, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = lis.Close() }) + + p := &fakeConnectProxy{addr: lis.Addr().String()} + go func() { + for { + conn, err := lis.Accept() + if err != nil { + return + } + go p.handle(conn, statusLine, tunnel) + } + }() + return p +} + +func (p *fakeConnectProxy) handle(conn net.Conn, statusLine string, tunnel bool) { + defer func() { _ = conn.Close() }() + br := bufio.NewReader(conn) + req, err := http.ReadRequest(br) + if err != nil { + return + } + p.mu.Lock() + p.connectHits++ + p.lastHost = req.Host + p.lastAuth = req.Header.Get("Proxy-Authorization") + p.mu.Unlock() + + if _, err := io.WriteString(conn, "HTTP/1.1 "+statusLine+"\r\n\r\n"); err != nil { + return + } + if !tunnel { + return + } + // Echo tunneled bytes, honoring anything already buffered past the request headers. + if n := br.Buffered(); n > 0 { + buffered := make([]byte, n) + if _, err := io.ReadFull(br, buffered); err == nil { + _, _ = conn.Write(buffered) + } + } + _, _ = io.Copy(conn, conn) +} + +func (p *fakeConnectProxy) stats() (hits int, host, auth string) { + p.mu.Lock() + defer p.mu.Unlock() + return p.connectHits, p.lastHost, p.lastAuth +} + +func roundTrip(t *testing.T, conn net.Conn, payload string) string { + t.Helper() + require.NoError(t, conn.SetDeadline(time.Now().Add(5*time.Second))) + _, err := io.WriteString(conn, payload) + require.NoError(t, err) + buf := make([]byte, len(payload)) + _, err = io.ReadFull(conn, buf) + require.NoError(t, err) + return string(buf) +} + +func TestDialProxyAwareContext_NoProxy(t *testing.T) { + clearProxyEnv(t) + echoAddr := startEchoServer(t) + + conn, err := dialProxyAwareContext(context.Background(), echoAddr) + require.NoError(t, err) + defer func() { _ = conn.Close() }() + + assert.Equal(t, "ping", roundTrip(t, conn, "ping")) +} + +func TestDialProxyAwareContext_HTTPConnect(t *testing.T) { + clearProxyEnv(t) + proxy := startFakeConnectProxy(t, "200 Connection established", true) + t.Setenv("HTTPS_PROXY", "http://"+proxy.addr) + + const target = "central.example.com:443" + conn, err := dialProxyAwareContext(context.Background(), target) + require.NoError(t, err) + defer func() { _ = conn.Close() }() + + assert.Equal(t, "hello", roundTrip(t, conn, "hello")) + + hits, host, auth := proxy.stats() + assert.Equal(t, 1, hits, "connection must go through the proxy") + assert.Equal(t, target, host, "CONNECT must target the requested endpoint") + assert.Empty(t, auth) +} + +func TestDialProxyAwareContext_HTTPConnectBasicAuth(t *testing.T) { + clearProxyEnv(t) + proxy := startFakeConnectProxy(t, "200 Connection established", true) + t.Setenv("HTTPS_PROXY", "http://user:pass@"+proxy.addr) + + conn, err := dialProxyAwareContext(context.Background(), "central.example.com:443") + require.NoError(t, err) + defer func() { _ = conn.Close() }() + _ = roundTrip(t, conn, "hi") + + _, _, auth := proxy.stats() + assert.Equal(t, proxyBasicAuth("user", "pass"), auth) +} + +func TestDialProxyAwareContext_NoProxyMatch(t *testing.T) { + clearProxyEnv(t) + proxy := startFakeConnectProxy(t, "200 Connection established", true) + t.Setenv("HTTPS_PROXY", "http://"+proxy.addr) + echoAddr := startEchoServer(t) + // Exclude the echo server from proxying; it must be dialed directly. + host, _, err := net.SplitHostPort(echoAddr) + require.NoError(t, err) + t.Setenv("NO_PROXY", host) + + conn, err := dialProxyAwareContext(context.Background(), echoAddr) + require.NoError(t, err) + defer func() { _ = conn.Close() }() + + assert.Equal(t, "direct", roundTrip(t, conn, "direct")) + hits, _, _ := proxy.stats() + assert.Zero(t, hits, "NO_PROXY host must bypass the proxy") +} + +func TestDialProxyAwareContext_ConnectRefused(t *testing.T) { + clearProxyEnv(t) + proxy := startFakeConnectProxy(t, "403 Forbidden", false) + t.Setenv("HTTPS_PROXY", "http://"+proxy.addr) + + _, err := dialProxyAwareContext(context.Background(), "central.example.com:443") + require.Error(t, err) + assert.Contains(t, err.Error(), "403") +} + +func TestDialProxyAwareContext_UnsupportedScheme(t *testing.T) { + clearProxyEnv(t) + t.Setenv("HTTPS_PROXY", "ftp://proxy.example.com:2121") + + _, err := dialProxyAwareContext(context.Background(), "central.example.com:443") + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported proxy scheme") +} + +func TestProxyHostPort(t *testing.T) { + for name, tc := range map[string]struct { + raw string + expected string + }{ + "http default port": {raw: "http://proxy.example.com", expected: "proxy.example.com:80"}, + "https default port": {raw: "https://proxy.example.com", expected: "proxy.example.com:443"}, + "explicit port": {raw: "http://proxy.example.com:8080", expected: "proxy.example.com:8080"}, + } { + t.Run(name, func(t *testing.T) { + u, err := url.Parse(tc.raw) + require.NoError(t, err) + assert.Equal(t, tc.expected, proxyHostPort(u)) + }) + } +} diff --git a/client/side_channel_creds.go b/client/side_channel_creds.go index 689fde1..b47e2ef 100644 --- a/client/side_channel_creds.go +++ b/client/side_channel_creds.go @@ -47,7 +47,10 @@ func (c *sideChannelCreds) ClientHandshake(ctx context.Context, authority string return rawConn, c.authInfo, nil } - sideChannelConn, err := (&net.Dialer{}).DialContext(ctx, "tcp", c.endpoint) + // Dial through the configured HTTP(S)/SOCKS proxy, if any. A plain net.Dialer here + // would bypass the proxy and hang when direct egress to the endpoint is blocked but + // a proxy is available (ROX-37149). + sideChannelConn, err := dialProxyAwareContext(ctx, c.endpoint) if err != nil { return nil, nil, err } diff --git a/client/ws_proxy.go b/client/ws_proxy.go index 33c654c..b92b651 100644 --- a/client/ws_proxy.go +++ b/client/ws_proxy.go @@ -298,6 +298,9 @@ func createClientWSProxy(endpoint string, tlsClientConf *tls.Config) (*http.Serv endpoint: endpoint, httpClient: &http.Client{ Transport: &http.Transport{ + // Honor HTTP(S)_PROXY/NO_PROXY; a hand-built transport otherwise dials + // the endpoint directly, bypassing the proxy (ROX-37149). + Proxy: http.ProxyFromEnvironment, TLSClientConfig: tlsClientConf, }, },