Respect 'SSLContext.hostname_checks_common_name' when available and set
MODIFY
.github/workflows/ci.yml
.github/workflows/ci.yml
@@ -40,6 +40,10 @@ jobs:
os: ubuntu-latest
experimental: false
nox-session: test-pypy
+ - python-version: "pypy-3.8"
+ os: ubuntu-latest
+ experimental: false
+ nox-session: test-pypy
- python-version: "2.7"
os: ubuntu-latest
experimental: false

MODIFY
src/urllib3/connection.py
src/urllib3/connection.py
@@ -25,6 +25,8 @@ from typing import (
if TYPE_CHECKING:
from typing_extensions import Literal
+ from .util.ssl_ import _TYPE_PEER_CERT_RET_DICT
+
from .util.proxy import create_proxy_ssl_context
from .util.timeout import _DEFAULT_TIMEOUT, _TYPE_TIMEOUT, Timeout
from .util.util import to_bytes, to_str
@@ -50,7 +52,6 @@ from .exceptions import (
)
from .util import SKIP_HEADER, SKIPPABLE_HEADERS, connection, ssl_
from .util.ssl_ import (
- _TYPE_PEER_CERT_RET,
assert_fingerprint,
create_urllib3_context,
resolve_cert_reqs,
@@ -537,8 +538,23 @@ class HTTPSConnection(HTTPConnection):
and not context.check_hostname
and self.assert_hostname is not False
):
- cert = self.sock.getpeercert()
- _match_hostname(cert, self.assert_hostname or server_hostname)
+ cert: "_TYPE_PEER_CERT_RET_DICT" = self.sock.getpeercert() # type: ignore[assignment]
+
+ # Need to signal to our match_hostname whether to use 'commonName' or not.
+ # If we're using our own constructed SSLContext we explicitly set 'False'
+ # because PyPy hard-codes 'True' from SSLContext.hostname_checks_common_name.
+ if default_ssl_context:
+ hostname_checks_common_name = False
+ else:
+ hostname_checks_common_name = (
+ getattr(context, "hostname_checks_common_name", False) or False
+ )
+
+ _match_hostname(
+ cert,
+ self.assert_hostname or server_hostname,
+ hostname_checks_common_name,
+ )
self.is_verified = context.verify_mode == ssl.CERT_REQUIRED or bool(
self.assert_fingerprint
@@ -583,9 +599,13 @@ class HTTPSConnection(HTTPConnection):
)
-def _match_hostname(cert: _TYPE_PEER_CERT_RET, asserted_hostname: str) -> None:
+def _match_hostname(
+ cert: Optional["_TYPE_PEER_CERT_RET_DICT"],
+ asserted_hostname: str,
+ hostname_checks_common_name: bool = False,
+) -> None:
try:
- match_hostname(cert, asserted_hostname)
+ match_hostname(cert, asserted_hostname, hostname_checks_common_name)
except CertificateError as e:
log.warning(
"Certificate did not match expected hostname: %s. Certificate: %s",

MODIFY
src/urllib3/util/ssl_.py
src/urllib3/util/ssl_.py
@@ -5,7 +5,7 @@ import sys
import warnings
from binascii import unhexlify
from hashlib import md5, sha1, sha256
-from typing import TYPE_CHECKING, Dict, Mapping, Optional, Tuple, Union, cast, overload
+from typing import TYPE_CHECKING, Dict, Optional, Tuple, Union, cast, overload
from ..exceptions import ProxySchemeUnsupported, SNIMissingWarning, SSLError
from .url import _BRACELESS_IPV6_ADDRZ_RE, _IPV4_RE
@@ -59,13 +59,22 @@ def _is_openssl_issue_14579_fixed(
def _is_bpo_43522_fixed(
implementation_name: str, version_info: _TYPE_VERSION_INFO
) -> bool:
- """Return True if PyPy or CPython 3.8.9+, 3.9.3+ or 3.10+ where setting
+ """Return True for CPython 3.8.9+, 3.9.3+ or 3.10+ where setting
SSLContext.hostname_checks_common_name to False works.
+
+ PyPy 7.3.7 doesn't work as it doesn't ship with OpenSSL 1.1.1l+
+ so we're waiting for a version of PyPy that works before
+ allowing this function to return 'True'.
+
+ Outside of CPython and PyPy we don't know which implementations work
+ or not so we conservatively use our hostname matching as we know that works
+ on all implementations.
+
https://github.com/urllib3/urllib3/issues/2192#issuecomment-821832963
https://foss.heptapod.net/pypy/pypy/-/issues/3539#
"""
if implementation_name != "cpython":
- return True
+ return False
major_minor = version_info[:2]
micro = version_info[2]
@@ -90,10 +99,16 @@ def _is_has_never_check_common_name_reliable(
if TYPE_CHECKING:
from ssl import VerifyMode
- from typing_extensions import Literal
+ from typing_extensions import Literal, TypedDict
from .ssltransport import SSLTransport as SSLTransportType
+ class _TYPE_PEER_CERT_RET_DICT(TypedDict, total=False):
+ subjectAltName: Tuple[Tuple[str, str], ...]
+ subject: Tuple[Tuple[Tuple[str, str], ...], ...]
+ serialNumber: str
+
+
# Mapping from 'ssl.PROTOCOL_TLSX' to 'TLSVersion.X'
_SSL_VERSION_TO_TLS_VERSION: Dict[int, int] = {}
@@ -150,10 +165,7 @@ except ImportError:
PROTOCOL_TLS_CLIENT = 16 # type: ignore[assignment]
-_PCTRTT = Tuple[Tuple[str, str], ...]
-_PCTRTTT = Tuple[_PCTRTT, ...]
-_TYPE_PEER_CERT_RET_DICT = Mapping[str, Union[str, _PCTRTTT, _PCTRTT]]
-_TYPE_PEER_CERT_RET = Union[_TYPE_PEER_CERT_RET_DICT, bytes, None]
+_TYPE_PEER_CERT_RET = Union["_TYPE_PEER_CERT_RET_DICT", bytes, None]
# A secure default.
# Sources for more information on TLS ciphers:
@@ -396,15 +408,19 @@ def create_urllib3_context(
# The order of the below lines setting verify_mode and check_hostname
# matter due to safe-guards SSLContext has to prevent an SSLContext with
# check_hostname=True, verify_mode=NONE/OPTIONAL.
- if cert_reqs == ssl.CERT_REQUIRED:
+ # We always set 'check_hostname=False' for pyOpenSSL so we rely on our own
+ # 'ssl.match_hostname()' implementation.
+ if cert_reqs == ssl.CERT_REQUIRED and not IS_PYOPENSSL:
context.verify_mode = cert_reqs
context.check_hostname = True
else:
context.check_hostname = False
context.verify_mode = cert_reqs
- if HAS_NEVER_CHECK_COMMON_NAME:
+ try:
context.hostname_checks_common_name = False
+ except AttributeError:
+ pass
# Enable logging of TLS session keys via defacto standard environment variable
# 'SSLKEYLOGFILE', if the feature is available (Python 3.8+). Skip empty values.

MODIFY
src/urllib3/util/ssl_match_hostname.py
src/urllib3/util/ssl_match_hostname.py
import ipaddress
import re
-from typing import Any, Match, Optional, Union
+from typing import TYPE_CHECKING, Any, Match, Optional, Tuple, Union
-from .ssl_ import _TYPE_PEER_CERT_RET
+if TYPE_CHECKING:
+ from .ssl_ import _TYPE_PEER_CERT_RET_DICT
__version__ = "3.5.0.1"
@@ -85,7 +86,11 @@ def _ipaddress_match(ipname: Any, host_ip: str) -> bool:
return bool(ip == host_ip)
-def match_hostname(cert: _TYPE_PEER_CERT_RET, hostname: str) -> None:
+def match_hostname(
+ cert: Optional["_TYPE_PEER_CERT_RET_DICT"],
+ hostname: str,
+ hostname_checks_common_name: bool = False,
+) -> None:
"""Verify that *cert* (in decoded format as returned by
SSLSocket.getpeercert()) matches the *hostname*. RFC 2818 and RFC 6125
rules are followed, but IP addresses are not accepted for *hostname*.
@@ -106,8 +111,10 @@ def match_hostname(cert: _TYPE_PEER_CERT_RET, hostname: str) -> None:
# Not an IP address (common case)
host_ip = None
dnsnames = []
- san = cert.get("subjectAltName", ()) # type: ignore[union-attr]
- for key, value in san: # type: ignore[misc]
+ san: Tuple[Tuple[str, str], ...] = cert.get("subjectAltName", ())
+ key: str
+ value: str
+ for key, value in san:
if key == "DNS":
if host_ip is None and _dnsname_match(value, hostname):
return
@@ -116,6 +123,17 @@ def match_hostname(cert: _TYPE_PEER_CERT_RET, hostname: str) -> None:
if host_ip is not None and _ipaddress_match(value, host_ip):
return
dnsnames.append(value)
+
+ # We only check 'commonName' if it's enabled and we're not verifying
+ # an IP address. IP addresses aren't valid within 'commonName'.
+ if hostname_checks_common_name and host_ip is None and not dnsnames:
+ for sub in cert.get("subject", ()):
+ for key, value in sub:
+ if key == "commonName":
+ if _dnsname_match(value, hostname):
+ return
+ dnsnames.append(value)
+
if len(dnsnames) > 1:
raise CertificateError(
"hostname %r "

MODIFY
src/urllib3/util/ssltransport.py
src/urllib3/util/ssltransport.py
@@ -194,14 +194,8 @@ class SSLTransport:
def getpeercert(self, binary_form: "Literal[True]") -> Optional[bytes]:
...
- @overload
- def getpeercert(self, binary_form: bool) -> "_TYPE_PEER_CERT_RET":
- ...
-
- def getpeercert(
- self, binary_form: bool = False
- ) -> Union[None, bytes, "_TYPE_PEER_CERT_RET_DICT", "_TYPE_PEER_CERT_RET"]:
- return self.sslobj.getpeercert(binary_form)
+ def getpeercert(self, binary_form: bool = False) -> "_TYPE_PEER_CERT_RET":
+ return self.sslobj.getpeercert(binary_form) # type: ignore[return-value]
def version(self) -> Optional[str]:
return self.sslobj.version()

MODIFY
test/conftest.py
test/conftest.py
@@ -100,6 +100,18 @@ def no_san_server(
yield cfg
+@pytest.fixture()
+def no_san_server_with_different_commmon_name(
+ tmp_path_factory: pytest.TempPathFactory,
+) -> Generator[ServerConfig, None, None]:
+ tmpdir = tmp_path_factory.mktemp("certs")
+ ca = trustme.CA()
+ server_cert = ca.issue_cert(common_name="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: pytest.TempPathFactory,

MODIFY
test/test_connection.py
test/test_connection.py
import datetime
import socket
+import typing
from unittest import mock
import pytest
@@ -10,12 +11,14 @@ from urllib3.connection import ( # type: ignore[attr-defined]
HTTPSConnection,
_match_hostname,
)
-from urllib3.util.ssl_ import _TYPE_PEER_CERT_RET
from urllib3.util.ssl_match_hostname import (
CertificateError as ImplementationCertificateError,
)
from urllib3.util.ssl_match_hostname import _dnsname_match, match_hostname
+if typing.TYPE_CHECKING:
+ from urllib3.util.ssl_ import _TYPE_PEER_CERT_RET_DICT
+
class TestConnection:
"""
@@ -29,18 +32,18 @@ class TestConnection:
_match_hostname(cert, asserted_hostname)
def test_match_hostname_empty_cert(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {}
asserted_hostname = "foo"
with pytest.raises(ValueError):
_match_hostname(cert, asserted_hostname)
def test_match_hostname_match(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {"subjectAltName": (("DNS", "foo"),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {"subjectAltName": (("DNS", "foo"),)}
asserted_hostname = "foo"
_match_hostname(cert, asserted_hostname)
def test_match_hostname_mismatch(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {"subjectAltName": (("DNS", "foo"),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {"subjectAltName": (("DNS", "foo"),)}
asserted_hostname = "bar"
try:
with mock.patch("urllib3.connection.log.warning") as mock_log:
@@ -55,7 +58,7 @@ class TestConnection:
assert e._peer_cert == cert
def test_match_hostname_no_dns(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {"subjectAltName": (("DNS", ""),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {"subjectAltName": (("DNS", ""),)}
asserted_hostname = "bar"
try:
with mock.patch("urllib3.connection.log.warning") as mock_log:
@@ -70,24 +73,24 @@ class TestConnection:
assert e._peer_cert == cert
def test_match_hostname_startwith_wildcard(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {"subjectAltName": (("DNS", "*"),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {"subjectAltName": (("DNS", "*"),)}
asserted_hostname = "foo"
_match_hostname(cert, asserted_hostname)
def test_match_hostname_dnsname(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {
"subjectAltName": (("DNS", "xn--p1b6ci4b4b3a*.xn--11b5bs8d"),)
}
asserted_hostname = "xn--p1b6ci4b4b3a*.xn--11b5bs8d"
_match_hostname(cert, asserted_hostname)
def test_match_hostname_include_wildcard(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {"subjectAltName": (("DNS", "foo*"),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {"subjectAltName": (("DNS", "foo*"),)}
asserted_hostname = "foobar"
_match_hostname(cert, asserted_hostname)
def test_match_hostname_more_than_one_dnsname_error(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {
"subjectAltName": (("DNS", "foo*"), ("DNS", "fo*"))
}
asserted_hostname = "bar"
@@ -99,7 +102,7 @@ class TestConnection:
_dnsname_match("foo**", "foobar")
def test_match_hostname_ignore_common_name(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {"subject": (("commonName", "foo"),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {"subject": ((("commonName", "foo"),),)}
asserted_hostname = "foo"
with pytest.raises(
ImplementationCertificateError,
@@ -107,8 +110,15 @@ class TestConnection:
):
match_hostname(cert, asserted_hostname)
+ def test_match_hostname_check_common_name(self) -> None:
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {"subject": ((("commonName", "foo"),),)}
+ asserted_hostname = "foo"
+ match_hostname(cert, asserted_hostname, True)
+
def test_match_hostname_ip_address(self) -> None:
- cert: _TYPE_PEER_CERT_RET = {"subjectAltName": (("IP Address", "1.1.1.1"),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {
+ "subjectAltName": (("IP Address", "1.1.1.1"),)
+ }
asserted_hostname = "1.1.1.2"
try:
with mock.patch("urllib3.connection.log.warning") as mock_log:
@@ -123,7 +133,9 @@ class TestConnection:
assert e._peer_cert == cert
def test_match_hostname_ip_address_ipv6(self) -> None:
- cert = {"subjectAltName": (("IP Address", "1:2::2:1"),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {
+ "subjectAltName": (("IP Address", "1:2::2:1"),)
+ }
asserted_hostname = "1:2::2:2"
try:
with mock.patch("urllib3.connection.log.warning") as mock_log:
@@ -138,7 +150,9 @@ class TestConnection:
assert e._peer_cert == cert
def test_match_hostname_ip_address_ipv6_brackets(self) -> None:
- cert = {"subjectAltName": (("IP Address", "1:2::2:1"),)}
+ cert: "_TYPE_PEER_CERT_RET_DICT" = {
+ "subjectAltName": (("IP Address", "1:2::2:1"),)
+ }
asserted_hostname = "[1:2::2:1]"
# Assert no error is raised
_match_hostname(cert, asserted_hostname)

MODIFY
test/test_util.py
test/test_util.py
@@ -1017,8 +1017,8 @@ class TestUtilSSL:
("OpenSSL 1.1.1l", 0x101010CF, "cpython", (3, 9, 3), True),
# Python OK -> reliable
("OpenSSL 1.1.1", 0x10101000, "cpython", (3, 9, 3), True),
- ("OpenSSL 1.1.1", 0x10101000, "pypy", (3, 6, 9), True),
- ("LibreSSL 3.3.5", 0x101010CF, "pypy", (3, 6, 9), True),
+ ("OpenSSL 1.1.1", 0x10101000, "pypy", (3, 6, 9), False),
+ ("LibreSSL 3.3.5", 0x101010CF, "pypy", (3, 6, 9), False),
# OpenSSL OK -> reliable
("OpenSSL", 0x101010CF, "cpython", (3, 9, 2), True),
# unreliable

MODIFY
test/with_dummyserver/test_https.py
test/with_dummyserver/test_https.py
@@ -42,6 +42,7 @@ from urllib3.exceptions import (
SSLError,
SystemTimeWarning,
)
+from urllib3.util.ssl_match_hostname import CertificateError
from urllib3.util.timeout import Timeout
from .. import has_alpn
@@ -988,6 +989,68 @@ class TestHTTPS_Hostname:
e.value
) or "no appropriate subjectAltName" in str(e.value)
+ def test_common_name_without_san_with_different_common_name(
+ self, no_san_server_with_different_commmon_name: ServerConfig
+ ) -> None:
+ ctx = urllib3.util.ssl_.create_urllib3_context()
+ try:
+ ctx.hostname_checks_common_name = True
+ except AttributeError:
+ pytest.skip("Couldn't set 'SSLContext.hostname_checks_common_name'")
+
+ with HTTPSConnectionPool(
+ no_san_server_with_different_commmon_name.host,
+ no_san_server_with_different_commmon_name.port,
+ cert_reqs="CERT_REQUIRED",
+ ca_certs=no_san_server_with_different_commmon_name.ca_certs,
+ ssl_context=ctx,
+ ) as https_pool:
+ with pytest.raises(MaxRetryError) as e:
+ https_pool.request("GET", "/")
+ assert "mismatch, certificate is not valid for 'localhost'" in str(
+ e.value
+ ) or "hostname 'localhost' doesn't match 'example.com'" in str(e.value)
+
+ @pytest.mark.parametrize("use_assert_hostname", [True, False])
+ def test_hostname_checks_common_name_respected(
+ self, no_san_server: ServerConfig, use_assert_hostname: bool
+ ) -> None:
+ ctx = urllib3.util.ssl_.create_urllib3_context()
+ if not hasattr(ctx, "hostname_checks_common_name"):
+ pytest.skip("Test requires 'SSLContext.hostname_checks_common_name'")
+ ctx.load_verify_locations(no_san_server.ca_certs)
+ try:
+ ctx.hostname_checks_common_name = True
+ except AttributeError:
+ pytest.skip("Couldn't set 'SSLContext.hostname_checks_common_name'")
+
+ err: Optional[MaxRetryError]
+ try:
+ with HTTPSConnectionPool(
+ no_san_server.host,
+ no_san_server.port,
+ cert_reqs="CERT_REQUIRED",
+ ssl_context=ctx,
+ assert_hostname=no_san_server.host if use_assert_hostname else None,
+ ) as https_pool:
+ https_pool.request("GET", "/")
+ except MaxRetryError as e:
+ err = e
+ else:
+ err = None
+
+ # commonName is only valid for DNS names, not IP addresses.
+ if no_san_server.host == "localhost":
+ assert err is None
+
+ # IP addresses should fail for commonName.
+ else:
+ assert err is not None
+ assert type(err.reason) == SSLError
+ assert isinstance(
+ err.reason.args[0], (ssl.SSLCertVerificationError, CertificateError)
+ )
+
def test_strip_square_brackets_before_validating(
self, ipv6_san_server: ServerConfig
) -> None:

Respect 'SSLContext.hostname_checks_common_name' when available and set

Co-authored-by: Seth Michael Larson <sethmichaellarson@gmail.com>