added tests for valid & invalid certificates

added SAN checking (incomplete though)

Author: nitneuqr

src/cryptography/hazmat/primitives/serialization/pkcs7.py

@@ -89,16 +89,38 @@ def _validate_key_usage(
                 raise ValueError(
                     "Key Usage, if specified, must have at least one of the "
                     "digital signature or content commitment (formerly non "
-                    "repudiation) bits set"
+                    "repudiation) bits set."
                 )
 
     def _validate_subject_alternative_name(
         policy: Policy,
         cert: Certificate,
-        san: x509.SubjectAlternativeName | None,
+        san: x509.SubjectAlternativeName,
     ) -> None:
-        if san is not None:
-            pass
+        """
+        For each general name in the SAN, for those which are email addresses:
+        - If it is an RFC822Name, general part must be ascii.
+        - If it is an OtherName, general part must be non-ascii.
+        """
+        for general_name in san:
+            if (
+                isinstance(general_name, x509.RFC822Name)
+                and "@" in general_name.value
+                and not general_name.value.split("@")[0].isascii()
+            ):
+                raise ValueError(
+                    f"RFC822Name {general_name.value} contains non-ASCII "
+                    "characters."
+                )
+            if (
+                isinstance(general_name, x509.OtherName)
+                and "@" in general_name.value.decode()
+                and general_name.value.decode().split("@")[0].isascii()
+            ):
+                raise ValueError(
+                    f"OtherName {general_name.value.decode()} is ASCII, "
+                    "so must be stored in RFC822Name."
+                )
 
     def _validate_extended_key_usage(
         policy: Policy, cert: Certificate, eku: x509.ExtendedKeyUsage | None
@@ -109,11 +131,11 @@ def _validate_extended_key_usage(
             if not (ep or aeku):
                 raise ValueError(
                     "Extended Key Usage, if specified, must include "
-                    "emailProtection or anyExtendedKeyUsage"
+                    "emailProtection or anyExtendedKeyUsage."
                 )
 
     ee_policy = (
-        ExtensionPolicy.permit_all()
+        ExtensionPolicy.webpki_defaults_ee()
         .may_be_present(
             x509.BasicConstraints,
             Criticality.AGNOSTIC,
@@ -124,7 +146,7 @@ def _validate_extended_key_usage(
             Criticality.CRITICAL,
             _validate_key_usage,
         )
-        .may_be_present(
+        .require_present(
             x509.SubjectAlternativeName,
             Criticality.AGNOSTIC,
             _validate_subject_alternative_name,

tests/hazmat/primitives/test_pkcs7.py

@@ -18,7 +18,11 @@
 from cryptography.hazmat.primitives.asymmetric import ed25519, padding, rsa
 from cryptography.hazmat.primitives.ciphers import algorithms
 from cryptography.hazmat.primitives.serialization import pkcs7
-from cryptography.x509.verification import PolicyBuilder, Store
+from cryptography.x509.verification import (
+    PolicyBuilder,
+    Store,
+    VerificationError,
+)
 from tests.x509.test_x509 import _generate_ca_and_leaf
 
 from ...hazmat.primitives.fixtures_rsa import (
@@ -140,18 +144,113 @@ def _load_cert_key():
     return cert, key
 
 
-def test_verify_pkcs7_certificate():
-    certificate, _ = _load_cert_key()
-    ca_policy, ee_policy = pkcs7.pkcs7_x509_extension_policies()
+class TestPKCS7VerifyCertificate:
+    def test_verify_pkcs7_certificate(self):
+        certificate, _ = _load_cert_key()
+        ca_policy, ee_policy = pkcs7.pkcs7_x509_extension_policies()
 
-    builder = (
-        PolicyBuilder()
-        .store(Store([certificate]))
-        .extension_policies(ca_policy, ee_policy)
-    )
+        verifier = (
+            PolicyBuilder()
+            .store(Store([certificate]))
+            .extension_policies(ca_policy, ee_policy)
+            .build_client_verifier()
+        )
+        verifier.verify(certificate, [])
+
+    @pytest.fixture(name="certificate_builder")
+    def fixture_certificate_builder(self) -> x509.CertificateBuilder:
+        certificate, private_key = _load_cert_key()
+        return (
+            x509.CertificateBuilder()
+            .serial_number(certificate.serial_number)
+            .subject_name(certificate.subject)
+            .issuer_name(certificate.issuer)
+            .public_key(private_key.public_key())
+            .not_valid_before(certificate.not_valid_before)
+            .not_valid_after(certificate.not_valid_after)
+        )
+
+    def test_verify_pkcs7_certificate_wrong_bc(self, certificate_builder):
+        certificate, private_key = _load_cert_key()
+
+        # Add an invalid extension
+        extension = x509.BasicConstraints(ca=True, path_length=None)
+        certificate_builder = certificate_builder.add_extension(
+            extension, True
+        )
+
+        # Build the certificate
+        pkcs7_certificate = certificate_builder.sign(
+            private_key, certificate.signature_hash_algorithm, None
+        )
+
+        # Verify the certificate
+        self.verify_invalid_pkcs7_certificate(pkcs7_certificate)
+
+    def test_verify_pkcs7_certificate_wrong_ku(self, certificate_builder):
+        certificate, private_key = _load_cert_key()
+
+        # Add an invalid extension
+        extension = x509.KeyUsage(
+            digital_signature=False,
+            content_commitment=False,
+            key_encipherment=True,
+            data_encipherment=True,
+            key_agreement=True,
+            key_cert_sign=True,
+            crl_sign=True,
+            encipher_only=False,
+            decipher_only=False,
+        )
+        certificate_builder = certificate_builder.add_extension(
+            extension, True
+        )
+
+        # Build the certificate
+        pkcs7_certificate = certificate_builder.sign(
+            private_key, certificate.signature_hash_algorithm, None
+        )
+
+        # Verify the certificate
+        self.verify_invalid_pkcs7_certificate(pkcs7_certificate)
+
+    def test_verify_pkcs7_certificate_wrong_eku(self, certificate_builder):
+        certificate, private_key = _load_cert_key()
+
+        # Add an invalid extension
+        usages = [x509.ExtendedKeyUsageOID.CLIENT_AUTH]  # type: ignore[attr-defined]
+        extension = x509.ExtendedKeyUsage(usages)
+        certificate_builder = certificate_builder.add_extension(
+            extension, True
+        )
+
+        # Add an invalid extension
+        usages = [x509.ExtendedKeyUsageOID.CLIENT_AUTH]  # type: ignore[attr-defined]
+        extension = x509.ExtendedKeyUsage(usages)
+        certificate_builder = certificate_builder.add_extension(
+            extension, True
+        )
+
+        # Build the certificate
+        pkcs7_certificate = certificate_builder.sign(
+            private_key, certificate.signature_hash_algorithm, None
+        )
+
+        # Verify the certificate
+        self.verify_invalid_pkcs7_certificate(pkcs7_certificate)
+
+    @staticmethod
+    def verify_invalid_pkcs7_certificate(certificate: x509.Certificate):
+        ca_policy, ee_policy = pkcs7.pkcs7_x509_extension_policies()
+        verifier = (
+            PolicyBuilder()
+            .store(Store([certificate]))
+            .extension_policies(ca_policy, ee_policy)
+            .build_client_verifier()
+        )
 
-    verifier = builder.build_client_verifier()
-    verifier.verify(certificate, [])
+        with pytest.raises(VerificationError):
+            verifier.verify(certificate, [])
 
 
 @pytest.mark.supported(

vectors/cryptography_vectors/pkcs7/ca.pem

@@ -1,10 +1,11 @@
 -----BEGIN CERTIFICATE-----
-MIIBZzCCAQ6gAwIBAgICAwkwCgYIKoZIzj0EAwIwJzELMAkGA1UEBhMCVVMxGDAW
+MIIBhjCCASygAwIBAgICAwkwCgYIKoZIzj0EAwIwJzELMAkGA1UEBhMCVVMxGDAW
 BgNVBAMMD2NyeXB0b2dyYXBoeSBDQTAgFw0xNzAxMDEwMTAwMDBaGA8yMTAwMDEw
 MTAwMDAwMFowJzELMAkGA1UEBhMCVVMxGDAWBgNVBAMMD2NyeXB0b2dyYXBoeSBD
 QTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABBj/z7v5Obj13cPuwECLBnUGq0/N
-2CxSJE4f4BBGZ7VfFblivTvPDG++Gve0oQ+0uctuhrNQ+WxRv8GC177F+QWjKDAm
-MAwGA1UdEwEB/wQCMAAwFgYDVR0lAQH/BAwwCgYIKwYBBQUHAwQwCgYIKoZIzj0E
-AwIDRwAwRAIgeUqOfJV3+l/GAUYwEnrfpks43W81QpUO9bcKml3N2bECIDZ1vM5p
-Wm+IO/fjVtpIPa3UwcfSguUb3Dut0PpxKPOF
+2CxSJE4f4BBGZ7VfFblivTvPDG++Gve0oQ+0uctuhrNQ+WxRv8GC177F+QWjRjBE
+MCEGA1UdEQEB/wQXMBWBE2V4YW1wbGVAZXhhbXBsZS5jb20wHwYDVR0jBBgwFoAU
+/Ou02BLyyT2Zwzxn9H03feYT7fowCgYIKoZIzj0EAwIDSAAwRQIgUwIdC0Emkd6f
+17DeOXTlmTAhwSDJ2FTuyHESwei7wJcCIQCnr9NpBxbtJfEzxHGGyd7PxgpOLi5u
+rk+8QfzGMmg/fw==
 -----END CERTIFICATE-----