-
-
Notifications
You must be signed in to change notification settings - Fork 996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Problems with use of X-Forwarded-For and other headers to get the "real" client IP #711
Comments
There was a similar proposal opened in #708 to implement a more advanced middleware, maybe worth taking a look at that proposal and jotting down your thought? |
I put this in #708 but I'm going to repeat it here because this issue is also about httprate... I wrote a library to help with extracting the "real" client IP from a request. I intended that it be useful in cases like this, so please let me know if it's suitable (and if it's not we'll figure out how to make it better). |
I wasn't aware of this issue directly, but I did reach out to @pkieltyka a few days ago around this topic specifically in relation to httprate, about always reading real-ip related headers in some of the default methods as not secure due to how easy it is to bypass rate limiting. He implemented a fix in go-chi/httprate#17 -- simply by moving the logic into separate methods (
|
Please, have a look at this proposal: #967 |
go-chi/chi/middleware/RealIP and go-chi/httprate suffer from problems getting the "real" client IP. I'll try to break it down into categories.
RealIP isn't necessarily "security-related", but it depends on how the dev uses it. I would argue that rate limiting -- and so httprate -- is security-related.
For a full explanation of the problems mentioned here, please see my blog post about it. (Here's the chi-specific section, but all the rest is informative.)
Headers are untrustworthy, unless added by a reverse proxy you control
The RealIP middleware has this comment:
This is a good warning, but a) there's no such warning on chi's httprate.KeyByIP (even though it does the same thing), and b) it's not a comprehensive enough warning.
Some reverse proxies, like AWS ALB, lets all header values through that it doesn't set itself. So it will let through
True-Client-IP
andX-Real-IP
. An attacker can spoof either of those headers, and that will end up as the IP that is reported by RealIP or the IP that's gets rate-limited by httprate.X-Forwarded-For
is trickyLeftmost vs Rightmost
Also,
X-Forwarded-For
is only appended to by reverse proxies, so the only IPs in it that are trustworthy are the ones that were added by reverse proxies that you control. httprate, however, takes the first IP in XFF. So an attacker can trivially spoof it. The leftmost XFF IP should never be used for security-related purposes.Switching to rightmost will introduce an XFF multiple-headers problem
Go's
http.Header.Get
returns the first instance of a header, but to get the rightmost IP from XFF, you need to get the last. (More info here.)Leftmost values can be garbage
If you really want to use the leftmost IP, you need to be aware that it could be either a private-space IP address or complete garbage.
A suggested algorithm for how to get it is here.
Parsing the XFF IP list
httprate splits the XFF list by comma-space rather than just comma. RFC 2616 only says "comma-separated" for list headers. (And RealIP splits by comma. And I've seen reverse proxies append without a space. And I've looked at about ten other projects and haven't seen another example of splitting by comma-space.)
Results
RealIP can easily be made to return a spoofed header.
httprate can be bypassed by spoofing different IPs. Possibly the server memory can be exhausted by spoofing very large fake IP values.
The text was updated successfully, but these errors were encountered: