diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f32b86..47df772 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/conf.py b/docs/conf.py index d072d82..66ebecc 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -18,6 +18,7 @@ "sphinx.ext.doctest", "sphinx.ext.autodoc", "sphinx.ext.intersphinx", + "sphinx.ext.napoleon", "myst_parser", "notfound.extension", ] diff --git a/src/service_identity/cryptography.py b/src/service_identity/cryptography.py index 9397fcf..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), @@ -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. diff --git a/src/service_identity/hazmat.py b/src/service_identity/hazmat.py index 6b9bf7b..53dc04e 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 30b5d58..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( @@ -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. diff --git a/tests/test_hazmat.py b/tests/test_hazmat.py index 3d28ca6..75ae389 100644 --- a/tests/test_hazmat.py +++ b/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. diff --git a/tox.ini b/tox.ini index 0d73684..d90a32b 100644 --- a/tox.ini +++ b/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