enhance: validate forwarded client IPs - #39190
Open
silverwind wants to merge 1 commit into
Open
Conversation
The X-Real-IP and X-Forwarded-For values a trusted proxy reports went into req.RemoteAddr unparsed, so a header the proxy did not overwrite could put an arbitrary string there. That string reaches the "Failed authentication attempt from %s" lines the documented fail2ban filters read, and InitialIP on new user records. X-Forwarded-For was also split on ", " rather than ",", and only the first of several header lines was read. Both shift the hop index into client-controlled entries. Replace the unmaintained chi-middleware/proxy with an in-tree implementation that parses every candidate as an IP and fails closed, merges repeated headers, and folds mapped IPv4 and IPv6 zones so one client maps to one address. The trusted proxy gate, the hop limit, "*" and unix socket handling are unchanged. Assisted-by: Claude:Opus 5
GiteaBot
added
the
lgtm/need 2
github-actions
Bot
added
topic/security
silverwind
changed the title
github-actions
Bot
added
type/enhancement
Member
Author
|
This is something that was noticed while doing recent chi upgrade, no security impact, seems just nice to have. cc: @lafriks, author of https://github.com/chi-middleware/proxy btw, there alo exists the standardized https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Forwarded which is currently unsupported. |
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
2 participants
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.
X-Real-IPandX-Forwarded-Forfrom a trusted proxy went intoreq.RemoteAddrunparsed, so a header the proxy did not overwrite could put an arbitrary string into the auth failure logs fail2ban parses.X-Forwarded-Forwas also split on", "not",", and only the first header line was read, shifting the hop index into client-controlled entries.Replaces
chi-middleware/proxywith an in-tree version that fails closed on non-IP input. Trust gate, hop limit and config unchanged.Written by Claude.