VAULT-191: Validate X-Forwarded-For IP (#29774)

* validate the IPs in x-forwarded-for header

* switch to only checking one IP

* remove newline

* changelog

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 
+```

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()
+	})
 }

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