diff --git a/changelog/29774.txt b/changelog/29774.txt new file mode 100644 index 0000000000..0e0b2ad689 --- /dev/null +++ b/changelog/29774.txt @@ -0,0 +1,3 @@ +```release-note:change +core: Verify that the client IP address extracted from an X-Forwarded-For header is a valid IPv4 or IPv6 address +``` \ No newline at end of file diff --git a/http/forwarded_for_test.go b/http/forwarded_for_test.go index ce6a514470..00c9555107 100644 --- a/http/forwarded_for_test.go +++ b/http/forwarded_for_test.go @@ -14,6 +14,7 @@ import ( sockaddr "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/vault" + "github.com/stretchr/testify/require" ) func getListenerConfigForMarshalerTest(addr sockaddr.IPAddr) *configutil.Listener { @@ -333,4 +334,60 @@ func TestHandler_XForwardedFor(t *testing.T) { t.Fatalf("bad body: %v vs %v", buf.String(), testcertificate) } }) + t.Run("reject invalid IP", func(t *testing.T) { + t.Parallel() + testHandler := func(props *vault.HandlerProperties) http.Handler { + origHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(r.RemoteAddr)) + }) + listenerConfig := getListenerConfigForMarshalerTest(goodAddr) + listenerConfig.XForwardedForRejectNotPresent = true + listenerConfig.XForwardedForRejectNotAuthorized = true + return WrapForwardedForHandler(origHandler, listenerConfig) + } + + cluster := vault.NewTestCluster(t, nil, &vault.TestClusterOptions{ + HandlerFunc: HandlerFunc(testHandler), + }) + cluster.Start() + defer cluster.Cleanup() + client := cluster.Cores[0].Client + + req := client.NewRequest("GET", "/") + req.Headers = make(http.Header) + req.Headers.Set("x-forwarded-for", "5.6.7.8, 4.5.6.7, 8.8.8.8") + req.Headers.Set("x-forwarded-for", "10.9.8.7, 1.2.3.4, 256.1.1.1") + _, err := client.RawRequest(req) + require.ErrorContains(t, err, "malformed x-forwarded-for IP address") + }) + + t.Run("ignore invalid proxy IP", func(t *testing.T) { + t.Parallel() + testHandler := func(props *vault.HandlerProperties) http.Handler { + origHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(r.RemoteAddr)) + }) + listenerConfig := getListenerConfigForMarshalerTest(goodAddr) + listenerConfig.XForwardedForRejectNotPresent = true + listenerConfig.XForwardedForRejectNotAuthorized = true + return WrapForwardedForHandler(origHandler, listenerConfig) + } + + cluster := vault.NewTestCluster(t, nil, &vault.TestClusterOptions{ + HandlerFunc: HandlerFunc(testHandler), + }) + cluster.Start() + defer cluster.Cleanup() + client := cluster.Cores[0].Client + + req := client.NewRequest("GET", "/") + req.Headers = make(http.Header) + req.Headers.Set("x-forwarded-for", "5.6.7.8, 4.5.6.7, 8.8.8.8") + req.Headers.Set("x-forwarded-for", "10.9.8.7, 256.1.1.1, 1.2.3.4") + resp, err := client.RawRequest(req) + require.NoError(t, err) + resp.Body.Close() + }) } diff --git a/http/handler.go b/http/handler.go index 4be68dbd08..628ca3025b 100644 --- a/http/handler.go +++ b/http/handler.go @@ -598,7 +598,13 @@ func WrapForwardedForHandler(h http.Handler, l *configutil.Listener) http.Handle return } - r.RemoteAddr = net.JoinHostPort(acc[indexToUse], port) + // check that the chosen address is a valid IP address + remoteAddr := acc[indexToUse] + if _, err := sockaddr.NewIPAddr(remoteAddr); err != nil { + respondError(w, http.StatusBadRequest, fmt.Errorf("malformed x-forwarded-for IP address %s", remoteAddr)) + return + } + r.RemoteAddr = net.JoinHostPort(remoteAddr, port) // Import the Client Certificate forwarded by the reverse proxy // There should be only 1 instance of the header, but looping allows for more flexibility