From d79534409bb6d3fd43a0067832fcfb41bc4da346 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Mon, 19 Mar 2018 17:01:53 -0400 Subject: [PATCH] Update `RemoteAddrRoutePredicateFactory` to optionally respect the `X-Forwarded-For` header. Addresses #155. (#156) * Update `RemoteAddrRoutePredicateFactory` to optionally respect the `X-Forwarded-For` header. Fixes gh-155. * Make function to extract client IP configurable. Add additional method for extracting client IP with is safer from spoofing of X-Forwarded-For header. --- .../RemoteAddrRoutePredicateFactory.java | 14 +- .../DefaultRemoteAddressResolver.java | 16 ++ .../ipresolver/RemoteAddressResolver.java | 13 ++ .../XForwardedRemoteAddressResolver.java | 117 +++++++++++++++ .../XForwardedRemoteAddressResolverTest.java | 138 ++++++++++++++++++ 5 files changed, 296 insertions(+), 2 deletions(-) create mode 100644 spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/DefaultRemoteAddressResolver.java create mode 100644 spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/RemoteAddressResolver.java create mode 100644 spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/XForwardedRemoteAddressResolver.java create mode 100644 spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/support/ipresolver/XForwardedRemoteAddressResolverTest.java diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/handler/predicate/RemoteAddrRoutePredicateFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/handler/predicate/RemoteAddrRoutePredicateFactory.java index f8cd892fd3..de0750aee5 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/handler/predicate/RemoteAddrRoutePredicateFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/handler/predicate/RemoteAddrRoutePredicateFactory.java @@ -25,10 +25,12 @@ import java.util.function.Predicate; import javax.validation.constraints.NotEmpty; +import javax.validation.constraints.NotNull; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.jetbrains.annotations.NotNull; +import org.springframework.cloud.gateway.support.ipresolver.DefaultRemoteAddressResolver; +import org.springframework.cloud.gateway.support.ipresolver.RemoteAddressResolver; import org.springframework.validation.annotation.Validated; import org.springframework.web.server.ServerWebExchange; @@ -72,7 +74,7 @@ public Predicate apply(Config config) { List sources = convert(config.sources); return exchange -> { - InetSocketAddress remoteAddress = exchange.getRequest().getRemoteAddress(); + InetSocketAddress remoteAddress = config.remoteAddressResolver.resolve(exchange); if (remoteAddress != null) { String hostAddress = remoteAddress.getAddress().getHostAddress(); String host = exchange.getRequest().getURI().getHost(); @@ -109,6 +111,9 @@ public static class Config { @NotEmpty private List sources = new ArrayList<>(); + @NotNull + private RemoteAddressResolver remoteAddressResolver = new DefaultRemoteAddressResolver(); + public List getSources() { return sources; } @@ -123,5 +128,10 @@ public Config setSources(String... sources) { return this; } + + public Config setRemoteAddressResolver(RemoteAddressResolver remoteAddressResolver) { + this.remoteAddressResolver = remoteAddressResolver; + return this; + } } } diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/DefaultRemoteAddressResolver.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/DefaultRemoteAddressResolver.java new file mode 100644 index 0000000000..63e82e70c8 --- /dev/null +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/DefaultRemoteAddressResolver.java @@ -0,0 +1,16 @@ +package org.springframework.cloud.gateway.support.ipresolver; + +import java.net.InetSocketAddress; + +import org.springframework.web.server.ServerWebExchange; + +/** + * @author Andrew Fitzgerald + */ +public class DefaultRemoteAddressResolver implements RemoteAddressResolver { + + @Override + public InetSocketAddress resolve(ServerWebExchange exchange) { + return exchange.getRequest().getRemoteAddress(); + } +} diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/RemoteAddressResolver.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/RemoteAddressResolver.java new file mode 100644 index 0000000000..4bb630f9bc --- /dev/null +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/RemoteAddressResolver.java @@ -0,0 +1,13 @@ +package org.springframework.cloud.gateway.support.ipresolver; + +import java.net.InetSocketAddress; + +import org.springframework.web.server.ServerWebExchange; + +/** + * @author Andrew Fitzgerald + */ +public interface RemoteAddressResolver { + + InetSocketAddress resolve(ServerWebExchange exchange); +} diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/XForwardedRemoteAddressResolver.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/XForwardedRemoteAddressResolver.java new file mode 100644 index 0000000000..e36e2c5f38 --- /dev/null +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ipresolver/XForwardedRemoteAddressResolver.java @@ -0,0 +1,117 @@ +package org.springframework.cloud.gateway.support.ipresolver; + +import java.net.InetSocketAddress; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.apache.logging.log4j.util.Strings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.util.Assert; +import org.springframework.web.server.ServerWebExchange; + +/** + * Parses the client address from the X-Forwarded-For header. If header is not present, + * falls back to {@link DefaultRemoteAddressResolver} and + * {@link ServerHttpRequest#getRemoteAddress()}. Use the static constructor methods which + * meets your security requirements. + * + * @see X-Forwarded-For + * reference + * @author Andrew Fitzgerald + */ +public class XForwardedRemoteAddressResolver implements RemoteAddressResolver { + + public static final String X_FORWARDED_FOR = "X-Forwarded-For"; + private static final Logger log = LoggerFactory + .getLogger(XForwardedRemoteAddressResolver.class); + private final DefaultRemoteAddressResolver defaultRemoteIpResolver = new DefaultRemoteAddressResolver(); + + private final int maxTrustedIndex; + + private XForwardedRemoteAddressResolver(int maxTrustedIndex) { + this.maxTrustedIndex = maxTrustedIndex; + } + + /** + * @return a {@link XForwardedRemoteAddressResolver} which always extracts the first + * IP address found in the X-Forwarded-For header (when present). Equivalent to + * calling {@link #maxTrustedIndexXForwardedRemoteAddressResolver(int)} with a + * {@link #maxTrustedIndex} of {@link Integer#MAX_VALUE}. This configuration is + * vulnerable to spoofing via manually setting the X-Forwarded-For header. If the + * resulting IP address is used for security purposes, use + * {@link #maxTrustedIndexXForwardedRemoteAddressResolver(int)} instead. + */ + public static XForwardedRemoteAddressResolver trustAllXForwardedRemoteAddressResolver() { + return new XForwardedRemoteAddressResolver(Integer.MAX_VALUE); + } + + /** + * @return a {@link XForwardedRemoteAddressResolver} which extracts the last + * trusted IP address found in the X-Forwarded-For header (when present). + * This configuration exists to prevent a malicious actor from spoofing the value of + * the X-Forwarded-For header. If you know that your gateway application is only + * accessible from a a trusted load balancer, then you can trust that the load + * balancer will append a valid client IP address to the X-Forwarded-For header, and + * should use a value of `1` for the `maxTrustedIndex`. + * + * + * Given the X-Forwarded-For value of [0.0.0.1, 0.0.0.2, 0.0.0.3]: + * + * 
+	 * maxTrustedIndex -> result
+	 *
+	 * [MIN_VALUE,0] -> IllegalArgumentException
+	 * 1 -> 0.0.0.3
+	 * 2 -> 0.0.0.2
+	 * 3 -> 0.0.0.1
+	 * [4, MAX_VALUE] -> 0.0.0.1
+	 *
+ * + * @param maxTrustedIndex correlates to the number of trusted proxies expected in + * front of Spring Cloud Gateway (index starts at 1). + */ + public static XForwardedRemoteAddressResolver maxTrustedIndexXForwardedRemoteAddressResolver( + int maxTrustedIndex) { + Assert.isTrue(maxTrustedIndex > 0, "An index greater than 0 is required"); + return new XForwardedRemoteAddressResolver(maxTrustedIndex); + } + + /** + * The X-Forwarded-For header contains a comma separated list of IP addresses. This + * method parses those IP addresses into a list. If no X-Forwarded-For header is + * found, an empty list is returned. If multiple X-Forwarded-For headers are found, an + * empty list is returned out of caution. + * @return The parsed values of the X-Forwarded-Header. + */ + @Override + public InetSocketAddress resolve(ServerWebExchange exchange) { + List xForwardedValues = extractXForwardedValues(exchange); + Collections.reverse(xForwardedValues); + if (xForwardedValues.size() != 0) { + int index = Math.min(xForwardedValues.size(), maxTrustedIndex) - 1; + return InetSocketAddress.createUnresolved(xForwardedValues.get(index), 0); + } + return defaultRemoteIpResolver.resolve(exchange); + } + + private List extractXForwardedValues(ServerWebExchange exchange) { + List xForwardedValues = exchange.getRequest().getHeaders() + .get(X_FORWARDED_FOR); + if (xForwardedValues == null || xForwardedValues.size() == 0) { + return Collections.emptyList(); + } + if (xForwardedValues.size() > 1) { + log.warn("Multiple X-Forwarded-For headers found, discarding all"); + return Collections.emptyList(); + } + List values = Arrays.asList(xForwardedValues.get(0).split(", ")); + if (values.size() == 1 && Strings.isBlank(values.get(0))) { + return Collections.emptyList(); + } + return values; + } +} diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/support/ipresolver/XForwardedRemoteAddressResolverTest.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/support/ipresolver/XForwardedRemoteAddressResolverTest.java new file mode 100644 index 0000000000..b851581ca4 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/support/ipresolver/XForwardedRemoteAddressResolverTest.java @@ -0,0 +1,138 @@ +package org.springframework.cloud.gateway.support.ipresolver; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.InetSocketAddress; + +import org.junit.Test; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; + +public class XForwardedRemoteAddressResolverTest { + + private final InetSocketAddress remote0000Address = InetSocketAddress + .createUnresolved("0.0.0.0", 1234); + + private final XForwardedRemoteAddressResolver trustOne = XForwardedRemoteAddressResolver + .maxTrustedIndexXForwardedRemoteAddressResolver(1); + + private final XForwardedRemoteAddressResolver trustAll = XForwardedRemoteAddressResolver + .trustAllXForwardedRemoteAddressResolver(); + + @Test + public void maxIndexOneReturnsLastForwardedIp() { + ServerWebExchange exchange = buildExchange(oneTwoThreeBuilder()); + + InetSocketAddress address = trustOne.resolve(exchange); + + assertThat(address.getHostName()).isEqualTo("0.0.0.3"); + } + + @Test + public void maxIndexOneFallsBackToRemoteIp() { + ServerWebExchange exchange = buildExchange(remoteAddressOnlyBuilder()); + + InetSocketAddress address = trustOne.resolve(exchange); + + assertThat(address.getHostName()).isEqualTo("0.0.0.0"); + } + + @Test + public void maxIndexOneReturnsNullIfNoForwardedOrRemoteIp() { + ServerWebExchange exchange = buildExchange(emptyBuilder()); + + InetSocketAddress address = trustOne.resolve(exchange); + + assertThat(address).isEqualTo(null); + } + + @Test + public void trustOneFallsBackOnEmptyHeader() { + ServerWebExchange exchange = buildExchange( + remoteAddressOnlyBuilder().header("X-Forwarded-For", "")); + + InetSocketAddress address = trustOne.resolve(exchange); + + assertThat(address.getHostName()).isEqualTo("0.0.0.0"); + + } + + @Test + public void trustOneFallsBackOnMultipleHeaders() { + ServerWebExchange exchange = buildExchange( + remoteAddressOnlyBuilder().header("X-Forwarded-For", "0.0.0.1") + .header("X-Forwarded-For", "0.0.0.2")); + + InetSocketAddress address = trustOne.resolve(exchange); + + assertThat(address.getHostName()).isEqualTo("0.0.0.0"); + } + + @Test + public void trustAllReturnsFirstForwardedIp() { + ServerWebExchange exchange = buildExchange(oneTwoThreeBuilder()); + + InetSocketAddress address = trustAll.resolve(exchange); + + assertThat(address.getHostName()).isEqualTo("0.0.0.1"); + } + + @Test + public void trustAllFinalFallsBackToRemoteIp() { + ServerWebExchange exchange = buildExchange(remoteAddressOnlyBuilder()); + + InetSocketAddress address = trustAll.resolve(exchange); + + assertThat(address.getHostName()).isEqualTo("0.0.0.0"); + } + + @Test + public void trustAllReturnsNullIfNoForwardedOrRemoteIp() { + ServerWebExchange exchange = buildExchange(emptyBuilder()); + + InetSocketAddress address = trustAll.resolve(exchange); + + assertThat(address).isEqualTo(null); + } + + @Test + public void trustAllFallsBackOnEmptyHeader() { + ServerWebExchange exchange = buildExchange( + remoteAddressOnlyBuilder().header("X-Forwarded-For", "")); + + InetSocketAddress address = trustAll.resolve(exchange); + + assertThat(address.getHostName()).isEqualTo("0.0.0.0"); + + } + + @Test + public void trustAllFallsBackOnMultipleHeaders() { + ServerWebExchange exchange = buildExchange( + remoteAddressOnlyBuilder().header("X-Forwarded-For", "0.0.0.1") + .header("X-Forwarded-For", "0.0.0.2")); + + InetSocketAddress address = trustAll.resolve(exchange); + + assertThat(address.getHostName()).isEqualTo("0.0.0.0"); + } + + private MockServerHttpRequest.BaseBuilder emptyBuilder() { + return MockServerHttpRequest.get("someUrl"); + } + + private MockServerHttpRequest.BaseBuilder remoteAddressOnlyBuilder() { + return MockServerHttpRequest.get("someUrl").remoteAddress(remote0000Address); + } + + private MockServerHttpRequest.BaseBuilder oneTwoThreeBuilder() { + return MockServerHttpRequest.get("someUrl").remoteAddress(remote0000Address) + .header("X-Forwarded-For", "0.0.0.1, 0.0.0.2, 0.0.0.3"); + } + + private ServerWebExchange buildExchange( + MockServerHttpRequest.BaseBuilder requestBuilder) { + return MockServerWebExchange.from(requestBuilder.build()); + } +} \ No newline at end of file