You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2021/10/27 20:53:31 UTC
[trafficserver] 02/04: Reject Transfer-Encoding in pre-HTTP/1.1
requests (#8451)
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 9.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 420935b7d7c61ab94a8addaaa43b6affb1fa9ab7
Author: Brian Neradt <br...@verizonmedia.com>
AuthorDate: Wed Oct 27 13:34:46 2021 -0500
Reject Transfer-Encoding in pre-HTTP/1.1 requests (#8451)
Per spec, Transfer-Encoding is only supported in HTTP/1.1. For earlier
versions, we must reject Transfer-Encoding rather than interpret it
since downstream proxies may ignore the chunk header and rely upon the
Content-Length, or interpret the body some other way. These differences
in interpretation may open up the door to compatibility issues. To
protect against this, we reply with a 4xx if the client uses
Transfer-Encoding with HTTP versions that do not support it.
(cherry picked from commit 6e5070118a20772a30c3fccee2cf1c44f0a21fc0)
---
proxy/http/HttpTransact.cc | 11 +++++
.../chunked_encoding/bad_chunked_encoding.test.py | 53 ++++++++++++++++++++--
.../replays/chunked_in_http_1_0.replay.yaml | 48 ++++++++++++++++++++
3 files changed, 109 insertions(+), 3 deletions(-)
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 813dfda..380184d 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -5358,6 +5358,17 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr)
return BAD_CONNECT_PORT;
}
+ if (s->client_info.transfer_encoding == CHUNKED_ENCODING && incoming_hdr->version_get() < HTTP_1_1) {
+ // Per spec, Transfer-Encoding is only supported in HTTP/1.1. For earlier
+ // versions, we must reject Transfer-Encoding rather than interpret it
+ // since downstream proxies may ignore the chunk header and rely upon the
+ // Content-Length, or interpret the body some other way. These
+ // differences in interpretation may open up the door to compatibility
+ // issues. To protect against this, we reply with a 4xx if the client
+ // uses Transfer-Encoding with HTTP versions that do not support it.
+ return UNACCEPTABLE_TE_REQUIRED;
+ }
+
// Require Content-Length/Transfer-Encoding for POST/PUSH/PUT
if ((scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_HTTPS) &&
(method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) &&
diff --git a/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py b/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py
index 38e4206..d64daa5 100644
--- a/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py
+++ b/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py
@@ -71,6 +71,53 @@ tr.StillRunningAfter = server
tr.StillRunningAfter = ts
+class HTTP10Test:
+ chunkedReplayFile = "replays/chunked_in_http_1_0.replay.yaml"
+
+ def __init__(self):
+ self.setupOriginServer()
+ self.setupTS()
+
+ def setupOriginServer(self):
+ self.server = Test.MakeVerifierServerProcess("verifier-server1", self.chunkedReplayFile)
+
+ def setupTS(self):
+ self.ts = Test.MakeATSProcess("ts2", enable_tls=True, enable_cache=False)
+ self.ts.addDefaultSSLFiles()
+ self.ts.Disk.records_config.update({
+ "proxy.config.diags.debug.enabled": 1,
+ "proxy.config.diags.debug.tags": "http",
+ "proxy.config.ssl.server.cert.path": f'{self.ts.Variables.SSLDir}',
+ "proxy.config.ssl.server.private_key.path": f'{self.ts.Variables.SSLDir}',
+ "proxy.config.ssl.client.verify.server.policy": 'PERMISSIVE',
+ })
+ self.ts.Disk.ssl_multicert_config.AddLine(
+ 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
+ )
+ self.ts.Disk.remap_config.AddLine(
+ f"map / http://127.0.0.1:{self.server.Variables.http_port}/",
+ )
+
+ def runChunkedTraffic(self):
+ tr = Test.AddTestRun()
+ tr.AddVerifierClientProcess(
+ "client1",
+ self.chunkedReplayFile,
+ http_ports=[self.ts.Variables.port],
+ https_ports=[self.ts.Variables.ssl_port],
+ other_args='--thread-limit 1')
+ tr.Processes.Default.StartBefore(self.server)
+ tr.Processes.Default.StartBefore(self.ts)
+ tr.StillRunningAfter = self.server
+ tr.StillRunningAfter = self.ts
+
+ def run(self):
+ self.runChunkedTraffic()
+
+
+HTTP10Test().run()
+
+
class MalformedChunkHeaderTest:
chunkedReplayFile = "replays/malformed_chunked_header.replay.yaml"
@@ -79,7 +126,7 @@ class MalformedChunkHeaderTest:
self.setupTS()
def setupOriginServer(self):
- self.server = Test.MakeVerifierServerProcess("verifier-server", self.chunkedReplayFile)
+ self.server = Test.MakeVerifierServerProcess("verifier-server2", self.chunkedReplayFile)
# The server's responses will fail the first two transactions
# because ATS will close the connection due to the malformed
@@ -98,7 +145,7 @@ class MalformedChunkHeaderTest:
"Verify that the body never got through.")
def setupTS(self):
- self.ts = Test.MakeATSProcess("ts2", enable_tls=True, enable_cache=False)
+ self.ts = Test.MakeATSProcess("ts3", enable_tls=True, enable_cache=False)
self.ts.addDefaultSSLFiles()
self.ts.Disk.records_config.update({
"proxy.config.diags.debug.enabled": 1,
@@ -120,7 +167,7 @@ class MalformedChunkHeaderTest:
def runChunkedTraffic(self):
tr = Test.AddTestRun()
tr.AddVerifierClientProcess(
- "client",
+ "client2",
self.chunkedReplayFile,
http_ports=[self.ts.Variables.port],
https_ports=[self.ts.Variables.ssl_port],
diff --git a/tests/gold_tests/chunked_encoding/replays/chunked_in_http_1_0.replay.yaml b/tests/gold_tests/chunked_encoding/replays/chunked_in_http_1_0.replay.yaml
new file mode 100644
index 0000000..b432c66
--- /dev/null
+++ b/tests/gold_tests/chunked_encoding/replays/chunked_in_http_1_0.replay.yaml
@@ -0,0 +1,48 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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
+#
+# http://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.
+
+meta:
+ version: "1.0"
+
+sessions:
+- transactions:
+ - client-request:
+ method: "POST"
+ # HTTP/1.0 does not support Transfer-Encoding. ATS should therefore
+ # reject it with a 4xx response.
+ version: "1.0"
+ url: /unexpected/chunk/header
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ Transfer-Encoding, chunked ]
+ - [ uuid, 51 ]
+ content:
+ size: 32
+
+ # This request should not make it to the server, but if it does reply with
+ # a 200 response so that we detect the non-4xx response we expect.
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 16 ]
+ - [ X-Response, "Unexpected origin response." ]
+
+ proxy-response:
+ status: 406
+ reason: "Transcoding Not Available"