Trusted proxies option allows passing a port which seems hard or even impossible to get working

[ahus1]

Describe the bug

As part of #29888 (cc: @sberyozkin, @michalvavrik, @Doctor-love) a check was added for the address of the proxy which the proxy uses to connect to Quarkus.

I see that the PR goes beyond the original request to add also a proxy port number which is IMHO hard or even impossible to get working and should probably be removed. As we're now exposing this option to the users o Keycloak, I'm having a hard time to explain it to our users and to see where this can be used successfully.

Expected behavior

A trusted proxy configuration with and without a port should work as expected.

Actual behavior

If you specify a trusted proxy with a port, IMHO only a single connection can be established at any given time:

A TCP connection is identified by the the four elements source IP, source port, target port and target IP. The target port and target IP are where Quarkus listens for incoming connections and are thereby fixed. Source host is the proxy. If I also limit the source port, the proxy can only establish a single connection at a time. And even if that connection is closed, it might take a while until it can open a new connection while it is still in CLOSE_WAIT state AFAIK.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

https://github.com/keycloak/keycloak/pull/32346#discussion_r1731299359

[michalvavrik]

hello @ahus1 ,

I see that the PR goes beyond the original request to add also a proxy port number which is IMHO hard or even impossible to get working and should probably be removed.

please allow me quick questions / checks just to be sure that I understand situation correctly

1. do you acknowledge that specifying port is valid option, here are few examples https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#examples ? quick search, sorry wrong example
2. it is possible configure it with single configuration property
3. it works without specifying port as well
4. the reason why you opened this issue is that you consider it impractical, which is the reason you propose removal

also I believe we need @cescoffier

[quarkus-bot[bot]]

/cc @pedroigor (bearer-token), @sberyozkin (bearer-token,jwt,security)

[michalvavrik]

Updated my question in regards of port as I found previous example in hurry.

[michalvavrik]

Anyway, I re-checked code and tests which seems alright to me. I think I understand Actual behavior from description in a way that you consider it unlikely to be used. I am bit confused by port which seems hard or even impossible to get working, therefore maybe I still miss other points of yours.

[ahus1]

do you acknowledge that specifying port is valid option

No.

While the Forwarded HTTP header might include a host and a port in the for, this is then the client's host and possibly port, or an intermediate proxy.

The check implemented here is about the source IP and source port Quarkus sees in the in the incoming request.

// In ForwardedProxyHandler.java
InetAddress proxyIP = ((SocketAddressImpl) event.remoteAddress()).ipAddress();

As the proxy will pick the source port from a port range, it will never be a single port due to the nature of TCP. If it would use only a single port, it would be able to establish only a single TCP connection at the time.

You can check by specifying a source port with curl that the first call will succeed, and follow-up calls with fail with Address already in use.

curl --local-port 9999 http://localhost:8080 -v

it is possible configure it with single configuration property

I don't understand this question. Please elaborate.

it works without specifying port as well

Yes, I agree it works without specifying the port. Using it without a port is IMHO the only valid thing to do.

the reason why you opened this issue is that you consider it impractical, which is the reason you propose removal

Yes. Either impractical or rather impossible.

[michalvavrik]

it is possible configure it with single configuration property I don't understand this question. Please elaborate.

I think you answered by other responses. Thank you

As the proxy will pick the source port from a port range, it will never be a single port due to the nature of TCP. If it would use only a single port, it would be able to establish only a single TCP connection at the time. You can check by specifying a source port with curl that the first call will succeed, and follow-up calls with fail with Address already in use.

I understand port can be only used once at the time. Thank you for the example and explanation, I appreciate your patience. In the end, it is not my decision and I am absolutely fine if the option is removed. If you don't mind, I have one more question, if you don't want to answer, please feel free to ignore it. We need to wait for Clement and Sergey to decide. Question:

The quarkus.http.proxy.trusted-proxies configuration property allows to specify list of ip/ports and if at least one if the configuration values is matched, it means that check passes. When you say, citing, If it would use only a single port, it would be able to establish only a single TCP connection at the time. I think it is true if you specified only single port for single IP. You are free to specify 200 ports as far as I am concerned. Therefore theoretically, admins can restrict port range that can be assigned to requests. Would it make sense to allow specify port range instead?

If you say it is no, I trust you as I am sure you have more experience than me. Thanks for raising the issue.

[Doctor-love]

I agree with @ahus1 that allowing configuration of a source port for the list of trusted proxies makes no sense. The idea behind the feature was to prevent malicious clients from simply adding an X-Forwarded-For header and thereby spoof their source address.

[michalvavrik]

I agree with @ahus1 that allowing configuration of a source port for the list of trusted proxies makes no sense. The idea behind the feature was to prevent malicious clients from simply adding an X-Forwarded-For header and thereby spoof their source address.

Alright, let's drop it.

[ahus1]

I trust you as I am sure you have more experience than me.

That's very kind of you. Still consider me just another engineer who is trying to find out how Keycloak users should apply this to their environments. While I've been exposed to some production environment over time as an engineer and being on-call, I've never owned one myself. I'm happy to be proven wrong, and will then take that new knowledge to the docs of Keycloak.

The quarkus.http.proxy.trusted-proxies configuration property allows to specify list of ip/ports and if at least one if the configuration values is matched, it means that check passes. [...] Would it make sense to allow specify port range instead?

That could be possible, although I've never seen it configured like this. I expect a port range to be in the area of maybe 1000 ports, and for that I'd rather see a notation of a port range than specifying each single port. I found that a port can be configured with Nginx using proxy_bind, while most are using it just with an IP address. I didn't find an example with a port number in the wild when searching with Google.

My conclusion here: I'm very glad for the responses I received here. For Keycloak, we'll then restrict the use to specifying just an IP address (list) or an IP address range (list), but not a port.

UPDATE: This would then be similar to what is described and linked in the original issue: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from

[ahus1]

I've opened another bug issue against Quarkus around passing in a hostname instead of an IP address. See #42782 for the details.

[sberyozkin]

@ahus1, @michalvavrik IMHO it is not a bug, it may be hard to set the port right, but where is the bug ? Are we sure admins can not restrict it to a single port ? CC @cescoffier

[michalvavrik]

@ahus1, @michalvavrik IMHO it is not a bug, it may be hard to set the port right, but where is the bug ? Are we sure admins can not restrict it to a single port ? CC @cescoffier

Yes, I agree it is not a bug. How I understand @ahus1 is that he patiently tried to explain that specifying one part is highly unlikely in prod env. You can specify more ports of course, but in case we would expect users to do that, I'd suggest it needs optimization (instead of one check per port do port range per one check to stay effective). I have tried to find usages out there and I can't say I found materials where admins restrict request port ranges. That is why I think Alexander is right saying it is edge feature.

I don't think there is something wrong about that option though, I think we could just tweak the docs to make situation clear to users. Let's gather opinions, we are in no hurry.

[ahus1]

Given the characteristics of the HTTP protocol, I think and admin can make it work by allocating a port range of some hundreds ports. Still the current syntax is a bit clunky for that as it doesn't allow for port ranges. I'm not sure if admins would limit the ports on the proxy side to limit the resource usage of ports (which are a scarce resource on the proxy side if it is handling multiple downstream services), or if it is used as a means of security when handling the requests on the downstream.

I agree that there is no hurry on this one. If you think Quarkus should evolve to support port ranges, it could be re-classified as an enhancement. To drop the ports, I think we might need more evidence that this is not needed. One piece of evidence is https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from as it only specifies the upstream proxy by IP without a port.

[sberyozkin]

Supporting port ranges sounds interesting and indeed, is likely to be more realistic. Thanks