From ba17bcfe52012a3c760ca2ac391d14c5ffd39ca3 Mon Sep 17 00:00:00 2001 From: Zhengxi Li Date: Fri, 2 Jun 2023 17:33:56 +0000 Subject: [PATCH 1/3] Added HTTP/2 CONNECT test with PV Co-authored-by: Masakazu Kitajo --- tests/gold_tests/connect/connect.test.py | 64 +++++++++++++++++++ .../connect/replays/connect_h2.replay.yaml | 61 ++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 tests/gold_tests/connect/replays/connect_h2.replay.yaml diff --git a/tests/gold_tests/connect/connect.test.py b/tests/gold_tests/connect/connect.test.py index 95b25958a7c..90638ae3d44 100644 --- a/tests/gold_tests/connect/connect.test.py +++ b/tests/gold_tests/connect/connect.test.py @@ -165,3 +165,67 @@ def run(self): ConnectViaPVTest().run() + + +class ConnectViaPVTest2: + # This test executes a HTTP/2 CONNECT request with Proxy Verifier. + connectReplayFile = "replays/connect_h2.replay.yaml" + + def __init__(self): + self.setupOriginServer() + self.setupTS() + + def setupOriginServer(self): + self.server = Test.MakeVerifierServerProcess( + "connect-verifier-server2", + self.connectReplayFile) + # Verify server output + self.server.Streams.stdout += Testers.ExcludesExpression( + "test: connect-request", + "Verify the CONNECT request doesn't reach the server.") + self.server.Streams.stdout += Testers.ContainsExpression( + "GET /get HTTP/1.1\nuuid: 1\ntest: real-request", reflags=re.MULTILINE, + description="Verify the server gets the second(tunneled) request.") + + def setupTS(self): + self.ts = Test.MakeATSProcess("connect-ts2", enable_tls=True) + + self.ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|hpack', + '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.http.server_ports': f"{self.ts.Variables.ssl_port}:ssl", + 'proxy.config.http.connect_ports': f"{self.server.Variables.http_port}", + }) + + self.ts.addDefaultSSLFiles() + self.ts.Disk.ssl_multicert_config.AddLine( + 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key' + ) + + self.ts.Disk.remap_config.AddLines([ + f"map / http://127.0.0.1:{self.server.Variables.http_port}/", + ]) + # Verify ts logs + self.ts.Disk.traffic_out.Content += Testers.ContainsExpression( + f"Proxy's Request.*\n.*\nCONNECT 127.0.0.1:{self.server.Variables.http_port} HTTP/1.1", reflags=re.MULTILINE, + description="Verify that ATS recognizes the CONNECT request.") + + def runTraffic(self): + tr = Test.AddTestRun("Verify correct handling of CONNECT request on HTTP/2") + tr.AddVerifierClientProcess( + "connect-client2", + self.connectReplayFile, + 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.runTraffic() + + +ConnectViaPVTest2().run() diff --git a/tests/gold_tests/connect/replays/connect_h2.replay.yaml b/tests/gold_tests/connect/replays/connect_h2.replay.yaml new file mode 100644 index 00000000000..140137730d7 --- /dev/null +++ b/tests/gold_tests/connect/replays/connect_h2.replay.yaml @@ -0,0 +1,61 @@ +# 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. + +# +# This replay file executes a HTTP/2 CONNECT request, whose DATA frame contains +# a tunnelled HTTP/1 GET request. +# +meta: + version: "1.0" + +sessions: + - protocol: + - name: http + version: 2 + - name: tls + sni: www.example.com + - name: tcp + - name: ip + + transactions: + - client-request: + frames: + - HEADERS: + headers: + fields: + - [:method, CONNECT] + - [:authority, www.example.com:80] + - [uuid, 1] + - [test, connect-request] + - DATA: + content: + encoding: plain + data: "GET /get HTTP/1.1\r\nuuid: 1\r\ntest: real-request\r\n\r\n" + # This is the server response for the tunnelled HTTP/1 request rather + # than for the CONNECT request. + server-response: + status: 200 + reason: OK + content: + encoding: plain + data: response_to_tunnelled_request + size: 29 + # Verify the client receives the response for the tunneled GET request + # from the origin server. + proxy-response: + status: 200 + content: + verify: { value: "response_to_tunnelled_request", as: contains } From e72d0154a777edf5b8ad39525c8609df547efedc Mon Sep 17 00:00:00 2001 From: Zhengxi Li Date: Tue, 6 Jun 2023 17:37:55 +0000 Subject: [PATCH 2/3] Fix a crash trigger by H2 CONNECT --- proxy/http/HttpSM.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 9b8896fd7ed..a51f5c2c734 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -4119,6 +4119,11 @@ HttpSM::tunnel_handler_ssl_producer(int event, HttpTunnelProducer *p) STATE_ENTER(&HttpSM::tunnel_handler_ssl_producer, event); switch (event) { + case VC_EVENT_READ_COMPLETE: + // This event is triggered during an HTTP/2 CONNECT request when a DATA + // frame with the END_STREAM flag set is received, indicating the end of the + // stream. + [[fallthrough]]; case VC_EVENT_EOS: // The write side of this connection is still alive // so half-close the read @@ -4150,7 +4155,11 @@ HttpSM::tunnel_handler_ssl_producer(int event, HttpTunnelProducer *p) } } break; - case VC_EVENT_READ_COMPLETE: + case VC_EVENT_READ_READY: + // This event is triggered when receiving DATA frames without the END_STREAM + // flag set in a HTTP/2 CONNECT request. Breaking as there are more DATA + // frames to come. + break; case HTTP_TUNNEL_EVENT_PRECOMPLETE: // We should never get these event since we don't know // how long the stream is From 745c61a9819ebd68d1deb4df0bd9e3a9d6f20783 Mon Sep 17 00:00:00 2001 From: Zhengxi Li Date: Wed, 7 Jun 2023 16:35:27 +0000 Subject: [PATCH 3/3] Put VC_EVENT_READ_READY on top per review comment --- proxy/http/HttpSM.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index a51f5c2c734..c5672c7f857 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -4119,6 +4119,11 @@ HttpSM::tunnel_handler_ssl_producer(int event, HttpTunnelProducer *p) STATE_ENTER(&HttpSM::tunnel_handler_ssl_producer, event); switch (event) { + case VC_EVENT_READ_READY: + // This event is triggered when receiving DATA frames without the END_STREAM + // flag set in a HTTP/2 CONNECT request. Breaking as there are more DATA + // frames to come. + break; case VC_EVENT_READ_COMPLETE: // This event is triggered during an HTTP/2 CONNECT request when a DATA // frame with the END_STREAM flag set is received, indicating the end of the @@ -4155,11 +4160,6 @@ HttpSM::tunnel_handler_ssl_producer(int event, HttpTunnelProducer *p) } } break; - case VC_EVENT_READ_READY: - // This event is triggered when receiving DATA frames without the END_STREAM - // flag set in a HTTP/2 CONNECT request. Breaking as there are more DATA - // frames to come. - break; case HTTP_TUNNEL_EVENT_PRECOMPLETE: // We should never get these event since we don't know // how long the stream is