Add strong warnings about XFF headers

[novln]

No description provided.

[adam-p]

I like the warnings you've added and the increased emphasis on using a custom KeyGetter to get exactly the right value for their network setup. I also like the combining of all the XFF headers and ensuring the returned value is an actual IP.

Things I like less:

• Why is X-Real-IP treated specially? Why is it a fallback from XFF when TrustForwardHeader is true? I think X-Real-IP should be treated exactly the same as CF-Connecting-IP, etc. And there's no way for the user to just use XFF (if present). (But I don't see this as a spoofing problem. If the XFF header is allowed to not be present, then the server is accepting direct connections. But in that case XFF could have been spoofed anyway, so the possibility of a spoofed X-Real-IP doesn't matter.)
• I don't love falling back by default to RemoteAddr, for reasons I explained in the post. Of course, if you didn't then you would have to figure out what to do if there was no IP available in the configured locations. (Which is a topic I talk about in the didip/tollbooth PR about this topic.
• You're still using the leftmost XFF IP and telling the user to only use if they have configured their first reverse proxy to strip the header and start it fresh. That's technically correct, but I feel like it'll still cause problems for a lot of people who don't read carefully enough. I also think that most people with that much control over their first proxy will also just put the IP into X-Real-IP or whatever, because the list-ness of XFF makes no sense at that point.

[novln]

Why is X-Real-IP treated specially?

To ensure backward compatibility with previous version for people using a correctly configured reverse proxy.
This will changes in limiter v4 and we'll only rely on the header provided by the users.
For the moment I keep it as-is.

I don't love falling back by default to RemoteAddr [...]

You could use WithExcludedKey to define the list of your reverse proxy in this case.
It was made for this use case (it happened to us once).

You're still using the leftmost XFF IP and telling the user to only use if they have configured their first reverse proxy to strip the header and start it fresh. That's technically correct, but I feel like it'll still cause problems for a lot of people who don't read carefully enough.

You're absolutely right. But I have to find the right trade-off between keeping running what is already deployed (assuming it's correctly configured) and introduce breaking change.
Without feedback from our users, I don't want to break everything.

Anyway, thank you for reporting this. Maybe this will give me enough motivation to work on this v4.

Thomas.

[adam-p]

@novln 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). Hopefully it'll be suitable for your v4.
https://github.com/realclientip/realclientip-go