From 8888f75a41ff59784dfea6ea52d886ac45b1cefb Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 27 Nov 2019 14:30:08 +0100 Subject: [PATCH] Polish "Add dedicated namespace for RemoteIpValve properties" See gh-18489 --- .../autoconfigure/web/ServerProperties.java | 131 +++++++----------- .../TomcatWebServerFactoryCustomizer.java | 16 +-- .../web/ServerPropertiesTests.java | 6 +- ...TomcatWebServerFactoryCustomizerTests.java | 28 ++-- .../src/main/asciidoc/howto.adoc | 10 +- 5 files changed, 80 insertions(+), 111 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java index 4c4ded84f25..b92f8bc217e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java @@ -300,43 +300,6 @@ public class ServerProperties { */ private final Accesslog accesslog = new Accesslog(); - /** - * Regular expression that matches proxies that are to be trusted. - */ - private String internalProxies = "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}|" // 10/8 - + "192\\.168\\.\\d{1,3}\\.\\d{1,3}|" // 192.168/16 - + "169\\.254\\.\\d{1,3}\\.\\d{1,3}|" // 169.254/16 - + "127\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}|" // 127/8 - + "172\\.1[6-9]{1}\\.\\d{1,3}\\.\\d{1,3}|" // 172.16/12 - + "172\\.2[0-9]{1}\\.\\d{1,3}\\.\\d{1,3}|172\\.3[0-1]{1}\\.\\d{1,3}\\.\\d{1,3}|" // - + "0:0:0:0:0:0:0:1|::1"; - - /** - * Header that holds the incoming protocol, usually named "X-Forwarded-Proto". - */ - private String protocolHeader; - - /** - * Value of the protocol header indicating whether the incoming request uses SSL. - */ - private String protocolHeaderHttpsValue = "https"; - - /** - * Name of the HTTP header used to override the original port value. - */ - private String portHeader = "X-Forwarded-Port"; - - /** - * Name of the HTTP header from which the remote IP is extracted. For instance, - * `X-FORWARDED-FOR`. - */ - private String remoteIpHeader; - - /** - * Name of the HTTP header from which the remote host is extracted. - */ - private String hostHeader = "X-Forwarded-Host"; - /** * Tomcat base directory. If not specified, a temporary directory is used. */ @@ -444,7 +407,7 @@ public class ServerProperties { /** * Remote Ip Valve configuration. */ - private final RemoteIpValve remoteIpValve = new RemoteIpValve(); + private final Remoteip remoteip = new Remoteip(); public int getMaxThreads() { return this.maxThreads; @@ -501,40 +464,58 @@ public class ServerProperties { this.basedir = basedir; } - @DeprecatedConfigurationProperty(replacement = "server.tomcat.remote-ip-valve.internal-proxies") + @DeprecatedConfigurationProperty(replacement = "server.tomcat.remoteip.internal-proxies") public String getInternalProxies() { - return this.remoteIpValve.getInternalProxies(); + return this.remoteip.getInternalProxies(); } public void setInternalProxies(String internalProxies) { - this.remoteIpValve.setInternalProxies(internalProxies); + this.remoteip.setInternalProxies(internalProxies); } - @DeprecatedConfigurationProperty(replacement = "server.tomcat.remote-ip-valve.protocol-header") + @DeprecatedConfigurationProperty(replacement = "server.tomcat.remoteip.protocol-header") public String getProtocolHeader() { - return this.remoteIpValve.getProtocolHeader(); + return this.remoteip.getProtocolHeader(); } public void setProtocolHeader(String protocolHeader) { - this.remoteIpValve.setProtocolHeader(protocolHeader); + this.remoteip.setProtocolHeader(protocolHeader); } - @DeprecatedConfigurationProperty(replacement = "server.tomcat.remote-ip-valve.protocol-header-https-value") + @DeprecatedConfigurationProperty(replacement = "server.tomcat.remoteip.protocol-header-https-value") public String getProtocolHeaderHttpsValue() { - return this.remoteIpValve.getProtocolHeaderHttpsValue(); + return this.remoteip.getProtocolHeaderHttpsValue(); } public void setProtocolHeaderHttpsValue(String protocolHeaderHttpsValue) { - this.remoteIpValve.setProtocolHeaderHttpsValue(protocolHeaderHttpsValue); + this.remoteip.setProtocolHeaderHttpsValue(protocolHeaderHttpsValue); } - @DeprecatedConfigurationProperty(replacement = "server.tomcat.remote-ip-valve.port-header") + @DeprecatedConfigurationProperty(replacement = "server.tomcat.remoteip.host-header") + public String getHostHeader() { + return this.remoteip.getHostHeader(); + } + + public void setHostHeader(String hostHeader) { + this.remoteip.setHostHeader(hostHeader); + } + + @DeprecatedConfigurationProperty(replacement = "server.tomcat.remote.port-header") public String getPortHeader() { - return this.remoteIpValve.getPortHeader(); + return this.remoteip.getPortHeader(); } public void setPortHeader(String portHeader) { - this.remoteIpValve.setPortHeader(portHeader); + this.remoteip.setPortHeader(portHeader); + } + + @DeprecatedConfigurationProperty(replacement = "server.tomcat.remoteip.remote-ip-header") + public String getRemoteIpHeader() { + return this.remoteip.getRemoteIpHeader(); + } + + public void setRemoteIpHeader(String remoteIpHeader) { + this.remoteip.setRemoteIpHeader(remoteIpHeader); } public Boolean getRedirectContextRoot() { @@ -553,24 +534,6 @@ public class ServerProperties { this.useRelativeRedirects = useRelativeRedirects; } - @DeprecatedConfigurationProperty(replacement = "server.tomcat.remote-ip-valve.remote-ip-header") - public String getRemoteIpHeader() { - return this.remoteIpValve.getRemoteIpHeader(); - } - - public void setRemoteIpHeader(String remoteIpHeader) { - this.remoteIpValve.setRemoteIpHeader(remoteIpHeader); - } - - @DeprecatedConfigurationProperty(replacement = "server.tomcat.remote-ip-valve.host-header") - public String getHostHeader() { - return this.remoteIpValve.getHostHeader(); - } - - public void setHostHeader(String hostHeader) { - this.remoteIpValve.setHostHeader(hostHeader); - } - public Charset getUriEncoding() { return this.uriEncoding; } @@ -651,8 +614,8 @@ public class ServerProperties { return this.mbeanregistry; } - public RemoteIpValve getRemoteIpValve() { - return this.remoteIpValve; + public Remoteip getRemoteip() { + return this.remoteip; } /** @@ -941,12 +904,7 @@ public class ServerProperties { } - public static class RemoteIpValve { - - /** - * Name of the HTTP header from which the remote host is extracted. - */ - private String hostHeader = "X-Forwarded-Host"; + public static class Remoteip { /** * Regular expression that matches proxies that are to be trusted. @@ -970,6 +928,11 @@ public class ServerProperties { */ private String protocolHeaderHttpsValue = "https"; + /** + * Name of the HTTP header from which the remote host is extracted. + */ + private String hostHeader = "X-Forwarded-Host"; + /** * Name of the HTTP header used to override the original port value. */ @@ -981,14 +944,6 @@ public class ServerProperties { */ private String remoteIpHeader; - public String getHostHeader() { - return this.hostHeader; - } - - public void setHostHeader(String hostHeader) { - this.hostHeader = hostHeader; - } - public String getInternalProxies() { return this.internalProxies; } @@ -1009,6 +964,14 @@ public class ServerProperties { return this.protocolHeaderHttpsValue; } + public String getHostHeader() { + return this.hostHeader; + } + + public void setHostHeader(String hostHeader) { + this.hostHeader = hostHeader; + } + public void setProtocolHeaderHttpsValue(String protocolHeaderHttpsValue) { this.protocolHeaderHttpsValue = protocolHeaderHttpsValue; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java index 266e3624cfb..ed7d034b633 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java @@ -31,8 +31,8 @@ import org.apache.coyote.http11.AbstractHttp11Protocol; import org.springframework.boot.autoconfigure.web.ErrorProperties; import org.springframework.boot.autoconfigure.web.ErrorProperties.IncludeStacktrace; import org.springframework.boot.autoconfigure.web.ServerProperties; -import org.springframework.boot.autoconfigure.web.ServerProperties.Tomcat; import org.springframework.boot.autoconfigure.web.ServerProperties.Tomcat.Accesslog; +import org.springframework.boot.autoconfigure.web.ServerProperties.Tomcat.Remoteip; import org.springframework.boot.cloud.CloudPlatform; import org.springframework.boot.context.properties.PropertyMapper; import org.springframework.boot.web.embedded.tomcat.ConfigurableTomcatWebServerFactory; @@ -173,9 +173,9 @@ public class TomcatWebServerFactoryCustomizer } private void customizeRemoteIpValve(ConfigurableTomcatWebServerFactory factory) { - Tomcat tomcatProperties = this.serverProperties.getTomcat(); - String protocolHeader = tomcatProperties.getRemoteIpValve().getProtocolHeader(); - String remoteIpHeader = tomcatProperties.getRemoteIpValve().getRemoteIpHeader(); + Remoteip remoteIpProperties = this.serverProperties.getTomcat().getRemoteip(); + String protocolHeader = remoteIpProperties.getProtocolHeader(); + String remoteIpHeader = remoteIpProperties.getRemoteIpHeader(); // For back compatibility the valve is also enabled if protocol-header is set if (StringUtils.hasText(protocolHeader) || StringUtils.hasText(remoteIpHeader) || getOrDeduceUseForwardHeaders()) { @@ -186,10 +186,10 @@ public class TomcatWebServerFactoryCustomizer } // The internal proxies default to a white list of "safe" internal IP // addresses - valve.setInternalProxies(tomcatProperties.getRemoteIpValve().getInternalProxies()); - valve.setHostHeader(tomcatProperties.getRemoteIpValve().getHostHeader()); - valve.setPortHeader(tomcatProperties.getRemoteIpValve().getPortHeader()); - valve.setProtocolHeaderHttpsValue(tomcatProperties.getRemoteIpValve().getProtocolHeaderHttpsValue()); + valve.setInternalProxies(remoteIpProperties.getInternalProxies()); + valve.setHostHeader(remoteIpProperties.getHostHeader()); + valve.setPortHeader(remoteIpProperties.getPortHeader()); + valve.setProtocolHeaderHttpsValue(remoteIpProperties.getProtocolHeaderHttpsValue()); // ... so it's safe to add this valve by default. factory.addEngineValves(valve); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java index 17b82914c92..96d91dd1720 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java @@ -125,9 +125,9 @@ class ServerPropertiesTests { map.put("server.tomcat.accesslog.rename-on-rotate", "true"); map.put("server.tomcat.accesslog.ipv6Canonical", "true"); map.put("server.tomcat.accesslog.request-attributes-enabled", "true"); - map.put("server.tomcat.protocol-header", "X-Forwarded-Protocol"); - map.put("server.tomcat.remote-ip-header", "Remote-Ip"); - map.put("server.tomcat.internal-proxies", "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}"); + map.put("server.tomcat.remoteip.protocol-header", "X-Forwarded-Protocol"); + map.put("server.tomcat.remoteip.remote-ip-header", "Remote-Ip"); + map.put("server.tomcat.remoteip.internal-proxies", "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}"); map.put("server.tomcat.background-processor-delay", "10"); map.put("server.tomcat.relaxed-path-chars", "|,<"); map.put("server.tomcat.relaxed-query-chars", "^ , | "); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java index 2c997e4b19e..b2798b9cc49 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java @@ -178,9 +178,12 @@ class TomcatWebServerFactoryCustomizerTests { @Test void customRemoteIpValve() { - bind("server.tomcat.remote-ip-header=x-my-remote-ip-header", - "server.tomcat.protocol-header=x-my-protocol-header", "server.tomcat.internal-proxies=192.168.0.1", - "server.tomcat.port-header=x-my-forward-port", "server.tomcat.protocol-header-https-value=On"); + bind("server.tomcat.remoteip.remote-ip-header=x-my-remote-ip-header", + "server.tomcat.remoteip.protocol-header=x-my-protocol-header", + "server.tomcat.remoteip.internal-proxies=192.168.0.1", + "server.tomcat.remoteip.host-header=x-my-forward-host", + "server.tomcat.remoteip.port-header=x-my-forward-port", + "server.tomcat.remoteip.protocol-header-https-value=On"); TomcatServletWebServerFactory factory = customizeAndGetFactory(); assertThat(factory.getEngineValves()).hasSize(1); Valve valve = factory.getEngineValves().iterator().next(); @@ -189,17 +192,18 @@ class TomcatWebServerFactoryCustomizerTests { assertThat(remoteIpValve.getProtocolHeader()).isEqualTo("x-my-protocol-header"); assertThat(remoteIpValve.getProtocolHeaderHttpsValue()).isEqualTo("On"); assertThat(remoteIpValve.getRemoteIpHeader()).isEqualTo("x-my-remote-ip-header"); + assertThat(remoteIpValve.getHostHeader()).isEqualTo("x-my-forward-host"); assertThat(remoteIpValve.getPortHeader()).isEqualTo("x-my-forward-port"); assertThat(remoteIpValve.getInternalProxies()).isEqualTo("192.168.0.1"); } @Test - void customNewPropertiesForRemoteIpValve() { - bind("server.tomcat.remote-ip-valve.remote-ip-header=x-my-remote-ip-header", - "server.tomcat.remote-ip-valve.protocol-header=x-my-protocol-header", - "server.tomcat.remote-ip-valve.internal-proxies=192.168.0.1", - "server.tomcat.remote-ip-valve.port-header=x-my-forward-port", - "server.tomcat.remote-ip-valve.protocol-header-https-value=On"); + @Deprecated + void customRemoteIpValveWithDeprecatedProperties() { + bind("server.tomcat.remote-ip-header=x-my-remote-ip-header", + "server.tomcat.protocol-header=x-my-protocol-header", "server.tomcat.internal-proxies=192.168.0.1", + "server.tomcat.host-header=x-my-forward-host", "server.tomcat.port-header=x-my-forward-port", + "server.tomcat.protocol-header-https-value=On"); TomcatServletWebServerFactory factory = customizeAndGetFactory(); assertThat(factory.getEngineValves()).hasSize(1); Valve valve = factory.getEngineValves().iterator().next(); @@ -208,6 +212,7 @@ class TomcatWebServerFactoryCustomizerTests { assertThat(remoteIpValve.getProtocolHeader()).isEqualTo("x-my-protocol-header"); assertThat(remoteIpValve.getProtocolHeaderHttpsValue()).isEqualTo("On"); assertThat(remoteIpValve.getRemoteIpHeader()).isEqualTo("x-my-remote-ip-header"); + assertThat(remoteIpValve.getHostHeader()).isEqualTo("x-my-forward-host"); assertThat(remoteIpValve.getPortHeader()).isEqualTo("x-my-forward-port"); assertThat(remoteIpValve.getInternalProxies()).isEqualTo("192.168.0.1"); } @@ -257,7 +262,8 @@ class TomcatWebServerFactoryCustomizerTests { @Test void defaultRemoteIpValve() { // Since 1.1.7 you need to specify at least the protocol - bind("server.tomcat.protocol-header=X-Forwarded-Proto", "server.tomcat.remote-ip-header=X-Forwarded-For"); + bind("server.tomcat.remoteip.protocol-header=X-Forwarded-Proto", + "server.tomcat.remoteip.remote-ip-header=X-Forwarded-For"); testRemoteIpValveConfigured(); } @@ -297,7 +303,7 @@ class TomcatWebServerFactoryCustomizerTests { @Test void disableRemoteIpValve() { - bind("server.tomcat.remote-ip-header=", "server.tomcat.protocol-header="); + bind("server.tomcat.remoteip.remote-ip-header=", "server.tomcat.remoteip.protocol-header="); TomcatServletWebServerFactory factory = customizeAndGetFactory(); assertThat(factory.getEngineValves()).isEmpty(); } diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc index f9949711a1d..1e496f69452 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc @@ -804,8 +804,8 @@ If you use Tomcat, you can additionally configure the names of the headers used [indent=0] ---- - server.tomcat.remote-ip-header=x-your-remote-ip-header - server.tomcat.protocol-header=x-your-protocol-header + server.tomcat.remoteip.remote-ip-header=x-your-remote-ip-header + server.tomcat.remoteip.protocol-header=x-your-protocol-header ---- Tomcat is also configured with a default regular expression that matches internal proxies that are to be trusted. @@ -814,7 +814,7 @@ You can customize the valve's configuration by adding an entry to `application.p [indent=0] ---- - server.tomcat.internal-proxies=192\\.168\\.\\d{1,3}\\.\\d{1,3} + server.tomcat.remoteip.internal-proxies=192\\.168\\.\\d{1,3}\\.\\d{1,3} ---- NOTE: The double backslashes are required only when you use a properties file for configuration. @@ -2203,8 +2203,8 @@ You can switch on the valve by adding some entries to `application.properties`, [source,properties,indent=0,configprops] ---- - server.tomcat.remote-ip-header=x-forwarded-for - server.tomcat.protocol-header=x-forwarded-proto + server.tomcat.remoteip.remote-ip-header=x-forwarded-for + server.tomcat.remoteip.protocol-header=x-forwarded-proto ---- (The presence of either of those properties switches on the valve.