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_chain_validation.py b/pyasice/tests/test_chain_validation.py new file mode 100644 index 0000000..d4cd3b1 --- /dev/null +++ b/pyasice/tests/test_chain_validation.py @@ -0,0 +1,281 @@ +""" +Tests demonstrating missing certificate chain validation (Issue #17). + +These tests PASS but SHOULD FAIL - they prove that pyasice accepts signatures from: +- Self-signed certificates +- Expired certificates +- Certificates signed by untrusted CAs + +Each test is marked with pytest.mark.xfail(reason="...") to document expected behavior. +When chain validation is implemented, these tests should start failing (which means +the xfail marks should be removed and assertions inverted). +""" + +from datetime import datetime, timedelta + +import pytest + +from cryptography import x509 +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.asymmetric import padding, rsa +from cryptography.hazmat.primitives.serialization import Encoding +from cryptography.x509 import NameOID + +from pyasice import XmlSignature + + +def generate_key_pair(): + """Generate a new RSA key pair.""" + private_key = rsa.generate_private_key( + public_exponent=65537, + key_size=2048, + backend=default_backend() + ) + return private_key + + +def create_self_signed_cert(private_key, subject="Test User", days_valid=5): + """Create a self-signed certificate (subject signs itself).""" + public_key = private_key.public_key() + builder = x509.CertificateBuilder() + builder = builder.subject_name(x509.Name([ + x509.NameAttribute(NameOID.COMMON_NAME, subject), + ])) + # Self-signed: issuer == subject + builder = builder.issuer_name(x509.Name([ + x509.NameAttribute(NameOID.COMMON_NAME, subject), + ])) + builder = builder.not_valid_before(datetime.today()) + builder = builder.not_valid_after(datetime.today() + timedelta(days=days_valid)) + builder = builder.serial_number(x509.random_serial_number()) + builder = builder.public_key(public_key) + builder = builder.add_extension( + x509.BasicConstraints(ca=False, path_length=None), + critical=True, + ) + # Signed with own private key = self-signed + certificate = builder.sign( + private_key=private_key, + algorithm=hashes.SHA256(), + backend=default_backend() + ) + return certificate + + +def create_expired_cert(private_key, subject="Expired User"): + """Create a certificate that expired yesterday.""" + public_key = private_key.public_key() + builder = x509.CertificateBuilder() + builder = builder.subject_name(x509.Name([ + x509.NameAttribute(NameOID.COMMON_NAME, subject), + ])) + builder = builder.issuer_name(x509.Name([ + x509.NameAttribute(NameOID.COMMON_NAME, "Some CA"), + ])) + # Expired: was valid 10 days ago, expired yesterday + builder = builder.not_valid_before(datetime.today() - timedelta(days=10)) + builder = builder.not_valid_after(datetime.today() - timedelta(days=1)) + builder = builder.serial_number(x509.random_serial_number()) + builder = builder.public_key(public_key) + builder = builder.add_extension( + x509.BasicConstraints(ca=False, path_length=None), + critical=True, + ) + certificate = builder.sign( + private_key=private_key, + algorithm=hashes.SHA256(), + backend=default_backend() + ) + return certificate + + +def create_untrusted_ca_cert(ca_key, subject_key, subject="User from Untrusted CA"): + """Create a certificate signed by an untrusted CA.""" + # First create the "untrusted CA" certificate + ca_builder = x509.CertificateBuilder() + ca_builder = ca_builder.subject_name(x509.Name([ + x509.NameAttribute(NameOID.COMMON_NAME, "Malicious CA"), + x509.NameAttribute(NameOID.ORGANIZATION_NAME, "Evil Corp"), + ])) + ca_builder = ca_builder.issuer_name(x509.Name([ + x509.NameAttribute(NameOID.COMMON_NAME, "Malicious CA"), + ])) + ca_builder = ca_builder.not_valid_before(datetime.today()) + ca_builder = ca_builder.not_valid_after(datetime.today() + timedelta(days=365)) + ca_builder = ca_builder.serial_number(x509.random_serial_number()) + ca_builder = ca_builder.public_key(ca_key.public_key()) + ca_builder = ca_builder.add_extension( + x509.BasicConstraints(ca=True, path_length=0), + critical=True, + ) + + # Now create a user certificate signed by this untrusted CA + builder = x509.CertificateBuilder() + builder = builder.subject_name(x509.Name([ + x509.NameAttribute(NameOID.COMMON_NAME, subject), + ])) + builder = builder.issuer_name(x509.Name([ + x509.NameAttribute(NameOID.COMMON_NAME, "Malicious CA"), + x509.NameAttribute(NameOID.ORGANIZATION_NAME, "Evil Corp"), + ])) + builder = builder.not_valid_before(datetime.today()) + builder = builder.not_valid_after(datetime.today() + timedelta(days=5)) + builder = builder.serial_number(x509.random_serial_number()) + builder = builder.public_key(subject_key.public_key()) + builder = builder.add_extension( + x509.BasicConstraints(ca=False, path_length=None), + critical=True, + ) + # Signed by the untrusted CA's key + certificate = builder.sign( + private_key=ca_key, + algorithm=hashes.SHA256(), + backend=default_backend() + ) + return certificate + + +def sign_xml_signature(xml_sig, private_key): + """Sign an XmlSignature with the given private key.""" + signed_data = xml_sig.signed_data() + signature = private_key.sign(signed_data, padding.PKCS1v15(), hashes.SHA256()) + xml_sig.set_signature_value(signature) + return xml_sig + + +class TestSelfSignedCertificateAccepted: + """ + Demonstrates that self-signed certificates pass verification. + + A self-signed certificate has no chain of trust - it vouches for itself. + This should be rejected unless explicitly trusted as a root. + """ + + @pytest.mark.xfail( + reason="Chain validation not implemented - self-signed certs should be rejected", + strict=True + ) + def test_self_signed_cert_should_be_rejected(self): + """Self-signed certificate should NOT pass signature verification.""" + private_key = generate_key_pair() + cert = create_self_signed_cert(private_key, subject="Self-Signed User") + + xml_sig = ( + XmlSignature.create() + .add_document("test.txt", b"test content", "text/plain") + .set_certificate(cert.public_bytes(Encoding.DER)) + .update_signed_info() + ) + sign_xml_signature(xml_sig, private_key) + + # This SHOULD raise an exception but currently doesn't + xml_sig.verify() + + # If we reach here, the test "passes" but that's the bug + pytest.fail("Self-signed certificate was accepted - chain validation missing") + + +class TestExpiredCertificateAccepted: + """ + Demonstrates that expired certificates pass verification. + + An expired certificate should be rejected regardless of signature validity. + """ + + @pytest.mark.xfail( + reason="Certificate expiration not checked during verification", + strict=True + ) + def test_expired_cert_should_be_rejected(self): + """Expired certificate should NOT pass signature verification.""" + private_key = generate_key_pair() + cert = create_expired_cert(private_key, subject="Expired User") + + xml_sig = ( + XmlSignature.create() + .add_document("test.txt", b"test content", "text/plain") + .set_certificate(cert.public_bytes(Encoding.DER)) + .update_signed_info() + ) + sign_xml_signature(xml_sig, private_key) + + # This SHOULD raise an exception but currently doesn't + xml_sig.verify() + + # If we reach here, the test "passes" but that's the bug + pytest.fail("Expired certificate was accepted - expiration check missing") + + +class TestUntrustedCAAccepted: + """ + Demonstrates that certificates from untrusted CAs pass verification. + + A certificate signed by an unknown/untrusted CA should be rejected + because there's no chain of trust to a known root. + """ + + @pytest.mark.xfail( + reason="Chain validation not implemented - untrusted CA certs should be rejected", + strict=True + ) + def test_untrusted_ca_cert_should_be_rejected(self): + """Certificate from untrusted CA should NOT pass signature verification.""" + ca_key = generate_key_pair() + user_key = generate_key_pair() + cert = create_untrusted_ca_cert(ca_key, user_key, subject="User from Evil Corp") + + xml_sig = ( + XmlSignature.create() + .add_document("test.txt", b"test content", "text/plain") + .set_certificate(cert.public_bytes(Encoding.DER)) + .update_signed_info() + ) + sign_xml_signature(xml_sig, user_key) + + # This SHOULD raise an exception but currently doesn't + xml_sig.verify() + + # If we reach here, the test "passes" but that's the bug + pytest.fail("Certificate from untrusted CA was accepted - chain validation missing") + + +class TestExistingTestsUseSelfSignedCerts: + """ + Documents that the existing test suite uses self-signed certificates. + + The conftest.py cert_builder() creates certificates where: + - subject = "signing user" + - issuer = "issuer CA" + - BUT it's signed with the subject's own private key + + This is effectively self-signed (the issuer name is a lie). + These pass because only cryptographic verification is performed. + """ + + def test_existing_fixture_is_self_signed(self, certificate_rsa, private_key_rsa): + """ + Prove that the existing test fixture creates self-signed certificates. + + The certificate claims issuer="issuer CA" but is signed with the + subject's private key, making it cryptographically self-signed. + """ + # The certificate says it was issued by "issuer CA" + issuer_cn = certificate_rsa.issuer.get_attributes_for_oid(NameOID.COMMON_NAME)[0].value + assert issuer_cn == "issuer CA" + + # But it's actually signed with the subject's own key + subject_cn = certificate_rsa.subject.get_attributes_for_oid(NameOID.COMMON_NAME)[0].value + assert subject_cn == "signing user" + + # Verify the certificate signature using the subject's own public key + # (this would fail if it was truly signed by a different "issuer CA") + public_key = private_key_rsa.public_key() + public_key.verify( + certificate_rsa.signature, + certificate_rsa.tbs_certificate_bytes, + padding.PKCS1v15(), + certificate_rsa.signature_hash_algorithm, + ) + # If we get here, the certificate is self-signed (signed with subject's key) + # A proper certificate would need the issuer CA's key to verify \ No newline at end of file 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)