diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f32b86..f9aa8c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ You can find out backwards-compatibility policy [here](https://github.com/pyca/s ## [Unreleased](https://github.com/pyca/service-identity/compare/23.1.0...HEAD) +### Changed + +- If a certificate doesn't contain any `subjectAltName`s, we now raise `service_identity.exceptions.CertificateError` instead of `service_identity.exceptions.VerificationError` to make the problem easier to debug. + [#67](https://github.com/pyca/service-identity/pull/67) + + ## [23.1.0](https://github.com/pyca/service-identity/compare/21.1.0...23.1.0) - 2023-06-14 ### Removed diff --git a/src/service_identity/cryptography.py b/src/service_identity/cryptography.py index 2e20061..4585525 100644 --- a/src/service_identity/cryptography.py +++ b/src/service_identity/cryptography.py @@ -40,22 +40,31 @@ def verify_certificate_hostname( certificate: Certificate, hostname: str ) -> None: - """ + r""" Verify whether *certificate* is valid for *hostname*. - .. note:: Nothing is verified about the *authority* of the certificate; - the caller must verify that the certificate chains to an appropriate - trust root themselves. + .. note:: + Nothing is verified about the *authority* of the certificate; + the caller must verify that the certificate chains to an appropriate + trust root themselves. + + Args: + certificate: A *cryptography* X509 certificate object. + + hostname: The hostname that *certificate* should be valid for. - :param certificate: A *cryptography* X509 certificate object. - :param hostname: The hostname that *certificate* should be valid for. + Raises: + service_identity.VerificationError: + If *certificate* is not valid for *hostname*. - :raises service_identity.VerificationError: If *certificate* is not valid - for *hostname*. - :raises service_identity.CertificateError: If *certificate* contains - invalid / unexpected data. + service_identity.CertificateError: + If *certificate* contains invalid / unexpected data. This includes + the case where the certificate contains no `subjectAltName`\ s. - :returns: ``None`` + .. versionchanged:: 24.1.0 + :exc:`~service_identity.CertificateError` is raised if the certificate + contains no ``subjectAltName``\ s instead of + :exc:`~service_identity.VerificationError`. """ verify_service_identity( cert_patterns=extract_patterns(certificate), @@ -67,25 +76,35 @@ def verify_certificate_hostname( def verify_certificate_ip_address( certificate: Certificate, ip_address: str ) -> None: - """ + r""" Verify whether *certificate* is valid for *ip_address*. - .. note:: Nothing is verified about the *authority* of the certificate; - the caller must verify that the certificate chains to an appropriate - trust root themselves. + .. note:: + Nothing is verified about the *authority* of the certificate; + the caller must verify that the certificate chains to an appropriate + trust root themselves. + + Args: + certificate: A *cryptography* X509 certificate object. - :param certificate: A *cryptography* X509 certificate object. - :param ip_address: The IP address that *connection* should be valid - for. Can be an IPv4 or IPv6 address. + ip_address: + The IP address that *connection* should be valid for. Can be an + IPv4 or IPv6 address. - :raises service_identity.VerificationError: If *certificate* is not valid - for *ip_address*. - :raises service_identity.CertificateError: If *certificate* contains - invalid / unexpected data. + Raises: + service_identity.VerificationError: + If *certificate* is not valid for *ip_address*. - :returns: ``None`` + service_identity.CertificateError: + If *certificate* contains invalid / unexpected data. This includes + the case where the certificate contains no ``subjectAltName``\ s. .. versionadded:: 18.1.0 + + .. versionchanged:: 24.1.0 + :exc:`~service_identity.CertificateError` is raised if the certificate + contains no ``subjectAltName``\ s instead of + :exc:`~service_identity.VerificationError`. """ verify_service_identity( cert_patterns=extract_patterns(certificate), diff --git a/src/service_identity/hazmat.py b/src/service_identity/hazmat.py index 19611d8..e8d5e75 100644 --- a/src/service_identity/hazmat.py +++ b/src/service_identity/hazmat.py @@ -50,6 +50,11 @@ def verify_service_identity( *obligatory_ids* must be both present and match. *optional_ids* must match if a pattern of the respective type is present. """ + if not cert_patterns: + raise CertificateError( + "Certificate does not contain any `subjectAltName`s." + ) + errors = [] matches = _find_matches(cert_patterns, obligatory_ids) + _find_matches( cert_patterns, optional_ids diff --git a/src/service_identity/pyopenssl.py b/src/service_identity/pyopenssl.py index 05b535b..0ed88bc 100644 --- a/src/service_identity/pyopenssl.py +++ b/src/service_identity/pyopenssl.py @@ -37,19 +37,28 @@ def verify_hostname(connection: Connection, hostname: str) -> None: - """ + r""" Verify whether the certificate of *connection* is valid for *hostname*. - :param connection: A pyOpenSSL connection object. - :param hostname: The hostname that *connection* should be connected to. + Args: + connection: A pyOpenSSL connection object. + + hostname: The hostname that *connection* should be connected to. + + Raises: + service_identity.VerificationError: + If *connection* does not provide a certificate that is valid for + *hostname*. - :raises service_identity.VerificationError: If *connection* does not - provide a certificate that is valid for *hostname*. - :raises service_identity.CertificateError: If the certificate chain of - *connection* contains a certificate that contains invalid/unexpected - data. + service_identity.CertificateError: + If certificate provided by *connection* contains invalid / + unexpected data. This includes the case where the certificate + contains no ``subjectAltName``\ s. - :returns: ``None`` + .. versionchanged:: 24.1.0 + :exc:`~service_identity.CertificateError` is raised if the certificate + contains no ``subjectAltName``\ s instead of + :exc:`~service_identity.VerificationError`. """ verify_service_identity( cert_patterns=extract_patterns( @@ -61,22 +70,31 @@ def verify_hostname(connection: Connection, hostname: str) -> None: def verify_ip_address(connection: Connection, ip_address: str) -> None: - """ + r""" Verify whether the certificate of *connection* is valid for *ip_address*. - :param connection: A pyOpenSSL connection object. - :param ip_address: The IP address that *connection* should be connected to. - Can be an IPv4 or IPv6 address. + Args: + connection: A pyOpenSSL connection object. + + ip_address: + The IP address that *connection* should be connected to. Can be an + IPv4 or IPv6 address. - :raises service_identity.VerificationError: If *connection* does not - provide a certificate that is valid for *ip_address*. - :raises service_identity.CertificateError: If the certificate chain of - *connection* contains a certificate that contains invalid/unexpected - data. + Raises: + service_identity.VerificationError: + If *connection* does not provide a certificate that is valid for + *ip_address*. - :returns: ``None`` + service_identity.CertificateError: + If the certificate chain of *connection* contains a certificate + that contains invalid/unexpected data. .. versionadded:: 18.1.0 + + .. versionchanged:: 24.1.0 + :exc:`~service_identity.CertificateError` is raised if the certificate + contains no ``subjectAltName``\ s instead of + :exc:`~service_identity.VerificationError`. """ verify_service_identity( cert_patterns=extract_patterns( diff --git a/tests/util.py b/tests/certificates.py similarity index 100% rename from tests/util.py rename to tests/certificates.py diff --git a/tests/test_cryptography.py b/tests/test_cryptography.py index 2c738ad..9a6e0dd 100644 --- a/tests/test_cryptography.py +++ b/tests/test_cryptography.py @@ -12,6 +12,7 @@ verify_certificate_ip_address, ) from service_identity.exceptions import ( + CertificateError, DNSMismatch, IPAddressMismatch, VerificationError, @@ -24,7 +25,12 @@ URIPattern, ) -from .util import PEM_CN_ONLY, PEM_DNS_ONLY, PEM_EVERYTHING, PEM_OTHER_NAME +from .certificates import ( + PEM_CN_ONLY, + PEM_DNS_ONLY, + PEM_EVERYTHING, + PEM_OTHER_NAME, +) backend = default_backend() @@ -35,6 +41,29 @@ class TestPublicAPI: + def test_no_cert_patterns_hostname(self): + """ + A certificate without subjectAltNames raises a helpful + CertificateError. + """ + with pytest.raises( + CertificateError, + match="Certificate does not contain any `subjectAltName`s.", + ): + verify_certificate_hostname(X509_CN_ONLY, "example.com") + + @pytest.mark.parametrize("ip", ["203.0.113.0", "2001:db8::"]) + def test_no_cert_patterns_ip_address(self, ip): + """ + A certificate without subjectAltNames raises a helpful + CertificateError. + """ + with pytest.raises( + CertificateError, + match="Certificate does not contain any `subjectAltName`s.", + ): + verify_certificate_ip_address(X509_CN_ONLY, ip) + def test_certificate_verify_hostname_ok(self): """ verify_certificate_hostname succeeds if the hostnames match. diff --git a/tests/test_hazmat.py b/tests/test_hazmat.py index 3d28ca6..6c70f1f 100644 --- a/tests/test_hazmat.py +++ b/tests/test_hazmat.py @@ -30,8 +30,8 @@ verify_service_identity, ) +from .certificates import DNS_IDS from .test_cryptography import CERT_EVERYTHING -from .util import DNS_IDS try: @@ -45,6 +45,18 @@ class TestVerifyServiceIdentity: Simple integration tests for verify_service_identity. """ + def test_no_cert_patterns(self): + """ + Empty cert patterns raise a helpful CertificateError. + """ + with pytest.raises( + CertificateError, + match="Certificate does not contain any `subjectAltName`s.", + ): + verify_service_identity( + cert_patterns=[], obligatory_ids=[], optional_ids=[] + ) + def test_dns_id_success(self): """ Return pairs of certificate ids and service ids on matches. diff --git a/tests/test_pyopenssl.py b/tests/test_pyopenssl.py index 2afbad4..582e107 100644 --- a/tests/test_pyopenssl.py +++ b/tests/test_pyopenssl.py @@ -21,7 +21,12 @@ verify_ip_address, ) -from .util import PEM_CN_ONLY, PEM_DNS_ONLY, PEM_EVERYTHING, PEM_OTHER_NAME +from .certificates import ( + PEM_CN_ONLY, + PEM_DNS_ONLY, + PEM_EVERYTHING, + PEM_OTHER_NAME, +) if pytest.importorskip("OpenSSL"):