strfry icon indicating copy to clipboard operation
strfry copied to clipboard

Parser for x-forwarded-for does not work with IPv6 or multiproxy

Open petrkr opened this issue 2 years ago • 4 comments

I have setup haproxy (ssl term) ---> Nginx --> strfry So x-forwarded-for header contains all proxies in the way, but parser is not happy from it, do not know if it is because of multiple IP or because of client uses IPv6

Websocket ]WARN| Couldn't parse IP from header x-forwarded-for: 2a00:abcd:401a:abcd:ca43:f3a7:c615:aaaa, 192.168.0.1 Websocket ]WARN| Couldn't parse IP from header x-forwarded-for: ffff:123.83.123.203, 192.168.0.1

petrkr avatar Jun 16 '23 08:06 petrkr

As of now, strfry doesn't support X-Forwarded-For. Personally I try to avoid this header when possible, because often first-level proxies get configured to append their IP onto this header if it exists. In this case clients can spoof their IP to be whatever they want by providing the header themselves. More info here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#security_and_privacy_concerns

In general, I think the X-Real-IP header ( https://nginx.org/en/docs/http/ngx_http_realip_module.html ) is less likely to have this problem. The typical configuration is to overwrite this header if provided, meaning that there will never be a list of IPs.

I believe strfry's current implementation will fail with x-forwarded-for headers that have multiple IPs, although this depends on the OS's inet_pton() implementation. We could make it support x-forwarded-for and pull out the ~final~ first IP address. If this is important to you let me know and I'll add it to my TODO list.

On the topic of IPv6, yes, strfry should support IPv6 addresses. I'll note that the two IP6 addresses you pasted seem to actually be invalid, though. The first has some non-printable characters in it (?), and the second should probably be ::ffff:123.83.123.203 (missing ::)

hoytech avatar Jun 16 '23 14:06 hoytech

Github replaces some : with emojisc next time i'll use code block

And that ffff is right, it's ipv4 noted in ipv6 format, i suppose haproxy and nginx have it right, since they are widely used.

Btw if someone will inject IP to headers, haproxy will just add right IP after it.

But yes, I can use some header like X-real-ip or even some random string

petrkr avatar Jun 16 '23 15:06 petrkr

My issue wasn't with the ffff, but that it was missing a ::, see this website:

Bad: http://v6decode.com/#address=ffff%3A123.83.123.203 Good: http://v6decode.com/#address=%3A%3Affff%3A123.83.123.203

But probably you're right and it is just github mangling your pasted text.

With X-Forwarded-For I think ideally you would configure haproxy to delete the untrusted IP(s), not just add the right IP to the end of the list. At the final end-point downstream, how will you decide which one of the IPs in the list is the original client IP?

It's not just the header name, X-Real-IP also has a different security profile, because it does not support a list of IPs, meaning that a proxy will always overwrite any (possibly untrusted) upstream values. In your case this means you would only want to have the X-Real-IP set at the HAProxy level (not nginx)

hoytech avatar Jun 16 '23 17:06 hoytech