x509 Verification: Ensure SAN is always checked for servers.

src/rust/cryptography-x509-verification/src/policy/extension.rs

@@ -4,7 +4,7 @@
 
 use std::sync::Arc;
 
-use cryptography_x509::extensions::{Extension, Extensions};
+use cryptography_x509::extensions::{Extension, Extensions, SubjectAlternativeName};
 use cryptography_x509::oid::{
     AUTHORITY_INFORMATION_ACCESS_OID, AUTHORITY_KEY_IDENTIFIER_OID, BASIC_CONSTRAINTS_OID,
     EXTENDED_KEY_USAGE_OID, KEY_USAGE_OID, NAME_CONSTRAINTS_OID, SUBJECT_ALTERNATIVE_NAME_OID,
@@ -16,6 +16,13 @@ use crate::{
     ops::CryptoOps, policy::Policy, ValidationError, ValidationErrorKind, ValidationResult,
 };
 
+use super::Subject;
+
+pub(crate) enum CertificateType {
+    EE,
+    CA,
+}
+
 #[derive(Clone)]
 pub struct ExtensionPolicy<'cb, B: CryptoOps> {
     pub authority_information_access: ExtensionValidator<'cb, B>,
@@ -116,12 +123,13 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
                 Some(Arc::new(ee::key_usage)),
             ),
             // CA/B 7.1.2.7.12 Subscriber Certificate Subject Alternative Name
-            // This validator handles both client and server cases by only matching against
-            // the SAN if the profile contains a subject, which it won't in the client
-            // validation case.
+            // This validator only handles the criticality checks. Matching
+            // SANs against the subject in the profile is handled by
+            // `validate_subject_alternative_name_match` which is
+            // invoked for all EE certificates, irrespective of this field's contents.
             subject_alternative_name: ExtensionValidator::present(
                 Criticality::Agnostic,
-                Some(Arc::new(ee::subject_alternative_name)),
+                Some(Arc::new(ee::subject_alternative_name_criticality)),
             ),
             // 5280 4.2.1.9: Basic Constraints
             basic_constraints: ExtensionValidator::maybe_present(
@@ -142,6 +150,7 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
     pub(crate) fn permits<'chain>(
         &self,
         policy: &Policy<'_, B>,
+        certificate_type: CertificateType,
         cert: &VerificationCertificate<'chain, B>,
         extensions: &Extensions<'_>,
     ) -> ValidationResult<'chain, (), B> {
@@ -180,6 +189,12 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
                     subject_alternative_name_seen = true;
                     self.subject_alternative_name
                         .permits(policy, cert, Some(&ext))?;
+
+                    if let CertificateType::EE = certificate_type {
+                        // This ensures that even custom ExtensionPolicies will always
+                        // check the SAN against the policy's subject
+                        validate_subject_alternative_name_match(&policy.subject, &ext)?;
+                    }
                 }
                 BASIC_CONSTRAINTS_OID => {
                     basic_constraints_seen = true;
@@ -231,10 +246,38 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
             self.extended_key_usage.permits(policy, cert, None)?;
         }
 
+        if let CertificateType::EE = certificate_type {
+            // SAN is always required if the policy contains a specific subject (server profile).
+            if policy.subject.is_some() && !subject_alternative_name_seen {
+                return Err(ValidationError::new(ValidationErrorKind::Other(
+                    "leaf server certificate has no subjectAltName".into(),
+                )));
+            }
+        }
+
         Ok(())
     }
 }
 
+/// This only verifies the SAN against `subject` if `subject` is not None.
+/// This allows us to handle both client and server profiles,
+/// **with the expectation** that `subject` is always set for server profiles.
+pub(crate) fn validate_subject_alternative_name_match<'chain, B: CryptoOps>(
+    subject: &Option<Subject<'_>>,
+    extn: &Extension<'_>,
+) -> ValidationResult<'chain, (), B> {
+    if let Some(sub) = subject {
+        let san: SubjectAlternativeName<'_> = extn.value()?;
+        if !sub.matches(&san) {
+            return Err(ValidationError::new(ValidationErrorKind::Other(
+                "leaf certificate has no matching subjectAltName".into(),
+            )));
+        }
+    }
+
+    Ok(())
+}
+
 /// Represents different criticality states for an extension.
 #[derive(Clone)]
 pub enum Criticality {
@@ -389,9 +432,7 @@ impl<'cb, B: CryptoOps> ExtensionValidator<'cb, B> {
 }
 
 mod ee {
-    use cryptography_x509::extensions::{
-        BasicConstraints, ExtendedKeyUsage, Extension, KeyUsage, SubjectAlternativeName,
-    };
+    use cryptography_x509::extensions::{BasicConstraints, ExtendedKeyUsage, Extension, KeyUsage};
 
     use crate::ops::{CryptoOps, VerificationCertificate};
     use crate::policy::{Policy, ValidationError, ValidationErrorKind, ValidationResult};
@@ -414,8 +455,8 @@ mod ee {
         Ok(())
     }
 
-    pub(crate) fn subject_alternative_name<'chain, B: CryptoOps>(
-        policy: &Policy<'_, B>,
+    pub(crate) fn subject_alternative_name_criticality<'chain, B: CryptoOps>(
+        _: &Policy<'_, B>,
         cert: &VerificationCertificate<'chain, B>,
         extn: &Extension<'_>,
     ) -> ValidationResult<'chain, (), B> {
@@ -435,19 +476,6 @@ mod ee {
             _ => (),
         };
 
-        // NOTE: We only verify the SAN against the policy's subject if the
-        // policy actually contains one. This enables both client and server
-        // profiles to use this validator, **with the expectation** that
-        // server profile construction requires a subject to be present.
-        if let Some(sub) = policy.subject.as_ref() {
-            let san: SubjectAlternativeName<'_> = extn.value()?;
-            if !sub.matches(&san) {
-                return Err(ValidationError::new(ValidationErrorKind::Other(
-                    "leaf certificate has no matching subjectAltName".into(),
-                )));
-            }
-        }
-
         Ok(())
     }
 

src/rust/cryptography-x509-verification/src/policy/mod.rs

@@ -22,6 +22,7 @@ use cryptography_x509::oid::{
     BASIC_CONSTRAINTS_OID, EC_SECP256R1, EC_SECP384R1, EC_SECP521R1, EKU_CLIENT_AUTH_OID,
     EKU_SERVER_AUTH_OID,
 };
+use extension::CertificateType;
 use once_cell::sync::Lazy;
 
 use crate::ops::CryptoOps;
@@ -424,7 +425,8 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
             }
         }
 
-        self.ca_extension_policy.permits(self, cert, extensions)?;
+        self.ca_extension_policy
+            .permits(self, CertificateType::CA, cert, extensions)?;
 
         Ok(())
     }
@@ -437,7 +439,8 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
     ) -> ValidationResult<'chain, (), B> {
         self.permits_basic(cert.certificate())?;
 
-        self.ee_extension_policy.permits(self, cert, extensions)?;
+        self.ee_extension_policy
+            .permits(self, CertificateType::EE, cert, extensions)?;
 
         Ok(())
     }

tests/x509/verification/test_verification.py

@@ -214,33 +214,6 @@ def test_verify_fails_renders_oid(self):
         ):
             verifier.verify(leaf, [])
 
-    def test_custom_ext_policy_no_san(self):
-        leaf = _load_cert(
-            os.path.join("x509", "custom", "no_sans.pem"),
-            x509.load_pem_x509_certificate,
-        )
-
-        store = Store([leaf])
-        validation_time = datetime.datetime.fromisoformat(
-            "2025-04-14T00:00:00+00:00"
-        )
-
-        builder = PolicyBuilder().store(store)
-        builder = builder.time(validation_time)
-
-        with pytest.raises(
-            VerificationError,
-            match="missing required extension",
-        ):
-            builder.build_client_verifier().verify(leaf, [])
-
-        builder = builder.extension_policies(
-            ExtensionPolicy.webpki_defaults_ca(), ExtensionPolicy.permit_all()
-        )
-
-        verified_client = builder.build_client_verifier().verify(leaf, [])
-        assert verified_client.subjects is None
-
 
 class TestServerVerifier:
     @pytest.mark.parametrize(
@@ -553,3 +526,68 @@ def validator(*_):
                 match="Python validator must return None.",
             ):
                 verifier.verify(self.leaf, [])
+
+    def test_no_subject_alt_name(self):
+        leaf = _load_cert(
+            os.path.join("x509", "custom", "no_sans.pem"),
+            x509.load_pem_x509_certificate,
+        )
+
+        store = Store([leaf])
+        validation_time = datetime.datetime.fromisoformat(
+            "2025-04-14T00:00:00+00:00"
+        )
+
+        builder = PolicyBuilder().store(store)
+        builder = builder.time(validation_time)
+
+        with pytest.raises(
+            VerificationError,
+            match="missing required extension",
+        ):
+            builder.build_client_verifier().verify(leaf, [])
+        with pytest.raises(
+            VerificationError,
+            match="missing required extension",
+        ):
+            builder.build_server_verifier(DNSName("example.com")).verify(
+                leaf, []
+            )
+
+        builder = builder.extension_policies(
+            ExtensionPolicy.webpki_defaults_ca(), ExtensionPolicy.permit_all()
+        )
+
+        verified_client = builder.build_client_verifier().verify(leaf, [])
+        assert verified_client.subjects is None
+
+        # For ServerVerifier, SAN must be present,
+        # even if the ExtensionPolicy permits it's absence.
+        with pytest.raises(
+            VerificationError,
+            match="leaf server certificate has no subjectAltName",
+        ):
+            # SAN must be present for ServerVerifier,
+            #
+            builder.build_server_verifier(DNSName("example.com")).verify(
+                leaf, []
+            )
+
+    def test_wrong_subject_alt_name(self):
+        builder = PolicyBuilder().store(self.store)
+        builder = builder.time(self.validation_time)
+        builder = builder.extension_policies(
+            ExtensionPolicy.webpki_defaults_ca(), ExtensionPolicy.permit_all()
+        )
+
+        builder.build_client_verifier().verify(self.leaf, [])
+
+        # For ServerVerifier, SAN must be matched against the subject
+        # even if the extension policy permits any SANs.
+        with pytest.raises(
+            VerificationError,
+            match="leaf certificate has no matching subjectAltName",
+        ):
+            builder.build_server_verifier(DNSName("wrong.io")).verify(
+                self.leaf, []
+            )