Sanitize claim-derived identifier on auth-bypass headers (round 48)

middleware.go

@@ -99,11 +99,23 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) b
 		return false
 	}
 
-	req.Header.Set("X-Forwarded-User", userIdentifier)
+	// Sanitize the claim-derived identifier before it is injected as a
-	if !t.minimalHeaders {
+	// header, matching forwardAuthorized (which uses safeIdentifier). The
-		req.Header.Set("X-Auth-Request-User", userIdentifier)
+	// value originates from an IdP-controlled claim and can contain CRLF
+	// or delimiter characters that would otherwise inject or confuse
+	// downstream header parsing. On failure, drop the header but still
+	// honor the bypass (the identity headers are decoration here).
+	safeIdentifier, ok := sanitizeHeaderClaimValue(userIdentifier, t.headerClaimMaxLen())
+	if !ok {
+		t.logger.Debugf("%s bypass: dropping unsafe user-identifier header: %s", reason, headerClaimValueReason(userIdentifier, t.headerClaimMaxLen()))
+		return true
 	}
-	t.logger.Debugf("%s bypass: forwarded user %s from session", reason, userIdentifier)
+
+	req.Header.Set("X-Forwarded-User", safeIdentifier)
+	if !t.minimalHeaders {
+		req.Header.Set("X-Auth-Request-User", safeIdentifier)
+	}
+	t.logger.Debugf("%s bypass: forwarded user %s from session", reason, safeIdentifier)
 	return true
 }
 

review_r48_regression_test.go

@@ -0,0 +1,92 @@
+package traefikoidc
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// TestApplyBypassUserHeaders_SanitizesIdentifier is a regression test for the
+// auth-bypass header path (applyBypassUserHeaders). The claim-derived user
+// identifier is injected into X-Forwarded-User / X-Auth-Request-User without
+// sanitization, unlike forwardAuthorized which uses sanitizeHeaderClaimValue.
+// An IdP-controlled identifier containing CRLF (or a delimiter) would
+// otherwise inject or confuse downstream header parsing. Now the bypass path
+// sanitizes the identifier and drops the header on failure.
+func TestApplyBypassUserHeaders_SanitizesIdentifier(t *testing.T) {
+	sm := createTestSessionManager(t)
+
+	base := httptest.NewRequest(http.MethodGet, "/protected", nil)
+	baseRec := httptest.NewRecorder()
+
+	// Build an authenticated session whose user identifier contains CRLF.
+	session, err := sm.GetSession(base)
+	if err != nil {
+		t.Fatalf("GetSession: %v", err)
+	}
+	if err := session.SetAuthenticated(true); err != nil {
+		t.Fatalf("SetAuthenticated: %v", err)
+	}
+	session.SetUserIdentifier("user\nX-Injected: yes")
+	if err := session.Save(base, baseRec); err != nil {
+		t.Fatalf("Save: %v", err)
+	}
+
+	req := httptest.NewRequest(http.MethodGet, "/protected", nil)
+	for _, c := range baseRec.Result().Cookies() {
+		req.AddCookie(c)
+	}
+
+	oidc := &TraefikOidc{
+		sessionManager: sm,
+		logger:         NewLogger("debug"),
+	}
+
+	if ok := oidc.applyBypassUserHeaders(req, "test"); !ok {
+		t.Fatalf("applyBypassUserHeaders returned false; bypass should be honored")
+	}
+	if v := req.Header.Get("X-Forwarded-User"); v != "" {
+		t.Fatalf("X-Forwarded-User = %q, want empty (unsafe identifier dropped)", v)
+	}
+	if v := req.Header.Get("X-Auth-Request-User"); v != "" {
+		t.Fatalf("X-Auth-Request-User = %q, want empty (unsafe identifier dropped)", v)
+	}
+}
+
+// TestApplyBypassUserHeaders_KeepsCleanIdentifier ensures a normal identifier
+// still flows through, and is trimmed/sanitized.
+func TestApplyBypassUserHeaders_KeepsCleanIdentifier(t *testing.T) {
+	sm := createTestSessionManager(t)
+
+	base := httptest.NewRequest(http.MethodGet, "/protected", nil)
+	baseRec := httptest.NewRecorder()
+
+	session, err := sm.GetSession(base)
+	if err != nil {
+		t.Fatalf("GetSession: %v", err)
+	}
+	if err := session.SetAuthenticated(true); err != nil {
+		t.Fatalf("SetAuthenticated: %v", err)
+	}
+	session.SetUserIdentifier("  [email protected]  ")
+	if err := session.Save(base, baseRec); err != nil {
+		t.Fatalf("Save: %v", err)
+	}
+
+	req := httptest.NewRequest(http.MethodGet, "/protected", nil)
+	for _, c := range baseRec.Result().Cookies() {
+		req.AddCookie(c)
+	}
+
+	oidc := &TraefikOidc{
+		sessionManager: sm,
+		logger:         NewLogger("debug"),
+	}
+
+	if ok := oidc.applyBypassUserHeaders(req, "test"); !ok {
+		t.Fatalf("applyBypassUserHeaders returned false; bypass should be honored")
+	}
+	if v := req.Header.Get("X-Forwarded-User"); v != "[email protected]" {
+		t.Fatalf("X-Forwarded-User = %q, want %q (trimmed)", v, "[email protected]")
+	}
+}