Resolve client IP via trusted proxies instead of trusting all X-Forwarded-For#245
Open
artemrootman wants to merge 3 commits into
Open
Resolve client IP via trusted proxies instead of trusting all X-Forwarded-For#245artemrootman wants to merge 3 commits into
artemrootman wants to merge 3 commits into
Conversation
…rded-For ContextWithIps unioned every X-Forwarded-For token into the client IP set, and the allowed-ips check (local/drpc key PreCheckSetting) matched if ANY token equaled an allowed IP. A client connecting directly could therefore send X-Forwarded-For: <allowed-ip> and bypass a key's allowed-ips restriction. Resolve a single client IP: use the direct peer (RemoteAddr) unless it is a configured trusted proxy (server.trusted-proxies, CIDRs or bare IPs), in which case take the right-most X-Forwarded-For entry that is not itself trusted. With no trusted proxies configured (the default), X-Forwarded-For is ignored, so the behavior is secure by default and opt-in for proxied deployments.
artemrootman
changed the title
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.
Problem
ContextWithIpsunioned everyX-Forwarded-Fortoken into the client IP set, and theallowed-ipscheck (local/drpckeyPreCheckSetting) matched if any token equaled an allowed IP. A client connecting directly could therefore sendX-Forwarded-For: <allowed-ip>and bypass a key'sallowed-ipsrestriction.Fix
Resolve a single client IP: use the direct peer (
RemoteAddr) unless it is a configured trusted proxy (server.trusted-proxies, CIDRs or bare IPs), in which case take the right-mostX-Forwarded-Forentry that is not itself trusted.With no trusted proxies configured (the default),
X-Forwarded-Foris ignored entirely, so the behavior is secure by default and opt-in for proxied deployments.Tests
Table-driven coverage in
pkg/utils: untrusted peer ignores a spoofed XFF, trusted peer takes the right-most untrusted hop, attacker-prepended entries are ignored, all-trusted hops fall back to the peer, plusParseTrustedProxiesvalidation.