diff --git a/docs/modules/ROOT/nav.adoc b/docs/modules/ROOT/nav.adoc index 523f84b159a..02279e4dc9f 100644 --- a/docs/modules/ROOT/nav.adoc +++ b/docs/modules/ROOT/nav.adoc @@ -147,6 +147,7 @@ *** xref:reactive/exploits/csrf.adoc[CSRF] *** xref:reactive/exploits/headers.adoc[Headers] *** xref:reactive/exploits/http.adoc[HTTP Requests] +*** xref:reactive/exploits/firewall.adoc[] ** Integrations *** xref:reactive/integrations/cors.adoc[CORS] *** xref:reactive/integrations/rsocket.adoc[RSocket] diff --git a/docs/modules/ROOT/pages/reactive/exploits/firewall.adoc b/docs/modules/ROOT/pages/reactive/exploits/firewall.adoc new file mode 100644 index 00000000000..dce7fb05f7d --- /dev/null +++ b/docs/modules/ROOT/pages/reactive/exploits/firewall.adoc @@ -0,0 +1,202 @@ +[[webflux-serverwebexchangefirewall]] += ServerWebExchangeFirewall + +There are various ways a request can be created by malicious users that can exploit applications. +Spring Security provides the `ServerWebExchangeFirewall` to allow rejecting requests that look malicious. +The default implementation is `StrictServerWebExchangeFirewall` which rejects malicious requests. + +For example a request could contain path-traversal sequences (such as `/../`) or multiple forward slashes (`//`) that could also cause pattern-matches to fail. +Some containers normalize these out before performing the servlet mapping, but others do not. +To protect against issues like these, `WebFilterChainProxy` uses a `ServerWebExchangeFirewall` strategy to check and wrap the request. +By default, un-normalized requests are automatically rejected, and path parameters are removed for matching purposes. +(So, for example, an original request path of `/secure;hack=1/somefile.html;hack=2` is returned as `/secure/somefile.html`.) +It is, therefore, essential that a `WebFilterChainProxy` is used. + +In practice, we recommend that you use method security at your service layer, to control access to your application, rather than rely entirely on the use of security constraints defined at the web-application level. +URLs change, and it is difficult to take into account all the possible URLs that an application might support and how requests might be manipulated. +You should restrict yourself to using a few simple patterns that are simple to understand. +Always try to use a "`deny-by-default`" approach, where you have a catch-all wildcard (`/**` or `**`) defined last to deny access. + +Security defined at the service layer is much more robust and harder to bypass, so you should always take advantage of Spring Security's method security options. + +You can customize the `ServerWebExchangeFirewall` by exposing it as a Bean. + +.Allow Matrix Variables +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +public StrictServerWebExchangeFirewall httpFirewall() { + StrictServerWebExchangeFirewall firewall = new StrictServerWebExchangeFirewall(); + firewall.setAllowSemicolon(true); + return firewall; +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun httpFirewall(): StrictServerWebExchangeFirewall { + val firewall = StrictServerWebExchangeFirewall() + firewall.setAllowSemicolon(true) + return firewall +} +---- +====== + +To protect against https://www.owasp.org/index.php/Cross_Site_Tracing[Cross Site Tracing (XST)] and https://www.owasp.org/index.php/Test_HTTP_Methods_(OTG-CONFIG-006)[HTTP Verb Tampering], the `StrictServerWebExchangeFirewall` provides an allowed list of valid HTTP methods that are allowed. +The default valid methods are `DELETE`, `GET`, `HEAD`, `OPTIONS`, `PATCH`, `POST`, and `PUT`. +If your application needs to modify the valid methods, you can configure a custom `StrictServerWebExchangeFirewall` bean. +The following example allows only HTTP `GET` and `POST` methods: + + +.Allow Only GET & POST +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +public StrictServerWebExchangeFirewall httpFirewall() { + StrictServerWebExchangeFirewall firewall = new StrictServerWebExchangeFirewall(); + firewall.setAllowedHttpMethods(Arrays.asList("GET", "POST")); + return firewall; +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun httpFirewall(): StrictServerWebExchangeFirewall { + val firewall = StrictServerWebExchangeFirewall() + firewall.setAllowedHttpMethods(listOf("GET", "POST")) + return firewall +} +---- +====== + +If you must allow any HTTP method (not recommended), you can use `StrictServerWebExchangeFirewall.setUnsafeAllowAnyHttpMethod(true)`. +Doing so entirely disables validation of the HTTP method. + + +[[webflux-serverwebexchangefirewall-headers-parameters]] +`StrictServerWebExchangeFirewall` also checks header names and values and parameter names. +It requires that each character have a defined code point and not be a control character. + +This requirement can be relaxed or adjusted as necessary by using the following methods: + +* `StrictServerWebExchangeFirewall#setAllowedHeaderNames(Predicate)` +* `StrictServerWebExchangeFirewall#setAllowedHeaderValues(Predicate)` +* `StrictServerWebExchangeFirewall#setAllowedParameterNames(Predicate)` + +[NOTE] +==== +Parameter values can be also controlled with `setAllowedParameterValues(Predicate)`. +==== + +For example, to switch off this check, you can wire your `StrictServerWebExchangeFirewall` with `Predicate` instances that always return `true`: + +.Allow Any Header Name, Header Value, and Parameter Name +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +public StrictServerWebExchangeFirewall httpFirewall() { + StrictServerWebExchangeFirewall firewall = new StrictServerWebExchangeFirewall(); + firewall.setAllowedHeaderNames((header) -> true); + firewall.setAllowedHeaderValues((header) -> true); + firewall.setAllowedParameterNames((parameter) -> true); + return firewall; +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun httpFirewall(): StrictServerWebExchangeFirewall { + val firewall = StrictServerWebExchangeFirewall() + firewall.setAllowedHeaderNames { true } + firewall.setAllowedHeaderValues { true } + firewall.setAllowedParameterNames { true } + return firewall +} +---- +====== + +Alternatively, there might be a specific value that you need to allow. + +For example, iPhone Xʀ uses a `User-Agent` that includes a character that is not in the ISO-8859-1 charset. +Due to this fact, some application servers parse this value into two separate characters, the latter being an undefined character. + +You can address this with the `setAllowedHeaderValues` method: + +.Allow Certain User Agents +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +public StrictServerWebExchangeFirewall httpFirewall() { + StrictServerWebExchangeFirewall firewall = new StrictServerWebExchangeFirewall(); + Pattern allowed = Pattern.compile("[\\p{IsAssigned}&&[^\\p{IsControl}]]*"); + Pattern userAgent = ...; + firewall.setAllowedHeaderValues((header) -> allowed.matcher(header).matches() || userAgent.matcher(header).matches()); + return firewall; +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun httpFirewall(): StrictServerWebExchangeFirewall { + val firewall = StrictServerWebExchangeFirewall() + val allowed = Pattern.compile("[\\p{IsAssigned}&&[^\\p{IsControl}]]*") + val userAgent = Pattern.compile(...) + firewall.setAllowedHeaderValues { allowed.matcher(it).matches() || userAgent.matcher(it).matches() } + return firewall +} +---- +====== + +In the case of header values, you may instead consider parsing them as UTF-8 at verification time: + +.Parse Headers As UTF-8 +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +firewall.setAllowedHeaderValues((header) -> { + String parsed = new String(header.getBytes(ISO_8859_1), UTF_8); + return allowed.matcher(parsed).matches(); +}); +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +firewall.setAllowedHeaderValues { + val parsed = String(header.getBytes(ISO_8859_1), UTF_8) + return allowed.matcher(parsed).matches() +} +---- +====== diff --git a/web/src/main/java/org/springframework/security/web/server/WebFilterChainProxy.java b/web/src/main/java/org/springframework/security/web/server/WebFilterChainProxy.java index f4654ab25fd..0600cbd07dd 100644 --- a/web/src/main/java/org/springframework/security/web/server/WebFilterChainProxy.java +++ b/web/src/main/java/org/springframework/security/web/server/WebFilterChainProxy.java @@ -22,6 +22,12 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import org.springframework.security.web.server.firewall.HttpStatusExchangeRejectedHandler; +import org.springframework.security.web.server.firewall.ServerExchangeRejectedException; +import org.springframework.security.web.server.firewall.ServerExchangeRejectedHandler; +import org.springframework.security.web.server.firewall.ServerWebExchangeFirewall; +import org.springframework.security.web.server.firewall.StrictServerWebExchangeFirewall; +import org.springframework.util.Assert; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; import org.springframework.web.server.WebFilterChain; @@ -38,6 +44,10 @@ public class WebFilterChainProxy implements WebFilter { private final List filters; + private ServerWebExchangeFirewall firewall = new StrictServerWebExchangeFirewall(); + + private ServerExchangeRejectedHandler exchangeRejectedHandler = new HttpStatusExchangeRejectedHandler(); + public WebFilterChainProxy(List filters) { this.filters = filters; } @@ -48,12 +58,41 @@ public WebFilterChainProxy(SecurityWebFilterChain... filters) { @Override public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { + return this.firewall.getFirewalledExchange(exchange) + .flatMap((firewalledExchange) -> filterFirewalledExchange(firewalledExchange, chain)) + .onErrorResume(ServerExchangeRejectedException.class, + (rejected) -> this.exchangeRejectedHandler.handle(exchange, rejected).then(Mono.empty())); + } + + private Mono filterFirewalledExchange(ServerWebExchange firewalledExchange, WebFilterChain chain) { return Flux.fromIterable(this.filters) - .filterWhen((securityWebFilterChain) -> securityWebFilterChain.matches(exchange)).next() - .switchIfEmpty(chain.filter(exchange).then(Mono.empty())) + .filterWhen((securityWebFilterChain) -> securityWebFilterChain.matches(firewalledExchange)).next() + .switchIfEmpty(chain.filter(firewalledExchange).then(Mono.empty())) .flatMap((securityWebFilterChain) -> securityWebFilterChain.getWebFilters().collectList()) .map((filters) -> new FilteringWebHandler(chain::filter, filters)).map(DefaultWebFilterChain::new) - .flatMap((securedChain) -> securedChain.filter(exchange)); + .flatMap((securedChain) -> securedChain.filter(firewalledExchange)); + } + + /** + * Protects the application using the provided + * {@link StrictServerWebExchangeFirewall}. + * @param firewall the {@link StrictServerWebExchangeFirewall} to use. Cannot be null. + * @since 5.7.13 + */ + public void setFirewall(ServerWebExchangeFirewall firewall) { + Assert.notNull(firewall, "firewall cannot be null"); + this.firewall = firewall; + } + + /** + * Handles {@link ServerExchangeRejectedException} when the + * {@link ServerWebExchangeFirewall} rejects the provided {@link ServerWebExchange}. + * @param exchangeRejectedHandler the {@link ServerExchangeRejectedHandler} to use. + * @since 5.7.13 + */ + public void setExchangeRejectedHandler(ServerExchangeRejectedHandler exchangeRejectedHandler) { + Assert.notNull(exchangeRejectedHandler, "exchangeRejectedHandler cannot be null"); + this.exchangeRejectedHandler = exchangeRejectedHandler; } } diff --git a/web/src/main/java/org/springframework/security/web/server/firewall/HttpStatusExchangeRejectedHandler.java b/web/src/main/java/org/springframework/security/web/server/firewall/HttpStatusExchangeRejectedHandler.java new file mode 100644 index 00000000000..a59c2d1c831 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/server/firewall/HttpStatusExchangeRejectedHandler.java @@ -0,0 +1,66 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.server.firewall; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import reactor.core.publisher.Mono; + +import org.springframework.core.log.LogMessage; +import org.springframework.http.HttpStatus; +import org.springframework.web.server.ServerWebExchange; + +/** + * A simple implementation of {@link ServerExchangeRejectedHandler} that sends an error + * with configurable status code. + * + * @author Rob Winch + * @since 5.7.13 + */ +public class HttpStatusExchangeRejectedHandler implements ServerExchangeRejectedHandler { + + private static final Log logger = LogFactory.getLog(HttpStatusExchangeRejectedHandler.class); + + private final HttpStatus status; + + /** + * Constructs an instance which uses {@code 400} as response code. + */ + public HttpStatusExchangeRejectedHandler() { + this(HttpStatus.BAD_REQUEST); + } + + /** + * Constructs an instance which uses a configurable http code as response. + * @param status http status code to use + */ + public HttpStatusExchangeRejectedHandler(HttpStatus status) { + this.status = status; + } + + @Override + public Mono handle(ServerWebExchange exchange, + ServerExchangeRejectedException serverExchangeRejectedException) { + return Mono.fromRunnable(() -> { + logger.debug( + LogMessage.format("Rejecting request due to: %s", serverExchangeRejectedException.getMessage()), + serverExchangeRejectedException); + exchange.getResponse().setStatusCode(this.status); + }); + } + +} diff --git a/web/src/main/java/org/springframework/security/web/server/firewall/ServerExchangeRejectedException.java b/web/src/main/java/org/springframework/security/web/server/firewall/ServerExchangeRejectedException.java new file mode 100644 index 00000000000..39018c49c94 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/server/firewall/ServerExchangeRejectedException.java @@ -0,0 +1,31 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.server.firewall; + +/** + * Thrown when a {@link org.springframework.web.server.ServerWebExchange} is rejected. + * + * @author Rob Winch + * @since 5.7.13 + */ +public class ServerExchangeRejectedException extends RuntimeException { + + public ServerExchangeRejectedException(String message) { + super(message); + } + +} diff --git a/web/src/main/java/org/springframework/security/web/server/firewall/ServerExchangeRejectedHandler.java b/web/src/main/java/org/springframework/security/web/server/firewall/ServerExchangeRejectedHandler.java new file mode 100644 index 00000000000..dd7317c4c4d --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/server/firewall/ServerExchangeRejectedHandler.java @@ -0,0 +1,39 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.server.firewall; + +import reactor.core.publisher.Mono; + +import org.springframework.web.server.ServerWebExchange; + +/** + * Handles {@link ServerExchangeRejectedException} thrown by + * {@link ServerWebExchangeFirewall}. + * + * @author Rob Winch + * @since 5.7.13 + */ +public interface ServerExchangeRejectedHandler { + + /** + * Handles an request rejected failure. + * @param exchange the {@link ServerWebExchange} that was rejected + * @param serverExchangeRejectedException that caused the invocation + */ + Mono handle(ServerWebExchange exchange, ServerExchangeRejectedException serverExchangeRejectedException); + +} diff --git a/web/src/main/java/org/springframework/security/web/server/firewall/ServerWebExchangeFirewall.java b/web/src/main/java/org/springframework/security/web/server/firewall/ServerWebExchangeFirewall.java new file mode 100644 index 00000000000..db4107e4843 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/server/firewall/ServerWebExchangeFirewall.java @@ -0,0 +1,46 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.server.firewall; + +import reactor.core.publisher.Mono; + +import org.springframework.web.server.ServerWebExchange; + +/** + * Interface which can be used to reject potentially dangerous requests and/or wrap them + * to control their behaviour. + * + * @author Rob Winch + * @since 5.7.13 + */ +public interface ServerWebExchangeFirewall { + + /** + * An implementation of {@link StrictServerWebExchangeFirewall} that does nothing. + * This is considered insecure and not recommended. + */ + ServerWebExchangeFirewall INSECURE_NOOP = (exchange) -> Mono.just(exchange); + + /** + * Get a {@link ServerWebExchange} that has firewall rules applied to it. + * @param exchange the {@link ServerWebExchange} to apply firewall rules to. + * @return the {@link ServerWebExchange} that has firewall rules applied to it. + * @throws ServerExchangeRejectedException when a rule is broken. + */ + Mono getFirewalledExchange(ServerWebExchange exchange); + +} diff --git a/web/src/main/java/org/springframework/security/web/server/firewall/StrictServerWebExchangeFirewall.java b/web/src/main/java/org/springframework/security/web/server/firewall/StrictServerWebExchangeFirewall.java new file mode 100644 index 00000000000..889628036c3 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/server/firewall/StrictServerWebExchangeFirewall.java @@ -0,0 +1,791 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.server.firewall; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Predicate; +import java.util.regex.Pattern; + +import reactor.core.publisher.Mono; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.http.server.reactive.ServerHttpRequestDecorator; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.ServerWebExchangeDecorator; + +/** + *

+ * A strict implementation of {@link ServerWebExchangeFirewall} that rejects any + * suspicious requests with a {@link ServerExchangeRejectedException}. + *

+ *

+ * The following rules are applied to the firewall: + *

+ *
    + *
  • Rejects HTTP methods that are not allowed. This specified to block + * HTTP Verb + * tampering and XST attacks. See {@link #setAllowedHttpMethods(Collection)}
    • + *
  • Rejects URLs that are not normalized to avoid bypassing security constraints. There + * is no way to disable this as it is considered extremely risky to disable this + * constraint. A few options to allow this behavior is to normalize the request prior to + * the firewall or using + * {@link org.springframework.security.web.firewall.DefaultHttpFirewall} instead. Please + * keep in mind that normalizing the request is fragile and why requests are rejected + * rather than normalized.
    • + *
  • Rejects URLs that contain characters that are not printable ASCII characters. There + * is no way to disable this as it is considered extremely risky to disable this + * constraint.
    • + *
  • Rejects URLs that contain semicolons. See {@link #setAllowSemicolon(boolean)}
    • + *
  • Rejects URLs that contain a URL encoded slash. See + * {@link #setAllowUrlEncodedSlash(boolean)}
    • + *
  • Rejects URLs that contain a backslash. See {@link #setAllowBackSlash(boolean)}
    • + *
  • Rejects URLs that contain a null character. See {@link #setAllowNull(boolean)}
    • + *
  • Rejects URLs that contain a URL encoded percent. See + * {@link #setAllowUrlEncodedPercent(boolean)}
    • + *
  • Rejects hosts that are not allowed. See {@link #setAllowedHostnames(Predicate)} + *
    • + *
  • Reject headers names that are not allowed. See + * {@link #setAllowedHeaderNames(Predicate)}
    • + *
  • Reject headers values that are not allowed. See + * {@link #setAllowedHeaderValues(Predicate)}
    • + *
  • Reject parameter names that are not allowed. See + * {@link #setAllowedParameterNames(Predicate)}
    • + *
  • Reject parameter values that are not allowed. See + * {@link #setAllowedParameterValues(Predicate)}
    • + *
+ * + * @author Rob Winch + * @since 5.7.13 + */ +public class StrictServerWebExchangeFirewall implements ServerWebExchangeFirewall { + + /** + * Used to specify to {@link #setAllowedHttpMethods(Collection)} that any HTTP method + * should be allowed. + */ + private static final Set ALLOW_ANY_HTTP_METHOD = Collections.emptySet(); + + private static final String ENCODED_PERCENT = "%25"; + + private static final String PERCENT = "%"; + + private static final List FORBIDDEN_ENCODED_PERIOD = Collections + .unmodifiableList(Arrays.asList("%2e", "%2E")); + + private static final List FORBIDDEN_SEMICOLON = Collections + .unmodifiableList(Arrays.asList(";", "%3b", "%3B")); + + private static final List FORBIDDEN_FORWARDSLASH = Collections + .unmodifiableList(Arrays.asList("%2f", "%2F")); + + private static final List FORBIDDEN_DOUBLE_FORWARDSLASH = Collections + .unmodifiableList(Arrays.asList("//", "%2f%2f", "%2f%2F", "%2F%2f", "%2F%2F")); + + private static final List FORBIDDEN_BACKSLASH = Collections + .unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); + + private static final List FORBIDDEN_NULL = Collections.unmodifiableList(Arrays.asList("\0", "%00")); + + private static final List FORBIDDEN_LF = Collections.unmodifiableList(Arrays.asList("\n", "%0a", "%0A")); + + private static final List FORBIDDEN_CR = Collections.unmodifiableList(Arrays.asList("\r", "%0d", "%0D")); + + private static final List FORBIDDEN_LINE_SEPARATOR = Collections.unmodifiableList(Arrays.asList("\u2028")); + + private static final List FORBIDDEN_PARAGRAPH_SEPARATOR = Collections + .unmodifiableList(Arrays.asList("\u2029")); + + private Set encodedUrlBlocklist = new HashSet<>(); + + private Set decodedUrlBlocklist = new HashSet<>(); + + private Set allowedHttpMethods = createDefaultAllowedHttpMethods(); + + private Predicate allowedHostnames = (hostname) -> true; + + private static final Pattern ASSIGNED_AND_NOT_ISO_CONTROL_PATTERN = Pattern + .compile("[\\p{IsAssigned}&&[^\\p{IsControl}]]*"); + + private static final Predicate ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE = ( + s) -> ASSIGNED_AND_NOT_ISO_CONTROL_PATTERN.matcher(s).matches(); + + private static final Pattern HEADER_VALUE_PATTERN = Pattern.compile("[\\p{IsAssigned}&&[[^\\p{IsControl}]||\\t]]*"); + + private static final Predicate HEADER_VALUE_PREDICATE = (s) -> s == null + || HEADER_VALUE_PATTERN.matcher(s).matches(); + + private Predicate allowedHeaderNames = ALLOWED_HEADER_NAMES; + + public static final Predicate ALLOWED_HEADER_NAMES = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE; + + private Predicate allowedHeaderValues = ALLOWED_HEADER_VALUES; + + public static final Predicate ALLOWED_HEADER_VALUES = HEADER_VALUE_PREDICATE; + + private Predicate allowedParameterNames = ALLOWED_PARAMETER_NAMES; + + public static final Predicate ALLOWED_PARAMETER_NAMES = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE; + + private Predicate allowedParameterValues = ALLOWED_PARAMETER_VALUES; + + public static final Predicate ALLOWED_PARAMETER_VALUES = (value) -> true; + + public StrictServerWebExchangeFirewall() { + urlBlocklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlocklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlocklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH); + urlBlocklistsAddAll(FORBIDDEN_BACKSLASH); + urlBlocklistsAddAll(FORBIDDEN_NULL); + urlBlocklistsAddAll(FORBIDDEN_LF); + urlBlocklistsAddAll(FORBIDDEN_CR); + + this.encodedUrlBlocklist.add(ENCODED_PERCENT); + this.encodedUrlBlocklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.decodedUrlBlocklist.add(PERCENT); + this.decodedUrlBlocklist.addAll(FORBIDDEN_LINE_SEPARATOR); + this.decodedUrlBlocklist.addAll(FORBIDDEN_PARAGRAPH_SEPARATOR); + } + + public Set getEncodedUrlBlocklist() { + return this.encodedUrlBlocklist; + } + + public Set getDecodedUrlBlocklist() { + return this.decodedUrlBlocklist; + } + + @Override + public Mono getFirewalledExchange(ServerWebExchange exchange) { + return Mono.fromCallable(() -> { + ServerHttpRequest request = exchange.getRequest(); + rejectForbiddenHttpMethod(request); + rejectedBlocklistedUrls(request); + rejectedUntrustedHosts(request); + if (!isNormalized(request)) { + throw new ServerExchangeRejectedException( + "The request was rejected because the URL was not normalized"); + } + + exchange.getResponse().beforeCommit(() -> Mono.fromRunnable(() -> { + ServerHttpResponse response = exchange.getResponse(); + HttpHeaders headers = response.getHeaders(); + for (Map.Entry> header : headers.entrySet()) { + String headerName = header.getKey(); + List headerValues = header.getValue(); + for (String headerValue : headerValues) { + validateCrlf(headerName, headerValue); + } + } + })); + return new StrictFirewallServerWebExchange(exchange); + }); + } + + private static void validateCrlf(String name, String value) { + Assert.isTrue(!hasCrlf(name) && !hasCrlf(value), () -> "Invalid characters (CR/LF) in header " + name); + } + + private static boolean hasCrlf(String value) { + return value != null && (value.indexOf('\n') != -1 || value.indexOf('\r') != -1); + } + + /** + * Sets if any HTTP method is allowed. If this set to true, then no validation on the + * HTTP method will be performed. This can open the application up to + * HTTP + * Verb tampering and XST attacks + * @param unsafeAllowAnyHttpMethod if true, disables HTTP method validation, else + * resets back to the defaults. Default is false. + * @since 5.1 + * @see #setAllowedHttpMethods(Collection) + */ + public void setUnsafeAllowAnyHttpMethod(boolean unsafeAllowAnyHttpMethod) { + this.allowedHttpMethods = unsafeAllowAnyHttpMethod ? ALLOW_ANY_HTTP_METHOD : createDefaultAllowedHttpMethods(); + } + + /** + *

+ * Determines which HTTP methods should be allowed. The default is to allow "DELETE", + * "GET", "HEAD", "OPTIONS", "PATCH", "POST", and "PUT". + *

+ * @param allowedHttpMethods the case-sensitive collection of HTTP methods that are + * allowed. + * @since 5.1 + * @see #setUnsafeAllowAnyHttpMethod(boolean) + */ + public void setAllowedHttpMethods(Collection allowedHttpMethods) { + Assert.notNull(allowedHttpMethods, "allowedHttpMethods cannot be null"); + this.allowedHttpMethods = (allowedHttpMethods != ALLOW_ANY_HTTP_METHOD) ? new HashSet<>(allowedHttpMethods) + : ALLOW_ANY_HTTP_METHOD; + } + + /** + *

+ * Determines if semicolon is allowed in the URL (i.e. matrix variables). The default + * is to disable this behavior because it is a common way of attempting to perform + * Reflected File + * Download Attacks. It is also the source of many exploits which bypass URL based + * security. + *

+ *

+ * For example, the following CVEs are a subset of the issues related to ambiguities + * in the Servlet Specification on how to treat semicolons that led to CVEs: + *

+ * + * + *

+ * If you are wanting to allow semicolons, please reconsider as it is a very common + * source of security bypasses. A few common reasons users want semicolons and + * alternatives are listed below: + *

+ *
    + *
  • Including the JSESSIONID in the path - You should not include session id (or + * any sensitive information) in a URL as it can lead to leaking. Instead use Cookies. + *
    • + *
  • Matrix Variables - Users wanting to leverage Matrix Variables should consider + * using HTTP parameters instead.
    • + *
+ * @param allowSemicolon should semicolons be allowed in the URL. Default is false + */ + public void setAllowSemicolon(boolean allowSemicolon) { + if (allowSemicolon) { + urlBlocklistsRemoveAll(FORBIDDEN_SEMICOLON); + } + else { + urlBlocklistsAddAll(FORBIDDEN_SEMICOLON); + } + } + + /** + *

+ * Determines if a slash "/" that is URL encoded "%2F" should be allowed in the path + * or not. The default is to not allow this behavior because it is a common way to + * bypass URL based security. + *

+ *

+ * For example, due to ambiguities in the servlet specification, the value is not + * parsed consistently which results in different values in {@code HttpServletRequest} + * path related values which allow bypassing certain security constraints. + *

+ * @param allowUrlEncodedSlash should a slash "/" that is URL encoded "%2F" be allowed + * in the path or not. Default is false. + */ + public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) { + if (allowUrlEncodedSlash) { + urlBlocklistsRemoveAll(FORBIDDEN_FORWARDSLASH); + } + else { + urlBlocklistsAddAll(FORBIDDEN_FORWARDSLASH); + } + } + + /** + *

+ * Determines if double slash "//" that is URL encoded "%2F%2F" should be allowed in + * the path or not. The default is to not allow. + *

+ * @param allowUrlEncodedDoubleSlash should a slash "//" that is URL encoded "%2F%2F" + * be allowed in the path or not. Default is false. + */ + public void setAllowUrlEncodedDoubleSlash(boolean allowUrlEncodedDoubleSlash) { + if (allowUrlEncodedDoubleSlash) { + urlBlocklistsRemoveAll(FORBIDDEN_DOUBLE_FORWARDSLASH); + } + else { + urlBlocklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH); + } + } + + /** + *

+ * Determines if a period "." that is URL encoded "%2E" should be allowed in the path + * or not. The default is to not allow this behavior because it is a frequent source + * of security exploits. + *

+ *

+ * For example, due to ambiguities in the servlet specification a URL encoded period + * might lead to bypassing security constraints through a directory traversal attack. + * This is because the path is not parsed consistently which results in different + * values in {@code HttpServletRequest} path related values which allow bypassing + * certain security constraints. + *

+ * @param allowUrlEncodedPeriod should a period "." that is URL encoded "%2E" be + * allowed in the path or not. Default is false. + */ + public void setAllowUrlEncodedPeriod(boolean allowUrlEncodedPeriod) { + if (allowUrlEncodedPeriod) { + this.encodedUrlBlocklist.removeAll(FORBIDDEN_ENCODED_PERIOD); + } + else { + this.encodedUrlBlocklist.addAll(FORBIDDEN_ENCODED_PERIOD); + } + } + + /** + *

+ * Determines if a backslash "\" or a URL encoded backslash "%5C" should be allowed in + * the path or not. The default is not to allow this behavior because it is a frequent + * source of security exploits. + *

+ *

+ * For example, due to ambiguities in the servlet specification a URL encoded period + * might lead to bypassing security constraints through a directory traversal attack. + * This is because the path is not parsed consistently which results in different + * values in {@code HttpServletRequest} path related values which allow bypassing + * certain security constraints. + *

+ * @param allowBackSlash a backslash "\" or a URL encoded backslash "%5C" be allowed + * in the path or not. Default is false + */ + public void setAllowBackSlash(boolean allowBackSlash) { + if (allowBackSlash) { + urlBlocklistsRemoveAll(FORBIDDEN_BACKSLASH); + } + else { + urlBlocklistsAddAll(FORBIDDEN_BACKSLASH); + } + } + + /** + *

+ * Determines if a null "\0" or a URL encoded nul "%00" should be allowed in the path + * or not. The default is not to allow this behavior because it is a frequent source + * of security exploits. + *

+ * @param allowNull a null "\0" or a URL encoded null "%00" be allowed in the path or + * not. Default is false + * @since 5.4 + */ + public void setAllowNull(boolean allowNull) { + if (allowNull) { + urlBlocklistsRemoveAll(FORBIDDEN_NULL); + } + else { + urlBlocklistsAddAll(FORBIDDEN_NULL); + } + } + + /** + *

+ * Determines if a percent "%" that is URL encoded "%25" should be allowed in the path + * or not. The default is not to allow this behavior because it is a frequent source + * of security exploits. + *

+ *

+ * For example, this can lead to exploits that involve double URL encoding that lead + * to bypassing security constraints. + *

+ * @param allowUrlEncodedPercent if a percent "%" that is URL encoded "%25" should be + * allowed in the path or not. Default is false + */ + public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) { + if (allowUrlEncodedPercent) { + this.encodedUrlBlocklist.remove(ENCODED_PERCENT); + this.decodedUrlBlocklist.remove(PERCENT); + } + else { + this.encodedUrlBlocklist.add(ENCODED_PERCENT); + this.decodedUrlBlocklist.add(PERCENT); + } + } + + /** + * Determines if a URL encoded Carriage Return is allowed in the path or not. The + * default is not to allow this behavior because it is a frequent source of security + * exploits. + * @param allowUrlEncodedCarriageReturn if URL encoded Carriage Return is allowed in + * the URL or not. Default is false. + */ + public void setAllowUrlEncodedCarriageReturn(boolean allowUrlEncodedCarriageReturn) { + if (allowUrlEncodedCarriageReturn) { + urlBlocklistsRemoveAll(FORBIDDEN_CR); + } + else { + urlBlocklistsAddAll(FORBIDDEN_CR); + } + } + + /** + * Determines if a URL encoded Line Feed is allowed in the path or not. The default is + * not to allow this behavior because it is a frequent source of security exploits. + * @param allowUrlEncodedLineFeed if URL encoded Line Feed is allowed in the URL or + * not. Default is false. + */ + public void setAllowUrlEncodedLineFeed(boolean allowUrlEncodedLineFeed) { + if (allowUrlEncodedLineFeed) { + urlBlocklistsRemoveAll(FORBIDDEN_LF); + } + else { + urlBlocklistsAddAll(FORBIDDEN_LF); + } + } + + /** + * Determines if a URL encoded paragraph separator is allowed in the path or not. The + * default is not to allow this behavior because it is a frequent source of security + * exploits. + * @param allowUrlEncodedParagraphSeparator if URL encoded paragraph separator is + * allowed in the URL or not. Default is false. + */ + public void setAllowUrlEncodedParagraphSeparator(boolean allowUrlEncodedParagraphSeparator) { + if (allowUrlEncodedParagraphSeparator) { + this.decodedUrlBlocklist.removeAll(FORBIDDEN_PARAGRAPH_SEPARATOR); + } + else { + this.decodedUrlBlocklist.addAll(FORBIDDEN_PARAGRAPH_SEPARATOR); + } + } + + /** + * Determines if a URL encoded line separator is allowed in the path or not. The + * default is not to allow this behavior because it is a frequent source of security + * exploits. + * @param allowUrlEncodedLineSeparator if URL encoded line separator is allowed in the + * URL or not. Default is false. + */ + public void setAllowUrlEncodedLineSeparator(boolean allowUrlEncodedLineSeparator) { + if (allowUrlEncodedLineSeparator) { + this.decodedUrlBlocklist.removeAll(FORBIDDEN_LINE_SEPARATOR); + } + else { + this.decodedUrlBlocklist.addAll(FORBIDDEN_LINE_SEPARATOR); + } + } + + /** + *

+ * Determines which header names should be allowed. The default is to reject header + * names that contain ISO control characters and characters that are not defined. + *

+ * @param allowedHeaderNames the predicate for testing header names + * @since 5.4 + * @see Character#isISOControl(int) + * @see Character#isDefined(int) + */ + public void setAllowedHeaderNames(Predicate allowedHeaderNames) { + Assert.notNull(allowedHeaderNames, "allowedHeaderNames cannot be null"); + this.allowedHeaderNames = allowedHeaderNames; + } + + /** + *

+ * Determines which header values should be allowed. The default is to reject header + * values that contain ISO control characters and characters that are not defined. + *

+ * @param allowedHeaderValues the predicate for testing hostnames + * @since 5.4 + * @see Character#isISOControl(int) + * @see Character#isDefined(int) + */ + public void setAllowedHeaderValues(Predicate allowedHeaderValues) { + Assert.notNull(allowedHeaderValues, "allowedHeaderValues cannot be null"); + this.allowedHeaderValues = allowedHeaderValues; + } + + /** + * Determines which parameter names should be allowed. The default is to reject header + * names that contain ISO control characters and characters that are not defined. + * @param allowedParameterNames the predicate for testing parameter names + * @since 5.4 + * @see Character#isISOControl(int) + * @see Character#isDefined(int) + */ + public void setAllowedParameterNames(Predicate allowedParameterNames) { + Assert.notNull(allowedParameterNames, "allowedParameterNames cannot be null"); + this.allowedParameterNames = allowedParameterNames; + } + + /** + *

+ * Determines which parameter values should be allowed. The default is to allow any + * parameter value. + *

+ * @param allowedParameterValues the predicate for testing parameter values + * @since 5.4 + */ + public void setAllowedParameterValues(Predicate allowedParameterValues) { + Assert.notNull(allowedParameterValues, "allowedParameterValues cannot be null"); + this.allowedParameterValues = allowedParameterValues; + } + + /** + *

+ * Determines which hostnames should be allowed. The default is to allow any hostname. + *

+ * @param allowedHostnames the predicate for testing hostnames + * @since 5.2 + */ + public void setAllowedHostnames(Predicate allowedHostnames) { + Assert.notNull(allowedHostnames, "allowedHostnames cannot be null"); + this.allowedHostnames = allowedHostnames; + } + + private void urlBlocklistsAddAll(Collection values) { + this.encodedUrlBlocklist.addAll(values); + this.decodedUrlBlocklist.addAll(values); + } + + private void urlBlocklistsRemoveAll(Collection values) { + this.encodedUrlBlocklist.removeAll(values); + this.decodedUrlBlocklist.removeAll(values); + } + + private void rejectNonPrintableAsciiCharactersInFieldName(String toCheck, String propertyName) { + if (!containsOnlyPrintableAsciiCharacters(toCheck)) { + throw new ServerExchangeRejectedException(String.format( + "The %s was rejected because it can only contain printable ASCII characters.", propertyName)); + } + } + + private void rejectForbiddenHttpMethod(ServerHttpRequest request) { + if (this.allowedHttpMethods == ALLOW_ANY_HTTP_METHOD) { + return; + } + if (!this.allowedHttpMethods.contains(request.getMethod())) { + throw new ServerExchangeRejectedException( + "The request was rejected because the HTTP method \"" + request.getMethod() + + "\" was not included within the list of allowed HTTP methods " + this.allowedHttpMethods); + } + } + + private void rejectedBlocklistedUrls(ServerHttpRequest request) { + for (String forbidden : this.encodedUrlBlocklist) { + if (encodedUrlContains(request, forbidden)) { + throw new ServerExchangeRejectedException( + "The request was rejected because the URL contained a potentially malicious String \"" + + forbidden + "\""); + } + } + for (String forbidden : this.decodedUrlBlocklist) { + if (decodedUrlContains(request, forbidden)) { + throw new ServerExchangeRejectedException( + "The request was rejected because the URL contained a potentially malicious String \"" + + forbidden + "\""); + } + } + } + + private void rejectedUntrustedHosts(ServerHttpRequest request) { + String hostName = request.getURI().getHost(); + if (hostName != null && !this.allowedHostnames.test(hostName)) { + throw new ServerExchangeRejectedException( + "The request was rejected because the domain " + hostName + " is untrusted."); + } + } + + private static Set createDefaultAllowedHttpMethods() { + Set result = new HashSet<>(); + result.add(HttpMethod.DELETE); + result.add(HttpMethod.GET); + result.add(HttpMethod.HEAD); + result.add(HttpMethod.OPTIONS); + result.add(HttpMethod.PATCH); + result.add(HttpMethod.POST); + result.add(HttpMethod.PUT); + return result; + } + + private boolean isNormalized(ServerHttpRequest request) { + if (!isNormalized(request.getPath().value())) { + return false; + } + if (!isNormalized(request.getURI().getRawPath())) { + return false; + } + if (!isNormalized(request.getURI().getPath())) { + return false; + } + return true; + } + + private void validateAllowedHeaderName(String headerNames) { + if (!StrictServerWebExchangeFirewall.this.allowedHeaderNames.test(headerNames)) { + throw new ServerExchangeRejectedException( + "The request was rejected because the header name \"" + headerNames + "\" is not allowed."); + } + } + + private void validateAllowedHeaderValue(Object key, String value) { + if (!StrictServerWebExchangeFirewall.this.allowedHeaderValues.test(value)) { + throw new ServerExchangeRejectedException("The request was rejected because the header: \"" + key + + " \" has a value \"" + value + "\" that is not allowed."); + } + } + + private void validateAllowedParameterName(String name) { + if (!StrictServerWebExchangeFirewall.this.allowedParameterNames.test(name)) { + throw new ServerExchangeRejectedException( + "The request was rejected because the parameter name \"" + name + "\" is not allowed."); + } + } + + private void validateAllowedParameterValue(String name, String value) { + if (!StrictServerWebExchangeFirewall.this.allowedParameterValues.test(value)) { + throw new ServerExchangeRejectedException("The request was rejected because the parameter: \"" + name + + " \" has a value \"" + value + "\" that is not allowed."); + } + } + + private static boolean encodedUrlContains(ServerHttpRequest request, String value) { + if (valueContains(request.getPath().value(), value)) { + return true; + } + return valueContains(request.getURI().getRawPath(), value); + } + + private static boolean decodedUrlContains(ServerHttpRequest request, String value) { + return valueContains(request.getURI().getPath(), value); + } + + private static boolean containsOnlyPrintableAsciiCharacters(String uri) { + if (uri == null) { + return true; + } + int length = uri.length(); + for (int i = 0; i < length; i++) { + char ch = uri.charAt(i); + if (ch < '\u0020' || ch > '\u007e') { + return false; + } + } + return true; + } + + private static boolean valueContains(String value, String contains) { + return value != null && value.contains(contains); + } + + /** + * Checks whether a path is normalized (doesn't contain path traversal sequences like + * "./", "/../" or "/.") + * @param path the path to test + * @return true if the path doesn't contain any path-traversal character sequences. + */ + private static boolean isNormalized(String path) { + if (path == null) { + return true; + } + for (int i = path.length(); i > 0;) { + int slashIndex = path.lastIndexOf('/', i - 1); + int gap = i - slashIndex; + if (gap == 2 && path.charAt(slashIndex + 1) == '.') { + return false; // ".", "/./" or "/." + } + if (gap == 3 && path.charAt(slashIndex + 1) == '.' && path.charAt(slashIndex + 2) == '.') { + return false; + } + i = slashIndex; + } + return true; + } + + private final class StrictFirewallServerWebExchange extends ServerWebExchangeDecorator { + + private StrictFirewallServerWebExchange(ServerWebExchange delegate) { + super(delegate); + } + + @Override + public ServerHttpRequest getRequest() { + return new StrictFirewallHttpRequest(super.getRequest()); + } + + private final class StrictFirewallHttpRequest extends ServerHttpRequestDecorator { + + private StrictFirewallHttpRequest(ServerHttpRequest delegate) { + super(delegate); + } + + @Override + public HttpHeaders getHeaders() { + return new StrictFirewallHttpHeaders(super.getHeaders()); + } + + @Override + public MultiValueMap getQueryParams() { + MultiValueMap queryParams = super.getQueryParams(); + for (Map.Entry> paramEntry : queryParams.entrySet()) { + String paramName = paramEntry.getKey(); + validateAllowedParameterName(paramName); + for (String paramValue : paramEntry.getValue()) { + validateAllowedParameterValue(paramName, paramValue); + } + } + return queryParams; + } + + private final class StrictFirewallHttpHeaders extends HttpHeaders { + + private StrictFirewallHttpHeaders(HttpHeaders delegate) { + super(delegate); + } + + @Override + public String getFirst(String headerName) { + validateAllowedHeaderName(headerName); + String headerValue = super.getFirst(headerName); + validateAllowedHeaderValue(headerName, headerValue); + return headerValue; + } + + @Override + public List get(Object key) { + if (key instanceof String) { + String headerName = (String) key; + validateAllowedHeaderName(headerName); + } + List headerValues = super.get(key); + if (headerValues == null) { + return headerValues; + } + for (String headerValue : headerValues) { + validateAllowedHeaderValue(key, headerValue); + } + return headerValues; + } + + @Override + public Set keySet() { + Set headerNames = super.keySet(); + for (String headerName : headerNames) { + validateAllowedHeaderName(headerName); + } + return headerNames; + } + + } + + } + + } + +} diff --git a/web/src/test/java/org/springframework/security/web/server/WebFilterChainProxyTests.java b/web/src/test/java/org/springframework/security/web/server/WebFilterChainProxyTests.java index 4f527b68504..9025876eb00 100644 --- a/web/src/test/java/org/springframework/security/web/server/WebFilterChainProxyTests.java +++ b/web/src/test/java/org/springframework/security/web/server/WebFilterChainProxyTests.java @@ -23,6 +23,9 @@ import reactor.core.publisher.Mono; import org.springframework.http.HttpStatus; +import org.springframework.security.web.server.firewall.ServerExchangeRejectedException; +import org.springframework.security.web.server.firewall.ServerExchangeRejectedHandler; +import org.springframework.security.web.server.firewall.ServerWebExchangeFirewall; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher.MatchResult; import org.springframework.test.web.reactive.server.WebTestClient; @@ -30,6 +33,12 @@ import org.springframework.web.server.WebFilter; import org.springframework.web.server.WebFilterChain; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; + /** * @author Rob Winch * @since 5.0 @@ -47,6 +56,72 @@ public void filterWhenNoMatchThenContinuesChainAnd404() { .isNotFound(); } + @Test + void doFilterWhenFirewallThenBadRequest() { + List filters = Arrays.asList(new Http200WebFilter()); + ServerWebExchangeMatcher notMatch = (exchange) -> MatchResult.notMatch(); + MatcherSecurityWebFilterChain chain = new MatcherSecurityWebFilterChain(notMatch, filters); + WebFilterChainProxy filter = new WebFilterChainProxy(chain); + WebTestClient.bindToController(new Object()).webFilter(filter).build().get().uri("/invalid;/").exchange() + .expectStatus().isBadRequest(); + } + + @Test + void doFilterWhenCustomFirewallThenInvoked() { + List filters = Arrays.asList(new Http200WebFilter()); + ServerWebExchangeMatcher notMatch = (exchange) -> MatchResult.notMatch(); + MatcherSecurityWebFilterChain chain = new MatcherSecurityWebFilterChain(notMatch, filters); + WebFilterChainProxy filter = new WebFilterChainProxy(chain); + ServerExchangeRejectedHandler handler = mock(ServerExchangeRejectedHandler.class); + ServerWebExchangeFirewall firewall = mock(ServerWebExchangeFirewall.class); + filter.setFirewall(firewall); + filter.setExchangeRejectedHandler(handler); + WebTestClient.bindToController(new Object()).webFilter(filter).build().get().exchange(); + verify(firewall).getFirewalledExchange(any()); + verifyNoInteractions(handler); + } + + @Test + void doFilterWhenCustomExchangeRejectedHandlerThenInvoked() { + List filters = Arrays.asList(new Http200WebFilter()); + ServerWebExchangeMatcher notMatch = (exchange) -> MatchResult.notMatch(); + MatcherSecurityWebFilterChain chain = new MatcherSecurityWebFilterChain(notMatch, filters); + WebFilterChainProxy filter = new WebFilterChainProxy(chain); + ServerExchangeRejectedHandler handler = mock(ServerExchangeRejectedHandler.class); + ServerWebExchangeFirewall firewall = mock(ServerWebExchangeFirewall.class); + given(firewall.getFirewalledExchange(any())) + .willReturn(Mono.error(new ServerExchangeRejectedException("Oops"))); + filter.setFirewall(firewall); + filter.setExchangeRejectedHandler(handler); + WebTestClient.bindToController(new Object()).webFilter(filter).build().get().exchange(); + verify(firewall).getFirewalledExchange(any()); + verify(handler).handle(any(), any()); + } + + @Test + void doFilterWhenDelayedServerExchangeRejectedException() { + List filters = Arrays.asList(new WebFilter() { + @Override + public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { + // simulate a delayed error (e.g. reading parameters) + return Mono.error(new ServerExchangeRejectedException("Ooops")); + } + }); + ServerWebExchangeMatcher match = (exchange) -> MatchResult.match(); + MatcherSecurityWebFilterChain chain = new MatcherSecurityWebFilterChain(match, filters); + WebFilterChainProxy filter = new WebFilterChainProxy(chain); + ServerExchangeRejectedHandler handler = mock(ServerExchangeRejectedHandler.class); + filter.setExchangeRejectedHandler(handler); + // @formatter:off + WebTestClient.bindToController(new Object()) + .webFilter(filter) + .build() + .get() + .exchange(); + // @formatter:on + verify(handler).handle(any(), any()); + } + static class Http200WebFilter implements WebFilter { @Override diff --git a/web/src/test/java/org/springframework/security/web/server/firewall/StrictServerWebExchangeFirewallTests.java b/web/src/test/java/org/springframework/security/web/server/firewall/StrictServerWebExchangeFirewallTests.java new file mode 100644 index 00000000000..397b0ecfdcb --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/server/firewall/StrictServerWebExchangeFirewallTests.java @@ -0,0 +1,512 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.server.firewall; + +import java.net.URI; +import java.util.Arrays; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseCookie; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +/** + * Tests for {@link StrictServerWebExchangeFirewall}. + * + * @author Rob Winch + * @since 5.7.13 + */ +class StrictServerWebExchangeFirewallTests { + + public String[] unnormalizedPaths = { "http://exploit.example/..", "http://exploit.example/./path/", + "http://exploit.example/path/path/.", "http://exploit.example/path/path//.", + "http://exploit.example/./path/../path//.", "http://exploit.example/./path", + "http://exploit.example/.//path", "http://exploit.example/.", "http://exploit.example//path", + "http://exploit.example//path/path", "http://exploit.example//path//path", + "http://exploit.example/path//path" }; + + private StrictServerWebExchangeFirewall firewall = new StrictServerWebExchangeFirewall(); + + private MockServerHttpRequest.BaseBuilder request = get("/"); + + @Test + void cookieWhenHasNewLineThenThrowsException() { + assertThatIllegalArgumentException().isThrownBy(() -> ResponseCookie.from("test", "Something\nhere").build()); + } + + @Test + void cookieWhenHasLineFeedThenThrowsException() { + assertThatIllegalArgumentException().isThrownBy(() -> ResponseCookie.from("test", "Something\rhere").build()); + } + + @Test + void responseHeadersWhenValueHasNewLineThenThrowsException() { + this.request = MockServerHttpRequest.get("/"); + ServerWebExchange exchange = getFirewalledExchange(); + exchange.getResponse().getHeaders().set("FOO", "new\nline"); + assertThatIllegalArgumentException().isThrownBy(() -> exchange.getResponse().setComplete().block()); + } + + @Test + void responseHeadersWhenValueHasLineFeedThenThrowsException() { + this.request = MockServerHttpRequest.get("/"); + ServerWebExchange exchange = getFirewalledExchange(); + exchange.getResponse().getHeaders().set("FOO", "line\rfeed"); + assertThatIllegalArgumentException().isThrownBy(() -> exchange.getResponse().setComplete().block()); + } + + @Test + void responseHeadersWhenNameHasNewLineThenThrowsException() { + this.request = MockServerHttpRequest.get("/"); + ServerWebExchange exchange = getFirewalledExchange(); + exchange.getResponse().getHeaders().set("new\nline", "FOO"); + assertThatIllegalArgumentException().isThrownBy(() -> exchange.getResponse().setComplete().block()); + } + + @Test + void responseHeadersWhenNameHasLineFeedThenThrowsException() { + this.request = MockServerHttpRequest.get("/"); + ServerWebExchange exchange = getFirewalledExchange(); + exchange.getResponse().getHeaders().set("line\rfeed", "FOO"); + assertThatIllegalArgumentException().isThrownBy(() -> exchange.getResponse().setComplete().block()); + } + + @Test + void getFirewalledExchangeWhenInvalidMethodThenThrowsServerExchangeRejectedException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> MockServerHttpRequest.method(HttpMethod.valueOf("INVALID"), "/")); + } + + private ServerWebExchange getFirewalledExchange() { + MockServerWebExchange exchange = MockServerWebExchange.from(this.request.build()); + return this.firewall.getFirewalledExchange(exchange).block(); + } + + private MockServerHttpRequest.BodyBuilder get(String uri) { + URI url = URI.create(uri); + return MockServerHttpRequest.method(HttpMethod.GET, url); + } + + // blocks XST attacks + @Test + void getFirewalledExchangeWhenTraceMethodThenThrowsServerExchangeRejectedException() { + this.request = MockServerHttpRequest.method(HttpMethod.TRACE, "/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + // blocks XST attack if request is forwarded to a Microsoft IIS web server + void getFirewalledExchangeWhenTrackMethodThenThrowsServerExchangeRejectedException() { + this.request = MockServerHttpRequest.method(HttpMethod.TRACE, "/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + // HTTP methods are case sensitive + void getFirewalledExchangeWhenLowercaseGetThenThrowsServerExchangeRejectedException() { + assertThatIllegalArgumentException().isThrownBy(() -> HttpMethod.valueOf("get")); + } + + @Test + void getFirewalledExchangeWhenAllowedThenNoException() { + List allowedMethods = Arrays.asList("DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT"); + for (String allowedMethod : allowedMethods) { + this.request = MockServerHttpRequest.method(HttpMethod.valueOf(allowedMethod), "/"); + getFirewalledExchange(); + } + } + + @Test + void getFirewalledExchangeWhenInvalidMethodAndAnyMethodThenNoException() { + this.firewall.setUnsafeAllowAnyHttpMethod(true); + assertThatIllegalArgumentException() + .isThrownBy(() -> MockServerHttpRequest.method(HttpMethod.valueOf("INVALID"), "/")); + } + + @Test + void getFirewalledExchangeWhenURINotNormalizedThenThrowsServerExchangeRejectedException() { + for (String path : this.unnormalizedPaths) { + this.request = get(path); + assertThatExceptionOfType(ServerExchangeRejectedException.class) + .describedAs("The path '" + path + "' is not normalized").isThrownBy(() -> getFirewalledExchange()); + } + } + + @Test + void getFirewalledExchangeWhenSemicolonInRequestUriThenThrowsServerExchangeRejectedException() { + this.request = get("/path;/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenEncodedSemicolonInRequestUriThenThrowsServerExchangeRejectedException() { + this.request = get("/path%3B/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenLowercaseEncodedSemicolonInRequestUriThenThrowsServerExchangeRejectedException() { + this.request = MockServerHttpRequest.method(HttpMethod.GET, URI.create("/path%3b/")); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request = get("/path;/"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request = get("/path%3B/"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenLowercaseEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request = get("/path%3b/"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenLowercaseEncodedPeriodInThenThrowsServerExchangeRejectedException() { + this.request = get("/%2e/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenContainsLowerboundAsciiThenNoException() { + this.request = get("/%20"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenContainsUpperboundAsciiThenNoException() { + this.request = get("/~"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenJapaneseCharacterThenNoException() { + // FIXME: .method(HttpMethod.GET to .get and similar methods + this.request = get("/\u3042"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenContainsEncodedNullThenException() { + this.request = get("/something%00/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenContainsLowercaseEncodedLineFeedThenException() { + this.request = get("/something%0a/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenContainsUppercaseEncodedLineFeedThenException() { + this.request = get("/something%0A/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenContainsLowercaseEncodedCarriageReturnThenException() { + this.request = get("/something%0d/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenContainsUppercaseEncodedCarriageReturnThenException() { + this.request = get("/something%0D/"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenContainsLowercaseEncodedLineFeedAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedLineFeed(true); + this.request = get("/something%0a/"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenContainsUppercaseEncodedLineFeedAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedLineFeed(true); + this.request = get("/something%0A/"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenContainsLowercaseEncodedCarriageReturnAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedCarriageReturn(true); + this.request = get("/something%0d/"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenContainsUppercaseEncodedCarriageReturnAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedCarriageReturn(true); + this.request = get("/something%0D/"); + getFirewalledExchange(); + } + + /** + * On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on /a/b/c + * because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c while Spring MVC + * will strip the ; content from requestURI before the path is URL decoded. + */ + @Test + void getFirewalledExchangeWhenLowercaseEncodedPathThenException() { + this.request = get("/context-root/a/b;%2f1/c"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenUppercaseEncodedPathThenException() { + this.request = get("/context-root/a/b;%2F1/c"); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + this.request = get("/context-root/a/b;%2f1/c"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + this.request = get("/context-root/a/b;%2F1/c"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenAllowUrlLowerCaseEncodedDoubleSlashThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowUrlEncodedDoubleSlash(true); + this.request = get("/context-root/a/b%2f%2fc"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenAllowUrlUpperCaseEncodedDoubleSlashThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowUrlEncodedDoubleSlash(true); + this.request = get("/context-root/a/b%2F%2Fc"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenAllowUrlLowerCaseAndUpperCaseEncodedDoubleSlashThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowUrlEncodedDoubleSlash(true); + this.request = get("/context-root/a/b%2f%2Fc"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenAllowUrlUpperCaseAndLowerCaseEncodedDoubleSlashThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowUrlEncodedDoubleSlash(true); + this.request = get("/context-root/a/b%2F%2fc"); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenRemoveFromUpperCaseEncodedUrlBlocklistThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.request = get("/context-root/a/b%2Fc"); + this.firewall.getEncodedUrlBlocklist().removeAll(Arrays.asList("%2F%2F")); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenRemoveFromDecodedUrlBlocklistThenNoException() { + this.request = get("/a/b%2F%2Fc"); + this.firewall.getDecodedUrlBlocklist().removeAll(Arrays.asList("//")); + this.firewall.getEncodedUrlBlocklist().removeAll(Arrays.asList("%2F%2F")); + this.firewall.getEncodedUrlBlocklist().removeAll(Arrays.asList("%2F")); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenTrustedDomainThenNoException() { + this.request.header("Host", "example.org"); + this.firewall.setAllowedHostnames((hostname) -> hostname.equals("example.org")); + getFirewalledExchange(); + } + + @Test + void getFirewalledExchangeWhenUntrustedDomainThenException() { + this.request = get("https://example.org"); + this.firewall.setAllowedHostnames((hostname) -> hostname.equals("myexample.org")); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> getFirewalledExchange()); + } + + @Test + void getFirewalledExchangeGetHeaderWhenNotAllowedHeaderNameThenException() { + this.firewall.setAllowedHeaderNames((name) -> !name.equals("bad name")); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.get("bad name")); + } + + @Test + void getFirewalledExchangeWhenHeaderNameNotAllowedWithAugmentedHeaderNamesThenException() { + this.firewall.setAllowedHeaderNames( + StrictServerWebExchangeFirewall.ALLOWED_HEADER_NAMES.and((name) -> !name.equals("bad name"))); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.getFirst("bad name")); + } + + @Test + void getFirewalledExchangeGetHeaderWhenNotAllowedHeaderValueThenException() { + this.request.header("good name", "bad value"); + this.firewall.setAllowedHeaderValues((value) -> !value.equals("bad value")); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.get("good name")); + } + + @Test + void getFirewalledExchangeWhenHeaderValueNotAllowedWithAugmentedHeaderValuesThenException() { + this.request.header("good name", "bad value"); + this.firewall.setAllowedHeaderValues( + StrictServerWebExchangeFirewall.ALLOWED_HEADER_VALUES.and((value) -> !value.equals("bad value"))); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.get("good name")); + } + + @Test + void getFirewalledExchangeGetDateHeaderWhenControlCharacterInHeaderNameThenException() { + this.request.header("Bad\0Name", "some value"); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.get("Bad\0Name")); + } + + @Test + void getFirewalledExchangeGetHeaderWhenUndefinedCharacterInHeaderNameThenException() { + this.request.header("Bad\uFFFEName", "some value"); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.get("Bad\uFFFEName")); + } + + @Test + void getFirewalledExchangeGetHeadersWhenControlCharacterInHeaderNameThenException() { + this.request.header("Bad\0Name", "some value"); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.get("Bad\0Name")); + } + + @Test + void getFirewalledExchangeGetHeaderNamesWhenControlCharacterInHeaderNameThenException() { + this.request.header("Bad\0Name", "some value"); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class) + .isThrownBy(() -> headers.keySet().iterator().next()); + } + + @Test + void getFirewalledExchangeGetHeaderWhenControlCharacterInHeaderValueThenException() { + this.request.header("Something", "bad\0value"); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.get("Something")); + } + + @Test + void getFirewalledExchangeGetHeaderWhenHorizontalTabInHeaderValueThenNoException() { + this.request.header("Something", "tab\tvalue"); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThat(headers.getFirst("Something")).isEqualTo("tab\tvalue"); + } + + @Test + void getFirewalledExchangeGetHeaderWhenUndefinedCharacterInHeaderValueThenException() { + this.request.header("Something", "bad\uFFFEvalue"); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class) + .isThrownBy(() -> headers.getFirst("Something")); + } + + @Test + void getFirewalledExchangeGetHeadersWhenControlCharacterInHeaderValueThenException() { + this.request.header("Something", "bad\0value"); + ServerWebExchange exchange = getFirewalledExchange(); + HttpHeaders headers = exchange.getRequest().getHeaders(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> headers.get("Something")); + } + + @Test + void getFirewalledExchangeGetParameterWhenControlCharacterInParameterNameThenException() { + this.request.queryParam("Bad\0Name", "some value"); + ServerWebExchange exchange = getFirewalledExchange(); + ServerHttpRequest request = exchange.getRequest(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(request::getQueryParams); + } + + @Test + void getFirewalledExchangeGetParameterValuesWhenNotAllowedInParameterValueThenException() { + this.firewall.setAllowedParameterValues((value) -> !value.equals("bad value")); + this.request.queryParam("Something", "bad value"); + ServerWebExchange exchange = getFirewalledExchange(); + ServerHttpRequest request = exchange.getRequest(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> request.getQueryParams()); + } + + @Test + void getFirewalledExchangeGetParameterValuesWhenNotAllowedInParameterNameThenException() { + this.firewall.setAllowedParameterNames((value) -> !value.equals("bad name")); + this.request.queryParam("bad name", "good value"); + ServerWebExchange exchange = getFirewalledExchange(); + ServerHttpRequest request = exchange.getRequest(); + assertThatExceptionOfType(ServerExchangeRejectedException.class).isThrownBy(() -> request.getQueryParams()); + } + + // gh-9598 + @Test + void getFirewalledExchangeGetHeaderWhenNameIsNullThenNull() { + ServerWebExchange exchange = getFirewalledExchange(); + assertThat(exchange.getRequest().getHeaders().get(null)).isNull(); + } + +}