diff options
author | Francis Lavoie <[email protected]> | 2024-03-21 12:54:25 -0400 |
---|---|---|
committer | GitHub <[email protected]> | 2024-03-21 10:54:25 -0600 |
commit | 63d597c09d542f4344416b821dc742ddb5fda68f (patch) | |
tree | ca72a310a2ea8de6495d5d5f87320c6974040208 | |
parent | e65b97f55b6c5da8859fbd5e5b397cf089bf7716 (diff) | |
download | caddy-63d597c09d542f4344416b821dc742ddb5fda68f.tar.gz caddy-63d597c09d542f4344416b821dc742ddb5fda68f.zip |
caddyhttp: Accept XFF header values with ports, when parsing client IP (#6183)
-rw-r--r-- | modules/caddyhttp/server.go | 32 | ||||
-rw-r--r-- | 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 04ae003a7..1ebfde61f 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(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 } diff --git a/modules/caddyhttp/server_test.go b/modules/caddyhttp/server_test.go index fd0e1e349..d382a6963 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" |