ForwardedRequestCustomizer / getLeftMost

[gb6881]

Hi

in our code we overwrote getLeftMost so we could limit the Number of IPs in that Header as, e.g. AWS/ApplicationLB does not replace, but append to the Header if the outside request contains it already. But as that part is untrusted, we want to limit the how many levels we accept.

in #5247 this method is made static, so we can not override it anymore.

if your are interessted we can also submit a merge request with our depth-limiting code.

Best,
Georg

[gb6881]

(currently we use 9.4.31)

[joakime]

in our code we overwrote getLeftMost so we could limit the Number of IPs in that Header

What does that mean? Can you explain the issue around "limit the number of IPs in that header"?
getLeftMost is per spec, and will only return the left-most value of the header value list.

A small example of this would be a request that has gone through 2 proxies before hitting Jetty.

https://github.com/eclipse/jetty.project/blob/f4c55c65e07e7fd810a96e4137b7adfe7e9829eb/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java#L407-L421

in #5247 this method is made static, so we can not override it anymore.

That class has undergone a fair bit of a rework recently in 9.4.33.

Can you provide, instead, some example requests where this is an issue?
Something suitable to the ForwardedRequestCustomizerTest (headers plus expectations).

[gb6881]

Hi,

our case is the following:

http --header https://www.xxxx.com 'X-Forwarded-For: 10.1.1.1' going through the AWS/ALB will result in Headers

X-Forwarded-For:  10.1.1.1, x.x.x.x

while x.x.x.x being my IP-Address

getLeftMost() will now select 10.1.1.1, which i do not want, because i know, that that is injected from the outside, and in my logs i need the endpoints of the tcp connection to on the LoadBalancer (last point of our infrastructure)

so that test would look like:

Arguments.of(new Request("Multi Level Header Respect only 1 Level (example.com:80 to rp.example.com:82 to localhost:8080)")
                    .headers(
                        "GET / HTTP/1.1",
                        "Host: example.com",
                        "X-Forwarded-For: 10.20.30.40, 10.0.0.1",
                        "X-Forwarded-Proto: https"
                    ),
                new Expectations()
                    .scheme("https").serverName("example.com").serverPort(443)
                    .secure(true)
                    .remoteAddr("10.0.0.1")
                    .requestURL("https://example.com/")
            )

Arguments.of(new Request("Multi Level Header Respect only 2 Level (example.com:80 to rp.example.com:82  to rp3.example.com:83 to localhost:8080)")
                    .headers(
                        "GET / HTTP/1.1",
                        "Host: example.com",
                        "X-Forwarded-For: 10.50.60.70, 10.20.30.40, 10.0.0.1"
                        "X-Forwarded-Proto: https"
                    ),
                new Expectations()
                    .scheme("https").serverName("example.com").serverPort(443)
                    .secure(true)
                    .remoteAddr("10.20.30.40")
                    .requestURL("https://example.com/")
            )

[joakime]

Ugh, that's a gross violation of various specs. (even the ones that are not specs, like X-Forwarded-For).

Fixing it via getLeftMost isn't going to work reliably (as that method is used for many fields, not just X-Forwarded-For), plus getLeftMost cannot be brought back into extensibility as it's used from inner classes and whatnot now.

But all is not lost, do this instead ...

Keep your extension of ForwardRequestCustomizer.

But override public void customize(Connector connector, HttpConfiguration config, Request request).

Then fix the specific headers in request.getHttpFields() before you call super.customizer(connector, config, request)

Something like this ...

package jetty.forwarding;

import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.ForwardedRequestCustomizer;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.Request;

public class OutOfSpecForwardingHeaders extends ForwardedRequestCustomizer
{
    @Override
    public void customize(Connector connector, HttpConfiguration config, Request request)
    {
        HttpFields fields = request.getHttpFields();

        HttpField xForwardedForField = fields.getField(HttpHeader.X_FORWARDED_FOR);

        // make sure field exists
        if (xForwardedForField != null)
        {
            // We are interested in the second value.
            String[] values = xForwardedForField.getValues();
            if (values != null && values.length >= 2)
            {
                // Only update if there is actually 2 or more values present
                String second = values[1];
                fields.put(HttpHeader.X_FORWARDED_FOR, second);
            }
        }

        super.customize(connector, config, request);
    }
}

[gb6881]

ok, thanks.

[joakime]

I updated the example for you.

[joakime]

Heh, it is actually easier to implement then my prior example, I forgot we were dealing with HttpField (which has proper field value list logic built-in).
Example updated (again).