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.

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/handler/predicate/RemoteAddrRoutePredicateFactory.java

@@ -25,10 +25,12 @@ import java.util.List;
 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 class RemoteAddrRoutePredicateFactory extends AbstractRoutePredicateFacto
         List<IpSubnetFilterRule> 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 class RemoteAddrRoutePredicateFactory extends AbstractRoutePredicateFacto
 		@NotEmpty
 		private List<String> sources = new ArrayList<>();
 
+		@NotNull
+		private RemoteAddressResolver remoteAddressResolver = new DefaultRemoteAddressResolver();
+
 		public List<String> getSources() {
 			return sources;
 		}
@@ -123,5 +128,10 @@ public class RemoteAddrRoutePredicateFactory extends AbstractRoutePredicateFacto
 			return this;
 		}
 
+
+		public Config setRemoteAddressResolver(RemoteAddressResolver remoteAddressResolver) {
+			this.remoteAddressResolver = remoteAddressResolver;
+			return this;
+		}
 	}
 }

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();
+	}
+}

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);
+}

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 <a href=
+ * "https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For">X-Forwarded-For
+ * reference</a>
+ * @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
+	 * <em>trusted</em> 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]:
+	 * 
+	 * <pre>
+	 * 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
+	 * </pre>
+	 *
+	 * @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<String> 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<String> extractXForwardedValues(ServerWebExchange exchange) {
+		List<String> 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<String> values = Arrays.asList(xForwardedValues.get(0).split(", "));
+		if (values.size() == 1 && Strings.isBlank(values.get(0))) {
+			return Collections.emptyList();
+		}
+		return values;
+	}
+}

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());
+	}
+}