From 03e6e439dd07d048323cc39516146e0f04032186 Mon Sep 17 00:00:00 2001 From: XYenon Date: Wed, 11 Feb 2026 04:00:20 +0800 Subject: [PATCH] reverseproxy: fix X-Forwarded-* headers for Unix socket requests (#7463) When a request arrives via a Unix domain socket (RemoteAddr == "@"), net.SplitHostPort fails, causing addForwardedHeaders to strip all X-Forwarded-* headers even when the connection is trusted via trusted_proxies_unix. Handle Unix socket connections before parsing RemoteAddr: if untrusted, strip headers for security; if trusted, let clientIP remain empty (no peer IP for a Unix socket hop) and fall through to the shared header logic, preserving the existing XFF chain without appending a spurious entry. Amp-Thread-ID: https://ampcode.com/threads/T-019c4225-a0ad-7283-ac56-e2c01eae1103 Co-authored-by: Amp --- .../caddyhttp/reverseproxy/headers_test.go | 93 +++++++++++++++++++ .../caddyhttp/reverseproxy/reverseproxy.go | 86 ++++++++++------- 2 files changed, 146 insertions(+), 33 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/headers_test.go b/modules/caddyhttp/reverseproxy/headers_test.go index 22f589141..9385468f6 100644 --- a/modules/caddyhttp/reverseproxy/headers_test.go +++ b/modules/caddyhttp/reverseproxy/headers_test.go @@ -32,3 +32,96 @@ func TestAddForwardedHeadersNonIP(t *testing.T) { t.Errorf("expected no error for non-IP address, got: %v", err) } } + +func TestAddForwardedHeaders_UnixSocketTrusted(t *testing.T) { + h := Handler{} + + req := httptest.NewRequest("GET", "http://example.com/", nil) + req.RemoteAddr = "@" + req.Header.Set("X-Forwarded-For", "1.2.3.4, 10.0.0.1") + req.Header.Set("X-Forwarded-Proto", "https") + req.Header.Set("X-Forwarded-Host", "original.example.com") + + vars := map[string]interface{}{ + caddyhttp.TrustedProxyVarKey: true, + caddyhttp.ClientIPVarKey: "1.2.3.4", + } + ctx := context.WithValue(req.Context(), caddyhttp.VarsCtxKey, vars) + req = req.WithContext(ctx) + + err := h.addForwardedHeaders(req) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + + if got := req.Header.Get("X-Forwarded-For"); got != "1.2.3.4, 10.0.0.1" { + t.Errorf("X-Forwarded-For = %q, want %q", got, "1.2.3.4, 10.0.0.1") + } + if got := req.Header.Get("X-Forwarded-Proto"); got != "https" { + t.Errorf("X-Forwarded-Proto = %q, want %q", got, "https") + } + if got := req.Header.Get("X-Forwarded-Host"); got != "original.example.com" { + t.Errorf("X-Forwarded-Host = %q, want %q", got, "original.example.com") + } +} + +func TestAddForwardedHeaders_UnixSocketUntrusted(t *testing.T) { + h := Handler{} + + req := httptest.NewRequest("GET", "http://example.com/", nil) + req.RemoteAddr = "@" + req.Header.Set("X-Forwarded-For", "1.2.3.4") + req.Header.Set("X-Forwarded-Proto", "https") + req.Header.Set("X-Forwarded-Host", "spoofed.example.com") + + vars := map[string]interface{}{ + caddyhttp.TrustedProxyVarKey: false, + caddyhttp.ClientIPVarKey: "", + } + ctx := context.WithValue(req.Context(), caddyhttp.VarsCtxKey, vars) + req = req.WithContext(ctx) + + err := h.addForwardedHeaders(req) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + + if got := req.Header.Get("X-Forwarded-For"); got != "" { + t.Errorf("X-Forwarded-For should be deleted, got %q", got) + } + if got := req.Header.Get("X-Forwarded-Proto"); got != "" { + t.Errorf("X-Forwarded-Proto should be deleted, got %q", got) + } + if got := req.Header.Get("X-Forwarded-Host"); got != "" { + t.Errorf("X-Forwarded-Host should be deleted, got %q", got) + } +} + +func TestAddForwardedHeaders_UnixSocketTrustedNoExistingHeaders(t *testing.T) { + h := Handler{} + + req := httptest.NewRequest("GET", "http://example.com/", nil) + req.RemoteAddr = "@" + + vars := map[string]interface{}{ + caddyhttp.TrustedProxyVarKey: true, + caddyhttp.ClientIPVarKey: "5.6.7.8", + } + ctx := context.WithValue(req.Context(), caddyhttp.VarsCtxKey, vars) + req = req.WithContext(ctx) + + err := h.addForwardedHeaders(req) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + + if got := req.Header.Get("X-Forwarded-For"); got != "" { + t.Errorf("X-Forwarded-For should be empty when no prior XFF exists, got %q", got) + } + if got := req.Header.Get("X-Forwarded-Proto"); got != "http" { + t.Errorf("X-Forwarded-Proto = %q, want %q", got, "http") + } + if got := req.Header.Get("X-Forwarded-Host"); got != "example.com" { + t.Errorf("X-Forwarded-Host = %q, want %q", got, "example.com") + } +} diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 6f6a0f9f2..f9fdd164e 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -801,37 +801,53 @@ func (h Handler) prepareRequest(req *http.Request, repl *caddy.Replacer) (*http. // the headers at all, then they will be added with the values // that we can glean from the request. func (h Handler) addForwardedHeaders(req *http.Request) error { - // Parse the remote IP, ignore the error as non-fatal, - // but the remote IP is required to continue, so we - // just return early. This should probably never happen - // though, unless some other module manipulated the request's - // remote address and used an invalid value. - clientIP, _, err := net.SplitHostPort(req.RemoteAddr) - if err != nil { - // Remove the `X-Forwarded-*` headers to avoid upstreams - // potentially trusting a header that came from the client - req.Header.Del("X-Forwarded-For") - req.Header.Del("X-Forwarded-Proto") - req.Header.Del("X-Forwarded-Host") - return nil - } - - // Client IP may contain a zone if IPv6, so we need - // to pull that out before parsing the IP - clientIP, _, _ = strings.Cut(clientIP, "%") - ipAddr, err := netip.ParseAddr(clientIP) - // Check if the client is a trusted proxy trusted := caddyhttp.GetVar(req.Context(), caddyhttp.TrustedProxyVarKey).(bool) - // If ParseAddr fails (e.g. non-IP network like SCION), we cannot check - // if it is a trusted proxy by IP range. In this case, we ignore the - // error and treat the connection as untrusted (or retain existing status). - if err == nil { - for _, ipRange := range h.trustedProxies { - if ipRange.Contains(ipAddr) { - trusted = true - break + var clientIP string + + if req.RemoteAddr == "@" { + // For Unix socket connections, RemoteAddr is "@" which cannot + // be parsed as host:port. If untrusted, strip forwarded headers + // for security. If trusted, there is no peer IP to append to + // X-Forwarded-For, so clientIP stays empty. + if !trusted { + req.Header.Del("X-Forwarded-For") + req.Header.Del("X-Forwarded-Proto") + req.Header.Del("X-Forwarded-Host") + return nil + } + } else { + // Parse the remote IP, ignore the error as non-fatal, + // but the remote IP is required to continue, so we + // just return early. This should probably never happen + // though, unless some other module manipulated the request's + // remote address and used an invalid value. + var err error + clientIP, _, err = net.SplitHostPort(req.RemoteAddr) + if err != nil { + // Remove the `X-Forwarded-*` headers to avoid upstreams + // potentially trusting a header that came from the client + req.Header.Del("X-Forwarded-For") + req.Header.Del("X-Forwarded-Proto") + req.Header.Del("X-Forwarded-Host") + return nil + } + + // Client IP may contain a zone if IPv6, so we need + // to pull that out before parsing the IP + clientIP, _, _ = strings.Cut(clientIP, "%") + ipAddr, err := netip.ParseAddr(clientIP) + + // If ParseAddr fails (e.g. non-IP network like SCION), we cannot check + // if it is a trusted proxy by IP range. In this case, we ignore the + // error and treat the connection as untrusted (or retain existing status). + if err == nil { + for _, ipRange := range h.trustedProxies { + if ipRange.Contains(ipAddr) { + trusted = true + break + } } } } @@ -839,13 +855,17 @@ func (h Handler) addForwardedHeaders(req *http.Request) error { // If we aren't the first proxy, and the proxy is trusted, // retain prior X-Forwarded-For information as a comma+space // separated list and fold multiple headers into one. - clientXFF := clientIP prior, ok, omit := allHeaderValues(req.Header, "X-Forwarded-For") - if trusted && ok && prior != "" { - clientXFF = prior + ", " + clientXFF - } if !omit { - req.Header.Set("X-Forwarded-For", clientXFF) + if trusted && ok && prior != "" { + if clientIP != "" { + req.Header.Set("X-Forwarded-For", prior+", "+clientIP) + } else { + req.Header.Set("X-Forwarded-For", prior) + } + } else if clientIP != "" { + req.Header.Set("X-Forwarded-For", clientIP) + } } // Set X-Forwarded-Proto; many backend apps expect this,