From f949dbf9dc5e0ac81d820c1fca25b7a03ea95a38 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 100.64.0.0/10 IP range with relevant option The `ResilientClient` options `ResilientClientDisallowInternalIPs` and `ResilientClientAllowInternalIPRequestsTo` were not allowing to call the IP range, like 100.64.0.0/10, properly. Some IP ranges are still not possible to bypass. --- httpx/resilient_client_test.go | 157 ++++++++++++++++++++++++--------- httpx/ssrf.go | 2 + 2 files changed, 118 insertions(+), 41 deletions(-) diff --git a/httpx/resilient_client_test.go b/httpx/resilient_client_test.go index 8a90118b..ff763383 100644 --- a/httpx/resilient_client_test.go +++ b/httpx/resilient_client_test.go @@ -5,59 +5,134 @@ package httpx import ( "context" - "net" + "fmt" "net/http" - "net/http/httptest" "net/http/httptrace" "net/netip" - "net/url" "sync/atomic" "testing" + "time" + + "code.dny.dev/ssrf" "github.com/hashicorp/go-retryablehttp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestNoPrivateIPs(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) - - allowedURL := "http://localhost:" + port + "/foobar" - allowedGlob := "http://localhost:" + port + "/glob/*" - - c := NewResilientClient( - ResilientClientWithMaxRetry(1), - ResilientClientDisallowInternalIPs(), - ResilientClientAllowInternalIPRequestsTo(allowedURL, allowedGlob), - ) +func TestPrivateIPs(t *testing.T) { + testCases := []struct { + url string + disallowInternalIPs bool + allowedIP bool + }{ + { + url: "http://127.0.0.1/foobar", + disallowInternalIPs: true, + allowedIP: false, + }, + { + url: "http://localhost/foobar", + disallowInternalIPs: true, + allowedIP: false, + }, + { + url: "http://127.0.0.1:56789/test", + disallowInternalIPs: true, + allowedIP: false, + }, + { + url: "http://192.168.178.5:56789", + disallowInternalIPs: true, + allowedIP: false, + }, + { + url: "http://127.0.0.1:56789/foobar", + disallowInternalIPs: true, + allowedIP: true, + }, + { + url: "http://127.0.0.1:56789/glob/bar", + disallowInternalIPs: true, + allowedIP: true, + }, + { + url: "http://127.0.0.1:56789/glob/bar/baz", + disallowInternalIPs: true, + allowedIP: false, + }, + { + url: "http://127.0.0.1:56789/FOOBAR", + disallowInternalIPs: true, + allowedIP: false, + }, + { + url: "http://100.64.1.1:80/private", + disallowInternalIPs: true, + allowedIP: true, + }, + { + url: "http://100.64.1.1:80/route", + disallowInternalIPs: true, + allowedIP: false, + }, + { + url: "http://198.18.99.99/forbidden", + disallowInternalIPs: true, + allowedIP: false, + }, + { + // Even if in the allowed requests, no exceptions can be made. + url: "http://198.18.99.99/allowed", + disallowInternalIPs: true, + allowedIP: false, + }, + { + url: "http://127.0.0.1", + disallowInternalIPs: false, + allowedIP: true, + }, + { + url: "http://192.168.178.5", + disallowInternalIPs: false, + allowedIP: true, + }, + { + url: "http://127.0.0.1:80/glob/bar", + disallowInternalIPs: false, + allowedIP: true, + }, + { + url: "http://100.64.1.1:80/route", + disallowInternalIPs: false, + allowedIP: true, + }, + } + for _, tt := range testCases { + t.Run( + fmt.Sprintf("%s should be allowed %v when disallowed internal IPs is %v", tt.url, tt.allowedIP, tt.disallowInternalIPs), + func(t *testing.T) { + options := []ResilientOptions{ + ResilientClientWithMaxRetry(0), + ResilientClientWithConnectionTimeout(50 * time.Millisecond), + } + if tt.disallowInternalIPs { + options = append(options, ResilientClientDisallowInternalIPs()) + options = append(options, ResilientClientAllowInternalIPRequestsTo( + "http://127.0.0.1:56789/foobar", + "http://127.0.0.1:56789/glob/*", + "http://100.64.1.1:80/private", + "http://198.18.99.99/allowed")) + } - for i := 0; i < 10; i++ { - for destination, passes := range map[string]bool{ - "http://127.0.0.1:" + port: false, - "http://localhost:" + port: false, - "http://192.168.178.5:" + port: false, - allowedURL: true, - "http://localhost:" + port + "/glob/bar": true, - "http://localhost:" + port + "/glob/bar/baz": false, - "http://localhost:" + port + "/FOOBAR": 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 { - require.NoErrorf(t, err, "dest = %s", destination) - } - } + c := NewResilientClient(options...) + _, err := c.Get(tt.url) + if tt.allowedIP { + assert.NotErrorIs(t, err, ssrf.ErrProhibitedIP) + } else { + assert.ErrorIs(t, err, ssrf.ErrProhibitedIP) + } + }) } } diff --git a/httpx/ssrf.go b/httpx/ssrf.go index 99b16e9e..25700e13 100644 --- a/httpx/ssrf.go +++ b/httpx/ssrf.go @@ -89,6 +89,7 @@ func init() { ssrf.WithNetworks("tcp4", "tcp6"), ssrf.WithAllowedV4Prefixes( netip.MustParsePrefix("10.0.0.0/8"), // Private-Use (RFC 1918) + netip.MustParsePrefix("100.64.0.0/10"), // Shared Address Space (RFC 6598) 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) @@ -109,6 +110,7 @@ func init() { ssrf.WithNetworks("tcp4"), ssrf.WithAllowedV4Prefixes( netip.MustParsePrefix("10.0.0.0/8"), // Private-Use (RFC 1918) + netip.MustParsePrefix("100.64.0.0/10"), // Shared Address Space (RFC 6598) 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)