X-Forwarded-For with multiple IPs is not recognized

[kvvloten]

Hi,

When traffic is routed through multiple (Apache) reverse proxies, each one adds an IP-address to the header X-Forwarded-For. For example:

X-Forwarded-For: 192.168.1.106, 172.28.1.19

The real client address is the first one in the list. Unfortunately keycloak-ipaddress-authenticator does not interpret the header as a list, instead it logs a warning in keycloak.log:

{"timestamp":"2024-09-27T18:23:13.802671101+02:00","sequence":10831,"loggerClassName":"org.jboss.logging.Logger","loggerName":"org.keycloak.authentication.authenticators.conditional.ConditionalClientIpAddressAuthenticator","level":"WARN","message":"Ignoring invalid IP address 192.168.1.106, 172.28.1.19","threadName":"executor-thread-24","threadId":255,"mdc":{},"ndc":"","hostName":"testserver","processName":"QuarkusEntryPoint","processId":147149}

This warning is produced by ConditionalClientIpAddressAuthenticator.java, line 96.

Rewriting the X-Forwarded-For header in Apache to contain the first IP only is probably not a good idea, as it might be used elsewhere in Keycloak.

Ideas for a solution:

1. Try reading another header e.g. X-Remote-IP first, when it does not exist fall-back to X-Forwarded-For. The current situation remains unchanged, but users with this issue can fix it in Apache by setting the new header:

   SetEnvIf X-Forwarded-For (.*) CLIENT_IP=$1
   RequestHeader set "X-Remote-IP" %{CLIENT_IP}e env=CLIENT_IP

2. Change the code to treat X-Forwarded-For as a comma separated list and use the first element.

I would think idea no. 1. is the most flexible and the most compatible with the current behaviour.

-- Kees.

[Nikos410]

Hey Kees, thanks for your report!

Taking a closer look at the spec for the X-Forwarded-For header, it seem we have to do a little more work to implement it properly (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For). I will get back to you soon :)

Best Nikos

[Nikos410]

Hey @kvvloten,

I have opened PR #7 to properly rework the handling of the header.

• Respect all ip addresses if a header contains multiple addresses
• Respect all headers if it is present multiple times
• Add a toggle to enable using a header or always use the IP address from the client connection (Otherwise, a bad actor can just send a spoofed X-Forwarded-For header)
• Add an optional parameter to specify a custom header name
• Add an option to specify the number of trusted reverse proxies (If you are running 2 reverse proxies, only the last 2 IP addresses can be trusted, with the second last address being the client)

For information on why this is necessary, the MDN docs explain it pretty well (especially the section "Security and privacy concerns")

Example:

Any comments / suggestions on this?

[kvvloten]

Hi Nikos,

Given your explanation and the screenshot (and without running tests), it looks like it fixes the issue in a flexible manner, a really nice enhancement. Thank you!

-- Kees.

[Nikos410]

Hey Kees,

thanks for your reply! If you want to test the changes, I've attached the (locally built) jar files here

Best Nikos

---

keycloak-ipaddress-authenticator-25.0.2_1.zip keycloak-ipaddress-authenticator-25.0.2_1-jar-with-dependencies.zip

I cannot upload .jar files directly, just change the file extension form .zip to .jar ;)

[kvvloten]

I have tested it with my setup. The good news is that it technically works as expected.

But I was confused by the text on trusted proxies: Given this header:

X-Forwarded-For: 192.168.1.106, 172.28.1.19

My browser's IP is 192.168.1.106. In order to get the right IP interpreted I have to set Number of trusted proxies to 2. Thing is that I have only 1 trusted proxy in the header / in this list: 172.28.1.19, but indeed there are two: one in the dmz and one on the keycloak host (tomcat listens to 127.0.0.1 and apache proxies network traffic to it).

I am not sure about a better text, perhaps something like: Use Nth ip from the right?