Skip to content

Commit

Permalink
Replace deprecated ssl.match_hostname method (fixes: #368)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jlaine committed Jul 5, 2023
1 parent 33b7609 commit e8b1300
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 43 deletions.
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ dependencies = [
"certifi",
"pylsqpack>=0.3.3,<0.4.0",
"pyopenssl>=22",
"service-identity>=23.1.0",
]
dynamic = ["version"]

Expand Down
23 changes: 7 additions & 16 deletions src/aioquic/tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 9 additions & 3 deletions tests/test_asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 8 additions & 24 deletions tests/test_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -1295,7 +1295,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", curve=ec.SECP256R1
)

with patch("aioquic.tls.utcnow") as mock_utcnow:
Expand All @@ -1321,7 +1321,9 @@ 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",
curve=ec.SECP256R1,
)
cadata = certificate.public_bytes(serialization.Encoding.PEM)

Expand Down Expand Up @@ -1369,29 +1371,13 @@ def test_verify_subject(self):
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
# certificates with no SubjectAltName are rejected
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'",
)

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):
Expand Down Expand Up @@ -1419,7 +1405,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'"
)

0 comments on commit e8b1300

Please sign in to comment.