fix: allow configured HTTP proxy on private IPs in SSRF transport

pkg/httpclient/ssrf.go

@@ -1,9 +1,12 @@
 package httpclient
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"net/http"
+	"net/url"
+	"os"
 	"syscall"
 	"time"
 )
@@ -53,16 +56,111 @@ func SSRFDialControl(_, address string, _ syscall.RawConn) error {
 // user-configured API endpoints, etc.). It does not enforce HTTPS —
 // callers that require it must validate the request URL themselves
 // and/or supply a CheckRedirect on the surrounding *http.Client.
+//
+// As an exception, the explicitly-configured HTTP/HTTPS/ALL proxy is
+// always dialable, even if it lives on a private address. Refusing to
+// dial the operator-configured proxy adds no SSRF protection (the proxy
+// enforces destination policy itself) and breaks sandboxes — like
+// docker-agent's — whose mandatory egress proxy is on an RFC1918 IP.
 func NewSSRFSafeTransport() *http.Transport {
 	t := http.DefaultTransport.(*http.Transport).Clone()
+	proxies := proxyDialAllowlist()
 	t.DialContext = (&net.Dialer{
 		Timeout:   30 * time.Second,
 		KeepAlive: 30 * time.Second,
-		Control:   SSRFDialControl,
+		Control: func(network, address string, c syscall.RawConn) error {
+			if _, ok := proxies[canonicalHostPort(address)]; ok {
+				return nil
+			}
+			return SSRFDialControl(network, address, c)
+		},
 	}).DialContext
 	return t
 }
 
+// canonicalHostPort normalises a "host:port" dial address so equivalent
+// IPv4 forms compare equal. An IPv4-mapped IPv6 address (::ffff:a.b.c.d)
+// is folded back to its dotted-quad form, matching what proxyHostPorts
+// stores in the allowlist for an IPv4-literal proxy. Non-IP hosts and
+// malformed inputs are returned unchanged so the allowlist lookup
+// simply misses and the SSRF fall-through applies.
+func canonicalHostPort(address string) string {
+	host, port, err := net.SplitHostPort(address)
+	if err != nil {
+		return address
+	}
+	ip := net.ParseIP(host)
+	if ip == nil {
+		return address
+	}
+	if ip4 := ip.To4(); ip4 != nil {
+		return net.JoinHostPort(ip4.String(), port)
+	}
+	return net.JoinHostPort(ip.String(), port)
+}
+
+// proxyDialAllowlist returns the set of "host:port" strings that the
+// HTTP_PROXY / HTTPS_PROXY / ALL_PROXY environment variables resolve to.
+// Hostname-based proxies are resolved to all of their A/AAAA records so
+// the comparison can be done against the post-resolution dial address.
+// The result is a snapshot taken at call time — safe to capture in a
+// dialer's Control closure.
+func proxyDialAllowlist() map[string]struct{} {
+	out := map[string]struct{}{}
+	raw := []string{
+		os.Getenv("HTTP_PROXY"), os.Getenv("http_proxy"),
+		os.Getenv("HTTPS_PROXY"), os.Getenv("https_proxy"),
+		os.Getenv("ALL_PROXY"), os.Getenv("all_proxy"),
+	}
+	for _, s := range raw {
+		for addr := range proxyHostPorts(s) {
+			out[addr] = struct{}{}
+		}
+	}
+	return out
+}
+
+// proxyHostPorts parses a single proxy specification (matching the syntax
+// accepted by net/http.ProxyFromEnvironment: a full URL or a bare host[:port]
+// in which case http:// is implied) and returns each "ip:port" it can be
+// reached at. A nil/empty input yields an empty set.
+func proxyHostPorts(spec string) map[string]struct{} {
+	if spec == "" {
+		return nil
+	}
+	u, err := url.Parse(spec)
+	if err != nil || u.Host == "" {
+		u, err = url.Parse("http://" + spec)
+		if err != nil || u.Host == "" {
+			return nil
+		}
+	}
+	port := u.Port()
+	if port == "" {
+		switch u.Scheme {
+		case "https":
+			port = "443"
+		case "socks5", "socks5h":
+			port = "1080"
+		default:
+			port = "80"
+		}
+	}
+	out := map[string]struct{}{}
+	host := u.Hostname()
+	if ip := net.ParseIP(host); ip != nil {
+		out[net.JoinHostPort(ip.String(), port)] = struct{}{}
+		return out
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	ips, _ := net.DefaultResolver.LookupIPAddr(ctx, host)
+	for _, ip := range ips {
+		out[net.JoinHostPort(ip.IP.String(), port)] = struct{}{}
+	}
+	return out
+}
+
 // BoundedRedirects returns an http.Client.CheckRedirect that limits a
 // redirect chain to maxHops. SSRF on each redirect target is enforced
 // by the transport's dialer; this only prevents infinite loops.

pkg/httpclient/ssrf_test.go

@@ -215,3 +215,133 @@ func TestSSRFDialControl_IPv6ZoneID(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "not a valid IP")
 }
+
+// TestProxyHostPorts pins the parsing rules for HTTP_PROXY-style values.
+// We support full URLs and bare host[:port] (matching net/http's
+// ProxyFromEnvironment), and we always emit a port — dial addresses
+// have one and we compare against them verbatim.
+func TestProxyHostPorts(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		spec string
+		want []string
+	}{
+		{"empty", "", nil},
+		{"http URL with port", "http://172.17.0.0:3128", []string{"172.17.0.0:3128"}},
+		{"http URL without port", "http://10.0.0.1", []string{"10.0.0.1:80"}},
+		{"https URL without port", "https://10.0.0.1", []string{"10.0.0.1:443"}},
+		{"socks5 URL without port", "socks5://10.0.0.1", []string{"10.0.0.1:1080"}},
+		{"bare host:port", "172.17.0.0:3128", []string{"172.17.0.0:3128"}},
+		{"bare host", "172.17.0.0", []string{"172.17.0.0:80"}},
+		{"IPv6 with port", "http://[::1]:3128", []string{"[::1]:3128"}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := proxyHostPorts(tt.spec)
+			if tt.want == nil {
+				assert.Empty(t, got)
+				return
+			}
+			for _, w := range tt.want {
+				assert.Contains(t, got, w)
+			}
+		})
+	}
+}
+
+// TestNewSSRFSafeTransport_AllowsConfiguredProxy is the regression test
+// for the docker-agent sandbox bug: HTTP_PROXY=http://172.17.0.0:3128
+// must not be rejected by SSRFDialControl, because that's the only way
+// outbound traffic can reach the public internet from inside the
+// sandbox. We verify the dial is *attempted* (i.e. SSRF check passes)
+// by using a port we know is closed, then asserting the resulting
+// error is a connection error, not an SSRF rejection.
+func TestNewSSRFSafeTransport_AllowsConfiguredProxy(t *testing.T) {
+	// Bind a socket and immediately close it, then reuse the freed port:
+	// connections to it will fail with ECONNREFUSED, proving the dial
+	// passed the SSRF gate. We use 127.0.0.1 as a stand-in for any
+	// private-IP proxy — same code path.
+	var lc net.ListenConfig
+	ln, err := lc.Listen(t.Context(), "tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	addr := ln.Addr().String()
+	require.NoError(t, ln.Close())
+
+	t.Setenv("HTTP_PROXY", "http://"+addr)
+	t.Setenv("HTTPS_PROXY", "")
+	t.Setenv("http_proxy", "")
+	t.Setenv("https_proxy", "")
+
+	// Override the transport's Proxy directly so the test does not
+	// depend on http.ProxyFromEnvironment's sync.Once-cached env
+	// snapshot, which can be frozen by an earlier test in the same
+	// process before we get a chance to set HTTP_PROXY.
+	proxyURL, err := url.Parse("http://" + addr)
+	require.NoError(t, err)
+	transport := NewSSRFSafeTransport()
+	transport.Proxy = http.ProxyURL(proxyURL)
+
+	client := &http.Client{Transport: transport}
+	req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://example.com/", http.NoBody)
+	require.NoError(t, err)
+	resp, reqErr := client.Do(req)
+	if resp != nil {
+		_ = resp.Body.Close()
+	}
+	require.Error(t, reqErr)
+	assert.NotContains(t, reqErr.Error(), "non-public address",
+		"explicitly-configured proxy must bypass SSRF dial-control")
+	assert.NotContains(t, reqErr.Error(), "not a valid IP")
+}
+
+// TestCanonicalHostPort verifies that IPv4-mapped IPv6 dial addresses
+// (e.g. "[::ffff:10.0.0.1]:3128", which dual-stack Linux dialers may
+// emit even for IPv4-literal proxy URLs) collapse to the same key as
+// the dotted-quad form, so the allowlist match doesn't miss.
+func TestCanonicalHostPort(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		in, want string
+	}{
+		{"10.0.0.1:3128", "10.0.0.1:3128"},
+		{"[::ffff:10.0.0.1]:3128", "10.0.0.1:3128"},
+		{"[::ffff:0a00:0001]:3128", "10.0.0.1:3128"},
+		{"[2001:db8::1]:3128", "[2001:db8::1]:3128"},
+		{"example.com:3128", "example.com:3128"},
+		{"not a valid address", "not a valid address"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.in, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tt.want, canonicalHostPort(tt.in))
+		})
+	}
+}
+
+// TestNewSSRFSafeTransport_AllowlistFrozenAtConstruction documents that
+// the allowlist is captured when the transport is built, not on every
+// dial. Changing the env after construction has no effect — acceptable
+// because proxy env vars are set at process start.
+func TestNewSSRFSafeTransport_AllowlistFrozenAtConstruction(t *testing.T) {
+	t.Setenv("HTTP_PROXY", "")
+	t.Setenv("HTTPS_PROXY", "")
+	t.Setenv("http_proxy", "")
+	t.Setenv("https_proxy", "")
+
+	client := &http.Client{Transport: NewSSRFSafeTransport()}
+
+	t.Setenv("HTTP_PROXY", "http://10.0.0.1:3128")
+
+	req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://10.0.0.1:3128/", http.NoBody)
+	require.NoError(t, err)
+	resp, reqErr := client.Do(req)
+	if resp != nil {
+		_ = resp.Body.Close()
+	}
+	require.Error(t, reqErr)
+	assert.Contains(t, reqErr.Error(), "non-public address")
+}