diff options
author | Hasan Ramezani <hasan.r67@gmail.com> | 2020-12-24 02:42:11 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-12-23 19:42:11 -0600 |
commit | fd0c475cc2c51aedb6c89d7b9be58d850966ee6a (patch) | |
tree | 997b625b6651aa745273fb0ad7b9f3e582760e51 | |
parent | c09b25e840a0ab0a30d46866b4c774dcdb6dd763 (diff) | |
download | urllib3-fd0c475cc2c51aedb6c89d7b9be58d850966ee6a.tar.gz |
Remove fallback on commonName in match_hostname()
-rw-r--r-- | src/urllib3/__init__.py | 2 | ||||
-rw-r--r-- | src/urllib3/connection.py | 17 | ||||
-rw-r--r-- | src/urllib3/exceptions.py | 6 | ||||
-rw-r--r-- | src/urllib3/packages/ssl_match_hostname/_implementation.py | 15 | ||||
-rw-r--r-- | test/conftest.py | 14 | ||||
-rw-r--r-- | test/contrib/test_pyopenssl.py | 2 | ||||
-rw-r--r-- | test/test_connection.py | 13 | ||||
-rw-r--r-- | test/with_dummyserver/test_https.py | 40 |
8 files changed, 26 insertions, 83 deletions
diff --git a/src/urllib3/__init__.py b/src/urllib3/__init__.py index c43d7074..067e40a6 100644 --- a/src/urllib3/__init__.py +++ b/src/urllib3/__init__.py @@ -69,8 +69,6 @@ del NullHandler # mechanisms to silence them. # SecurityWarning's always go off by default. warnings.simplefilter("always", exceptions.SecurityWarning, append=True) -# SubjectAltNameWarning's should go off once per host -warnings.simplefilter("default", exceptions.SubjectAltNameWarning, append=True) # InsecurePlatformWarning's don't vary between requests, so we keep it default. warnings.simplefilter("default", exceptions.InsecurePlatformWarning, append=True) # SNIMissingWarnings should go off only once. diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py index c8becf8f..6e4c4709 100644 --- a/src/urllib3/connection.py +++ b/src/urllib3/connection.py @@ -23,12 +23,7 @@ except (ImportError, AttributeError): # Platform-specific: No SSL. from ._version import __version__ -from .exceptions import ( - ConnectTimeoutError, - NewConnectionError, - SubjectAltNameWarning, - SystemTimeWarning, -) +from .exceptions import ConnectTimeoutError, NewConnectionError, SystemTimeWarning from .packages.ssl_match_hostname import CertificateError, match_hostname from .util import SKIP_HEADER, SKIPPABLE_HEADERS, connection from .util.ssl_ import ( @@ -420,16 +415,6 @@ class HTTPSConnection(HTTPConnection): # the TLS library, this cannot always be done. So we check whether # the TLS Library still thinks it's matching hostnames. cert = self.sock.getpeercert() - if not cert.get("subjectAltName", ()): - warnings.warn( - ( - f"Certificate for {hostname} has no `subjectAltName`, falling back to check for a " - "`commonName` for now. This feature is being removed by major browsers and " - "deprecated by RFC 2818. (See https://github.com/urllib3/urllib3/issues/497 " - "for details.)" - ), - SubjectAltNameWarning, - ) _match_hostname(cert, self.assert_hostname or server_hostname) self.is_verified = ( diff --git a/src/urllib3/exceptions.py b/src/urllib3/exceptions.py index 2a7473e9..e28d720f 100644 --- a/src/urllib3/exceptions.py +++ b/src/urllib3/exceptions.py @@ -186,12 +186,6 @@ class SecurityWarning(HTTPWarning): pass -class SubjectAltNameWarning(SecurityWarning): - """Warned when connecting to a host with a certificate missing a SAN.""" - - pass - - class InsecureRequestWarning(SecurityWarning): """Warned when making an unverified HTTPS request.""" diff --git a/src/urllib3/packages/ssl_match_hostname/_implementation.py b/src/urllib3/packages/ssl_match_hostname/_implementation.py index 8c4d6d2f..169508d1 100644 --- a/src/urllib3/packages/ssl_match_hostname/_implementation.py +++ b/src/urllib3/packages/ssl_match_hostname/_implementation.py @@ -125,17 +125,6 @@ def match_hostname(cert, hostname): if host_ip is not None and _ipaddress_match(value, host_ip): return dnsnames.append(value) - if not dnsnames: - # The subject is only checked when there is no dNSName entry - # in subjectAltName - for sub in cert.get("subject", ()): - for key, value in sub: - # XXX according to RFC 2818, the most specific Common Name - # must be used. - if key == "commonName": - if _dnsname_match(value, hostname): - return - dnsnames.append(value) if len(dnsnames) > 1: raise CertificateError( "hostname %r " @@ -144,6 +133,4 @@ def match_hostname(cert, hostname): elif len(dnsnames) == 1: raise CertificateError(f"hostname {hostname!r} doesn't match {dnsnames[0]!r}") else: - raise CertificateError( - "no appropriate commonName or subjectAltName fields were found" - ) + raise CertificateError("no appropriate subjectAltName fields were found") diff --git a/test/conftest.py b/test/conftest.py index 3b4b1dd4..5921560e 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -76,20 +76,6 @@ def ip_san_server(tmp_path_factory): @pytest.fixture -def ipv6_addr_server(tmp_path_factory): - if not HAS_IPV6: - pytest.skip("Only runs on IPv6 systems") - - tmpdir = tmp_path_factory.mktemp("certs") - ca = trustme.CA() - # IP address in Common Name - server_cert = ca.issue_cert(common_name="::1") - - with run_server_in_thread("https", "::1", tmpdir, ca, server_cert) as cfg: - yield cfg - - -@pytest.fixture def ipv6_san_server(tmp_path_factory): if not HAS_IPV6: pytest.skip("Only runs on IPv6 systems") diff --git a/test/contrib/test_pyopenssl.py b/test/contrib/test_pyopenssl.py index 05d2c7e2..0f9d4751 100644 --- a/test/contrib/test_pyopenssl.py +++ b/test/contrib/test_pyopenssl.py @@ -34,9 +34,7 @@ from ..test_util import TestUtilSSL # noqa: E402, F401 from ..with_dummyserver.test_https import ( # noqa: E402, F401 TestHTTPS, TestHTTPS_IPSAN, - TestHTTPS_IPv6Addr, TestHTTPS_IPV6SAN, - TestHTTPS_NoSAN, TestHTTPS_TLSv1, TestHTTPS_TLSv1_1, TestHTTPS_TLSv1_2, diff --git a/test/test_connection.py b/test/test_connection.py index 6c7ad4a8..0bc761bf 100644 --- a/test/test_connection.py +++ b/test/test_connection.py @@ -4,6 +4,10 @@ from unittest import mock import pytest from urllib3.connection import RECENT_DATE, CertificateError, _match_hostname +from urllib3.packages.ssl_match_hostname._implementation import ( + CertificateError as ImplementationCertificateError, +) +from urllib3.packages.ssl_match_hostname._implementation import match_hostname class TestConnection: @@ -43,6 +47,15 @@ class TestConnection: ) assert e._peer_cert == cert + def test_match_hostname_ignore_common_name(self): + cert = {"subject": [("commonName", "foo")]} + asserted_hostname = "foo" + with pytest.raises( + ImplementationCertificateError, + match="no appropriate subjectAltName fields were found", + ): + match_hostname(cert, asserted_hostname) + def test_recent_date(self): # This test is to make sure that the RECENT_DATE value # doesn't get too far behind what the current date is. diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 4306c2ef..d06e8fb4 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -782,22 +782,6 @@ class TestHTTPS_TLSv1_3(TestHTTPS): certs = TLSv1_3_CERTS -class TestHTTPS_NoSAN: - def test_warning_for_certs_without_a_san(self, no_san_server): - """Ensure that a warning is raised when the cert from the server has - no Subject Alternative Name.""" - with mock.patch("warnings.warn") as warn: - with HTTPSConnectionPool( - no_san_server.host, - no_san_server.port, - cert_reqs="CERT_REQUIRED", - ca_certs=no_san_server.ca_certs, - ) as https_pool: - r = https_pool.request("GET", "/") - assert r.status == 200 - assert warn.called - - class TestHTTPS_IPSAN: def test_can_validate_ip_san(self, ip_san_server): """Ensure that urllib3 can validate SANs with IP addresses in them.""" @@ -816,19 +800,6 @@ class TestHTTPS_IPSAN: assert r.status == 200 -class TestHTTPS_IPv6Addr: - def test_strip_square_brackets_before_validating(self, ipv6_addr_server): - """Test that the fix for #760 works.""" - with HTTPSConnectionPool( - "[::1]", - ipv6_addr_server.port, - cert_reqs="CERT_REQUIRED", - ca_certs=ipv6_addr_server.ca_certs, - ) as https_pool: - r = https_pool.request("GET", "/") - assert r.status == 200 - - class TestHTTPS_IPV6SAN: def test_can_validate_ipv6_san(self, ipv6_san_server): """Ensure that urllib3 can validate SANs with IPv6 addresses in them.""" @@ -845,3 +816,14 @@ class TestHTTPS_IPV6SAN: ) as https_pool: r = https_pool.request("GET", "/") assert r.status == 200 + + def test_strip_square_brackets_before_validating(self, ipv6_san_server): + """Test that the fix for #760 works.""" + with HTTPSConnectionPool( + "[::1]", + ipv6_san_server.port, + cert_reqs="CERT_REQUIRED", + ca_certs=ipv6_san_server.ca_certs, + ) as https_pool: + r = https_pool.request("GET", "/") + assert r.status == 200 |