From 63d597c09d542f4344416b821dc742ddb5fda68f Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 21 Mar 2024 12:54:25 -0400 Subject: [PATCH] caddyhttp: Accept XFF header values with ports, when parsing client IP (#6183) --- modules/caddyhttp/server.go | 32 +++++++++++++++++++++++++------- modules/caddyhttp/server_test.go | 9 +++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 04ae003a..1ebfde61 100644 --- a/modules/caddyhttp/server.go +++ b/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(ips) - 1; i >= 0; i-- { - ip, _, _ := strings.Cut(strings.TrimSpace(ips[i]), "%") - ipAddr, err := netip.ParseAddr(ip) + 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), "%") + + // Parse the IP address + ipAddr, err := netip.ParseAddr(host) if err != nil { continue } diff --git a/modules/caddyhttp/server_test.go b/modules/caddyhttp/server_test.go index fd0e1e34..d382a696 100644 --- a/modules/caddyhttp/server_test.go +++ b/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"