From 96b53635e0ebec9a2055c5c456371f3394dd8f19 Mon Sep 17 00:00:00 2001 From: Tushar Gupta Date: Wed, 14 Jun 2023 03:18:54 +0530 Subject: [PATCH 1/7] Emit insecure proxy event when using unverified proxies --- src/urllib3/connectionpool.py | 18 +++++++++++++++++- src/urllib3/exceptions.py | 4 ++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index 2479405bd5..07def9ea84 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -28,6 +28,7 @@ EmptyPoolError, FullPoolError, HostChangedError, + InsecureProxyWarning, InsecureRequestWarning, LocationValueError, MaxRetryError, @@ -1091,7 +1092,22 @@ def _validate_conn(self, conn: BaseHTTPConnection) -> None: if conn.is_closed: conn.connect() - if not conn.is_verified: + if ( + conn.proxy + and conn.proxy.scheme == "https" + and conn.proxy_is_verified is False + ): + warnings.warn( + ( + f"Unverified HTTPS request is being made using proxy host '{conn.host}'. " + "Adding certificate verification is strongly advised. See: " + "https://urllib3.readthedocs.io/en/latest/advanced-usage.html" + "#tls-warnings" + ), + InsecureProxyWarning, + ) + + elif not conn.is_verified: warnings.warn( ( f"Unverified HTTPS request is being made to host '{conn.host}'. " diff --git a/src/urllib3/exceptions.py b/src/urllib3/exceptions.py index 5bb9236961..9816ac20e6 100644 --- a/src/urllib3/exceptions.py +++ b/src/urllib3/exceptions.py @@ -214,6 +214,10 @@ class InsecureRequestWarning(SecurityWarning): """Warned when making an unverified HTTPS request.""" +class InsecureProxyWarning(SecurityWarning): + """Warned when using an unverified proxy.""" + + class NotOpenSSLWarning(SecurityWarning): """Warned when using unsupported SSL library""" From 747e2333b6e870cc5deeaf16fa882eba6894f91b Mon Sep 17 00:00:00 2001 From: Tushar Gupta Date: Fri, 16 Jun 2023 20:19:37 +0530 Subject: [PATCH 2/7] Add tests and update docs for changes. --- changelog/2791.feature.rst | 1 + docs/advanced-usage.rst | 6 ++++++ src/urllib3/connectionpool.py | 2 +- test/with_dummyserver/test_no_ssl.py | 17 +++++++++++++---- 4 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 changelog/2791.feature.rst diff --git a/changelog/2791.feature.rst b/changelog/2791.feature.rst new file mode 100644 index 0000000000..96c68784a1 --- /dev/null +++ b/changelog/2791.feature.rst @@ -0,0 +1 @@ +Emit ``InsecureProxyWarning`` when the proxy connection is not verified but the proxy.scheme is https \ No newline at end of file diff --git a/docs/advanced-usage.rst b/docs/advanced-usage.rst index 6d6c176826..4e41954417 100644 --- a/docs/advanced-usage.rst +++ b/docs/advanced-usage.rst @@ -509,6 +509,12 @@ be resolved in different ways. verification enabled. Follow the :ref:`certificate verification ` guide to resolve this warning. +* :class:`~exceptions.InsecureProxyWarning` + This happens when a request is made to an HTTPS Proxy without certificate + verification enabled or the proxy connection is not verified. + Follow the :ref:`certificate verification ` guide to + resolve this warning. + .. _disable_ssl_warnings: Making unverified HTTPS requests is **strongly** discouraged, however, if you diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index 07def9ea84..c9abad9210 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -1099,7 +1099,7 @@ def _validate_conn(self, conn: BaseHTTPConnection) -> None: ): warnings.warn( ( - f"Unverified HTTPS request is being made using proxy host '{conn.host}'. " + f"Unverified HTTPS request is being made using proxy host '{conn.proxy.host}'. " "Adding certificate verification is strongly advised. See: " "https://urllib3.readthedocs.io/en/latest/advanced-usage.html" "#tls-warnings" diff --git a/test/with_dummyserver/test_no_ssl.py b/test/with_dummyserver/test_no_ssl.py index b89f703fac..a536e3ffb1 100644 --- a/test/with_dummyserver/test_no_ssl.py +++ b/test/with_dummyserver/test_no_ssl.py @@ -8,8 +8,8 @@ import pytest import urllib3 -from dummyserver.testcase import HTTPDummyServerTestCase, HTTPSDummyServerTestCase -from urllib3.exceptions import InsecureRequestWarning +from dummyserver.testcase import HTTPDummyProxyTestCase, HTTPDummyServerTestCase +from urllib3.exceptions import InsecureProxyWarning, InsecureRequestWarning from ..test_no_ssl import TestWithoutSSL @@ -21,13 +21,22 @@ def test_simple(self) -> None: assert r.status == 200, r.data -class TestHTTPSWithoutSSL(HTTPSDummyServerTestCase, TestWithoutSSL): +class TestHTTPSWithoutSSL(HTTPDummyProxyTestCase, TestWithoutSSL): def test_simple(self) -> None: with urllib3.HTTPSConnectionPool( - self.host, self.port, cert_reqs="NONE" + self.https_host, self.https_port, cert_reqs="NONE" ) as pool: with pytest.warns(InsecureRequestWarning): try: pool.request("GET", "/") except urllib3.exceptions.SSLError as e: assert "SSL module is not available" in str(e) + + def test_simple_proxy(self) -> None: + https_proxy_url = f"https://{self.proxy_host}:{int(self.https_proxy_port)}" + with urllib3.ProxyManager(https_proxy_url, cert_reqs="NONE") as pool: + with pytest.warns(InsecureProxyWarning): + try: + pool.request("GET", "/") + except urllib3.exceptions.SSLError as e: + assert "SSL module is not available" in str(e) From 505482958b3c91f61e5d0fea06068889da90d3f0 Mon Sep 17 00:00:00 2001 From: Tushar Gupta Date: Tue, 20 Jun 2023 02:04:02 +0530 Subject: [PATCH 3/7] fix: Update InsecureRequestWarning message to use _tunnel_host when HTTPs proxy is being used. fix: Raise warnings for both Unverified Proxy and Request --- src/urllib3/connectionpool.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index c9abad9210..02dea3f574 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -1107,10 +1107,10 @@ def _validate_conn(self, conn: BaseHTTPConnection) -> None: InsecureProxyWarning, ) - elif not conn.is_verified: + if not conn.is_verified: warnings.warn( ( - f"Unverified HTTPS request is being made to host '{conn.host}'. " + f"Unverified HTTPS request is being made to host '{conn._tunnel_host if conn.proxy and conn.proxy.scheme == 'https' else conn.host}'. " "Adding certificate verification is strongly advised. See: " "https://urllib3.readthedocs.io/en/latest/advanced-usage.html" "#tls-warnings" From 32c6a737846cde577f579b8084a4b11f4da3733c Mon Sep 17 00:00:00 2001 From: Tushar <30565750+tushar5526@users.noreply.github.com> Date: Tue, 29 Aug 2023 00:37:49 +0530 Subject: [PATCH 4/7] Use absolute URL in proxy test case --- test/with_dummyserver/test_no_ssl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/with_dummyserver/test_no_ssl.py b/test/with_dummyserver/test_no_ssl.py index a536e3ffb1..a8123d4e84 100644 --- a/test/with_dummyserver/test_no_ssl.py +++ b/test/with_dummyserver/test_no_ssl.py @@ -37,6 +37,6 @@ def test_simple_proxy(self) -> None: with urllib3.ProxyManager(https_proxy_url, cert_reqs="NONE") as pool: with pytest.warns(InsecureProxyWarning): try: - pool.request("GET", "/") + pool.request("GET", "https://urllib3.readthedocs.io") except urllib3.exceptions.SSLError as e: assert "SSL module is not available" in str(e) From 2a024b4709000258d2c37247a35bd6e6de6d4293 Mon Sep 17 00:00:00 2001 From: Tushar <30565750+tushar5526@users.noreply.github.com> Date: Tue, 29 Aug 2023 01:06:40 +0530 Subject: [PATCH 5/7] Assert host in warning strings --- test/with_dummyserver/test_no_ssl.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/with_dummyserver/test_no_ssl.py b/test/with_dummyserver/test_no_ssl.py index a8123d4e84..4e8d682954 100644 --- a/test/with_dummyserver/test_no_ssl.py +++ b/test/with_dummyserver/test_no_ssl.py @@ -35,8 +35,10 @@ def test_simple(self) -> None: def test_simple_proxy(self) -> None: https_proxy_url = f"https://{self.proxy_host}:{int(self.https_proxy_port)}" with urllib3.ProxyManager(https_proxy_url, cert_reqs="NONE") as pool: - with pytest.warns(InsecureProxyWarning): + with pytest.warns((InsecureProxyWarning, InsecureRequestWarning)) as record: try: pool.request("GET", "https://urllib3.readthedocs.io") except urllib3.exceptions.SSLError as e: assert "SSL module is not available" in str(e) + assert('localhost' in str(record[0].message)) + assert('urllib3.readthedocs.io' in str(record[1].message)) From 0a07cfc427290b5e4ba89107bdbfbcdd3a10581e Mon Sep 17 00:00:00 2001 From: Tushar <30565750+tushar5526@users.noreply.github.com> Date: Tue, 29 Aug 2023 02:14:42 +0530 Subject: [PATCH 6/7] Add testcases for HTTPs to HTTP proxies --- src/urllib3/connectionpool.py | 8 ++------ test/with_dummyserver/test_no_ssl.py | 12 ++++++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index 02dea3f574..20ee7b6888 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -1092,11 +1092,7 @@ def _validate_conn(self, conn: BaseHTTPConnection) -> None: if conn.is_closed: conn.connect() - if ( - conn.proxy - and conn.proxy.scheme == "https" - and conn.proxy_is_verified is False - ): + if conn.proxy and conn.proxy.scheme == "https" and not conn.proxy_is_verified: warnings.warn( ( f"Unverified HTTPS request is being made using proxy host '{conn.proxy.host}'. " @@ -1110,7 +1106,7 @@ def _validate_conn(self, conn: BaseHTTPConnection) -> None: if not conn.is_verified: warnings.warn( ( - f"Unverified HTTPS request is being made to host '{conn._tunnel_host if conn.proxy and conn.proxy.scheme == 'https' else conn.host}'. " + f"Unverified HTTPS request is being made to host '{conn._tunnel_host if conn._tunnel_host else conn.host}'. " "Adding certificate verification is strongly advised. See: " "https://urllib3.readthedocs.io/en/latest/advanced-usage.html" "#tls-warnings" diff --git a/test/with_dummyserver/test_no_ssl.py b/test/with_dummyserver/test_no_ssl.py index 4e8d682954..e92dddf0de 100644 --- a/test/with_dummyserver/test_no_ssl.py +++ b/test/with_dummyserver/test_no_ssl.py @@ -40,5 +40,13 @@ def test_simple_proxy(self) -> None: pool.request("GET", "https://urllib3.readthedocs.io") except urllib3.exceptions.SSLError as e: assert "SSL module is not available" in str(e) - assert('localhost' in str(record[0].message)) - assert('urllib3.readthedocs.io' in str(record[1].message)) + assert "localhost" in str(record[0].message) + assert "urllib3.readthedocs.io" in str(record[1].message) + + with pytest.warns((InsecureProxyWarning, InsecureRequestWarning)) as record: + try: + pool.request("GET", f"http://{self.http_host}:{self.http_port}") + except urllib3.exceptions.SSLError as e: + assert "SSL module is not available" in str(e) + assert "localhost" in str(record[0].message) + assert self.http_host in str(record[1].message) From 150172d569726397719fc8b3ba820202b1fbf6ef Mon Sep 17 00:00:00 2001 From: Tushar <30565750+tushar5526@users.noreply.github.com> Date: Tue, 29 Aug 2023 02:20:31 +0530 Subject: [PATCH 7/7] Fix lint errors --- src/urllib3/connectionpool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index 20ee7b6888..66650a6c69 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -1106,7 +1106,7 @@ def _validate_conn(self, conn: BaseHTTPConnection) -> None: if not conn.is_verified: warnings.warn( ( - f"Unverified HTTPS request is being made to host '{conn._tunnel_host if conn._tunnel_host else conn.host}'. " + f"Unverified HTTPS request is being made to host '{conn._tunnel_host if conn._tunnel_host else conn.host}'. " # type: ignore[attr-defined] "Adding certificate verification is strongly advised. See: " "https://urllib3.readthedocs.io/en/latest/advanced-usage.html" "#tls-warnings"