aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorFrancis Lavoie <[email protected]>2024-03-21 12:54:25 -0400
committerGitHub <[email protected]>2024-03-21 10:54:25 -0600
commit63d597c09d542f4344416b821dc742ddb5fda68f (patch)
treeca72a310a2ea8de6495d5d5f87320c6974040208
parente65b97f55b6c5da8859fbd5e5b397cf089bf7716 (diff)
downloadcaddy-63d597c09d542f4344416b821dc742ddb5fda68f.tar.gz
caddy-63d597c09d542f4344416b821dc742ddb5fda68f.zip
caddyhttp: Accept XFF header values with ports, when parsing client IP (#6183)
-rw-r--r--modules/caddyhttp/server.go32
-rw-r--r--modules/caddyhttp/server_test.go9
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"