Add support for secure proxies in the client

Resolves #3816
Resolves #4268

Co-Authored-By: Brian Bouterse <bmbouter@gmail.com>
Co-Authored-By: Jordan Borean <jborean93@gmail.com>
Co-Authored-By: Sviatoslav Sydorenko <webknjaz@redhat.com>

CHANGES/5992.feature

@@ -0,0 +1,3 @@
+Added support for HTTPS proxies to the extent CPython's
+:py:mod:`asyncio` supports it -- by @bmbouter, @jborean93
+and @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 to not only 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()