[PR #5992/c29e5fb5 backport][3.8] Add secure proxy support in the cli…

…ent (#6049)

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:
* bugs.python.org/issue37179
* python/cpython#28073
* docs.aiohttp.org/en/stable/client_advanced.html#proxy-support
* #6044

PR #5992
Resolves #3816
Resolves #4268

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

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

@@ -55,6 +55,7 @@ Bob Haddleton
 Boris Feld
 Boyi Chen
 Brett Cannon
+Brian Bouterse
 Brian C. Lane
 Brian Muller
 Bruce Merry
@@ -159,6 +160,7 @@ Jonas Obrist
 Jonathan Wright
 Jonny Tan
 Joongi Kim
+Jordan Borean
 Josep Cugat
 Josh Junon
 Joshu Coats

aiohttp/client_reqrep.py

@@ -547,8 +547,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

@@ -988,6 +988,144 @@ async def _wrap_create_connection(
         except OSError as exc:
             raise client_error(req.connection_key, exc) from exc
 
+    def _fail_on_no_start_tls(self, req: "ClientRequest") -> None:
+        """Raise a :py:exc:`RuntimeError` on missing ``start_tls()``.
+
+        One case is that :py:meth:`asyncio.loop.start_tls` is not yet
+        implemented under Python 3.6. It is necessary for TLS-in-TLS so
+        that it is possible to send HTTPS queries through HTTPS proxies.
+
+        This doesn't affect regular HTTP requests, though.
+        """
+        if not req.is_ssl():
+            return
+
+        proxy_url = req.proxy
+        assert proxy_url is not None
+        if proxy_url.scheme != "https":
+            return
+
+        self._check_loop_for_start_tls()
+
+    def _check_loop_for_start_tls(self) -> None:
+        try:
+            self._loop.start_tls
+        except AttributeError as attr_exc:
+            raise RuntimeError(
+                "An HTTPS request is being sent through an HTTPS proxy. "
+                "This needs support for TLS in TLS but it is not implemented "
+                "in your runtime for the stdlib asyncio.\n\n"
+                "Please upgrade to Python 3.7 or higher. For more details, "
+                "please see:\n"
+                "* https://bugs.python.org/issue37179\n"
+                "* https://github.com/python/cpython/pull/28073\n"
+                "* https://docs.aiohttp.org/en/stable/"
+                "client_advanced.html#proxy-support\n"
+                "* https://github.com/aio-libs/aiohttp/discussions/6044\n",
+            ) from attr_exc
+
+    def _loop_supports_start_tls(self) -> bool:
+        try:
+            self._check_loop_for_start_tls()
+        except RuntimeError:
+            return False
+        else:
+            return True
+
+    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",
@@ -1066,7 +1204,10 @@ 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]:
+        self._fail_on_no_start_tls(req)
+        runtime_has_start_tls = self._loop_supports_start_tls()
+
         headers = {}  # type: Dict[str, str]
         if req.proxy_headers is not None:
             headers = req.proxy_headers  # type: ignore[assignment]
@@ -1101,7 +1242,9 @@ async def _create_proxy_connection(
                 proxy_req.headers[hdrs.PROXY_AUTHORIZATION] = auth
 
         if req.is_ssl():
-            sslcontext = self._get_ssl_context(req)
+            if runtime_has_start_tls:
+                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:
@@ -1121,7 +1264,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=runtime_has_start_tls)
                 resp = await proxy_resp.start(conn)
             except BaseException:
                 proxy_resp.close()
@@ -1142,21 +1289,42 @@ 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:
+                    if not runtime_has_start_tls:
+                        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()
+                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()
-
-                transport, proto = await self._wrap_create_connection(
-                    self._factory,
-                    timeout=timeout,
-                    ssl=sslcontext,
-                    sock=rawsock,
-                    server_hostname=req.host,
+                    raise
+                finally:
+                    if not runtime_has_start_tls:
+                        transport.close()
+
+                if not runtime_has_start_tls:
+                    # HTTP proxy with support for upgrade to HTTPS
+                    sslcontext = self._get_ssl_context(req)
+                    return await self._wrap_create_connection(
+                        self._factory,
+                        timeout=timeout,
+                        ssl=sslcontext,
+                        sock=rawsock,
+                        server_hostname=req.host,
+                        req=req,
+                    )
+
+                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()

aiohttp/helpers.py

@@ -54,6 +54,9 @@
 
 __all__ = ("BasicAuth", "ChainMapProxy", "ETag")
 
+IS_MACOS = platform.system() == "Darwin"
+IS_WINDOWS = platform.system() == "Windows"
+
 PY_36 = sys.version_info >= (3, 6)
 PY_37 = sys.version_info >= (3, 7)
 PY_38 = sys.version_info >= (3, 8)
@@ -213,9 +216,7 @@ def netrc_from_env() -> Optional[netrc.netrc]:
             )
             return None
 
-        netrc_path = home_dir / (
-            "_netrc" if platform.system() == "Windows" else ".netrc"
-        )
+        netrc_path = home_dir / ("_netrc" if IS_WINDOWS else ".netrc")
 
     try:
         return netrc.netrc(str(netrc_path))

docs/client_advanced.rst

@@ -526,9 +526,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",
@@ -563,6 +565,33 @@ 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.
+
 Graceful Shutdown
 -----------------
 

tests/test_client_request.py

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