Spring-cloud-netflix: Redirects from backend service to internal IP

[sfussenegger]

The headers X-Forwarded-* headers set in PreDecorationFilter are ignored by default spring-boot services built on Tomcat. As a consequence, redirects to absolute URLs (due to context.setUseRelativeRedirects(false) in TomcatEmbeddedServletContainerFactory) will go to the value in Host rather than X-Forwarded-Host.

For instance, this happens with server.contextPath: /foo and a request to curl -H 'X-Forwarded-Host: api.example.com' http://127.0.0.1/foo with a redirect to http://127.0.0.1/foo/ (due to context.setMapperContextRootRedirectEnabled(true) in TomcatEmbeddedServletContainerFactory)

I think there are 2 workaround.

1) proxy: use a ZuulFilter to set Host additionally to X-Forwarded-Host

2) client: add a Valve to override the server name:

    @Bean
    public EmbeddedServletContainerCustomizer customizer() {
        return (final ConfigurableEmbeddedServletContainer container) -> {

            final TomcatEmbeddedServletContainerFactory tomcat = (TomcatEmbeddedServletContainerFactory) container;

            tomcat.addContextCustomizers((final Context context) -> {
                context.setMapperContextRootRedirectEnabled(false);
            });

            tomcat.addContextValves(new ValveBase() {

                @Override
                public void invoke(final Request request, final Response response) throws IOException, ServletException {

                    final MessageBytes serverNameMB = request.getCoyoteRequest().serverName();
                    String originalServerName = null;
                    final String forwardedHost = request.getHeader("X-Forwarded-Host");
                    if (forwardedHost != null) {
                        originalServerName = serverNameMB.getString();
                        serverNameMB.setString(forwardedHost);
                    }

                    int originalPort = -1;
                    final String forwardedPort = request.getHeader("X-Forwarded-Port");
                    if (forwardedPort != null) {
                        try {
                            originalPort = request.getServerPort();
                            request.setServerPort(Integer.valueOf(forwardedPort));
                        } catch (final NumberFormatException e) {
                            log.debug("ignoring forwarded port {}", forwardedPort);
                        }
                    }

                    try {
                        getNext().invoke(request, response);
                    } finally {
                        if (forwardedHost != null) {
                            serverNameMB.setString(originalServerName);
                        }
                        if (forwardedPort != null) {
                            request.setServerPort(originalPort);
                        }
                    }
                }
            });
        };
    }

Note: requires disabling mapperContextRootRedirectEnabled for Valve processing to kick in in order to work with the example above

[dsyer]

You shouldn't have to do anything if the backend is aware of the proxy headers (http://docs.spring.io/spring-boot/docs/current-SNAPSHOT/reference/htmlsingle/#howto-use-tomcat-behind-a-proxy-server).

[sfussenegger]

@dsyer please correct me if I'm wrong but from documentation and code it seems that server.use-forward-headers only enables Tomcat's RemoteIpValve which "replaces the apparent client remote IP address and hostname for the request with the IP address list presented by a proxy or a load balancer via a request headers (e.g. X-Forwarded-For)".

So by default, it only checks X-Forwarded-For and X-Forwarded-By and is configured to check X-Forwarded-Proto and X-Forwarded-Port as well.

It does however not check X-Forwarded-Host. I actually can't find any mention of this header in either spring-boot or Tomcat code.

[sfussenegger]

Ok, just found spring-projects/spring-boot#2603 which came to the conclusion that all spring libraries should build their own support for X-Forwarded-Host. It also mentions a pending Tomcat issue that nobody seems interested in. So basically spring libraries are relying on a feature that isn't implemented in Tomcat.

I discovered this issue when Tomcat was redirecting to a context (/foo to /foo/) using an absolute redirect (which is configured as default by spring-boot) without any involvement of other libraries than Tomcat. So this simply can't work as designed.

As long as Tomcat does not support X-Forwarded-Host out of the box it's possibly best to either set Host using a ZuulFitler or configure the Valve posted above (minus the X-Forwarded-Port part which is handled by Tomcat):

    @Bean
    public EmbeddedServletContainerCustomizer customizer() {
        return (final ConfigurableEmbeddedServletContainer container) -> {

            final TomcatEmbeddedServletContainerFactory tomcat = (TomcatEmbeddedServletContainerFactory) container;

            tomcat.addContextCustomizers((final Context context) -> {
                context.setMapperContextRootRedirectEnabled(false);
            });

            tomcat.addContextValves(new ValveBase() {

                @Override
                public void invoke(final Request request, final Response response) throws IOException, ServletException {

                    final MessageBytes serverNameMB = request.getCoyoteRequest().serverName();
                    String originalServerName = null;
                    final String forwardedHost = request.getHeader("X-Forwarded-Host");
                    if (forwardedHost != null) {
                        originalServerName = serverNameMB.getString();
                        serverNameMB.setString(forwardedHost);
                    }

                    try {
                        getNext().invoke(request, response);
                    } finally {
                        if (forwardedHost != null) {
                            serverNameMB.setString(originalServerName);
                        }
                    }
                }
            });
        };
    }

[dsyer]

I think the "standard" proxy behaviour is to set the Host header, so maybe that's what we should do in Zuul. I don't think there's anything else that we could usefully do here. Is that correct?

[sfussenegger]

Basically yes, because support for X-Forwarded-Host is unreliable. That's true for spring-boot apps and even more so for everything else. So the header name should probably be configurable.

[sfussenegger]

One more thing: Jetty accepts the X-Forwarded-Host header when configured with server.use-forward-headers. It does however ignore the X-Forwarded-Port header as it expects the port as part of the host header (i.e. the same format as the regular Host header which is compatible with the Forwarded HTTP Extension RFC). As a consequence, it's not possible to use non-default proxy ports with Jetty.

So there are obviously subtle differences in the implementations of proxy header handling in Tomcat and Jetty, adding to the mess here. Therefore I'd suggest that headers should even be configurable on a per-route basis.

[dsyer]

We can't just set the "Host" header unconditionally, and I don't think it should be the default either, because anyone running in a platform (Heroku, CloudFoundry, many others including in-house built environments) will need to have the "Host" header set to the target in order to route the request correctly. I think what we have works in the sense that it adds new information and doesn't take anything away, so the backend has to be able to figure it out. The fact that the existing server components don't do that and you have to rely on filters and custom configuration is regrettable, but from a Spring Cloud point of view it still just comes down to documentation. If you need to set another header it's easy in a ZuulFilter.

[sfussenegger]

I agree with you that Host shouldn't be set by default. I'd still recommend the following minor changes to simplify the issues:

1. add zuul.add-host-header property to set Host header
2. add port to X-Forwarded-Host as defined by RFC 7239
3. extract PreDecorationFilter.filterOrder() into a public constant so custom filters don't have to rely on a magic number to run before or after

[dsyer]

I'm not sure we should set the Host header at all, so we'll pass on 1. Good idea on 2. and 3., thanks.

[sfussenegger]

I was already working on the PR before I've read your comment, so I've already implemented Host support. Let me know in the PR comments section what else to change and I'll fix it all at once