Add secure proxy support in the client

This patch opens up the code path and adds the implementation that
allows end-users to start sending HTTPS requests through
HTTPS proxies.

The support for TLS-in-TLS (needed for this to work) in the stdlib is
kinda available since Python 3.7 but is disabled for `asyncio` with an
attribute/flag/toggle. When the upstream CPython enables it finally,
aiohttp v3.8+ will be able to work with it out of the box.

Currently the tests monkey-patch `asyncio` in order to verify that
this works. The users who are willing to do the same, will be able to
take advantage of it right now. Eventually (hopefully starting Python
3.11), the need for monkey-patching should be eliminated.

Refs:
* https://bugs.python.org/issue37179
* python/cpython#28073
* https://docs.aiohttp.org/en/stable/client_advanced.html#proxy-support
* aio-libs#6044

PR aio-libs#5992
Resolves aio-libs#3816
Resolves aio-libs#4268

Co-authored-by: Brian Bouterse <[email protected]>
Co-authored-by: Jordan Borean <[email protected]>
Co-authored-by: Sviatoslav Sydorenko <[email protected]>

Author: 3 people

CHANGES/5992.feature

@@ -0,0 +1,3 @@
+Added support for HTTPS proxies to the extent CPython's
+:py:mod:`asyncio` supports it -- by :user:`bmbouter`,
+:user:`jborean93` and :user:`webknjaz`.

CONTRIBUTORS.txt

@@ -57,6 +57,7 @@ Boris Feld
 Borys Vorona
 Boyi Chen
 Brett Cannon
+Brian Bouterse
 Brian C. Lane
 Brian Muller
 Bruce Merry
@@ -165,6 +166,7 @@ Jonas Obrist
 Jonathan Wright
 Jonny Tan
 Joongi Kim
+Jordan Borean
 Josep Cugat
 Josh Junon
 Joshu Coats

aiohttp/client_reqrep.py

@@ -481,8 +481,6 @@ def update_proxy(
         proxy_auth: Optional[BasicAuth],
         proxy_headers: Optional[LooseHeaders],
     ) -> None:
-        if proxy and not proxy.scheme == "http":
-            raise ValueError("Only http proxies are supported")
         if proxy_auth and not isinstance(proxy_auth, helpers.BasicAuth):
             raise ValueError("proxy_auth must be None or BasicAuth() tuple")
         self.proxy = proxy

aiohttp/connector.py

@@ -951,6 +951,100 @@ async def _wrap_create_connection(
         except OSError as exc:
             raise client_error(req.connection_key, exc) from exc
 
+    def _warn_about_tls_in_tls(
+        self,
+        underlying_transport: asyncio.Transport,
+        req: "ClientRequest",
+    ) -> None:
+        """Issue a warning if the requested URL has HTTPS scheme."""
+        if req.request_info.url.scheme != "https":
+            return
+
+        asyncio_supports_tls_in_tls = getattr(
+            underlying_transport,
+            "_start_tls_compatible",
+            False,
+        )
+
+        if asyncio_supports_tls_in_tls:
+            return
+
+        warnings.warn(
+            "An HTTPS request is being sent through an HTTPS proxy. "
+            "This support for TLS in TLS is known to be disabled "
+            "in the stdlib asyncio. This is why you'll probably see "
+            "an error in the log below.\n\n"
+            "It is possible to enable it via monkeypatching under "
+            "Python 3.7 or higher. For more details, see:\n"
+            "* https://bugs.python.org/issue37179\n"
+            "* https://github.com/python/cpython/pull/28073\n\n"
+            "You can temporarily patch this as follows:\n"
+            "* https://docs.aiohttp.org/en/stable/client_advanced.html#proxy-support\n"
+            "* https://github.com/aio-libs/aiohttp/discussions/6044\n",
+            RuntimeWarning,
+            source=self,
+            # Why `4`? At least 3 of the calls in the stack originate
+            # from the methods in this class.
+            stacklevel=3,
+        )
+
+    async def _start_tls_connection(
+        self,
+        underlying_transport: asyncio.Transport,
+        req: "ClientRequest",
+        timeout: "ClientTimeout",
+        client_error: Type[Exception] = ClientConnectorError,
+    ) -> Tuple[asyncio.BaseTransport, ResponseHandler]:
+        """Wrap the raw TCP transport with TLS."""
+        tls_proto = self._factory()  # Create a brand new proto for TLS
+
+        # Safety of the `cast()` call here is based on the fact that
+        # internally `_get_ssl_context()` only returns `None` when
+        # `req.is_ssl()` evaluates to `False` which is never gonna happen
+        # in this code path. Of course, it's rather fragile
+        # maintainability-wise but this is to be solved separately.
+        sslcontext = cast(ssl.SSLContext, self._get_ssl_context(req))
+
+        try:
+            async with ceil_timeout(timeout.sock_connect):
+                try:
+                    tls_transport = await self._loop.start_tls(
+                        underlying_transport,
+                        tls_proto,
+                        sslcontext,
+                        server_hostname=req.host,
+                        ssl_handshake_timeout=timeout.total,
+                    )
+                except BaseException:
+                    # We need to close the underlying transport since
+                    # `start_tls()` probably failed before it had a
+                    # chance to do this:
+                    underlying_transport.close()
+                    raise
+        except cert_errors as exc:
+            raise ClientConnectorCertificateError(req.connection_key, exc) from exc
+        except ssl_errors as exc:
+            raise ClientConnectorSSLError(req.connection_key, exc) from exc
+        except OSError as exc:
+            raise client_error(req.connection_key, exc) from exc
+        except TypeError as type_err:
+            # Example cause looks like this:
+            # TypeError: transport <asyncio.sslproto._SSLProtocolTransport
+            # object at 0x7f760615e460> is not supported by start_tls()
+
+            raise ClientConnectionError(
+                "Cannot initialize a TLS-in-TLS connection to host "
+                f"{req.host!s}:{req.port:d} through an underlying connection "
+                f"to an HTTPS proxy {req.proxy!s} ssl:{req.ssl or 'default'} "
+                f"[{type_err!s}]"
+            ) from type_err
+        else:
+            tls_proto.connection_made(
+                tls_transport
+            )  # Kick the state machine of the new TLS protocol
+
+        return tls_transport, tls_proto
+
     async def _create_direct_connection(
         self,
         req: "ClientRequest",
@@ -1028,7 +1122,7 @@ def drop_exception(fut: "asyncio.Future[List[Dict[str, Any]]]") -> None:
 
     async def _create_proxy_connection(
         self, req: "ClientRequest", traces: List["Trace"], timeout: "ClientTimeout"
-    ) -> Tuple[asyncio.Transport, ResponseHandler]:
+    ) -> Tuple[asyncio.BaseTransport, ResponseHandler]:
         headers = {}  # type: Dict[str, str]
         if req.proxy_headers is not None:
             headers = req.proxy_headers  # type: ignore[assignment]
@@ -1063,7 +1157,8 @@ async def _create_proxy_connection(
                 proxy_req.headers[hdrs.PROXY_AUTHORIZATION] = auth
 
         if req.is_ssl():
-            sslcontext = self._get_ssl_context(req)
+            self._warn_about_tls_in_tls(transport, req)
+
             # For HTTPS requests over HTTP proxy
             # we must notify proxy to tunnel connection
             # so we send CONNECT command:
@@ -1083,7 +1178,11 @@ async def _create_proxy_connection(
             try:
                 protocol = conn._protocol
                 assert protocol is not None
-                protocol.set_response_params()
+
+                # read_until_eof=True will ensure the connection isn't closed
+                # once the response is received and processed allowing
+                # START_TLS to work on the connection below.
+                protocol.set_response_params(read_until_eof=True)
                 resp = await proxy_resp.start(conn)
             except BaseException:
                 proxy_resp.close()
@@ -1104,21 +1203,19 @@ async def _create_proxy_connection(
                             message=message,
                             headers=resp.headers,
                         )
-                    rawsock = transport.get_extra_info("socket", default=None)
-                    if rawsock is None:
-                        raise RuntimeError("Transport does not expose socket instance")
-                    # Duplicate the socket, so now we can close proxy transport
-                    rawsock = rawsock.dup()
-                finally:
+                except BaseException:
+                    # It shouldn't be closed in `finally` because it's fed to
+                    # `loop.start_tls()` and the docs say not to touch it after
+                    # passing there.
                     transport.close()
+                    raise
 
-                transport, proto = await self._wrap_create_connection(
-                    self._factory,
-                    timeout=timeout,
-                    ssl=sslcontext,
-                    sock=rawsock,
-                    server_hostname=req.host,
+                return await self._start_tls_connection(
+                    # Access the old transport for the last time before it's
+                    # closed and forgotten forever:
+                    transport,
                     req=req,
+                    timeout=timeout,
                 )
             finally:
                 proxy_resp.close()

docs/client_advanced.rst

@@ -533,9 +533,11 @@ DER with e.g::
 Proxy support
 -------------
 
-aiohttp supports plain HTTP proxies and HTTP proxies that can be upgraded to HTTPS
-via the HTTP CONNECT method. aiohttp does not support proxies that must be
-connected to via ``https://``. To connect, use the *proxy* parameter::
+aiohttp supports plain HTTP proxies and HTTP proxies that can be
+upgraded to HTTPS via the HTTP CONNECT method. aiohttp has a limited
+support for proxies that must be connected to via ``https://`` — see
+the info box below for more details.
+To connect, use the *proxy* parameter::
 
    async with aiohttp.ClientSession() as session:
        async with session.get("http://python.org",
@@ -570,6 +572,33 @@ variables* (all are case insensitive)::
 Proxy credentials are given from ``~/.netrc`` file if present (see
 :class:`aiohttp.ClientSession` for more details).
 
+.. attention::
+
+   CPython has introduced the support for TLS in TLS around Python 3.7.
+   But, as of now (Python 3.10), it's disabled for the transports that
+   :py:mod:`asyncio` uses. If the further release of Python (say v3.11)
+   toggles one attribute, it'll *just work™*.
+
+   aiohttp v3.8 and higher is ready for this to happen and has code in
+   place supports TLS-in-TLS, hence sending HTTPS requests over HTTPS
+   proxy tunnels.
+
+   ⚠️ For as long as your Python runtime doesn't declare the support for
+   TLS-in-TLS, please don't file bugs with aiohttp but rather try to
+   help the CPython upstream enable this feature. Meanwhile, if you
+   *really* need this to work, there's a patch that may help you make
+   it happen, include it into your app's code base:
+   https://github.com/aio-libs/aiohttp/discussions/6044#discussioncomment-1432443.
+
+.. important::
+
+   When supplying a custom :py:class:`ssl.SSLContext` instance, bear in
+   mind that it will be used not only to establish a TLS session with
+   the HTTPS endpoint you're hitting but also to establish a TLS tunnel
+   to the HTTPS proxy. To avoid surprises, make sure to set up the trust
+   chain that would recognize TLS certificates used by both the endpoint
+   and the proxy.
+
 .. _aiohttp-persistent-session:
 
 Persistent session

tests/test_client_request.py

@@ -119,11 +119,6 @@ def test_version_err(make_request: Any) -> None:
         make_request("get", "http://python.org/", version="1.c")
 
 
-def test_https_proxy(make_request: Any) -> None:
-    with pytest.raises(ValueError):
-        make_request("get", "http://python.org/", proxy=URL("https://proxy.org"))
-
-
 def test_keep_alive(make_request: Any) -> None:
     req = make_request("get", "http://python.org/", version=(0, 9))
     assert not req.keep_alive()