Strip spoofable forwarded headers at the Fastly edge entry point#505
Open
ChristianPavilonis wants to merge 1 commit intomainfrom
Open
Strip spoofable forwarded headers at the Fastly edge entry point#505ChristianPavilonis wants to merge 1 commit intomainfrom
ChristianPavilonis wants to merge 1 commit intomainfrom
Conversation
Prevent X-Forwarded-Host / Forwarded header spoofing that allows attackers to hijack URL rewriting across HTML streaming, ad-proxy URLs, Prebid OpenRTB requests, and request signing payloads. Closes #409
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #409 —
X-Forwarded-Host/Forwardedheader spoofing enables URL rewrite hijack.Forwarded,X-Forwarded-Host,X-Forwarded-Proto, andFastly-SSLheaders at the top ofroute_request()before any routing or header inspection occurslog::debug!tracing when a spoofable header is actually stripped (aids incident response)RequestInfoandpublisher.rsdoc comments to reflect the new trust boundaryWhy it's safe to remove these headers
On Fastly Compute, the service is the edge — there is no legitimate upstream proxy setting forwarded headers. Any
X-Forwarded-HostorForwardedvalues present in the request were sent directly by the client and are therefore untrusted.After stripping, the existing fallback logic in
RequestInfo::from_requestnaturally lands on trustworthy signals:extract_request_hostfalls throughForwarded(gone) →X-Forwarded-Host(gone) →Hostheader — which Fastly validates against the service configurationdetect_request_schemehits Fastly SDK TLS detection first (priority 1, unchanged by this PR, not a header — it's SDK-level and unspoofable) → forwarded headers (gone) → defaulthttpThe forwarded header parsing code in
commonremains intact for potential non-edge deployment contexts, but the Fastly entry point now enforces the trust boundary.Impact on integrations
site.domain/site.pagesettings.publisher.domain, notRequestInfoHostheader is the actual browser-facing domainext.trusted_serverboth use the same sourceRequestInfoat allHostis the correct domainGit blame context
The forwarded header logic was introduced in commit
563c281("Improve forwarded header parsing for host and scheme") with no security review (PR #176 had an empty description and no review comments). It was speculative support for a hypothetical chained-proxy scenario (Client → Proxy A → Trusted Server) that does not apply to the Fastly Compute deployment model.Test plan
sanitize_removes_all_spoofable_headers,sanitize_then_request_info_falls_back_to_hostRequestInfoforwarded-header parsing tests, which validate thecommoncrate logic independently)cargo fmt,cargo clippyclean