From 8ce99b3a55abf2372d6ad98df83196d44991f183 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 21 May 2026 18:46:47 +0200 Subject: [PATCH] fix: allow configured HTTP proxy on private IPs in SSRF transport --- pkg/httpclient/ssrf.go | 100 ++++++++++++++++++++++++++- pkg/httpclient/ssrf_test.go | 130 ++++++++++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+), 1 deletion(-) diff --git a/pkg/httpclient/ssrf.go b/pkg/httpclient/ssrf.go index 3fed5289d..a1b0d109b 100644 --- a/pkg/httpclient/ssrf.go +++ b/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. diff --git a/pkg/httpclient/ssrf_test.go b/pkg/httpclient/ssrf_test.go index 958a3c24d..0bb56dc92 100644 --- a/pkg/httpclient/ssrf_test.go +++ b/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") +}