From 088f22d6dc21bb9b2376dfc9f89b90025a759e5a Mon Sep 17 00:00:00 2001 From: Lenno Nagel Date: Fri, 30 Jan 2026 15:15:37 +0200 Subject: [PATCH] fix OCSP.verify_response() to check cert_status Previously, verify_response() only checked response_status (whether the OCSP server processed the request) but not cert_status (whether the certificate is actually valid). This meant revoked and unknown certificates silently passed validation. Now raises OCSPCertificateRevokedError for revoked certificates and OCSPCertificateUnknownError for unknown status. Fixes #13 Co-Authored-By: Claude Opus 4.5 --- pyasice/ocsp.py | 33 ++++++++++++++++++++++++ pyasice/tests/test_ocsp.py | 53 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/pyasice/ocsp.py b/pyasice/ocsp.py index 728beb9..fc580f5 100644 --- a/pyasice/ocsp.py +++ b/pyasice/ocsp.py @@ -13,6 +13,11 @@ from .exceptions import PyAsiceError from .tsa import default_get_session +# OCSP certificate status values (RFC 6960) +CERT_STATUS_GOOD = "good" +CERT_STATUS_REVOKED = "revoked" +CERT_STATUS_UNKNOWN = "unknown" + class SKHackedTBSRequestExtension(TBSRequestExtension): """A workaround class for compatibility with old java libraries used in SK.ee @@ -33,6 +38,18 @@ class OCSPError(PyAsiceError): pass +class OCSPCertificateRevokedError(OCSPError): + """The certificate has been revoked.""" + + pass + + +class OCSPCertificateUnknownError(OCSPError): + """The certificate status is unknown to the OCSP responder.""" + + pass + + class OCSP(object): """ Certificate validation request via the OCSP protocol, using the asn1crypto/ocspbuilder stack. @@ -127,6 +144,22 @@ def verify_response(ocsp_response: Union[OCSPResponse, bytes]): basic_response: ocsp.BasicOCSPResponse = ocsp_response.basic_ocsp_response + # Check cert_status (the actual certificate validity) + single_response = ocsp_response.response_data["responses"][0] + cert_status = single_response["cert_status"] + status_name = cert_status.name + + if status_name == CERT_STATUS_GOOD: + pass # Certificate is valid, continue to signature verification + elif status_name == CERT_STATUS_REVOKED: + revoked_info = cert_status.chosen + revocation_time = revoked_info["revocation_time"].native + raise OCSPCertificateRevokedError(f"Certificate was revoked at {revocation_time}") + elif status_name == CERT_STATUS_UNKNOWN: + raise OCSPCertificateUnknownError("Certificate status is unknown to the OCSP responder") + else: + raise OCSPError(f"Unexpected certificate status: {status_name}") + # Signer's certificate certs = basic_response["certs"] cert: ASN1Certificate = certs[0] diff --git a/pyasice/tests/test_ocsp.py b/pyasice/tests/test_ocsp.py index bd0bc66..7aa53dd 100644 --- a/pyasice/tests/test_ocsp.py +++ b/pyasice/tests/test_ocsp.py @@ -1,13 +1,14 @@ +from datetime import datetime from unittest.mock import Mock, patch import pytest -from asn1crypto.ocsp import OCSPRequest +from asn1crypto.ocsp import OCSPRequest, OCSPResponse from asn1crypto.x509 import Certificate as ASN1Certificate from cryptography.hazmat.primitives.serialization import Encoding from oscrypto.asymmetric import load_certificate -from pyasice.ocsp import OCSP +from pyasice.ocsp import OCSP, OCSPCertificateRevokedError, OCSPCertificateUnknownError from .conftest import cert_builder @@ -63,3 +64,51 @@ def test_ocsp_validate(demo_ocsp_response): mock_build_ocsp_request.assert_called_once_with(b"subject cert", b"issuer cert", b"some-signature") mock_post.assert_called_once() + + +def test_verify_response_rejects_revoked_status(demo_ocsp_response): + ocsp_response = OCSPResponse.load(demo_ocsp_response) + real_response_data = ocsp_response.response_data + + mock_cert_status = Mock() + mock_cert_status.name = "revoked" + mock_revoked_info = Mock() + mock_revoked_info.__getitem__ = Mock( + side_effect=lambda key: Mock(native=datetime(2024, 1, 15, 12, 0, 0)) if key == "revocation_time" else None + ) + mock_cert_status.chosen = mock_revoked_info + + mock_single_response = Mock() + mock_single_response.__getitem__ = Mock(side_effect=lambda key: mock_cert_status if key == "cert_status" else None) + + mock_response_data = Mock() + mock_response_data.__getitem__ = Mock(side_effect=lambda key: [mock_single_response] if key == "responses" else None) + mock_response_data.dump = real_response_data.dump + + with patch.object(OCSPResponse, "response_data", new_callable=lambda: property(lambda self: mock_response_data)): + with pytest.raises(OCSPCertificateRevokedError, match="Certificate was revoked at"): + OCSP.verify_response(ocsp_response) + + +def test_verify_response_rejects_unknown_status(demo_ocsp_response): + ocsp_response = OCSPResponse.load(demo_ocsp_response) + real_response_data = ocsp_response.response_data + + mock_cert_status = Mock() + mock_cert_status.name = "unknown" + + mock_single_response = Mock() + mock_single_response.__getitem__ = Mock(side_effect=lambda key: mock_cert_status if key == "cert_status" else None) + + mock_response_data = Mock() + mock_response_data.__getitem__ = Mock(side_effect=lambda key: [mock_single_response] if key == "responses" else None) + mock_response_data.dump = real_response_data.dump + + with patch.object(OCSPResponse, "response_data", new_callable=lambda: property(lambda self: mock_response_data)): + with pytest.raises(OCSPCertificateUnknownError, match="Certificate status is unknown"): + OCSP.verify_response(ocsp_response) + + +def test_verify_response_accepts_good_status(demo_ocsp_response): + # The demo_ocsp_response has a 'good' status + OCSP.verify_response(demo_ocsp_response)