fix: don't trust forwarded headers from arbitrary clients#62
Merged
Conversation
UseAppFoundation cleared KnownIPNetworks and KnownProxies unconditionally, so the app honored X-Forwarded-For / -Proto from any caller. Without a trusted proxy always overwriting those headers, a client could spoof its source IP (falsifying audit logs, bypassing IP-based controls) or the request scheme. Make forwarded-header trust configurable and secure by default: - New AppFoundationOptions.KnownProxyNetworks / KnownProxies let a host declare the reverse-proxy CIDRs / IPs whose headers are trusted; when set, only those origins are trusted. - With none configured, trust every origin in Development (local convenience) but only loopback otherwise, and log a warning in non-Dev so operators behind a proxy know to configure it. - New ConfigureForwardedHeaders hook for full control (e.g. ForwardLimit, or deliberately trusting all when an ingress sanitizes the headers). Closes #51
Production proxy CIDRs are typically unknown at build time, so allow the KnownProxyNetworks / KnownProxies trust lists to be supplied via configuration (AppFoundation:KnownProxyNetworks / :KnownProxies) in addition to code — as a delimited scalar (friendly for a single env var or .env entry) or a configuration array. Config values augment and de-duplicate against anything set in the configure callback. IPv6 CIDRs are preserved (split never uses ':').
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
1 participant
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.
Closes #51 (F2 — forwarded headers trusted from all proxies).
What
UseAppFoundationpreviously didKnownIPNetworks.Clear()+KnownProxies.Clear()unconditionally, soX-Forwarded-For/X-Forwarded-Protowere honored from any caller. This makes forwarded-header trust configurable and secure by default:AppFoundationOptions.KnownProxyNetworks(CIDRs) andKnownProxies(IPs) — declare the reverse proxies whose headers are trusted. When either is set, only those origins are trusted (framework loopback defaults are cleared).ConfigureForwardedHeadershook for full control (ForwardLimit, or deliberately trusting all when an upstream ingress sanitizes the headers).The trust-building logic is extracted to an internal
BuildForwardedHeadersOptions(options, isDevelopment)helper and unit-tested.Why
With the lists cleared, a client could send
X-Forwarded-For: 1.2.3.4to forgeRemoteIpAddress(falsifying audit logs, bypassing any IP allowlist/rate limit) orX-Forwarded-Proto: httpsto confuse scheme-dependent logic. As a reusable foundation, "trust all proxies" is an unsafe default to ship to every consumer.Hosts running in a non-Development environment behind a reverse proxy must now set
KnownProxyNetworks/KnownProxies(or useConfigureForwardedHeaders) to keep seeing the real client IP/scheme — otherwise forwarded headers are honored only from loopback. The logged warning points this out. A host that truly wants the old behavior can clear the lists viaConfigureForwardedHeaders.Tests
ForwardedHeadersOptionsTestscovers: forwarded flags always set; non-Dev + no proxies keeps loopback-only (not wide open); Dev + no proxies trusts all; configured proxies trusted exactly (defaults cleared).Verification
dotnet build -c Release— 0 warnings, 0 errors.dotnet test -c Release— 61/61 passing (4 new).main(fix: don't allow runtime schema drops outside Development #61 merged).