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 c64887cd..8961f224 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 @@ -113,6 +113,7 @@ import org.springframework.cloud.gateway.filter.headers.GRPCRequestHeadersFilter import org.springframework.cloud.gateway.filter.headers.GRPCResponseHeadersFilter; 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; @@ -305,6 +306,11 @@ public class GatewayAutoConfiguration { return new GRPCResponseHeadersFilter(); } + @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/main/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiter.java index dfa4a00e..1e93a8d4 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiter.java @@ -16,7 +16,6 @@ package org.springframework.cloud.gateway.filter.ratelimit; -import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -250,8 +249,7 @@ public class RedisRateLimiter extends AbstractRateLimiter keys = getKeys(id); // The arguments to the LUA script. time() returns unixtime in seconds. - List scriptArgs = Arrays.asList(replenishRate + "", burstCapacity + "", - Instant.now().getEpochSecond() + "", requestedTokens + ""); + List scriptArgs = Arrays.asList(replenishRate + "", burstCapacity + "", "", requestedTokens + ""); // allowed, tokens_left = redis.eval(SCRIPT, keys, args) Flux> flux = this.redisTemplate.execute(this.script, keys, scriptArgs); // .log("redisratelimiter", Level.FINER); diff --git a/spring-cloud-gateway-server/src/main/resources/META-INF/scripts/request_rate_limiter.lua b/spring-cloud-gateway-server/src/main/resources/META-INF/scripts/request_rate_limiter.lua index de316dd2..36a01733 100644 --- a/spring-cloud-gateway-server/src/main/resources/META-INF/scripts/request_rate_limiter.lua +++ b/spring-cloud-gateway-server/src/main/resources/META-INF/scripts/request_rate_limiter.lua @@ -1,10 +1,12 @@ +redis.replicate_commands() + local tokens_key = KEYS[1] local timestamp_key = KEYS[2] --redis.log(redis.LOG_WARNING, "tokens_key " .. tokens_key) local rate = tonumber(ARGV[1]) local capacity = tonumber(ARGV[2]) -local now = tonumber(ARGV[3]) +local now = redis.call('TIME')[1] local requested = tonumber(ARGV[4]) local fill_time = capacity/rate @@ -12,7 +14,7 @@ local ttl = math.floor(fill_time*2) --redis.log(redis.LOG_WARNING, "rate " .. ARGV[1]) --redis.log(redis.LOG_WARNING, "capacity " .. ARGV[2]) ---redis.log(redis.LOG_WARNING, "now " .. ARGV[3]) +--redis.log(redis.LOG_WARNING, "now " .. now) --redis.log(redis.LOG_WARNING, "requested " .. ARGV[4]) --redis.log(redis.LOG_WARNING, "filltime " .. fill_time) --redis.log(redis.LOG_WARNING, "ttl " .. ttl) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilterIntegrationTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilterIntegrationTests.java new file mode 100644 index 00000000..66e74240 --- /dev/null +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilterIntegrationTests.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 TransferEncodingNormalizationHeadersFilterIntegrationTests { + + private static final Log log = LogFactory.getLog(TransferEncodingNormalizationHeadersFilterIntegrationTests.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/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilterTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilterTests.java new file mode 100644 index 00000000..07d7bbbb --- /dev/null +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/TransferEncodingNormalizationHeadersFilterTests.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 TransferEncodingNormalizationHeadersFilterTests { + + @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/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