Change error on empty subjectAltNames

CHANGELOG.md

@@ -15,6 +15,11 @@ 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.
+
+
 ## [23.1.0](https://github.com/pyca/service-identity/compare/21.1.0...23.1.0) - 2023-06-14
 
 ### Removed

docs/conf.py

@@ -18,6 +18,7 @@
     "sphinx.ext.doctest",
     "sphinx.ext.autodoc",
     "sphinx.ext.intersphinx",
+    "sphinx.ext.napoleon",
     "myst_parser",
     "notfound.extension",
 ]

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),
@@ -101,9 +120,11 @@ def extract_patterns(cert: Certificate) -> Sequence[CertificatePattern]:
     """
     Extract all valid ID patterns from a certificate for service verification.
 
-    :param cert: The certificate to be dissected.
+    Args:
+        cert: The certificate to be dissected.
 
-    :return: List of IDs.
+    Returns:
+        List of IDs.
 
     .. versionchanged:: 23.1.0
        ``commonName`` is not used as a fallback anymore.

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

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(
@@ -94,9 +112,11 @@ def extract_patterns(cert: X509) -> Sequence[CertificatePattern]:
     """
     Extract all valid ID patterns from a certificate for service verification.
 
-    :param cert: The certificate to be dissected.
+    Args:
+        cert: The certificate to be dissected.
 
-    :return: List of IDs.
+    Returns:
+        List of IDs.
 
     .. versionchanged:: 23.1.0
        ``commonName`` is not used as a fallback anymore.

tests/test_hazmat.py

@@ -45,6 +45,16 @@ 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([], [], [])
+
     def test_dns_id_success(self):
         """
         Return pairs of certificate ids and service ids on matches.

tox.ini

@@ -52,6 +52,19 @@ commands =
     sphinx-build -W -b html -d {envtmpdir}/doctrees docs docs/_build/html
     sphinx-build -W -b doctest -d {envtmpdir}/doctrees docs docs/_build/html
 
+[testenv:docs-watch]
+package = editable
+base_python = {[testenv:docs]base_python}
+extras = {[testenv:docs]extras}
+deps = watchfiles
+commands =
+    watchfiles \
+        --ignore-paths docs/_build/ \
+        'sphinx-build -W -n --jobs auto -b html -d {envtmpdir}/doctrees docs docs/_build/html' \
+        src \
+        docs
+
+
 
 [testenv:coverage-report]
 # keep in-sync with .python-version-default