diff --git a/docs/releases.rst b/docs/releases.rst index 198fa3d11..a1a78ab54 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -4,6 +4,7 @@ Release notes .. toctree:: :maxdepth: 2 + releases/v6.5.7 releases/v6.5.6 releases/v6.5.5 releases/v6.5.4 diff --git a/docs/releases/v6.5.7.rst b/docs/releases/v6.5.7.rst new file mode 100644 index 000000000..07e7c9364 --- /dev/null +++ b/docs/releases/v6.5.7.rst @@ -0,0 +1,13 @@ +What's new in Tornado 6.5.7 +=========================== + +Jun 8, 2026 +----------- + +Security fixes +~~~~~~~~~~~~~~ + +- ``CurlAsyncHTTPClient`` now fully resets the curl object before reusing it. This prevents + incorrectly reusing options from a previous request, specifically including client SSL and + credentials used for accessing proxies. Thanks to `Koh Jun Sheng `_ + for reporting this issue. \ No newline at end of file diff --git a/tornado/curl_httpclient.py b/tornado/curl_httpclient.py index b7f482033..98da8543c 100644 --- a/tornado/curl_httpclient.py +++ b/tornado/curl_httpclient.py @@ -221,6 +221,7 @@ def _process_queue(self) -> None: # _process_queue() is called from # _finish_pending_requests the exceptions have # nowhere to go. + curl.reset() self._free_list.append(curl) callback(HTTPResponse(request=request, code=599, error=e)) else: @@ -238,7 +239,6 @@ def _finish( info = curl.info # type: ignore curl.info = None # type: ignore self._multi.remove_handle(curl) - self._free_list.append(curl) buffer = info["buffer"] if curl_error: assert curl_message is not None @@ -282,12 +282,22 @@ def _finish( ) except Exception: self.handle_callback_exception(info["callback"]) + curl.reset() + self._free_list.append(curl) def handle_callback_exception(self, callback: Any) -> None: app_log.error("Exception in callback %r", callback, exc_info=True) def _curl_create(self) -> pycurl.Curl: - curl = pycurl.Curl() + return pycurl.Curl() + + def _curl_setup_request( + self, + curl: pycurl.Curl, + request: HTTPRequest, + buffer: BytesIO, + headers: httputil.HTTPHeaders, + ) -> None: if curl_log.isEnabledFor(logging.DEBUG): curl.setopt(pycurl.VERBOSE, 1) curl.setopt(pycurl.DEBUGFUNCTION, self._curl_debug) @@ -296,15 +306,7 @@ def _curl_create(self) -> pycurl.Curl: ): # PROTOCOLS first appeared in pycurl 7.19.5 (2014-07-12) curl.setopt(pycurl.PROTOCOLS, pycurl.PROTO_HTTP | pycurl.PROTO_HTTPS) curl.setopt(pycurl.REDIR_PROTOCOLS, pycurl.PROTO_HTTP | pycurl.PROTO_HTTPS) - return curl - def _curl_setup_request( - self, - curl: pycurl.Curl, - request: HTTPRequest, - buffer: BytesIO, - headers: httputil.HTTPHeaders, - ) -> None: curl.setopt(pycurl.URL, native_str(request.url)) # libcurl's magic "Expect: 100-continue" behavior causes delays diff --git a/tornado/test/curl_httpclient_test.py b/tornado/test/curl_httpclient_test.py index 0c4766fe3..dfbf69a17 100644 --- a/tornado/test/curl_httpclient_test.py +++ b/tornado/test/curl_httpclient_test.py @@ -1,10 +1,13 @@ -import unittest from hashlib import md5 +import os +import ssl +import unittest from tornado import gen from tornado.escape import utf8 +from tornado.netutil import ssl_options_to_context from tornado.test import httpclient_test -from tornado.testing import AsyncHTTPTestCase +from tornado.testing import AsyncHTTPSTestCase, AsyncHTTPTestCase from tornado.web import Application, RequestHandler try: @@ -139,3 +142,98 @@ async def _async_recv_chunk(chunk): with self.assertRaises(TypeError): self.fetch("/digest", streaming_callback=_async_recv_chunk) + + +class ProxyAuthEchoHandler(RequestHandler): + def get(self): + if self.request.headers.get("Proxy-Authorization", None) is not None: + self.write(f"proxy auth: {self.request.headers['Proxy-Authorization']}") + else: + self.write("no proxy auth") + + +@unittest.skipIf(pycurl is None, "pycurl module not present") +class CurlHTTPClientReuseProxyAuthTestCase(AsyncHTTPTestCase): + def get_app(self): + # Note that we don't properly support proxy-style requests, but it works well enough + # for this test if we start the url matcher with a wildcard. + return Application([(".*/proxy_auth", ProxyAuthEchoHandler)]) + + def get_http_client(self): + # max_clients=1 forces us to reuse curl "easy handles". This is a regression test for + # a bug in which proxy credentials were not cleared between requests. + return CurlAsyncHTTPClient( + force_instance=True, + defaults=dict( + allow_ipv6=False, + ), + max_clients=1, + ) + + def test_reuse_proxy_credentials(self): + # Proxy credentials used on one request should not be automatically reused + # by another request. + response = self.fetch( + "/proxy_auth", + proxy_host="127.0.0.1", + proxy_port=self.get_http_port(), + proxy_username="foo", + proxy_password="bar", + ) + self.assertEqual(response.body, b"proxy auth: Basic Zm9vOmJhcg==") + response = self.fetch( + "/proxy_auth", + proxy_host="127.0.0.1", + proxy_port=self.get_http_port(), + ) + self.assertEqual(response.body, b"no proxy auth") + + +class ClientCertEchoHandler(RequestHandler): + def get(self): + cert = self.request.get_ssl_certificate() + if cert is not None: + assert isinstance(cert, dict) + self.write(f"client cert: {cert['subject']}") + else: + self.write("no client cert") + + +@unittest.skipIf(pycurl is None, "pycurl module not present") +class CurlHTTPClientReuseCertsTestCase(AsyncHTTPSTestCase): + def get_app(self): + return Application([(".*/client_cert", ClientCertEchoHandler)]) + + def get_http_client(self): + return CurlAsyncHTTPClient( + force_instance=True, + defaults=dict( + allow_ipv6=False, + validate_cert=False, + ), + max_clients=1, + ) + + def get_httpserver_options(self): + ssl_ctx = ssl_options_to_context(self.get_ssl_options(), server_side=True) + ssl_ctx.verify_mode = ssl.CERT_OPTIONAL + return dict(ssl_options=ssl_ctx) + + def get_ssl_options(self): + opts = super().get_ssl_options() + opts["ca_certs"] = os.path.join(os.path.dirname(__file__), "test.crt") + return opts + + def test_reuse_certs(self): + # Client certs used on one request should not be automatically reused + # by another request. + response = self.fetch( + self.get_url("/client_cert"), + client_cert=os.path.join(os.path.dirname(__file__), "test.crt"), + client_key=os.path.join(os.path.dirname(__file__), "test.key"), + ) + self.assertEqual( + response.body, b"client cert: ((('commonName', 'foo.example.com'),),)" + ) + response = self.fetch(self.get_url("/client_cert")) + self.assertEqual(response.body, b"no client cert")