diff --git a/safehttp/safehttp.go b/safehttp/safehttp.go index 78d9035..f0a3212 100644 --- a/safehttp/safehttp.go +++ b/safehttp/safehttp.go @@ -8,6 +8,14 @@ // directly, so a rebind between check and connect cannot escape // the gate. // +// When the transport sends a request through a forward proxy (for +// example from HTTP_PROXY or HTTPS_PROXY), the dial reaches the +// forward proxy, so the destination is checked before the request +// is handed over: it is resolved and every address must pass the +// same gate. A name that cannot be resolved locally is passed to +// the forward proxy, since some networks resolve public names only +// there. +// // 2. Redirect cap. Go's default ignores chain length and re-trusts // each hop. CheckRedirect caps at 10 and re-validates every // redirect target. diff --git a/safehttp/transport_std.go b/safehttp/transport_std.go index 2244ec6..fed2b59 100644 --- a/safehttp/transport_std.go +++ b/safehttp/transport_std.go @@ -7,8 +7,13 @@ import ( "fmt" "net" "net/http" + "net/url" ) +// lookupIPAddr resolves the destination of a request sent through a forward +// proxy. It is a variable so tests can resolve names without DNS. +var lookupIPAddr = net.DefaultResolver.LookupIPAddr + func newClient(base *http.Client, opts Options) *http.Client { c := http.Client{Timeout: defaultTimeout} if base != nil { @@ -33,6 +38,7 @@ func newClient(base *http.Client, opts Options) *http.Client { transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { return gate.dial(ctx, network, addr, underlying) } + transport.Proxy = guardProxy(transport.Proxy, gate) c.Transport = transport c.CheckRedirect = func(req *http.Request, via []*http.Request) error { @@ -43,3 +49,42 @@ func newClient(base *http.Client, opts Options) *http.Client { } return &c } + +// guardProxy wraps proxy so that a request sent through a forward proxy is +// held to the same address rules as a direct one. The dial gate only sees the +// address it connects to, which for a proxied request is the forward proxy. +// +// The destination is resolved here and every address must pass. A host that +// cannot be resolved locally is passed on, since networks that route all +// traffic through a forward proxy often resolve public names only there. +func guardProxy(proxy func(*http.Request) (*url.URL, error), gate *ipGate) func(*http.Request) (*url.URL, error) { + if proxy == nil { + return nil + } + return func(req *http.Request) (*url.URL, error) { + proxyURL, err := proxy(req) + if err != nil || proxyURL == nil { + return proxyURL, err + } + if err := gate.checkProxied(req.Context(), req.URL.Hostname()); err != nil { + return nil, err + } + return proxyURL, nil + } +} + +func (g *ipGate) checkProxied(ctx context.Context, host string) error { + if ip := net.ParseIP(host); ip != nil { + return g.checkHost(host, ip) + } + addrs, err := lookupIPAddr(ctx, host) + if err != nil { + return nil //nolint:nilerr // the forward proxy resolves names this host cannot + } + for _, addr := range addrs { + if err := g.checkHost(host, addr.IP); err != nil { + return err + } + } + return nil +} diff --git a/safehttp/transport_std_test.go b/safehttp/transport_std_test.go new file mode 100644 index 0000000..0e5eb5d --- /dev/null +++ b/safehttp/transport_std_test.go @@ -0,0 +1,118 @@ +//go:build !tinygo + +package safehttp + +import ( + "context" + "errors" + "net" + "net/http" + "net/http/httptest" + "net/url" + "sync/atomic" + "testing" +) + +// routeThroughTestProxy starts a forward proxy that answers every request +// itself, returns a base client whose transport sends requests to it, and +// resolves hostnames from hosts. +func routeThroughTestProxy(t *testing.T, hosts map[string]string) (*http.Client, *atomic.Int32) { + t.Helper() + var received atomic.Int32 + forward := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + received.Add(1) + _, _ = w.Write([]byte("via proxy")) + })) + t.Cleanup(forward.Close) + forwardURL, err := url.Parse(forward.URL) + if err != nil { + t.Fatal(err) + } + + oldLookup := lookupIPAddr + lookupIPAddr = func(_ context.Context, host string) ([]net.IPAddr, error) { + if ip, ok := hosts[host]; ok { + return []net.IPAddr{{IP: net.ParseIP(ip)}}, nil + } + return nil, &net.DNSError{Err: "no such host", Name: host, IsNotFound: true} + } + t.Cleanup(func() { lookupIPAddr = oldLookup }) + + base := &http.Client{Transport: &http.Transport{Proxy: http.ProxyURL(forwardURL)}} + return base, &received +} + +func TestClient_ForwardProxyChecksDestination(t *testing.T) { + base, received := routeThroughTestProxy(t, map[string]string{ + "metadata.example.test": "169.254.169.254", + "intranet.example.test": "10.1.2.3", + "allowed.example.test": "10.1.2.4", + "public.example.test": "203.0.113.10", + }) + // The forward proxy listens on loopback, so the client has to be allowed + // to dial it. + c := New(base, Options{ + AllowLoopback: true, + AllowPrivateHosts: []string{"allowed.example.test", "10.9.9.9"}, + }) + + for _, tt := range []struct { + url string + blocked bool + }{ + {"http://169.254.169.254/latest/meta-data/", true}, + {"https://169.254.169.254/latest/meta-data/", true}, + {"http://[fe80::1]/", true}, + {"http://metadata.example.test/", true}, + {"http://10.1.2.3/", true}, + {"http://intranet.example.test/", true}, + {"http://allowed.example.test/", false}, + {"http://10.9.9.9/", false}, + {"http://203.0.113.10/", false}, + {"http://public.example.test/", false}, + {"http://unresolvable.example.test/", false}, + } { + before := received.Load() + resp, err := c.Get(tt.url) + if err == nil { + _ = resp.Body.Close() + } + reached := received.Load() > before + if tt.blocked { + if err == nil || reached { + t.Errorf("GET %s: err = %v, reached forward proxy = %v; want refused before the forward proxy", tt.url, err, reached) + } + continue + } + if err != nil || !reached { + t.Errorf("GET %s: err = %v, reached forward proxy = %v; want it passed to the forward proxy", tt.url, err, reached) + } + } +} + +func TestClient_DefaultTransportProxyIsGuarded(t *testing.T) { + if http.DefaultTransport.(*http.Transport).Proxy == nil { + t.Skip("http.DefaultTransport has no proxy func") + } + c := New(nil, Options{}) + if c.Transport.(*http.Transport).Proxy == nil { + t.Fatal("proxy func was dropped") + } +} + +func TestGuardProxyKeepsDirectRequests(t *testing.T) { + gate := newGate(Options{}) + if guardProxy(nil, gate) != nil { + t.Error("a transport without a proxy func got one") + } + req := httptest.NewRequest(http.MethodGet, "http://169.254.169.254/", nil) + proxyErr := errors.New("bad proxy config") + guarded := guardProxy(func(*http.Request) (*url.URL, error) { return nil, proxyErr }, gate) + if _, err := guarded(req); !errors.Is(err, proxyErr) { + t.Errorf("err = %v, want the proxy func's own error", err) + } + direct := guardProxy(func(*http.Request) (*url.URL, error) { return nil, nil }, gate) + if proxyURL, err := direct(req); proxyURL != nil || err != nil { + t.Errorf("direct request: proxy = %v, err = %v; want nil, nil so the dial gate checks it", proxyURL, err) + } +}