HttpServletRequest#getRemoteAddr is not adjusted if X-FORWARDED-FOR is present - regression from Jetty 11

[Spikhalskiy]

Jetty version(s)
Jetty 12.0.16

Jetty Environment
EE10

Description

With Jetty 11, if X-Forwarded-For header is present, HttpServletRequest#getRemoteAddr was getting adjusted by the following code in ForwardedRequestCustomizer:

            // Set Remote Address
            if (forwarded.hasFor())
            {
                int forPort = forwarded._for._port > 0 ? forwarded._for._port : request.getRemotePort();
                request.setRemoteAddr(InetSocketAddress.createUnresolved(forwarded._for._host, forPort));
            }

Jetty 12 does have a similar piece in ForwardedRequestCustomizer for ConnectionMetaData:

if (forwarded.hasFor())
        {
            int forPort = forwarded._for._port;
            if (forPort <= 0)
            {
                // TODO utility methods for this would be nice.
                SocketAddress addr = request.getConnectionMetaData().getRemoteSocketAddress();
                if (addr instanceof InetSocketAddress)
                    forPort = ((InetSocketAddress)addr).getPort();
            }
            remote = InetSocketAddress.createUnresolved(forwarded._for._host, forPort);
        }

But the computed result doesn't propagate to HttpServletRequest#getRemoteAddr correctly, leaving it with the original value.

This looks like a regression from Jetty 11 behavior. It may be a desired breaking change in behavior to leave the original value and make uses reach for X-Forwarded-For directly if needed, but I can't find this stated anywhere.

[joakime]

We have 42 testcases that work with X-Forwarded-For behaviors when ForwardedRequestCustomizer is present, that all pass.

I added another one specifically for the combination of ee10 + X-Forwarded-For (again, when ForwardedRequestCustomizer is present), that passes too.

I suspect you don't have ForwardedRequestCustomizer actually setup/enabled.

[Spikhalskiy]

Thank you for the pointers; during the UriCompliance investigation, I dropped httpConfig.addCustomizer(new ForwardedRequestCustomizer()) and forgot to return it back later. Not a bug.

[joakime]

Here's a different way to test this (posted here in case others come across this issue)

Setup the jetty.base for testing this with the jetty-home archive.

[joakim@hyperion bases]$ mkdir 12.0.16-demo
[joakim@hyperion bases]$ cd 12.0.16-demo/
[joakim@hyperion 12.0.16-demo]$ java -jar ../../jetty-home-12.0.16/start.jar --add-modules=http,http-forwarded,ee10-deploy,ee10-demo-jetty

[joakim@hyperion 12.0.16-demo]$ tree -F
./
├── etc/
│   ├── ee10-demo-rewrite-rules.xml
│   ├── jetty-demo-realm.properties
│   ├── jetty-demo-realm.xml
│   └── jetty-rewrite-rules.xml
├── lib/
│   └── ext/
├── resources/
│   └── jetty-logging.properties
├── start.d/
│   ├── ee10-demo-jetty.ini
│   ├── ee10-deploy.ini
│   ├── http-forwarded.ini
│   └── http.ini
└── webapps/
    ├── ee10-demo-jetty.d/
    │   └── ee10-demo-jetty-override-web.xml
    ├── ee10-demo-jetty.war
    └── ee10-demo-jetty.xml

Run the server, with dump, and verify that the ForwardedRequestCustomizer is there.

[joakim@hyperion 12.0.16-demo]$ java -jar ../../jetty-home-12.0.16/start.jar jetty.server.dumpAfterStart=true
...(snip)...
|  += HttpConnectionFactory@128d2484[HTTP/1.1] - STARTED
|  |  +- HttpConfiguration@782a4fff{32768/8192,8192/8192,https://:0,[ForwardedRequestCustomizer@46c670a6]}
|  |     +> customizers size=1
|  |     |  +> ForwardedRequestCustomizer@46c670a6
...(snip)...

On a different terminal, use the request dump endpoint to see the HttpServletRequest.getRemoteAddr value on ee10

[joakim@hyperion 12.0.16-demo]$ curl --no-progress-meter http://localhost:8080/ee10-test/dump/info | grep getRemote
<th align="right">getRemoteAddr:&nbsp;</th><td>127.0.0.1</td></tr><tr>
<th align="right">getRemoteHost:&nbsp;</th><td>127.0.0.1</td></tr><tr>
<th align="right">getRemotePort:&nbsp;</th><td>36230</td></tr><tr>
<th align="right">getRemoteUser:&nbsp;</th><td>user</td></tr><tr>

Now, lets try with X-Forwarded-For ...

[joakim@hyperion 12.0.16-demo]$ curl --no-progress-meter -H "X-Forwarded-For: 10.9.8.7" http://localhost:8080/ee10-test/dump/info
<html>
<head>
    <title>Jetty Demo</title>
    <meta http-equiv="Pragma" content="no-cache">
    <meta http-equiv="Cache-Control" content="no-cache,no-store">
    <link rel="stylesheet" href="demo.css"/>
</head>
<body>
    <div class="topnav">
      <a class="menu" href="http://localhost:8080/">Demo Home</a>
      <a class="menu" href="https://github.com/jetty/jetty.project/tree/jetty-12.0.x/jetty-ee10/jetty-ee10-demos/jetty-ee10-demo-jetty-webapp/">Source</a>
      <a class="menu" href="https://jetty.org/">Jetty Project Home</a>
      <a class="menu" href="https://jetty.org/docs/">Documentation</a>
      <a class="menu" href="https://webtide.com">Commercial Support</a>
    </div>

    <div class="content">
      <h1>Welcome to Jetty 11 - REMOTE ACCESS!!</h1>
      <p>
       This is a demo webapp for the Eclipse Jetty HTTP Server and Servlet Container.
      </p>
      <p>
       This test context serves several demo filters and servlets that are not safe for deployment on the internet, since (by design) they contain cross domain scripting vulnerabilities and reveal private information.  This page is displayed because you have accessed this context from a non local IP address.
      </p>
      <p>
       You can disable the remote address checking by editing demo-base/webapps/demo-jetty.d/demo-jetty-override-web.xml, uncommenting the declaration of the TestFilter, and changing the "remote" init parameter to "true".
      </p>
    </div>

    <div class="footer">
      <center><a href="https://jetty.org"><img style="border:0" src="small_powered_by.gif"/></a></center>
    </div>

</body>
</html>

Oops, the existing org.example.TestFilter present in that WAR is preventing remote access (as designed).

That filter is present in the WebApp due to a dynamic registration.

jetty.project/jetty-ee10/jetty-ee10-demos/jetty-ee10-demo-jetty-webapp/src/main/java/org/example/TestListener.java

Lines 149 to 162 in a171ad3

	FilterRegistration registration = sce.getServletContext().addFilter("TestFilter", TestFilter.class.getName());
	if (registration != null) //otherwise defined in web.xml
	{
	((FilterRegistration.Dynamic)registration).setAsyncSupported(true);
	}
	else
	{
	registration = sce.getServletContext().getFilterRegistration("TestFilter");
	}
	registration.setInitParameter("remote", "false");
	registration.addMappingForUrlPatterns(
	EnumSet.of(DispatcherType.ERROR, DispatcherType.ASYNC, DispatcherType.FORWARD, DispatcherType.INCLUDE, DispatcherType.REQUEST),
	true,
	new String[]{"/*"});

But we can override it with XML, there's a snippet that's commented out that we can just enable for that.

[joakim@hyperion 12.0.16-demo]$ grep -A11 "Allow remote" webapps/ee10-demo-jetty.d/ee10-demo-jetty-override-web.xml 
  <!-- Allow remote access to test webapp -->
  <!--
  <filter>
    <filter-name>TestFilter</filter-name>
    <filter-class>org.example.TestFilter</filter-class>
    <async-supported>true</async-supported>
    <init-param>
      <param-name>remote</param-name>
      <param-value>true</param-value>
    </init-param>
  </filter>
  -->
[joakim@hyperion 12.0.16-demo]$ edit webapps/ee10-demo-jetty.d/ee10-demo-jetty-override-web.xml 
[joakim@hyperion 12.0.16-demo]$ grep -A11 "Allow remote" webapps/ee10-demo-jetty.d/ee10-demo-jetty-override-web.xml 
  <!-- Allow remote access to test webapp -->
  <filter>
    <filter-name>TestFilter</filter-name>
    <filter-class>org.example.TestFilter</filter-class>
    <async-supported>true</async-supported>
    <init-param>
      <param-name>remote</param-name>
      <param-value>true</param-value>
    </init-param>
  </filter>
  
</web-app>

Restart the server and try again ...

[joakim@hyperion 12.0.16-demo]$ curl --no-progress-meter -H "X-Forwarded-For: 10.9.8.7" http://localhost:8080/ee10-test/dump/info | grep getRemote
<th align="right">getRemoteAddr:&nbsp;</th><td>10.9.8.7</td></tr><tr>
<th align="right">getRemoteHost:&nbsp;</th><td>10.9.8.7</td></tr><tr>
<th align="right">getRemotePort:&nbsp;</th><td>43156</td></tr><tr>
<th align="right">getRemoteUser:&nbsp;</th><td>user</td></tr><tr>

Yup it works, even on the 12.0.16 release.