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 <amp@ampcode.com>

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")
+	}
+}

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,