caddyhttp: Accept XFF header values with ports, when parsing client IP (#6183)

Author: francislavoie

modules/caddyhttp/server.go

@@ -902,9 +902,18 @@ func trustedRealClientIP(r *http.Request, headers []string, clientIP string) str
 	allValues := strings.Split(strings.Join(values, ","), ",")
 
 	// Get first valid left-most IP address
-	for _, ip := range allValues {
-		ip, _, _ = strings.Cut(strings.TrimSpace(ip), "%")
-		ipAddr, err := netip.ParseAddr(ip)
+	for _, part := range allValues {
+		// Some proxies may retain the port number, so split if possible
+		host, _, err := net.SplitHostPort(part)
+		if err != nil {
+			host = part
+		}
+
+		// Remove any zone identifier from the IP address
+		host, _, _ = strings.Cut(strings.TrimSpace(host), "%")
+
+		// Parse the IP address
+		ipAddr, err := netip.ParseAddr(host)
 		if err != nil {
 			continue
 		}
@@ -921,11 +930,20 @@ func trustedRealClientIP(r *http.Request, headers []string, clientIP string) str
 // remote address is returned.
 func strictUntrustedClientIp(r *http.Request, headers []string, trusted []netip.Prefix, clientIP string) string {
 	for _, headerName := range headers {
-		ips := strings.Split(strings.Join(r.Header.Values(headerName), ","), ",")
+		parts := strings.Split(strings.Join(r.Header.Values(headerName), ","), ",")
+
+		for i := len(parts) - 1; i >= 0; i-- {
+			// Some proxies may retain the port number, so split if possible
+			host, _, err := net.SplitHostPort(parts[i])
+			if err != nil {
+				host = parts[i]
+			}
+
+			// Remove any zone identifier from the IP address
+			host, _, _ = strings.Cut(strings.TrimSpace(host), "%")
 
-		for i := len(ips) - 1; i >= 0; i-- {
-			ip, _, _ := strings.Cut(strings.TrimSpace(ips[i]), "%")
-			ipAddr, err := netip.ParseAddr(ip)
+			// Parse the IP address
+			ipAddr, err := netip.ParseAddr(host)
 			if err != nil {
 				continue
 			}

modules/caddyhttp/server_test.go

@@ -188,6 +188,15 @@ func TestServer_TrustedRealClientIP_OneTrustedHeaderValidArray(t *testing.T) {
 	assert.Equal(t, ip, "1.1.1.1")
 }
 
+func TestServer_TrustedRealClientIP_IncludesPort(t *testing.T) {
+	req := httptest.NewRequest("GET", "/", nil)
+	req.RemoteAddr = "192.0.2.1:12345"
+	req.Header.Set("X-Forwarded-For", "1.1.1.1:1234")
+	ip := trustedRealClientIP(req, []string{"X-Forwarded-For"}, "192.0.2.1")
+
+	assert.Equal(t, ip, "1.1.1.1")
+}
+
 func TestServer_TrustedRealClientIP_SkipsInvalidIps(t *testing.T) {
 	req := httptest.NewRequest("GET", "/", nil)
 	req.RemoteAddr = "192.0.2.1:12345"