Merge commit from fork

* feat: drop headers with underscore in their names

* feat: Caddyfile binding and tests for underscore-in-header drop

Add the `allow_underscore_in_headers` global server option, refine the
doc comment, and cover the filter end-to-end: server-level unit tests
(drop, opt-out, debug log, RFC-7230 space rejection), a fastcgi unit
test for the trimmed header name replacer, and forward_auth integration
tests for both the default-drop and opt-out paths.

* remove allow_underscore_in_headers option for now

caddytest/integration/forwardauth_test.go

@@ -22,6 +22,8 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/caddyserver/caddy/v2/caddytest"
 )
 
@@ -204,3 +206,66 @@ func TestForwardAuthCopyHeadersAuthResponseWins(t *testing.T) {
 		t.Errorf("X-User-Role: want %q, got %q", wantUserRole, gotRole)
 	}
 }
+
+// TestForwardAuthCopyHeadersUnderscoreAlias guards GHSA-f59h-q822-g45g:
+// a client-supplied `Remote_user` alias of the copy_headers target
+// `Remote-User` must be stripped before the auth route runs, otherwise
+// a downstream CGI/FastCGI backend would fold both names into the same
+// HTTP_REMOTE_USER variable and the attacker would override the trusted
+// identity.
+func TestForwardAuthCopyHeadersUnderscoreAlias(t *testing.T) {
+	const wantRemoteUser = "alice"
+
+	authSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Remote-User", wantRemoteUser)
+		w.WriteHeader(http.StatusOK)
+	}))
+	t.Cleanup(authSrv.Close)
+
+	type received struct {
+		remoteUserHyphen, remoteUserUnderscore string
+	}
+	var (
+		mu   sync.Mutex
+		last received
+	)
+	backendSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		mu.Lock()
+		last = received{
+			remoteUserHyphen:     r.Header.Get("Remote-User"),
+			remoteUserUnderscore: strings.Join(r.Header["Remote_user"], ","),
+		}
+		mu.Unlock()
+		fmt.Fprint(w, "ok")
+	}))
+	t.Cleanup(backendSrv.Close)
+
+	tester := caddytest.NewTester(t)
+	tester.InitServer(fmt.Sprintf(`
+	{
+		skip_install_trust
+		admin localhost:2999
+		http_port 9080
+		https_port 9443
+		grace_period 1ns
+	}
+	http://localhost:9080 {
+		forward_auth %s {
+			uri /
+			copy_headers Remote-User
+		}
+		reverse_proxy %s
+	}
+	`, strings.TrimPrefix(authSrv.URL, "http://"), strings.TrimPrefix(backendSrv.URL, "http://")), "caddyfile")
+
+	req, _ := http.NewRequest(http.MethodGet, "http://localhost:9080/", nil)
+	// Set the underscore alias via raw map access to bypass http.Header
+	// canonicalization, as an attacker would on the wire.
+	req.Header["Remote_user"] = []string{"attacker"}
+	tester.AssertResponse(req, http.StatusOK, "ok")
+
+	mu.Lock()
+	defer mu.Unlock()
+	assert.Equal(t, wantRemoteUser, last.remoteUserHyphen, "trusted Remote-User must reach the backend")
+	assert.Empty(t, last.remoteUserUnderscore, "underscore alias must be dropped")
+}

modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go

@@ -507,7 +507,7 @@ var tlsProtocolStrings = map[uint16]string{
 	tls.VersionTLS13: "TLSv1.3",
 }
 
-var headerNameReplacer = strings.NewReplacer(" ", "_", "-", "_")
+var headerNameReplacer = strings.NewReplacer("-", "_")
 
 // Interface guards
 var (

modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go

@@ -304,6 +304,30 @@ func TestSplitPosUnicodeSecurityRegression(t *testing.T) {
 	}
 }
 
+// TestHeaderNameReplacer asserts the CGI header-to-env normalization rule:
+// hyphens are mapped to underscores while every other character (including
+// spaces) is passed through. Spaces are not RFC 7230 tokens, so they cannot
+// reach this function from the wire; the only header names that survive
+// untouched at the server layer are sanitized by the underscore filter in
+// caddyhttp.Server.serveHTTP (see GHSA-f59h-q822-g45g).
+func TestHeaderNameReplacer(t *testing.T) {
+	tests := []struct {
+		in, want string
+	}{
+		{"X-Forwarded-For", "X_Forwarded_For"},
+		{"Remote-User", "Remote_User"},
+		// Underscores are preserved (the server has already dropped any
+		// underscore-named headers when the filter is on).
+		{"Remote_User", "Remote_User"},
+		// Spaces are not rewritten because Go's HTTP parser rejects whitespace in
+		// header field names.
+		{"Foo Bar", "Foo Bar"},
+	}
+	for _, tt := range tests {
+		assert.Equal(t, tt.want, headerNameReplacer.Replace(tt.in), "input %q", tt.in)
+	}
+}
+
 // TestSplitPosSecurityRegressionUnicodeBypass guards against the FrankenPHP
 // advisories GHSA-3g8v-8r37-cgjm (uninitialized match flag on inner non-ASCII
 // byte) and GHSA-v4h7-cj44-8fc8 (Unicode equivalence via search.IgnoreCase

modules/caddyhttp/server.go

@@ -494,6 +494,19 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) error {
 		}
 	}
 
+	// Drop headers whose names contain `_`: once FastCGI/CGI/FrankenPHP etc. rewrites `-` to
+	// `_`, an underscore alias collides with the legitimate hyphenated header
+	// and can bypass `forward_auth copy_headers` (GHSA-f59h-q822-g45g).
+	for k := range r.Header {
+		if strings.ContainsRune(k, '_') {
+			delete(r.Header, k)
+
+			if c := s.logger.Check(zapcore.DebugLevel, "dropping header containing underscore"); c != nil {
+				c.Write(zap.String("header", k))
+			}
+		}
+	}
+
 	// execute the primary handler chain
 	return s.primaryHandlerChain.ServeHTTP(w, r)
 }

modules/caddyhttp/server_test.go

@@ -1,16 +1,20 @@
 package caddyhttp
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 )
@@ -478,6 +482,79 @@ func TestServer_DetermineTrustedProxy_MatchRightMostUntrustedSkippingTrusted(t *
 	assert.Equal(t, clientIP, "45.54.45.54")
 }
 
+// TestServer_serveHTTP_DropsUnderscoreHeader covers GHSA-f59h-q822-g45g: an
+// underscore-named alias (e.g. `Remote_user`) of a hyphenated header must be
+// dropped before any handler runs.
+func TestServer_serveHTTP_DropsUnderscoreHeader(t *testing.T) {
+	got := &http.Header{}
+	s := &Server{
+		logger: zap.NewNop(),
+		primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+			*got = r.Header.Clone()
+			return nil
+		}),
+	}
+
+	req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
+	req.Header["X-Real-Header"] = []string{"ok"}
+	req.Header["Remote_user"] = []string{"attacker"}
+	req.Header["Remote_groups"] = []string{"admin"}
+
+	require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
+	assert.NotContains(t, *got, "Remote_user")
+	assert.NotContains(t, *got, "Remote_groups")
+	assert.Equal(t, "ok", got.Get("X-Real-Header"))
+}
+
+// TestServer_serveHTTP_LogsDroppedUnderscoreHeader verifies each dropped
+// header is emitted at debug level so operators can diagnose unexpectedly
+// missing headers without spamming the log on adversarial traffic.
+func TestServer_serveHTTP_LogsDroppedUnderscoreHeader(t *testing.T) {
+	var buf bytes.Buffer
+	s := &Server{
+		logger: testLogger(buf.Write),
+		primaryHandlerChain: HandlerFunc(func(http.ResponseWriter, *http.Request) error {
+			return nil
+		}),
+	}
+
+	req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
+	req.Header["Remote_user"] = []string{"attacker"}
+
+	require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
+	assert.Contains(t, buf.String(), `"level":"debug"`)
+	assert.Contains(t, buf.String(), `"msg":"dropping header containing underscore"`)
+	assert.Contains(t, buf.String(), `"header":"Remote_user"`)
+}
+
+// TestServer_SpaceInHeaderNameReturnsBadRequest documents why the underscore
+// filter does not also strip space-named headers: Go's HTTP parser rejects a
+// space in a field name with 400 before any handler runs, so such a request
+// can never reach Caddy's pipeline.
+func TestServer_SpaceInHeaderNameReturnsBadRequest(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Errorf("handler must not be reached; got headers %v", r.Header)
+	}))
+	t.Cleanup(srv.Close)
+
+	addr := strings.TrimPrefix(srv.URL, "http://")
+	conn, err := net.DialTimeout("tcp", addr, 5*time.Second)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = conn.Close() })
+	require.NoError(t, conn.SetDeadline(time.Now().Add(5*time.Second)))
+
+	_, err = conn.Write([]byte("GET / HTTP/1.1\r\n" +
+		"Host: " + addr + "\r\n" +
+		"Remote User: attacker\r\n" +
+		"Connection: close\r\n\r\n"))
+	require.NoError(t, err)
+
+	resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = resp.Body.Close() })
+	assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
+}
+
 func TestServer_DetermineTrustedProxy_MatchRightMostUntrustedFirst(t *testing.T) {
 	localPrivatePrefix, _ := netip.ParsePrefix("10.0.0.0/8")