HttpServletRequest.getServerName can include port when using ForwardedRequestCustomizer

[peterlynch]

Jetty version
9.4.30

Java version
Any supported

OS type/version
Any supported

Description

Nexus Repository Manager was upgraded from using Jetty 9.4.18 to 9.4.30.

Customers have reported certain combinations of previously working X-Forwarded-* headers to no longer work. ( https://issues.sonatype.org/browse/NEXUS-25158 ).

It appears a certain combination of X-Forwarded headers can cause HttpServletRequest.getServerName to return a value which is not a host name because it includes a port number. This fails the validation that NXRM does on host name value.

This test case:

Index: jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java	(revision 271836e4c1f4612f12b7bb13ef5a92a927634b0d)
+++ jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java	(date 1599052223742)
@@ -497,7 +497,21 @@
                     .requestURL("http://fw.example.com:4333/")
                     .remoteAddr("8.5.4.3").remotePort(2222)
             ),
-
+            Arguments.of(new Request("X-Forwarded-* ( Multiple Ports )")
+                    .headers(
+                        "GET / HTTP/1.1",
+                        "Host: myhost:10001",
+                        "X-Forwarded-For: 127.0.0.1:8888,127.0.0.2:9999",
+                        "X-Forwarded-Port: 10002",
+                        "X-Forwarded-Proto: https",
+                        "X-Forwarded-Host: sub1.example.com:10003",
+                        "X-Forwarded-Server: sub2.example.com"
+                    ),
+                new Expectations()
+                    .scheme("https").serverName("sub1.example.com").serverPort(10002) // Jetty 9.4.18 serverPort is 10003
+                    .requestURL("https://sub.example.com:10002/")
+                    .remoteAddr("127.0.0.1").remotePort(8888)
+            ),
             // =================================================================
             // Mixed Behavior
             Arguments.of(new Request("RFC7239 mixed with X-Forwarded-* headers")

Fails with:

java.lang.AssertionError: serverName
Expected: is "sub1.example.com"
     but: was "[sub1.example.com:10003]"
Expected :sub1.example.com
Actual   :[sub1.example.com:10003]

We noticed if X-Forwarded-Port header is not set, then the test will pass. Still since this was working in 9.4.18, we did not expect this to break on a micro version upgrade.

[joakime]

Thanks for filing this issue.

But before I get into it I need to do my usual spiel.
The X-Forwarded-* headers are not a standard, and have many different (and conflicting) interpretations.

If you want consistent behavior, you should be using the RFC7239 Forwarding headers only.

Since you stated going from Jetty 9.4.18 to 9.4.30 you have progressed through 7 reported issues with X-Forwarded-* header since 9.4.18.
The behavior in 9.4.18 had several bugs in itself, and didn't behave properly under certain heavy uses of X-Forwarded-* header combinations (and orders).

As for your reported example testcase ...
The X-Forwarded-Host: sub1.example.com:10003 header is invalid.
That's not a valid declared host (as it contains a port).
That header should never contain a port.

Your example would result in a Jetty HostPort of [sub1.example.com:10003]:10002, which is nonsensical.
The returned "serverName" is then reported as [sub1.example.com:10003].

I feel that our implementation should throw a BadMessageException and return an error 400 on this invalid header.

@gregw what do you think?

[joakime]

Branch jetty-9.4.x-5224-xforwarded-multiple-ports has testcase that replicates the report on this issue.

Adding port support to X-Forwarded-Host is possible, but what other things will we break (again)?

[joakime]

Opened PR #5226

[peterlynch]

Hi @joakime ,

If you want consistent behavior, you should be using the RFC7239 Forwarding headers only.

We have shared this message with some of our customers based on previous 'spiels' we have seen in other Jetty issues 😀 .

However based on some of our research, many of the common webservers and load balancers do not natively support setting the Forwarded header.

[sbordet]

However based on some of our research, many of the common webservers and load balancers do not natively support setting the Forwarded header.

Ah, the curse of being sooo good at riding the front wave!

😄

[joakime]

@peterlynch please see the commits on PR #5226, especially on the Test cases.

You'll see a few things.

First, there's a few variations of your headers, in different orders.
Which result in different values.

This is because once X-Forwarded-Port is seen, it has an impact on the provided authority as seen by the Host header (based on default configuration of the ForwardedRequestCustomizer).
The X-Forwarded-Host is then influenced by the newly calculated authority at the time at which X-Forwarded-Host is seen in the headers.
So if you have ...

Host: foo:1111
X-Forwarded-Port: 2222
X-Forwarded-Host: bar:3333

The resolution is ...

1. authority is foo:1111 (from Host header)
2. authority is now foo:2222 (from X-Forwarded-Port)
3. authority is now bar:2222 (as X-Forwarded-Port was declared, the X-Forwarded-Host port is ignored)

resulting authority is bar:2222 (which means the serverName will return bar)

But if the order of the headers is different (but the values are not), such as ...

Host: foo:1111
X-Forwarded-Host: bar:3333
X-Forwarded-Port: 2222

Then the resolution order is ...

1. authority is foo:1111 (from Host header)
2. authority is now bar:3333 (from X-Forwarded-Host)
3. authority is unchanged as X-Forwarded-Port was after X-Forwarded-Host.

Resulting in an authority of bar:3333

There's also a testcase where the ForwardedRequestCustomizer.setForwardedPortAsAuthority(false) is used to flip the default behavior of X-Forwarded-Port (from the default meaning that the provided port belongs to the authority, to it now belonging to the client)

So that if you have that setForwardedPortAsAuthority(false) set, you'll see the following behavior for a request like ...

Host: foo:1111
X-Forwarded-Port: 2222
X-Forwarded-Host: bar:3333

The resolution is ...

1. authority is foo:1111 (from Host header)
2. authority is unchanged by X-Forwarded-Port
3. authority is now bar:3333 (as X-Forwarded-Port does not belong to authority)

resulting in an authority of bar:3333 and a serverName of bar.

[peterlynch]

@joakime The tests look good to me - thank you for quickly addressing the issue.

As for your reported example testcase ...
The X-Forwarded-Host: sub1.example.com:10003 header is invalid.
That's not a valid declared host (as it contains a port).
That header should never contain a port.

Given that the Host header is allowed to contain a port, my initial reaction to these statements was disagreement. It sounds like support for port inside X-Forwarded-Host has now been added, and thus you have reversed your viewpoint?

authority is unchanged as X-Forwarded-Port was after X-Forwarded-Host.

Just curious - why does Jetty decide that order of request headers matters? According to https://tools.ietf.org/html/rfc7230#section-3.2.2

The order in which header fields with differing field names are received is not significant.

[joakime]

Given that the Host header is allowed to contain a port, my initial reaction to these statements was disagreement. It sounds like support for port inside X-Forwarded-Host has now been added, and thus you have reversed your viewpoint?

The X-Forwarded-Host header has many interpretations, approximately 20% of them say it's related to the Client provided Host header (most do not).

The change we made will likely cause us headaches with IPv6 users that don't properly format their IPv6 host addresses in X-Forwarded-Host (the majority of documented behavior for X-Forwarded-Host specifically calls this out as the why treating X-Forwarded-Host as the Host header is wrong, and why the X-Forwarded-Port header exists. RFC7239 Forwarding has addressed this for IPv6 and even IPvFuture)

This change we just made is actually against the majority interpretation, and will harm existing IPv6 users that don't properly format their IPv6 addresses for that header value. (they MUST now use IPv6reference ABNF as we no longer support IPv6address ABNF in X-Forwarded-Host)

authority is unchanged as X-Forwarded-Port was after X-Forwarded-Host.

Just curious - why does Jetty decide that order of request headers matters? According to https://tools.ietf.org/html/rfc7230#section-3.2.2

The order in which header fields with differing field names are received is not significant.

Again, this is wholly in "the X-Forwarded-* headers are not a standard" space.

The behavior you are asking about is what other implementations expect and perform like (this order behavior is especially common on older cloud provider configurations, eg: AWS).

To highlight how messed up this is, just look at X-Forwarded-Port ...

• We have a configurable ForwardedRequestCustomizer.setForwardedPortAsAuthority(boolean) to allow you to control the 2 main uses of the X-Forwarded-Port header. As an authority header, or as a client remote port header.
• X-Forwarded-Port can also belong to (based on other implementations) as a server port (but not the authority).
• Some implementations treat it as modifying the provided Host header.
• Some implementations treat it as modifying the X-Forwarded-Host header.
• Some implementations treat it as a hierarchy of modifications between 4 headers X-Fowarded-Host + X-Forwarded-For + Host + X-Forwarded-Server.
• Some implementations treat it as a final authoritative port.
• Some implementations allow it to be overridden by other non-X-Forwarded- headers.
• Most implementations consider WHERE the X-Forwarded-Port appears in the header list (in relation to the X-Forwarded-Host and Host headers)
• The behavior on some cloud providers is also dependent on what OS / Runtime / Webapp Language is being talked to. (eg: the behavior expected by a Java webserver is different then, php, or node.js)

In short, there's no X-Forwarded-* header standard, and there's no reliable core behavior across implementations for us to lean on.
It is a complete wild west of interpretation of those headers across the internet.

[joakime]

PR #5226 merged.

Opened Issue #5247 to address this in a more well documented way