From 0ca4115b148dc98216e024696dcbe33abde13509 Mon Sep 17 00:00:00 2001 From: David Wobrock Date: Wed, 21 Aug 2024 18:21:54 +0200 Subject: [PATCH] fix: allow calling internal IP ranges with relevant option The `ResilientClient` options `ResilientClientDisallowInternalIPs` and `ResilientClientAllowInternalIPRequestsTo` were not allowing to call certain IP ranges, like 100.64.0.0/10 properly. --- httpx/resilient_client_test.go | 47 +++++++++++++++++++++++++++++++--- httpx/ssrf.go | 19 +++----------- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/httpx/resilient_client_test.go b/httpx/resilient_client_test.go index 8a90118b..34a0cace 100644 --- a/httpx/resilient_client_test.go +++ b/httpx/resilient_client_test.go @@ -13,6 +13,7 @@ import ( "net/url" "sync/atomic" "testing" + "time" "github.com/hashicorp/go-retryablehttp" "github.com/stretchr/testify/assert" @@ -33,11 +34,13 @@ func TestNoPrivateIPs(t *testing.T) { allowedURL := "http://localhost:" + port + "/foobar" allowedGlob := "http://localhost:" + port + "/glob/*" + allowedPrivateIP := "http://100.64.1.1:80" + "/private" c := NewResilientClient( - ResilientClientWithMaxRetry(1), + ResilientClientWithMaxRetry(0), + ResilientClientWithConnectionTimeout(10*time.Millisecond), ResilientClientDisallowInternalIPs(), - ResilientClientAllowInternalIPRequestsTo(allowedURL, allowedGlob), + ResilientClientAllowInternalIPRequestsTo(allowedURL, allowedGlob, allowedPrivateIP), ) for i := 0; i < 10; i++ { @@ -49,13 +52,51 @@ func TestNoPrivateIPs(t *testing.T) { "http://localhost:" + port + "/glob/bar": true, "http://localhost:" + port + "/glob/bar/baz": false, "http://localhost:" + port + "/FOOBAR": false, + allowedPrivateIP: true, + "http://100.64.8.8:" + port + "/route": false, } { _, err := c.Get(destination) if !passes { require.Errorf(t, err, "dest = %s", destination) assert.Containsf(t, err.Error(), "is not a permitted destination", "dest = %s", destination) + } else if err != nil { + assert.NotContainsf(t, err.Error(), "is not a permitted destination", "dest = %s", destination) + } + } + } +} + +func TestAllowPrivateIPs(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("Hello, world!")) + })) + t.Cleanup(ts.Close) + + target, err := url.ParseRequestURI(ts.URL) + require.NoError(t, err) + + _, port, err := net.SplitHostPort(target.Host) + require.NoError(t, err) + + c := NewResilientClient( + ResilientClientWithMaxRetry(0), + ResilientClientWithConnectionTimeout(10*time.Millisecond), + ) + + for i := 0; i < 10; i++ { + for destination, handled := range map[string]bool{ + "http://127.0.0.1:" + port: true, + "http://localhost:" + port: true, + "http://192.168.178.5:" + port: false, + "http://localhost:" + port + "/glob/bar": true, + "http://100.64.1.1:" + port + "/route": false, + } { + _, err = c.Get(destination) + if handled { + require.NoError(t, err) } else { - require.NoErrorf(t, err, "dest = %s", destination) + require.Error(t, err) + assert.NotContainsf(t, err.Error(), "is not a permitted destination", "dest = %s", destination) } } } diff --git a/httpx/ssrf.go b/httpx/ssrf.go index 99b16e9e..a52f6ab3 100644 --- a/httpx/ssrf.go +++ b/httpx/ssrf.go @@ -8,7 +8,6 @@ import ( "net" "net/http" "net/http/httptrace" - "net/netip" "time" "code.dny.dev/ssrf" @@ -88,15 +87,10 @@ func init() { ssrf.WithAnyPort(), ssrf.WithNetworks("tcp4", "tcp6"), ssrf.WithAllowedV4Prefixes( - netip.MustParsePrefix("10.0.0.0/8"), // Private-Use (RFC 1918) - netip.MustParsePrefix("127.0.0.0/8"), // Loopback (RFC 1122, Section 3.2.1.3)) - netip.MustParsePrefix("169.254.0.0/16"), // Link Local (RFC 3927) - netip.MustParsePrefix("172.16.0.0/12"), // Private-Use (RFC 1918) - netip.MustParsePrefix("192.168.0.0/16"), // Private-Use (RFC 1918) + ssrf.IPv4DeniedPrefixes..., ), ssrf.WithAllowedV6Prefixes( - netip.MustParsePrefix("::1/128"), // Loopback (RFC 4193) - netip.MustParsePrefix("fc00::/7"), // Unique Local (RFC 4193) + ssrf.IPv6DeniedPrefixes..., ), ).Safe allowInternalAllowIPv6 = otelTransport(t) @@ -108,15 +102,10 @@ func init() { ssrf.WithAnyPort(), ssrf.WithNetworks("tcp4"), ssrf.WithAllowedV4Prefixes( - netip.MustParsePrefix("10.0.0.0/8"), // Private-Use (RFC 1918) - netip.MustParsePrefix("127.0.0.0/8"), // Loopback (RFC 1122, Section 3.2.1.3)) - netip.MustParsePrefix("169.254.0.0/16"), // Link Local (RFC 3927) - netip.MustParsePrefix("172.16.0.0/12"), // Private-Use (RFC 1918) - netip.MustParsePrefix("192.168.0.0/16"), // Private-Use (RFC 1918) + ssrf.IPv4DeniedPrefixes..., ), ssrf.WithAllowedV6Prefixes( - netip.MustParsePrefix("::1/128"), // Loopback (RFC 4193) - netip.MustParsePrefix("fc00::/7"), // Unique Local (RFC 4193) + ssrf.IPv6DeniedPrefixes..., ), ).Safe t.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {