spring-cloud-gateway icon indicating copy to clipboard operation
spring-cloud-gateway copied to clipboard

Gateway uses X-Forward-* headers for routing when Cloud plattform is present

Open capnrotbart opened this issue 11 months ago • 4 comments

TL;DR: When a Spring Gateway application detects that a cloud platform is present the forwarding behavior changes in potentially a dangerous way.

The bug We recently upgraded our Spring Cloud Gateway based API gateway from Spring Boot 3.2 / Spring Cloud 2023 to Spring Boot 3.4 / Spring Cloud 2024. For some requests the gateway resides behind another proxy that does some mild path rewriting and being a nice proxy sets these headers:

x-forwarded-prefix: /something-external
x-forwarded-scheme: https
x-forwarded-port: 443
x-forwarded-for: 123.123.23.2

After the upgrade I noticed those requests failing, as they were being rewritten in a strange manner, e.g. https://example.com/something-external/my-service/get/things should be rewritten to https://my-service/get/things but instead where changed to https://my-service/something-external/get/things and would 404.

https://example.com/something-external/my-service/get/things
https://my-service/get/things <- old behavior
https://my-service/something-external/get/things <- 404

Some testing showed that I could also set the x-forwarded-host header to coerce the gateway to contact outside servers. Spooky.

More digging revealed that this error was caused by the NettyWebServerFactoryCustomizer deciding to enable special forward header treatment when the presence of a cloud-plattform is detected (with isUsingForwardHeaders(), which all currently are). The DefaultHttpForwardedHeaderHandler would then rewrite the request URI to to a new host and path.

In total I found three code paths through which this would be enabled: Either straight through the ForwardedHeaderTransformer and server.forward-headers-strategy=framework, or via the NettyWebServerFactoryCustomizer setting setUseForwardHeaders to true when getOrDeduceUseForwardHeaders detected a cloud platform or the server.forward-headers-strategy=native. Only explicitly setting server.forward-headers-strategy=none protected us from this. Noteworthy, the default config is also dangerous if you use any cloud platform integration and it detects a cloud platform.

Why is this bad? For one, I could not find any mention of a behavior change in the documentation, so at minimum this is a documentation issue. Then, at least to me personally, this is an entirely unexpected behavior. Both the forwarding and that it engages only in some circumstances. The most worrying part is the rewrite of the host through the x-forwarded-host header, which opens up some potential attacks.

What should change?

  • Documentation
  • The default should never allow rewriting URIs by X-Forwarded headers
  • ???

Workarounds Obviously setting the server.forward-headers-strategy=none prevents this issue. Disabling spring.cloud.gateway.x-forwarded.* may work (not tested). Not setting the X-Forwarded headers also prevents this (duh). And not letting the X-Forwarded headers into your backend is probably a good practice anyway.

Versions affected

  • Spring Boot 3.4.1 / Cloud 2024.0.0: Present
  • Spring Boot 3.3.7 / Cloud 2023.0.5: No, dangerous only when explicitly setting server.forward-headers-strategy=framework.
  • Spring Boot 3.2.0 / Cloud 2023.0.5: No, dangerous only when explicitly setting server.forward-headers-strategy=framework.

Sample I wrote tests covering all configurations that seemed susceptible to this. The bad ones will fail for "evil" requests.

Image

package org.example.gatewayforwardrepo;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.cloud.gateway.route.RouteLocator;
import org.springframework.cloud.gateway.route.builder.RouteLocatorBuilder;
import org.springframework.context.annotation.Bean;
import org.springframework.http.HttpStatus;
import org.springframework.web.client.RestClient;

import static org.junit.jupiter.api.Assertions.*;

@SpringBootApplication
public class GatewayForwardRepoApplication {

    @Bean
    public RouteLocator customRouteLocator(RouteLocatorBuilder builder) {
        return builder
                .routes()
                .route(
                        "sample",
                        r -> r
                                .path("/get")
                                .uri("https://httpbin.org")
                )
                .build();
    }

    public static void main(String[] args) {
        SpringApplication.run(GatewayForwardRepoApplication.class, args);
    }

}

abstract class GatewayForwardRepoApplicationBaseTest {

    @LocalServerPort
    private int port;

    private String uri;

    @BeforeEach
    void setUp() {
        uri = "http://localhost:" + port + "/get?value=a+nice+response";
    }

    @Test
    void niceRequest() {
        var result = RestClient
                .create()
                .get()
                .uri(uri)
                .retrieve()
                .toEntity(String.class);

        assertEquals(HttpStatus.OK, result.getStatusCode(), "Body\n%s".formatted(result.getBody()));
        assertNotNull(result.getBody());
        assertTrue(result.getBody().contains("a nice response"), "Contained instead\n%s".formatted(result.getBody()));
    }

    @Test
    void evilRequest() {
        var result = RestClient
                .create()
                .get()
                .uri(uri)
                .header("X-Forwarded-For", "192.123.23.2")
                .header("X-Forwarded-Host", "example.org")
                .header("X-Forwarded-Port", "443")
                .header("X-Forwarded-Proto", "https")
                .header("X-Forwarded-Prefix", "/evil")
                .retrieve()
                .toEntity(String.class);

        assertEquals(HttpStatus.OK, result.getStatusCode(), "Body\n%s".formatted(result.getBody()));
        assertNotNull(result.getBody());
        assertTrue(result.getBody().contains("a nice response"), "Body\n%s".formatted(result.getBody()));
    }

}

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
class DefaultSettingsTest extends GatewayForwardRepoApplicationBaseTest {
    // Both will succeed
}

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
                properties = "server.forward-headers-strategy=framework")
class ForwardHeadersStrategyFrameworkTest extends GatewayForwardRepoApplicationBaseTest {
    // One will fail
}

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
                properties = "server.forward-headers-strategy=none")
class ForwardHeadersStrategyNoneTest extends GatewayForwardRepoApplicationBaseTest {
    // Both will succeed
}

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
                properties = "server.forward-headers-strategy=native")
class ForwardHeadersStrategyNativeTest extends GatewayForwardRepoApplicationBaseTest {
    // One will fail
}

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
                properties = "spring.main.cloud-platform=kubernetes")
class CloudPlattformK8sTest extends GatewayForwardRepoApplicationBaseTest {
    // One will fail
}

capnrotbart avatar Jan 22 '25 14:01 capnrotbart

While server.forward-headers-strategy=none worked for us in most cases, it still breaks things when we have a gateway running as a proxy between frontend and backend with Spring Security and CORS, so it's not ideal. Same issue mentioned here - https://github.com/spring-cloud/spring-cloud-gateway/issues/3662

Airidas36 avatar Jan 24 '25 07:01 Airidas36

@Airidas36 what do you think gateway should do here?

spencergibb avatar Jan 31 '25 18:01 spencergibb

@Airidas36 what do you think gateway should do here?

I do agree with @capnrotbart that this behavior should definitely be documented. I would expect that by default no rewrite/re-route logic is applied even if those x-forwarded-* headers are present because this is a breaking change for all spring services running in cloud platforms behind load balancers, so every service needs to be reconfigured with server.forward-headers-strategy=none. Having this rewrite/re-route behavior as a configurable property which is toggled off by default would make more sense I think, this feature could be toggled on if the need arises, having it disabled by default would also not break anything as the behaviour would be consistent with previous versions.

Airidas36 avatar Feb 06 '25 09:02 Airidas36

I would expect that by default no rewrite/re-route logic is applied even if those x-forwarded-* headers are present because this is a breaking change

@Airidas36 gateway is not doing any rewriting or rerouting. reactor-netty is doing this before reaching any gateway specific code. See the conversations below.

https://github.com/reactor/reactor-netty/issues/3432

https://github.com/spring-projects/spring-boot/issues/43577

spencergibb avatar Mar 31 '25 17:03 spencergibb