90 lines
3.4 KiB
Diff
90 lines
3.4 KiB
Diff
|
From 8d65ea1ecf6e2cdc27d42124e587c1b83a3118b0 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Jorge <JALopezSilva@gmail.com>
|
||
|
|
Date: Mon, 15 Mar 2021 06:49:49 -0700
|
||
|
|
Subject: [PATCH] Merge pull request from GHSA-5phf-pp7p-vc2r
|
||
|
|
|
||
|
|
* Enable hostname verification for HTTPS proxies with default cert.
|
||
|
|
|
||
|
|
Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
|
||
|
|
|
||
|
|
* Adjust exception check for Python 3.9+
|
||
|
|
|
||
|
|
Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
|
||
|
|
|
||
|
|
* Use a SAN instead of a common name.
|
||
|
|
|
||
|
|
Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
|
||
|
|
---
|
||
|
|
src/urllib3/connection.py | 4 ++++
|
||
|
|
test/conftest.py | 11 ++++++++++
|
||
|
|
.../test_proxy_poolmanager.py | 22 +++++++++++++++++++
|
||
|
|
3 files changed, 37 insertions(+)
|
||
|
|
|
||
|
|
diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py
|
||
|
|
index 9066e6ade4..45580b7e1e 100644
|
||
|
|
--- a/src/urllib3/connection.py
|
||
|
|
+++ b/src/urllib3/connection.py
|
||
|
|
@@ -490,6 +490,10 @@ def _connect_tls_proxy(self, hostname, conn):
|
||
|
|
self.ca_cert_dir,
|
||
|
|
self.ca_cert_data,
|
||
|
|
)
|
||
|
|
+ # By default urllib3's SSLContext disables `check_hostname` and uses
|
||
|
|
+ # a custom check. For proxies we're good with relying on the default
|
||
|
|
+ # verification.
|
||
|
|
+ ssl_context.check_hostname = True
|
||
|
|
|
||
|
|
# If no cert was provided, use only the default options for server
|
||
|
|
# certificate validation
|
||
|
|
diff --git a/test/conftest.py b/test/conftest.py
|
||
|
|
index ff8e463186..96c9b2b5bc 100644
|
||
|
|
--- a/test/conftest.py
|
||
|
|
+++ b/test/conftest.py
|
||
|
|
@@ -64,6 +64,17 @@ def no_san_server(tmp_path_factory):
|
||
|
|
yield cfg
|
||
|
|
|
||
|
|
|
||
|
|
+@pytest.fixture
|
||
|
|
+def no_localhost_san_server(tmp_path_factory):
|
||
|
|
+ tmpdir = tmp_path_factory.mktemp("certs")
|
||
|
|
+ ca = trustme.CA()
|
||
|
|
+ # non localhost common name
|
||
|
|
+ server_cert = ca.issue_cert(u"example.com")
|
||
|
|
+
|
||
|
|
+ with run_server_in_thread("https", "localhost", tmpdir, ca, server_cert) as cfg:
|
||
|
|
+ yield cfg
|
||
|
|
+
|
||
|
|
+
|
||
|
|
@pytest.fixture
|
||
|
|
def ip_san_server(tmp_path_factory):
|
||
|
|
tmpdir = tmp_path_factory.mktemp("certs")
|
||
|
|
diff --git a/test/with_dummyserver/test_proxy_poolmanager.py b/test/with_dummyserver/test_proxy_poolmanager.py
|
||
|
|
index 737e5f7afa..c1535bd087 100644
|
||
|
|
--- a/test/with_dummyserver/test_proxy_poolmanager.py
|
||
|
|
+++ b/test/with_dummyserver/test_proxy_poolmanager.py
|
||
|
|
@@ -543,3 +543,25 @@ def test_basic_ipv6_proxy(self):
|
||
|
|
|
||
|
|
r = http.request("GET", "%s/" % self.https_url)
|
||
|
|
assert r.status == 200
|
||
|
|
+
|
||
|
|
+
|
||
|
|
+class TestHTTPSProxyVerification:
|
||
|
|
+ @onlyPy3
|
||
|
|
+ def test_https_proxy_hostname_verification(self, no_localhost_san_server):
|
||
|
|
+ bad_server = no_localhost_san_server
|
||
|
|
+ bad_proxy_url = "https://%s:%s" % (bad_server.host, bad_server.port)
|
||
|
|
+
|
||
|
|
+ # An exception will be raised before we contact the destination domain.
|
||
|
|
+ test_url = "testing.com"
|
||
|
|
+ with proxy_from_url(bad_proxy_url, ca_certs=bad_server.ca_certs) as https:
|
||
|
|
+ with pytest.raises(MaxRetryError) as e:
|
||
|
|
+ https.request("GET", "http://%s/" % test_url)
|
||
|
|
+ assert isinstance(e.value.reason, SSLError)
|
||
|
|
+ assert "hostname 'localhost' doesn't match" in str(e.value.reason)
|
||
|
|
+
|
||
|
|
+ with pytest.raises(MaxRetryError) as e:
|
||
|
|
+ https.request("GET", "https://%s/" % test_url)
|
||
|
|
+ assert isinstance(e.value.reason, SSLError)
|
||
|
|
+ assert "hostname 'localhost' doesn't match" in str(
|
||
|
|
+ e.value.reason
|
||
|
|
+ ) or "Hostname mismatch" in str(e.value.reason)
|