X-Forwarded-Host header resets the request port

[vmuzikar]

Describe the bug

When a request contains the X-Forwarded-Host header, it resets the port ignoring the original request port. This is particularly problematic when the X-Forwarded-Host header is used without the port and the app is running on a non-default port. In this case the request port should be used, but the port is effectively removed defaulting to protocol port (80 or 443).

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

3.18.0, 3.18.1

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

No response

Additional information

This is a regression in 3.18 caused by #44601 and it is blocking Keycloak from upgrading to Quarkus 3.18+.

It is not affecting the Forwarded header where the port is not reset.

[sberyozkin]

@vmuzikar Is it blocked because you have got a test failing or for any other reasons ?

This issue is a duplicate of the closed #44776, with the rationale given at #44777

[quarkus-bot]

/cc @pedroigor (bearer-token)

[vmuzikar]

We have a failing test but IMHO for a good reason. :) I believe when the port is explicitly not provided (or hinted by other proxy headers like proto), it should fall back to the request port. But I understand this is not covered by any specs so it could be either way.

Why I'm raising this is mainly because this is basically a breaking change for our users and I'm afraid it might backfire on us. Hostname/proxy is an extremely fragile area in Keycloak, lot's of users were struggling with it as it's quite hard to get it right when external proxy is involved. We even had to rewrite the whole UX, making it consistent and simple. Hence I'm a bit concerned about this is a new (AFAICT undocumented) breaking change. The main problem I see is that it's now inconsistent – X-Forwarded-Host resets the port while Forwarded: host does not.

[sberyozkin]

@vmuzikar This test shows it actually has always been the case for Forwarded as well.

The problem with returning the actual request port is that it is leaked, if the user requests an access to www.keycloak.com, then the follow up links this user works with should not include www.keycloak.com:8081, etc

IMHO, we can definitely keep tuning that code, but without any concrete user specific requirements, we'd be just theorizing.
I believe the test which is failing may be no longer valid, but please don't be concerned, if proves necessary, we'd deal with tuning it...

[vmuzikar]

The problem with returning the actual request port is that it is leaked, if the user requests an access to www.keycloak.com, then the follow up links this user works with should not include www.keycloak.com:8081, etc

Yes, I won't be arguing about whether the port should be reset or not. From the Keycloak perspective, we treat port independently from the host, so if port is not explicitly provided by the proxy, we expect it to fallback to request port. So I tend to say the port should not reset.

However, my main concern now is that X-Forwarded-Host now behaves differently from Forwarded: host.

[sberyozkin]

From the Keycloak perspective, we treat port independently from the host, so if port is not explicitly provided by the proxy, we expect it to fallback to request port. So I tend to say the port should not reset.

But this is not backed up by any text or recommendation anywhere, getting the local port added to the URI that this endpoint will be accessed from outside the proxy space does not work IMHO...

However, my main concern now is that X-Forwarded-Host now behaves differently from Forwarded: host.

What I'm seeing that, at the test level, there is no difference, in both cases, the local port is not included to the produced URL.

Can you please give me a favor and create 2 tests confirming it is not the case ?

[vmuzikar]

I can try and create a test but looking at the existing one you mentioned, the port is reset there because it is also setting proto which resets the port (this is not new, it was like that even before). The use case I'm talking about is when only Forwarded: host=... is set without other parts like proto.

[sberyozkin]

The use case I'm talking about is when only Forwarded: host=... is set without other parts like proto.

If this is the case then I'd prefer making sure the local port is not leaked in case of Forwarded as well, as opposed to getting back to the old behavior and returning the local port for X-Forwarded-Host

[sberyozkin]

@vmuzikar I believe this recommendation at https://stackoverflow.com/questions/61429542/http-x-forwarded-host-behavior-without-port is sound

[vmuzikar]

If this is the case then I'd prefer making sure the local port is not leaked in case of Forwarded as well, as opposed to getting back to the old behavior and returning the local port for X-Forwarded-Host

Ok, thinking about it more, the new behaviour (port is reset) sounds good. We just need to make it consistent.

Do you want me to open a PR, or do you plan to fix it yourself?

[sberyozkin]

Hi @vmuzikar , sure, glad you are finding it reasonable as well,

Do you want me to open a PR, or do you plan to fix it yourself?

If you can find a bit of time then it would be very appreciated, I might get a chance to look after doing it for Forwarded as well, but with a bit of a delay, thanks