Restore Forwarded header in reverse proxy Rewrite mode

[lodener]

• Read the Contributing document.

Summary

The standard RFC 7239 Forwarded header was being silently dropped by gorouter and never reaching backend apps.

The cause is the switch from the deprecated ReverseProxy.Director to ReverseProxy.Rewrite in ebe6fed (#573), done for Go 1.26 staticcheck compliance. Go's Rewrite mode strips Forwarded, X-Forwarded-For, X-Forwarded-Host, and X-Forwarded-Proto from the outbound request before it calls the rewrite func (see outreq.Header.Del("Forwarded") in net/http/httputil/reverseproxy.go). Director mode never stripped any of these. That commit put back the three X-Forwarded-* headers but missed Forwarded, so client-set Forwarded headers stopped getting
forwarded.

This change copies Forwarded from the inbound request to the outbound request as-is, keeping multiple header values intact per RFC 7239. It follows the same restore logic already used for the X-Forwarded-* headers in proxy/proxy.go. It also adds regression tests for a single value, multiple values, and the case where the client sends no header.

• Introducing commit: ebe6fed (Amelia/fix staticcheck go126 #573)
• Upstream behavior: net/http/httputil ReverseProxy.Rewrite strips forwarding headers before calling the rewrite func

Backward Compatibility

Breaking Change? No

This puts back the pass-through behavior that existed before ebe6fed (#573), so it returns gorouter to how it used to work rather than adding new behavior. It is a plain verbatim pass-through with no new config, applies right away to all deployments, and needs no opt-in.

[geofffranks]

This brings to light the fact that we don't have any proper sanitization of the Forwarded header like we do for the others. Can you please add this?

sanitize_forwarded_proto and force_forwarded_proto_https (config.go:423-424, both global and per-domain) exist for exactly one purpose: guarantee the backend can't be lied to about whether the connection was really TLS. handlers/x_forwarded_proto.go overwrites the client's value with the real newReq.TLS state.

Set that flag and you have closed the hole on one channel while leaving Forwarded: proto=https wide open on another. Gorouter now emits both headers, and they can disagree — one hardened, one attacker-controlled. Which one wins is decided by the backend framework, not by your config.

That last point is the crux. RFC 7239 is the standards-track header, and frameworks implementing it commonly give it precedence over the X-Forwarded-* legacy set when present (Spring's ForwardedHeaderFilter is the canonical example — worth confirming against your specific app stack). Where that's true, your sanitize_forwarded_proto setting is not weakened, it's bypassed: the attacker just uses the channel that wins.

Concrete consequences per parameter

• proto=https — app believes TLS over cleartext: Secure cookies emitted on an insecure hop, redirect-to-HTTPS and HSTS logic skipped, https:// absolute URLs generated. Matters most when something terminates TLS ahead of gorouter and speaks HTTP to it.
• host= — host-header-injection class: poisoned password-reset links, cache poisoning, wrong absolute URLs. Least novel, since gorouter already forwards X-Forwarded-Host verbatim ("preserve whatever the client/middleware set").
• for= — this is the sharpest asymmetry. Gorouter appends the real client IP to X-Forwarded-For, so the trustworthy value is always present and always last. Forwarded: for= gets no such treatment — nothing is appended, so an app reading client IP from Forwarded has zero trustworthy element to anchor on. IP allowlists, rate limits, and audit logs all become attacker-controlled.

Relatedly, gorouter appends no by= element, so there's no record it was a hop — a minor RFC 7239 deviation (a conformant proxy should extend the list).

[lodener]

Sanitization is now added in 8c05b5b, using the existing sanitize_forwarded_proto / force_forwarded_proto_https flags rather than a new property, since the same trust decision applies.

When either flag is set, a client-supplied Forwarded field is replaced rather than appended:

client:  Forwarded: for=1.2.3.4;host=evil.example.com;proto=https
backend: Forwarded: for=10.0.0.1;proto=http

I considered appending a sanitized element, but that leaves the client's element in place, so a consumer that gives precedence to it can still see attacker-controlled values. Rewriting only proto would require parsing the RFC 7239 element syntax and would leave the client's for and host in place. Replacing the whole field is also consistent with how XFCC is handled under SANITIZE_SET.

Per parameter:

• proto is the sanitized scheme, so the existing flag now covers both X-Forwarded-Proto and Forwarded: proto=.
• for is the immediate peer from RemoteAddr, the same address gorouter adds to X-Forwarded-For. Behind a load balancer, this is the LB rather than the original client.
• host is omitted intentionally. A consumer without a host parameter can fall back to the Host header, which is the value gorouter routed on. Adding it would duplicate that value and introduce another field requiring correct quoting and escaping. X-Forwarded-Host remains unchanged.

On by: I left it out because RFC 7239 makes it optional. I was also wary of choosing a fixed identifier that applications or log tooling might start relying on. I'm happy to add it if we think identifying the gorouter hop is useful.

With neither flag set, nothing changes: the client's Forwarded header passes through as-is, and gorouter does not add one. This preserves the pre-#573 behavior.

Two remarks:

• With these flags enabled, trusted Forwarded information from an upstream LB is discarded rather than merged. This is consistent with what the flags already do to X-Forwarded-Proto.
• I only checked precedence against Spring's ForwardedHeaderUtils (the example in the review). Other frameworks may handle precedence differently.

[geofffranks]

Thanks!