From 2c40dd51942e676ce3c3b405eb9f200704eb68d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jeremy=20Lain=C3=A9?= Date: Wed, 5 Jul 2023 08:01:24 +0200 Subject: [PATCH] Replace deprecated `ssl.match_hostname` method (fixes: #368) The standard libraries's `ssl.match_hostname` method was marked as deprecated in Python 3.10. Rather than implementing this critical piece of code ourselves, make use of the Python Cryptographic Authority's `service-identity` package. One notable behaviour change is that validation is performed *only* against the `subjectAltName` extension instead of the `commonName`. This is the same behaviour as web browsers use. --- pyproject.toml | 1 + src/aioquic/tls.py | 23 +++++++---------------- tests/test_asyncio.py | 12 +++++++++--- tests/test_tls.py | 38 ++++++++------------------------------ 4 files changed, 25 insertions(+), 49 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9afd26474..4a44d531f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,6 +29,7 @@ dependencies = [ "certifi", "pylsqpack>=0.3.3,<0.4.0", "pyopenssl>=22", + "service-identity>=23.1.0", ] dynamic = ["version"] diff --git a/src/aioquic/tls.py b/src/aioquic/tls.py index 9e28c6340..bd8ff66de 100644 --- a/src/aioquic/tls.py +++ b/src/aioquic/tls.py @@ -21,6 +21,7 @@ ) import certifi +import service_identity from cryptography import x509 from cryptography.exceptions import InvalidSignature from cryptography.hazmat.backends import default_backend @@ -216,24 +217,14 @@ def verify_certificate( # verify subject if server_name is not None: - subject = [] - subjectAltName: List[Tuple[str, str]] = [] - for attr in certificate.subject: - if attr.oid == x509.NameOID.COMMON_NAME: - subject.append((("commonName", attr.value),)) - for ext in certificate.extensions: - if isinstance(ext.value, x509.SubjectAlternativeName): - for name in ext.value: - if isinstance(name, x509.DNSName): - subjectAltName.append(("DNS", name.value)) - try: - ssl.match_hostname( - {"subject": tuple(subject), "subjectAltName": tuple(subjectAltName)}, - server_name, + service_identity.cryptography.verify_certificate_hostname( + certificate, server_name ) - except ssl.CertificateError as exc: - raise AlertBadCertificate("\n".join(exc.args)) from exc + except service_identity.VerificationError as exc: + raise AlertBadCertificate( + "Certificate does not match hostname '%s'" % server_name + ) from exc # load CAs store = crypto.X509Store() diff --git a/tests/test_asyncio.py b/tests/test_asyncio.py index 198d0f36f..1a4ff9ae6 100644 --- a/tests/test_asyncio.py +++ b/tests/test_asyncio.py @@ -149,19 +149,25 @@ async def _test_connect_and_serve_with_certificate(self, certificate, private_ke @asynctest async def test_connect_and_serve_with_ec_certificate(self): await self._test_connect_and_serve_with_certificate( - *generate_ec_certificate(common_name="localhost") + *generate_ec_certificate( + alternative_names=["localhost"], common_name="localhost" + ) ) @asynctest async def test_connect_and_serve_with_ed25519_certificate(self): await self._test_connect_and_serve_with_certificate( - *generate_ed25519_certificate(common_name="localhost") + *generate_ed25519_certificate( + alternative_names=["localhost"], common_name="localhost" + ) ) @asynctest async def test_connect_and_serve_with_ed448_certificate(self): await self._test_connect_and_serve_with_certificate( - *generate_ed448_certificate(common_name="localhost") + *generate_ed448_certificate( + alternative_names=["localhost"], common_name="localhost" + ) ) @asynctest diff --git a/tests/test_tls.py b/tests/test_tls.py index 882dce258..4d5a94de8 100644 --- a/tests/test_tls.py +++ b/tests/test_tls.py @@ -37,7 +37,6 @@ ) from cryptography.exceptions import UnsupportedAlgorithm from cryptography.hazmat.primitives import serialization -from cryptography.hazmat.primitives.asymmetric import ec from .utils import ( SERVER_CACERTFILE, @@ -1295,7 +1294,7 @@ def test_verify_certificate_chain(self): def test_verify_certificate_chain_self_signed(self): certificate, _ = generate_ec_certificate( - common_name="localhost", curve=ec.SECP256R1 + alternative_names=["localhost"], common_name="localhost" ) with patch("aioquic.tls.utcnow") as mock_utcnow: @@ -1321,7 +1320,7 @@ def test_verify_certificate_chain_self_signed(self): def test_verify_dates(self): certificate, _ = generate_ec_certificate( - common_name="example.com", curve=ec.SECP256R1 + alternative_names=["example.com"], common_name="example.com" ) cadata = certificate.public_bytes(serialization.Encoding.PEM) @@ -1360,45 +1359,26 @@ def test_verify_dates(self): ) self.assertEqual(str(cm.exception), "Certificate is no longer valid") - def test_verify_subject(self): - certificate, _ = generate_ec_certificate( - common_name="example.com", curve=ec.SECP256R1 - ) + def test_verify_subject_no_subjaltname(self): + certificate, _ = generate_ec_certificate(common_name="example.com") cadata = certificate.public_bytes(serialization.Encoding.PEM) with patch("aioquic.tls.utcnow") as mock_utcnow: mock_utcnow.return_value = certificate.not_valid_before - # valid - verify_certificate( - cadata=cadata, certificate=certificate, server_name="example.com" - ) - - # invalid - with self.assertRaises(tls.AlertBadCertificate) as cm: - verify_certificate( - cadata=cadata, - certificate=certificate, - server_name="test.example.com", - ) - self.assertEqual( - str(cm.exception), - "hostname 'test.example.com' doesn't match 'example.com'", - ) - + # certificates with no SubjectAltName are rejected with self.assertRaises(tls.AlertBadCertificate) as cm: verify_certificate( - cadata=cadata, certificate=certificate, server_name="acme.com" + cadata=cadata, certificate=certificate, server_name="example.com" ) self.assertEqual( - str(cm.exception), "hostname 'acme.com' doesn't match 'example.com'" + str(cm.exception), "Certificate does not match hostname 'example.com'" ) def test_verify_subject_with_subjaltname(self): certificate, _ = generate_ec_certificate( alternative_names=["*.example.com", "example.com"], common_name="example.com", - curve=ec.SECP256R1, ) cadata = certificate.public_bytes(serialization.Encoding.PEM) @@ -1419,7 +1399,5 @@ def test_verify_subject_with_subjaltname(self): cadata=cadata, certificate=certificate, server_name="acme.com" ) self.assertEqual( - str(cm.exception), - "hostname 'acme.com' doesn't match either of '*.example.com', " - "'example.com'", + str(cm.exception), "Certificate does not match hostname 'acme.com'" )