From 186b1fae6d44d5ee2506b1842c51e2937eac9045 Mon Sep 17 00:00:00 2001 From: Rafiullah Hamedy Date: Wed, 20 Mar 2019 23:31:01 -0400 Subject: [PATCH] Add support for configuring remaining Undertow server options This commit adds support for configuring Undertow's server options that were previously not configurable via application properties. The additions are the following: - allow-encoded-slash - always-set-keep-alive - decode-url - max-cookies - max-headers - max-parameters, - url-charset See gh-16278 --- .../autoconfigure/web/ServerProperties.java | 101 ++++++++++++++++++ .../UndertowWebServerFactoryCustomizer.java | 69 +++++++----- ...dertowWebServerFactoryCustomizerTests.java | 36 +++++++ 3 files changed, 182 insertions(+), 24 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 be11fbfb01..b8ee50e26d 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 @@ -58,6 +58,8 @@ import org.springframework.util.unit.DataSize; * @author Chentao Qu * @author Artsiom Yudovin * @author Andrew McGhie + * @author Rafiullah Hamedy + * */ @ConfigurationProperties(prefix = "server", ignoreUnknownFields = true) public class ServerProperties { @@ -1114,6 +1116,49 @@ public class ServerProperties { */ private boolean eagerFilterInit = true; + /** + * The maximum number of query or path parameters that are allowed. This limit + * exists to prevent hash collision based DOS attacks. + */ + private Integer maxParameters; + + /** + * The maximum number of headers that are allowed. This limit exists to prevent + * hash collision based DOS attacks. + */ + private Integer maxHeaders; + + /** + * The maximum number of cookies that are allowed. This limit exists to prevent + * hash collision based DOS attacks. + */ + private Integer maxCookies; + + /** + * Set this to true if you want the server to decode percent encoded slash + * characters. This is probably a bad idea, as it can have security implications, + * due to different servers interpreting the slash differently. Only enable this + * if you have a legacy application that requires it. + */ + private Boolean allowEncodedSlash; + + /** + * If the URL should be decoded. If this is not set to true then percent encoded + * characters in the URL will be left as is. + */ + private Boolean decodeUrl; + + /** + * The charset to decode the URL to. + */ + private String urlCharset; + + /** + * If the 'Connection: keep-alive' header should be added to all responses, even + * if not required by spec. + */ + private Boolean alwaysSetKeepAlive; + private final Accesslog accesslog = new Accesslog(); public DataSize getMaxHttpPostSize() { @@ -1164,6 +1209,62 @@ public class ServerProperties { this.eagerFilterInit = eagerFilterInit; } + public Integer getMaxParameters() { + return this.maxParameters; + } + + public void setMaxParameters(Integer maxParameters) { + this.maxParameters = maxParameters; + } + + public Integer getMaxHeaders() { + return this.maxHeaders; + } + + public void setMaxHeaders(Integer maxHeaders) { + this.maxHeaders = maxHeaders; + } + + public Integer getMaxCookies() { + return this.maxCookies; + } + + public void setMaxCookies(Integer maxCookies) { + this.maxCookies = maxCookies; + } + + public Boolean isAllowEncodedSlash() { + return this.allowEncodedSlash; + } + + public void setAllowEncodedSlash(Boolean allowEncodedSlash) { + this.allowEncodedSlash = allowEncodedSlash; + } + + public Boolean isDecodeUrl() { + return this.decodeUrl; + } + + public void setDecodeUrl(Boolean decodeUrl) { + this.decodeUrl = decodeUrl; + } + + public String getUrlCharset() { + return this.urlCharset; + } + + public void setUrlCharset(String urlCharset) { + this.urlCharset = urlCharset; + } + + public Boolean isAlwaysSetKeepAlive() { + return this.alwaysSetKeepAlive; + } + + public void setAlwaysSetKeepAlive(Boolean alwaysSetKeepAlive) { + this.alwaysSetKeepAlive = alwaysSetKeepAlive; + } + public Accesslog getAccesslog() { return this.accesslog; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java index 3859d4e71f..0bd6f897b6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java @@ -16,9 +16,8 @@ package org.springframework.boot.autoconfigure.web.embedded; -import java.time.Duration; - import io.undertow.UndertowOptions; +import org.xnio.Option; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.cloud.CloudPlatform; @@ -38,6 +37,7 @@ import org.springframework.util.unit.DataSize; * @author Stephane Nicoll * @author Phillip Webb * @author Arstiom Yudovin + * @author Rafiullah Hamedy * @since 2.0.0 */ public class UndertowWebServerFactoryCustomizer implements @@ -86,17 +86,50 @@ public class UndertowWebServerFactoryCustomizer implements .to(factory::setAccessLogRotate); propertyMapper.from(this::getOrDeduceUseForwardHeaders) .to(factory::setUseForwardHeaders); + propertyMapper.from(properties::getMaxHttpHeaderSize).whenNonNull() .asInt(DataSize::toBytes).when(this::isPositive) - .to((maxHttpHeaderSize) -> customizeMaxHttpHeaderSize(factory, - maxHttpHeaderSize)); + .to((maxHttpHeaderSize) -> customizeProperties(factory, + UndertowOptions.MAX_HEADER_SIZE, maxHttpHeaderSize)); + propertyMapper.from(undertowProperties::getMaxHttpPostSize) .asInt(DataSize::toBytes).when(this::isPositive) - .to((maxHttpPostSize) -> customizeMaxHttpPostSize(factory, - maxHttpPostSize)); + .to((maxHttpPostSize) -> customizeProperties(factory, + UndertowOptions.MAX_HEADER_SIZE, maxHttpPostSize)); + propertyMapper.from(properties::getConnectionTimeout) - .to((connectionTimeout) -> customizeConnectionTimeout(factory, - connectionTimeout)); + .to((connectionTimeout) -> customizeProperties(factory, + UndertowOptions.NO_REQUEST_TIMEOUT, + (int) connectionTimeout.toMillis())); + + propertyMapper.from(undertowProperties::getMaxParameters) + .to((maxParameters) -> customizeProperties(factory, + UndertowOptions.MAX_PARAMETERS, maxParameters)); + + propertyMapper.from(undertowProperties::getMaxHeaders) + .to((maxHeaders) -> customizeProperties(factory, + UndertowOptions.MAX_HEADERS, maxHeaders)); + + propertyMapper.from(undertowProperties::getMaxCookies) + .to((maxCookies) -> customizeProperties(factory, + UndertowOptions.MAX_COOKIES, maxCookies)); + + propertyMapper.from(undertowProperties::isAllowEncodedSlash) + .to((allowEncodedSlash) -> customizeProperties(factory, + UndertowOptions.ALLOW_ENCODED_SLASH, allowEncodedSlash)); + + propertyMapper.from(undertowProperties::isDecodeUrl) + .to((isDecodeUrl) -> customizeProperties(factory, + UndertowOptions.DECODE_URL, isDecodeUrl)); + + propertyMapper.from(undertowProperties::getUrlCharset) + .to((urlCharset) -> customizeProperties(factory, + UndertowOptions.URL_CHARSET, urlCharset)); + + propertyMapper.from(undertowProperties::isAlwaysSetKeepAlive) + .to((alwaysSetKeepAlive) -> customizeProperties(factory, + UndertowOptions.ALWAYS_SET_KEEP_ALIVE, alwaysSetKeepAlive)); + factory.addDeploymentInfoCustomizers((deploymentInfo) -> deploymentInfo .setEagerFilterInit(undertowProperties.isEagerFilterInit())); } @@ -105,22 +138,10 @@ public class UndertowWebServerFactoryCustomizer implements return value.longValue() > 0; } - private void customizeConnectionTimeout(ConfigurableUndertowWebServerFactory factory, - Duration connectionTimeout) { - factory.addBuilderCustomizers((builder) -> builder.setServerOption( - UndertowOptions.NO_REQUEST_TIMEOUT, (int) connectionTimeout.toMillis())); - } - - private void customizeMaxHttpHeaderSize(ConfigurableUndertowWebServerFactory factory, - int maxHttpHeaderSize) { - factory.addBuilderCustomizers((builder) -> builder - .setServerOption(UndertowOptions.MAX_HEADER_SIZE, maxHttpHeaderSize)); - } - - private void customizeMaxHttpPostSize(ConfigurableUndertowWebServerFactory factory, - long maxHttpPostSize) { - factory.addBuilderCustomizers((builder) -> builder - .setServerOption(UndertowOptions.MAX_ENTITY_SIZE, maxHttpPostSize)); + private void customizeProperties(ConfigurableUndertowWebServerFactory factory, + Option propType, T prop) { + factory.addBuilderCustomizers( + (builder) -> builder.setServerOption(propType, prop)); } private boolean getOrDeduceUseForwardHeaders() { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizerTests.java index ed40f56463..fa0d62db76 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizerTests.java @@ -48,6 +48,8 @@ import static org.mockito.Mockito.verify; * @author Brian Clozel * @author Phillip Webb * @author Artsiom Yudovin + * @author Rafiullah Hamedy + * */ public class UndertowWebServerFactoryCustomizerTests { @@ -85,6 +87,40 @@ public class UndertowWebServerFactoryCustomizerTests { verify(factory).setAccessLogRotate(false); } + @Test + public void customizeUndertowConnectionCommonSettings() { + bind("server.undertow.maxParameters=50", "server.undertow.maxHeaders=60", + "server.undertow.maxCookies=70", "server.undertow.allowEncodedSlash=true", + "server.undertow.decodeUrl=true", "server.undertow.urlCharset=UTF-8", + "server.undertow.alwaysSetKeepAlive=true"); + Builder builder = Undertow.builder(); + ConfigurableUndertowWebServerFactory factory = mockFactory(builder); + this.customizer.customize(factory); + OptionMap map = ((OptionMap.Builder) ReflectionTestUtils.getField(builder, + "serverOptions")).getMap(); + assertThat(map.get(UndertowOptions.MAX_PARAMETERS)).isEqualTo(50); + assertThat(map.get(UndertowOptions.MAX_HEADERS)).isEqualTo(60); + assertThat(map.get(UndertowOptions.MAX_COOKIES)).isEqualTo(70); + assertThat(map.get(UndertowOptions.ALLOW_ENCODED_SLASH)).isTrue(); + assertThat(map.get(UndertowOptions.DECODE_URL)).isTrue(); + assertThat(map.get(UndertowOptions.URL_CHARSET)).isEqualTo("UTF-8"); + assertThat(map.get(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)).isTrue(); + } + + @Test + public void customizeUndertowCommonConnectionCommonBoolSettings() { + bind("server.undertow.allowEncodedSlash=false", "server.undertow.decodeUrl=false", + "server.undertow.alwaysSetKeepAlive=false"); + Builder builder = Undertow.builder(); + ConfigurableUndertowWebServerFactory factory = mockFactory(builder); + this.customizer.customize(factory); + OptionMap map = ((OptionMap.Builder) ReflectionTestUtils.getField(builder, + "serverOptions")).getMap(); + assertThat(map.get(UndertowOptions.ALLOW_ENCODED_SLASH)).isFalse(); + assertThat(map.get(UndertowOptions.DECODE_URL)).isFalse(); + assertThat(map.get(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)).isFalse(); + } + @Test public void deduceUseForwardHeaders() { this.environment.setProperty("DYNO", "-");