diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 0d54ad376d7..7fa59ac9f2c 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -2145,6 +2145,16 @@ HttpSM::state_read_server_response_header(int event, void *data) // If there is a post body in transit, give up on it if (tunnel.is_tunnel_alive()) { tunnel.abort_tunnel(); + // abort_tunnel() cancels I/O but does not close VCs or clean up + // vc_table entries. When a request transform is active the + // post_transform_info entry still references the TransformVConnection + // with in_tunnel=true, which causes cleanup_entry() to skip + // do_io_close() — leaking the VC. Close it explicitly here. + if (post_transform_info.entry != nullptr) { + post_transform_info.vc->do_io_close(); + vc_table.cleanup_entry(post_transform_info.entry); + post_transform_info.entry = nullptr; + } // Make sure client connection is closed when we are done in case there is cruft left over t_state.client_info.keep_alive = HTTPKeepAlive::NO_KEEPALIVE; // Similarly the server connection should also be closed diff --git a/tests/gold_tests/slow_post/partial_post_client.py b/tests/gold_tests/slow_post/partial_post_client.py new file mode 100644 index 00000000000..7cd5ab98fdf --- /dev/null +++ b/tests/gold_tests/slow_post/partial_post_client.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 +"""Send a partial POST to trigger the abort_tunnel code path. + +Sends POST headers claiming a large Content-Length but only sends a small +chunk of body data. When a request transform plugin is active, this causes +ATS to call abort_tunnel() while the transform entry is still in the vc_table. +""" + +# 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. + +import socket +import sys + + +def main() -> int: + """Run the client.""" + host = sys.argv[1] if len(sys.argv) > 1 else '127.0.0.1' + port = int(sys.argv[2]) if len(sys.argv) > 2 else 8080 + + request = ( + f'POST / HTTP/1.1\r\n' + f'Host: quick.server.com\r\n' + f'Content-Type: application/octet-stream\r\n' + f'Content-Length: 100000\r\n' + f'\r\n').encode() + + partial_body = b'x' * 4096 + + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.settimeout(5) + try: + sock.connect((host, port)) + sock.sendall(request + partial_body) + print(f'Sent POST headers (Content-Length: 100000) + {len(partial_body)} bytes') + + # Read whatever response ATS sends back. + try: + response = sock.recv(4096) + if response: + first_line = response.split(b'\r\n')[0].decode(errors='replace') + print(f'Got response: {first_line}') + else: + print('Got response: connection closed') + except socket.timeout: + print('Got response: timeout (server may still be processing)') + except ConnectionError as e: + print(f'Got response: connection error ({e})') + finally: + sock.close() + + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/tests/gold_tests/slow_post/post_early_response_transform.test.py b/tests/gold_tests/slow_post/post_early_response_transform.test.py new file mode 100644 index 00000000000..8ed47ac83a6 --- /dev/null +++ b/tests/gold_tests/slow_post/post_early_response_transform.test.py @@ -0,0 +1,112 @@ +"""Verify ATS does not crash when a server replies before receiving the full POST body and a request transform plugin is active. + +When a POST request has a request transform and the origin responds before the +full body is forwarded through the transform chain, abort_tunnel() is called. +Without the fix, post_transform_info.entry is left stale in the vc_table, +causing a use-after-free in cleanup_all(). +""" + +# 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. + +import os +from ports import get_port +import sys + +Test.Summary = __doc__ + + +class PostEarlyResponseTransformTest: + """Verify abort_tunnel with a request transform does not crash ATS.""" + + _partial_post_client = 'partial_post_client.py' + _quick_server = 'quick_server.py' + _init_file = '__init__.py' + _http_utils = 'http_utils.py' + + def __init__(self): + """Configure and run the test.""" + tr = Test.AddTestRun('Partial POST with request transform and early server response') + self._configure_dns(tr) + self._configure_server(tr) + self._configure_traffic_server(tr) + self._configure_client(tr) + + def _configure_dns(self, tr: 'TestRun') -> None: + """Configure the DNS process. + + :param tr: The test run to associate the DNS process with. + """ + self._dns = tr.MakeDNServer('dns', default='127.0.0.1') + + def _configure_server(self, tr: 'TestRun') -> None: + """Configure the quick-responding origin server. + + The server responds immediately after receiving the request headers, + before the full POST body arrives. + + :param tr: The test run to associate the server process with. + """ + server = tr.Processes.Process('server') + server_port = get_port(server, 'http_port') + server.Command = f'{sys.executable} {self._quick_server} 127.0.0.1 {server_port}' + server.Ready = When.PortOpenv4(server_port) + self._server = server + + def _configure_traffic_server(self, tr: 'TestRun') -> None: + """Configure ATS with the null_transform_request plugin. + + :param tr: The test run to associate the ATS process with. + """ + self._ts = tr.MakeATSProcess('ts') + self._ts.Disk.remap_config.AddLine(f'map / http://quick.server.com:{self._server.Variables.http_port}') + self._ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http', + 'proxy.config.dns.nameservers': f'127.0.0.1:{self._dns.Variables.Port}', + 'proxy.config.dns.resolv_conf': 'NULL', + }) + Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'null_transform_request.so'), self._ts) + + def _configure_client(self, tr: 'TestRun') -> None: + """Configure the partial POST client. + + Sends a POST with a large Content-Length but only a small body, + triggering abort_tunnel when the origin responds early. + + :param tr: The test run to associate the client process with. + """ + tools_dir = self._ts.Variables.AtsTestToolsDir + http_utils = os.path.join(tools_dir, 'http_utils.py') + tr.Setup.CopyAs(self._init_file, Test.RunDirectory) + tr.Setup.CopyAs(http_utils, Test.RunDirectory) + tr.Setup.CopyAs(self._quick_server, Test.RunDirectory) + tr.Setup.CopyAs(self._partial_post_client, Test.RunDirectory) + + p = tr.Processes.Default + p.Command = (f'{sys.executable} {self._partial_post_client} ' + f'127.0.0.1 {self._ts.Variables.port}') + p.ReturnCode = 0 + p.Streams.All += Testers.ContainsExpression('Got response', 'Verify client received a response from ATS') + + self._ts.StartBefore(self._dns) + self._ts.StartBefore(self._server) + p.StartBefore(self._ts) + tr.Timeout = 10 + + +PostEarlyResponseTransformTest() diff --git a/tests/tools/plugins/CMakeLists.txt b/tests/tools/plugins/CMakeLists.txt index b7f18109ef0..856d00e6a9c 100644 --- a/tests/tools/plugins/CMakeLists.txt +++ b/tests/tools/plugins/CMakeLists.txt @@ -42,6 +42,7 @@ add_autest_plugin(hook_tunnel_plugin hook_tunnel_plugin.cc) add_autest_plugin(tunnel_transform tunnel_transform.cc) add_autest_plugin(http2_close_connection http2_close_connection.cc) add_autest_plugin(redirect_rearm redirect_rearm.cc) +add_autest_plugin(null_transform_request null_transform_request.cc) target_link_libraries(continuations_verify PRIVATE OpenSSL::SSL) target_link_libraries(ssl_client_verify_test PRIVATE OpenSSL::SSL) diff --git a/tests/tools/plugins/null_transform_request.cc b/tests/tools/plugins/null_transform_request.cc new file mode 100644 index 00000000000..63401cb73b3 --- /dev/null +++ b/tests/tools/plugins/null_transform_request.cc @@ -0,0 +1,162 @@ +/** @file + + Null request transform plugin hooked at TS_HTTP_READ_REQUEST_HDR_HOOK. + + Used by post_early_response_transform.test.py to reproduce a use-after-free + in HttpSM::state_read_server_response_header() when abort_tunnel() is called + while a request transform is active. The transform passes request body data + through unmodified. + + @section license License + + 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. + */ + +#include +#include + +#include "ts/ts.h" + +#define PLUGIN_NAME "null_transform_request" + +typedef struct { + TSVIO output_vio; + TSIOBuffer output_buffer; + TSIOBufferReader output_reader; +} TransformData; + +static TransformData * +transform_data_alloc() +{ + auto *data = static_cast(TSmalloc(sizeof(TransformData))); + data->output_vio = nullptr; + data->output_buffer = nullptr; + data->output_reader = nullptr; + return data; +} + +static void +transform_data_destroy(TransformData *data) +{ + if (data) { + if (data->output_buffer) { + TSIOBufferDestroy(data->output_buffer); + } + TSfree(data); + } +} + +static void +handle_transform(TSCont contp) +{ + TSVConn output_conn = TSTransformOutputVConnGet(contp); + TSVIO input_vio = TSVConnWriteVIOGet(contp); + TransformData *data = static_cast(TSContDataGet(contp)); + + if (!data) { + data = transform_data_alloc(); + data->output_buffer = TSIOBufferCreate(); + data->output_reader = TSIOBufferReaderAlloc(data->output_buffer); + data->output_vio = TSVConnWrite(output_conn, contp, data->output_reader, TSVIONBytesGet(input_vio)); + TSContDataSet(contp, data); + } + + if (!TSVIOBufferGet(input_vio)) { + TSVIONBytesSet(data->output_vio, TSVIONDoneGet(input_vio)); + TSVIOReenable(data->output_vio); + return; + } + + int64_t towrite = TSVIONTodoGet(input_vio); + if (towrite > 0) { + int64_t avail = TSIOBufferReaderAvail(TSVIOReaderGet(input_vio)); + if (towrite > avail) { + towrite = avail; + } + if (towrite > 0) { + TSIOBufferCopy(TSVIOBufferGet(data->output_vio), TSVIOReaderGet(input_vio), towrite, 0); + TSIOBufferReaderConsume(TSVIOReaderGet(input_vio), towrite); + TSVIONDoneSet(input_vio, TSVIONDoneGet(input_vio) + towrite); + } + } + + if (TSVIONTodoGet(input_vio) > 0) { + if (towrite > 0) { + TSVIOReenable(data->output_vio); + TSContCall(TSVIOContGet(input_vio), TS_EVENT_VCONN_WRITE_READY, input_vio); + } + } else { + TSVIONBytesSet(data->output_vio, TSVIONDoneGet(input_vio)); + TSVIOReenable(data->output_vio); + TSContCall(TSVIOContGet(input_vio), TS_EVENT_VCONN_WRITE_COMPLETE, input_vio); + } +} + +static int +null_transform(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */) +{ + if (TSVConnClosedGet(contp)) { + transform_data_destroy(static_cast(TSContDataGet(contp))); + TSContDestroy(contp); + return 0; + } + + switch (event) { + case TS_EVENT_ERROR: { + TSVIO input_vio = TSVConnWriteVIOGet(contp); + TSContCall(TSVIOContGet(input_vio), TS_EVENT_ERROR, input_vio); + break; + } + case TS_EVENT_VCONN_WRITE_COMPLETE: + TSVConnShutdown(TSTransformOutputVConnGet(contp), 0, 1); + break; + default: + handle_transform(contp); + break; + } + + return 0; +} + +static int +transform_plugin(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) +{ + if (event == TS_EVENT_HTTP_READ_REQUEST_HDR) { + TSHttpTxn txnp = static_cast(edata); + TSVConn connp = TSTransformCreate(null_transform, txnp); + TSHttpTxnHookAdd(txnp, TS_HTTP_REQUEST_TRANSFORM_HOOK, connp); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + } + return 0; +} + +void +TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */) +{ + TSPluginRegistrationInfo info; + + info.plugin_name = PLUGIN_NAME; + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Unable to initialize plugin (disabled)", PLUGIN_NAME); + return; + } + + TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, TSContCreate(transform_plugin, nullptr)); +}