summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHasan Ramezani <hasan.r67@gmail.com>2020-12-24 02:42:11 +0100
committerGitHub <noreply@github.com>2020-12-23 19:42:11 -0600
commitfd0c475cc2c51aedb6c89d7b9be58d850966ee6a (patch)
tree997b625b6651aa745273fb0ad7b9f3e582760e51
parentc09b25e840a0ab0a30d46866b4c774dcdb6dd763 (diff)
downloadurllib3-fd0c475cc2c51aedb6c89d7b9be58d850966ee6a.tar.gz
Remove fallback on commonName in match_hostname()
-rw-r--r--src/urllib3/__init__.py2
-rw-r--r--src/urllib3/connection.py17
-rw-r--r--src/urllib3/exceptions.py6
-rw-r--r--src/urllib3/packages/ssl_match_hostname/_implementation.py15
-rw-r--r--test/conftest.py14
-rw-r--r--test/contrib/test_pyopenssl.py2
-rw-r--r--test/test_connection.py13
-rw-r--r--test/with_dummyserver/test_https.py40
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