Skip to content

forward_auth: copy_headers does not strip client-supplied identity headers (Fixes GHSA-7r4p-vjf4-gxv4)#7545

Merged
francislavoie merged 1 commit intocaddyserver:masterfrom
NucleiAv:fix/forward-auth-copy-headers-injection
Mar 4, 2026
Merged

forward_auth: copy_headers does not strip client-supplied identity headers (Fixes GHSA-7r4p-vjf4-gxv4)#7545
francislavoie merged 1 commit intocaddyserver:masterfrom
NucleiAv:fix/forward-auth-copy-headers-injection

Conversation

@NucleiAv
Copy link
Contributor

@NucleiAv NucleiAv commented Mar 4, 2026

What is the problem?

The forward_auth directive with copy_headers does not remove client-supplied headers before forwarding the request to the backend.

An attacker with any valid authentication token can send a request that includes forged identity headers like X-User-Id: admin or X-User-Role: superadmin. If the auth service validates the token and returns 200 OK without including those headers in its response, Caddy skips the Set operation (the MatchNot guard added by PR #6608 fires) and never removes the original client-supplied values. The backend receives the attacker's values and may grant elevated access.

This is a regression introduced by PR #6608 (November 2024). All stable releases from v2.10.0 onward are affected.

Common patterns that trigger this:

  • Stateless JWT validators that verify the signature but do not return
    claims as response headers (the backend decodes the JWT itself)
  • Auth services that only return identity headers for some requests
  • Auth middleware like Authelia configured with bypass rules

What does this change?

For each entry in copy_headers, a new unconditional delete route is added immediately before the existing conditional set route. The delete runs always. The set still only runs when the auth service includes that header in its response.

Result:

  • Client-supplied headers are always removed
  • When the auth service returns the header, the backend gets that value
  • When the auth service does not return the header, the backend gets nothing

Existing behavior is unchanged for deployments where the auth service returns all configured copy_headers entries.

Changes

  • modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go: core fix
  • Updated forward_auth_authelia.caddyfiletest and forward_auth_rename_headers.caddyfiletest to reflect new delete routes
    in expected JSON output
  • Added forward_auth_copy_headers_strip.caddyfiletest for the security scenario
  • Added forwardauth_test.go with two integration tests: one that confirms injected headers are stripped when the auth service does not return them, and one that confirms the auth service value wins when it does return them

Fact-Check Report

1. Core Fix — caddyfile.go

  • to is the correct variable: to = http.CanonicalHeaderKey(headersToCopy[from]) is the destination header name. For A>1, to = "1". Clients inject the destination header (e.g., 1: admin), so we must delete to.
  • Unconditional delete is correct: The delete route has no MatcherSetsRawomitempty means no "match" key → route always executes.
  • Route order: The loop appends [delete, conditional-set] per header. Delete fires first (removes any client-supplied value), then set fires only if the auth service returned that header. When auth does not return it: delete removed the forge, set is skipped → backend sees absent header.
  • Scoped to 2xx only: All routes are inside goodResponseHandler which has StatusCode: []int{2}. The reverseproxy code (reverseproxy.go:1047) skips any handler whose rh.Match does not match the status code. So 4xx/5xx from auth bypasses the delete entirely — client sees the exact error response.

2. JSON Fixture Structure — Confirmed Mechanism

Resolved the "handle" before "match" ordering: routes embedded inside handlers go through caddyconfig.JSONModuleObject, which does json.Marshal → unmarshal to map[string]any → add "handler" key → json.Marshal(map). Go's json.Marshal on maps sorts keys alphabetically. "handle" (h) < "match" (m), so inner routes have "handle" first. This is verified by the pre-existing upstream fixtures that pass Caddy's CI.

  • forward_auth_rename_headers — 5 delete routes, destination names ("1","B","3","D","5"), 12-tab depth.
  • forward_auth_authelia — 4 delete routes, destination names (Remote-Email/Groups/Name/User sorted), 16-tab depth (4 more due to subroute wrapper from app.example.com hostname matcher).
  • forward_auth_copy_headers_strip (new) — deletes X-User-Id and X-User-Role, 12-tab depth, mirrors rename_headers structure exactly.
  • All delete routes have no "match" key (omitempty).

3. Integration Test — forwardauth_test.go

  • Module registration: caddytest imports _ "github.com/caddyserver/caddy/v2/modules/standard" which chains to caddyhttp/standard/imports.go which includes _ ".../reverseproxy/forwardauth". The forward_auth directive is registered.
  • Ports: admin localhost:2999 and http_port 9080 are the standard Caddy test ports used by all other integration tests.
  • No t.Run anti-pattern: Sequential assertions using the outer *testing.T. tc.t.Fatalf correctly fails the outer test.
  • Race safety: sync.Mutex protects last. AssertResponse waits for a complete HTTP round-trip; by the time it returns, the backend handler has already written to last.
  • Test logic:
    • Case 1 (no token): auth returns 401 → goodResponseHandler skipped → client sees 401 as-is
    • Case 2 (valid token, no forged headers): delete fires, set skipped → backend sees absent headers
    • Case 3 (valid token + forged headers): delete removes forge, set skipped → backend sees absent headers (regression test)
    • TestForwardAuthCopyHeadersAuthResponseWins: auth returns headers → delete removes forge, set copies auth value → backend gets auth value

4. No New Security Bugs

Property Verdict
Only fires on 2xx auth responses Scoped inside goodResponseHandler
Operates on request headers only ApplyToRequest → r.Header.Del()
No wildcard deletion Only specific named to values
Placeholder source repl.Set in reverseproxy from res.Header (Auth response, not request)
HTTP header canonicalization Go normalizes all incoming headers
Sequential route execution Single-threaded route chain; no races

Fixes GHSA-7r4p-vjf4-gxv4

…lied headers

When using copy_headers in a forward_auth block, client-supplied headers with
the same names were not being removed before being forwarded to the backend.

This happens because PR caddyserver#6608 added a MatchNot guard that skips the Set
operation when the auth service does not return a given header. That guard
prevents setting headers to empty strings, which is the correct behavior,
but it also means a client can send X-User-Id: admin in their request and
if the auth service validates the token without returning X-User-Id, Caddy
skips the Set and the client value passes through unchanged to the backend.

The fix adds an unconditional delete route for each copy_headers entry,
placed just before the existing conditional set route. The delete always runs
regardless of what the auth service returns. The conditional set still only
runs when the auth service provides that header.

The end result is:
  - Client-supplied headers are always removed
  - When the auth service returns the header, the backend gets that value
  - When the auth service does not return the header, the backend sees nothing

Existing behavior is unchanged for any deployment where the auth service
returns all of the configured copy_headers entries.

Fixes GHSA-7r4p-vjf4-gxv4
@CLAassistant
Copy link

CLAassistant commented Mar 4, 2026

CLA assistant check
All committers have signed the CLA.

@francislavoie francislavoie changed the title fix: forward_auth copy_headers does not strip client-supplied identity headers (Fixes GHSA-7r4p-vjf4-gxv4) forward_auth: copy_headers does not strip client-supplied identity headers (Fixes GHSA-7r4p-vjf4-gxv4) Mar 4, 2026
@francislavoie francislavoie enabled auto-merge (squash) March 4, 2026 04:29
@francislavoie francislavoie disabled auto-merge March 4, 2026 04:29
@francislavoie francislavoie merged commit 2dbcdef into caddyserver:master Mar 4, 2026
25 checks passed
@github-actions github-actions bot mentioned this pull request Mar 6, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants