diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java index 1feff526..ca9e43f1 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java @@ -107,6 +107,7 @@ import org.springframework.cloud.gateway.filter.factory.rewrite.ModifyResponseBo import org.springframework.cloud.gateway.filter.headers.ForwardedHeadersFilter; import org.springframework.cloud.gateway.filter.headers.HttpHeadersFilter; import org.springframework.cloud.gateway.filter.headers.RemoveHopByHopHeadersFilter; +import org.springframework.cloud.gateway.filter.headers.TransferEncodingNormalizationHeadersFilter; import org.springframework.cloud.gateway.filter.headers.XForwardedHeadersFilter; import org.springframework.cloud.gateway.filter.ratelimit.KeyResolver; import org.springframework.cloud.gateway.filter.ratelimit.PrincipalNameKeyResolver; @@ -286,6 +287,11 @@ public class GatewayAutoConfiguration { return new XForwardedHeadersFilter(); } + @Bean + public TransferEncodingNormalizationHeadersFilter transferEncodingNormalizationHeadersFilter() { + return new TransferEncodingNormalizationHeadersFilter(); + } + // GlobalFilter beans @Bean diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilter.java new file mode 100644 index 00000000..e393c1ec --- /dev/null +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilter.java @@ -0,0 +1,49 @@ +/* + * Copyright 2013-2021 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.cloud.gateway.filter.headers; + +import org.springframework.core.Ordered; +import org.springframework.http.HttpHeaders; +import org.springframework.web.server.ServerWebExchange; + +/** + * See https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3 for details. + */ +public class TransferEncodingNormalizationHeadersFilter implements HttpHeadersFilter, Ordered { + + @Override + public int getOrder() { + return 1000; + } + + @Override + public HttpHeaders filter(HttpHeaders input, ServerWebExchange exchange) { + String transferEncoding = input.getFirst(HttpHeaders.TRANSFER_ENCODING); + if (transferEncoding != null && "chunked".equalsIgnoreCase(transferEncoding.trim()) + && input.containsKey(HttpHeaders.CONTENT_LENGTH)) { + + HttpHeaders filtered = new HttpHeaders(); + // avoids read only if input is read only + filtered.addAll(input); + filtered.remove(HttpHeaders.CONTENT_LENGTH); + return filtered; + } + + return input; + } + +} diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingMarmalizationHeadersFilterTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingMarmalizationHeadersFilterTests.java new file mode 100644 index 00000000..85eaf51a --- /dev/null +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingMarmalizationHeadersFilterTests.java @@ -0,0 +1,64 @@ +/* + * Copyright 2013-2020 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.cloud.gateway.filter.headers; + +import org.junit.Test; + +import org.springframework.http.HttpHeaders; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Spencer Gibb + */ +public class TransferEncodingMarmalizationHeadersFilterTests { + + @Test + public void noTransferEncodingWithContentLength() { + MockServerHttpRequest.BaseBuilder builder = MockServerHttpRequest.post("http://localhost/post") + .header(HttpHeaders.CONTENT_LENGTH, "6"); + + HttpHeaders headers = testFilter(MockServerWebExchange.from(builder)); + assertThat(headers).containsKey(HttpHeaders.CONTENT_LENGTH).doesNotContainKey(HttpHeaders.TRANSFER_ENCODING); + } + + @Test + public void transferEncodingWithContentLength() { + MockServerHttpRequest.BaseBuilder builder = MockServerHttpRequest.post("http://localhost/post") + .header(HttpHeaders.CONTENT_LENGTH, "6").header(HttpHeaders.TRANSFER_ENCODING, "chunked"); + + HttpHeaders headers = testFilter(MockServerWebExchange.from(builder)); + assertThat(headers).doesNotContainKey(HttpHeaders.CONTENT_LENGTH).containsKey(HttpHeaders.TRANSFER_ENCODING); + } + + @Test + public void transferEncodingCaseInsensitiveWithContentLength() { + MockServerHttpRequest.BaseBuilder builder = MockServerHttpRequest.post("http://localhost/post") + .header(HttpHeaders.CONTENT_LENGTH, "6").header(HttpHeaders.TRANSFER_ENCODING, "Chunked "); + + HttpHeaders headers = testFilter(MockServerWebExchange.from(builder)); + assertThat(headers).doesNotContainKey(HttpHeaders.CONTENT_LENGTH).containsKey(HttpHeaders.TRANSFER_ENCODING); + } + + private HttpHeaders testFilter(MockServerWebExchange exchange) { + TransferEncodingNormalizationHeadersFilter filter = new TransferEncodingNormalizationHeadersFilter(); + return filter.filter(exchange.getRequest().getHeaders(), exchange); + } + +} diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeardsFilterIntegrationTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeardsFilterIntegrationTests.java new file mode 100644 index 00000000..92c08a18 --- /dev/null +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeardsFilterIntegrationTests.java @@ -0,0 +1,152 @@ +/* + * Copyright 2013-2020 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.cloud.gateway.filter.headers; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.net.Socket; + +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.web.server.LocalServerPort; +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.gateway.route.RouteLocator; +import org.springframework.cloud.gateway.route.builder.RouteLocatorBuilder; +import org.springframework.cloud.gateway.test.PermitAllSecurityConfiguration; +import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient; +import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier; +import org.springframework.cloud.loadbalancer.support.ServiceInstanceListSuppliers; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Import; +import org.springframework.core.log.LogMessage; +import org.springframework.http.MediaType; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.util.StreamUtils; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RestController; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +@SpringBootTest(properties = {}, webEnvironment = RANDOM_PORT) +@ActiveProfiles("transferencoding") +public class TransferEncodingNormalizationHeardsFilterIntegrationTests { + + private static final Log log = LogFactory.getLog(TransferEncodingNormalizationHeardsFilterIntegrationTests.class); + + @LocalServerPort + private int port; + + @Test + void legitRequestShouldNotFail() throws Exception { + final ClassLoader classLoader = this.getClass().getClassLoader(); + + // Issue a crafted request with smuggling attempt + assert200With("Should Fail", + StreamUtils.copyToByteArray(classLoader.getResourceAsStream("transfer-encoding/invalid-request.bin"))); + + // Issue a legit request, which should not fail + assert200With("Should Not Fail", + StreamUtils.copyToByteArray(classLoader.getResourceAsStream("transfer-encoding/valid-request.bin"))); + } + + private void assert200With(String name, byte[] payload) throws Exception { + final String response = execute("localhost", port, payload); + log.info(LogMessage.format("Request to localhost:%d %s\n%s", port, name, new String(payload))); + assertThat(response).isNotNull(); + log.info(LogMessage.format("Response %s\n%s", name, response)); + assertThat(response).matches("HTTP/1.\\d 200 OK"); + } + + private String execute(String target, int port, byte[] payload) throws IOException { + final Socket socket = new Socket(target, port); + + final OutputStream out = socket.getOutputStream(); + final BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream())); + + out.write(payload); + + final String headResponse = in.readLine(); + + out.close(); + in.close(); + + return headResponse; + } + + @EnableAutoConfiguration + @SpringBootConfiguration + @Import(PermitAllSecurityConfiguration.class) + @LoadBalancerClient(name = "xferenc", configuration = TestLoadBalancerConfig.class) + @RestController + public static class TestConfig { + + @PostMapping(value = "/echo", produces = { MediaType.APPLICATION_JSON_VALUE }) + public Message message(@RequestBody Message message) throws IOException { + return message; + } + + @Bean + public RouteLocator routeLocator(RouteLocatorBuilder builder) { + return builder.routes() + .route("echo", r -> r.path("/route/echo").filters(f -> f.stripPrefix(1)).uri("lb://xferenc")) + .build(); + } + + } + + public static class Message { + + private String message; + + public Message(@JsonProperty("message") String message) { + this.message = message; + } + + public String getMessage() { + return message; + } + + public void setMessage(String message) { + this.message = message; + } + + } + + public static class TestLoadBalancerConfig { + + @LocalServerPort + protected int port = 0; + + @Bean + public ServiceInstanceListSupplier staticServiceInstanceListSupplier() { + return ServiceInstanceListSuppliers.from("xferenc", + new DefaultServiceInstance("xferenc" + "-1", "xferenc", "localhost", port, false)); + } + + } + +} diff --git a/spring-cloud-gateway-server/src/test/resources/transfer-encoding/invalid-request.bin b/spring-cloud-gateway-server/src/test/resources/transfer-encoding/invalid-request.bin new file mode 100644 index 00000000..4248e357 --- /dev/null +++ b/spring-cloud-gateway-server/src/test/resources/transfer-encoding/invalid-request.bin @@ -0,0 +1,13 @@ +POST /route/echo HTTP/1.0 +Host: localhost:8080 +Content-Length: 19 +Transfer-encoding: Chunked +Content-Type: application/json +Connection: close + +22 +{"message":"3"} + +GET /nonexistantpath123 HTTP/1.0 +0 + diff --git a/spring-cloud-gateway-server/src/test/resources/transfer-encoding/valid-request.bin b/spring-cloud-gateway-server/src/test/resources/transfer-encoding/valid-request.bin new file mode 100644 index 00000000..d23a2bb3 --- /dev/null +++ b/spring-cloud-gateway-server/src/test/resources/transfer-encoding/valid-request.bin @@ -0,0 +1,7 @@ +POST /route/echo HTTP/1.1 +Host: localhost:8080 +Content-Type: application/json +Content-Length: 15 +Connection: close + +{"message":"3"} \ No newline at end of file