Impl Check: ServletRequest.getRemoteAddr and intermediaries

[edburns]

Hello Experts,

I have a customer with a question about ServletRequest.getRemoteAddr. Consider the following network layout.

Browser
   |
  \|/
CloudFlare
   |
  \|/
----Azure-boundary-----
NGINX A
   |
  \|/
NGINX B
   |
  \|/
Tomcat (Spring Cloud)

Please confirm that getRemoteAddr in the tomcat should return the CloudFlare not the NGINX B.

Thanks,

Ed

[edburns]

#Question

[jasonwee]

should be nginx b, but this should be easy to test if you are not sure?

and if you want to pass the cloudflare ip, what is stopping you by setting cloudflare ip in the http header?

[markt-asf]

From a pure Servlet spec perspective the Servlet container will see NGINX as the user agent. There are various ways to make the reverse proxies transparent so Servlet container sees the "real" user agent but they are all outside of the Servlet spec. The support channel for your Servlet container of choice is the place to see container specific advice.

[edburns]

This method was present in Servlet 2.3, in 2002. Back then, there was
no such thing as PaaS. The Servlet container could reasonably be
expected to be the service boundary. Flip forward 18 years and
running a Servlet container as a PaaS and the user expectation is still
the same. The Servlet container should be seen as the service boundary.

I am not contesting your closing this, but I do wonder what you think is
the right way for Servlet containers to behave when they are part of a
cloud PaaS with respect to interacting with things outside the PaaS
service boundary.

What do you think @markt-asf Mark?

Ed

[gregw]

Note that there are several variations of defacto standards for dealing with intermediaries: X-Forwarded-For headers etc.
However, there is now a good standard for such things in RFC7239 and most servers should implement that.

Perhaps the spec could encourage that to be supported as too many clients/intermediaries/servers are still futzing with the old badly defined X-Some-Proxies-Do-This headers.

Currently the javadoc for getRemoteAddr says:

    /**
     * Returns the Internet Protocol (IP) address of the client 
     * or last proxy that sent the request.
     * For HTTP servlets, same as the value of the 
     * CGI variable <code>REMOTE_ADDR</code>.
     *
     * @return a <code>String</code> containing the 
     * IP address of the client that sent the request
     */

So it is not even internally consistent in the description of the value returned and does not allow for the server to do such things as RFC7239.
Perhaps the javadoc could be improved to something like:

    /**
     * Returns the Internet Protocol (IP) address of the 
     * remote source of the request.  Depending on the server
     * configuration this may be of <ul>
     *   <li>the originating client; </li>
     *   <li>the last intermediary in the connection that delivered the request;</li>
     *   <li>the address provided by a protocol specific mechanism 
     *       such as <a href="https://tools.ietf.org/html/rfc7239">RFC7239</a>.</li>
     * <ul>
     * @return a <code>String</code> containing the 
     * IP address of the remote source of the request.
     */

So I'll reopen and depending on feedback may do a PR (or Ed feel free to do a PR if I don't get around to it).

[markt-asf]

To Ed's point, HTTP reverse proxies have been a common feature of Servlet container deployments from the beginning. Handling the scenario where the service boundary is the other side of one or more reverse proxies is nothing new. PaaS moves the hardware to someone else's server room but doesn't change the problem.

"Right way to behave" depends on your point of view and/or the requirements of any given situation.

I've no objection to clarifying the text. I'd add some text to Greg's proposal to specify what the default behaviour is expected to be in the absence of container specific configuration. Something like "The default behaviour is to return the IP address of the source of the IP connection (whether user agent or intermediary) to the Servlet container.". I'm sure that can be improved but you get the idea.

A separate question - probably for the Servlet 6.0 timescale - is should the Servlet spec require containers to support RFC 7239? I'm currently leaning towards making it a recommendation rather than a requirement.

[stuartwdouglas]

I think it should be a recommendation rather than a requirement. It's definitely not something that should be enabled by default as it has security implications if it is enabled and it is possible for the remote clients to craft the Forwarded header (e.g. you are deployed behind an older reverse proxy that does not know about Forwarded, a remote client adds its own Forwarded to pretend to be coming from inside your corporate netword, and the old proxy lets it through because it does not know any better).