From a97438b14c4745ef9de8a627cce1704deef82a5b Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 4 Jan 2024 12:41:22 -0500 Subject: [PATCH] Make extension handling in x.509 verifier less meta-programmed (#10054) We now iterate over the extensions only once. --- .../src/policy/extension.rs | 309 +++++++++++------- .../src/policy/mod.rs | 128 +++----- tests/x509/verification/test_limbo.py | 2 + 3 files changed, 223 insertions(+), 216 deletions(-) diff --git a/src/rust/cryptography-x509-verification/src/policy/extension.rs b/src/rust/cryptography-x509-verification/src/policy/extension.rs index fd5035d87ab5..7006ad5dd110 100644 --- a/src/rust/cryptography-x509-verification/src/policy/extension.rs +++ b/src/rust/cryptography-x509-verification/src/policy/extension.rs @@ -2,7 +2,11 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use asn1::ObjectIdentifier; +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, + SUBJECT_KEY_IDENTIFIER_OID, +}; use cryptography_x509::{ certificate::Certificate, extensions::{Extension, Extensions}, @@ -10,6 +14,114 @@ use cryptography_x509::{ use crate::{ops::CryptoOps, policy::Policy, ValidationError}; +pub(crate) struct ExtensionPolicy { + pub(crate) authority_information_access: ExtensionValidator, + pub(crate) authority_key_identifier: ExtensionValidator, + pub(crate) subject_key_identifier: ExtensionValidator, + pub(crate) key_usage: ExtensionValidator, + pub(crate) subject_alternative_name: ExtensionValidator, + pub(crate) basic_constraints: ExtensionValidator, + pub(crate) name_constraints: ExtensionValidator, + pub(crate) extended_key_usage: ExtensionValidator, +} + +impl ExtensionPolicy { + pub(crate) fn permits( + &self, + policy: &Policy<'_, B>, + cert: &Certificate<'_>, + extensions: &Extensions<'_>, + ) -> Result<(), ValidationError> { + let mut authority_information_access_seen = false; + let mut authority_key_identifier_seen = false; + let mut subject_key_identifier_seen = false; + let mut key_usage_seen = false; + let mut subject_alternative_name_seen = false; + let mut basic_constraints_seen = false; + let mut name_constraints_seen = false; + let mut extended_key_usage_seen = false; + + // Iterate over each extension and run its policy. + for ext in extensions.iter() { + match ext.extn_id { + AUTHORITY_INFORMATION_ACCESS_OID => { + authority_information_access_seen = true; + self.authority_information_access + .permits(policy, cert, Some(&ext))?; + } + AUTHORITY_KEY_IDENTIFIER_OID => { + authority_key_identifier_seen = true; + self.authority_key_identifier + .permits(policy, cert, Some(&ext))?; + } + SUBJECT_KEY_IDENTIFIER_OID => { + subject_key_identifier_seen = true; + self.subject_key_identifier + .permits(policy, cert, Some(&ext))?; + } + KEY_USAGE_OID => { + key_usage_seen = true; + self.key_usage.permits(policy, cert, Some(&ext))?; + } + SUBJECT_ALTERNATIVE_NAME_OID => { + subject_alternative_name_seen = true; + self.subject_alternative_name + .permits(policy, cert, Some(&ext))?; + } + BASIC_CONSTRAINTS_OID => { + basic_constraints_seen = true; + self.basic_constraints.permits(policy, cert, Some(&ext))?; + } + NAME_CONSTRAINTS_OID => { + name_constraints_seen = true; + self.name_constraints.permits(policy, cert, Some(&ext))?; + } + EXTENDED_KEY_USAGE_OID => { + extended_key_usage_seen = true; + self.extended_key_usage.permits(policy, cert, Some(&ext))?; + } + _ if ext.critical => { + return Err(ValidationError::Other(format!( + "certificate contains unaccounted-for critical extensions: {}", + ext.extn_id + ))); + } + _ => {} + } + } + + // Now we check if there were any required extensions that aren't + // present + if !authority_information_access_seen { + self.authority_information_access + .permits(policy, cert, None)?; + } + if !authority_key_identifier_seen { + self.authority_key_identifier.permits(policy, cert, None)?; + } + if !subject_key_identifier_seen { + self.subject_key_identifier.permits(policy, cert, None)?; + } + if !key_usage_seen { + self.key_usage.permits(policy, cert, None)?; + } + if !subject_alternative_name_seen { + self.subject_alternative_name.permits(policy, cert, None)?; + } + if !basic_constraints_seen { + self.basic_constraints.permits(policy, cert, None)?; + } + if !name_constraints_seen { + self.name_constraints.permits(policy, cert, None)?; + } + if !extended_key_usage_seen { + self.extended_key_usage.permits(policy, cert, None)?; + } + + Ok(()) + } +} + /// Represents different criticality states for an extension. pub(crate) enum Criticality { /// The extension MUST be marked as critical. @@ -58,46 +170,28 @@ pub(crate) enum ExtensionValidator { }, } -/// A "policy" for validating a specific X.509v3 extension, identified by -/// its OID. -pub(crate) struct ExtensionPolicy { - pub(crate) oid: asn1::ObjectIdentifier, - pub(crate) validator: ExtensionValidator, -} - -impl ExtensionPolicy { - pub(crate) fn not_present(oid: ObjectIdentifier) -> Self { - Self { - oid, - validator: ExtensionValidator::NotPresent, - } +impl ExtensionValidator { + pub(crate) fn not_present() -> Self { + Self::NotPresent } pub(crate) fn present( - oid: ObjectIdentifier, criticality: Criticality, validator: Option>, ) -> Self { - Self { - oid, - validator: ExtensionValidator::Present { - criticality, - validator, - }, + Self::Present { + criticality, + validator, } } pub(crate) fn maybe_present( - oid: ObjectIdentifier, criticality: Criticality, validator: Option>, ) -> Self { - Self { - oid, - validator: ExtensionValidator::MaybePresent { - criticality, - validator, - }, + Self::MaybePresent { + criticality, + validator, } } @@ -105,20 +199,19 @@ impl ExtensionPolicy { &self, policy: &Policy<'_, B>, cert: &Certificate<'_>, - extensions: &Extensions<'_>, + extension: Option<&Extension<'_>>, ) -> Result<(), ValidationError> { - match (&self.validator, extensions.get_extension(&self.oid)) { + match (self, extension) { // Extension MUST NOT be present and isn't; OK. (ExtensionValidator::NotPresent, None) => Ok(()), // Extension MUST NOT be present but is; NOT OK. (ExtensionValidator::NotPresent, Some(_)) => Err(ValidationError::Other( - "EE certificate contains prohibited extension".to_string(), + "Certificate contains prohibited extension".to_string(), )), // Extension MUST be present but is not; NOT OK. - (ExtensionValidator::Present { .. }, None) => Err(ValidationError::Other(format!( - "EE certificate is missing required extension: {}", - self.oid - ))), + (ExtensionValidator::Present { .. }, None) => Err(ValidationError::Other( + "Certificate is missing required extension".to_string(), + )), // Extension MUST be present and is; check it. ( ExtensionValidator::Present { @@ -129,12 +222,12 @@ impl ExtensionPolicy { ) => { if !criticality.permits(extn.critical) { return Err(ValidationError::Other( - "EE certificate extension has incorrect criticality".to_string(), + "Certificate extension has incorrect criticality".to_string(), )); } // If a custom validator is supplied, apply it. - validator.map_or(Ok(()), |v| v(policy, cert, &extn)) + validator.map_or(Ok(()), |v| v(policy, cert, extn)) } // Extension MAY be present. ( @@ -145,17 +238,14 @@ impl ExtensionPolicy { extn, ) => { // If the extension is present, apply our criticality check. - if extn - .as_ref() - .map_or(false, |extn| !criticality.permits(extn.critical)) - { + if extn.map_or(false, |extn| !criticality.permits(extn.critical)) { return Err(ValidationError::Other( - "EE certificate extension has incorrect criticality".to_string(), + "Certificate extension has incorrect criticality".to_string(), )); } // If a custom validator is supplied, apply it. - validator.map_or(Ok(()), |v| v(policy, cert, extn.as_ref())) + validator.map_or(Ok(()), |v| v(policy, cert, extn)) } } } @@ -448,10 +538,10 @@ pub(crate) mod common { mod tests { use asn1::{ObjectIdentifier, SimpleAsn1Writable}; use cryptography_x509::certificate::Certificate; - use cryptography_x509::extensions::{BasicConstraints, Extension, Extensions}; + use cryptography_x509::extensions::{BasicConstraints, Extension}; use cryptography_x509::oid::BASIC_CONSTRAINTS_OID; - use super::{Criticality, ExtensionPolicy}; + use super::{Criticality, ExtensionValidator}; use crate::certificate::tests::PublicKeyErrorOps; use crate::ops::tests::{cert, v1_cert_pem}; use crate::ops::CryptoOps; @@ -477,25 +567,18 @@ mod tests { asn1::DateTime::new(1970, 1, 1, 0, 0, 0).unwrap() } - fn create_encoded_extensions( + fn create_encoded_extension( oid: ObjectIdentifier, critical: bool, ext: &T, ) -> Vec { let ext_value = asn1::write_single(&ext).unwrap(); - let exts = vec![Extension { + let ext = Extension { extn_id: oid, critical, extn_value: &ext_value, - }]; - let der_exts = asn1::write_single(&asn1::SequenceOfWriter::new(exts)).unwrap(); - der_exts - } - - fn create_empty_encoded_extensions() -> Vec { - let exts: Vec> = vec![]; - let der_exts = asn1::write_single(&asn1::SequenceOfWriter::new(exts)).unwrap(); - der_exts + }; + asn1::write_single(&ext).unwrap() } fn present_extension_validator( @@ -507,7 +590,7 @@ mod tests { } #[test] - fn test_extension_policy_present() { + fn test_extension_validator_present() { // The certificate doesn't get used for this validator, so the certificate we use isn't important. let cert_pem = v1_cert_pem(); let cert = cert(&cert_pem); @@ -520,31 +603,22 @@ mod tests { ); // Test a policy that stipulates that a given extension MUST be present. - let extension_policy = ExtensionPolicy::present( - BASIC_CONSTRAINTS_OID, - Criticality::Critical, - Some(present_extension_validator), - ); + let extension_validator = + ExtensionValidator::present(Criticality::Critical, Some(present_extension_validator)); // Check the case where the extension is present. let bc = BasicConstraints { ca: true, path_length: Some(3), }; - let der_exts = create_encoded_extensions(BASIC_CONSTRAINTS_OID, true, &bc); - let raw_exts = asn1::parse_single(&der_exts).unwrap(); - let exts = Extensions::from_raw_extensions(Some(&raw_exts)) - .ok() - .unwrap(); - assert!(extension_policy.permits(&policy, &cert, &exts).is_ok()); + let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, true, &bc); + let raw_ext = asn1::parse_single(&der_ext).unwrap(); + assert!(extension_validator + .permits(&policy, &cert, Some(&raw_ext)) + .is_ok()); // Check the case where the extension isn't present. - let der_exts: Vec = create_empty_encoded_extensions(); - let raw_exts = asn1::parse_single(&der_exts).unwrap(); - let exts = Extensions::from_raw_extensions(Some(&raw_exts)) - .ok() - .unwrap(); - assert!(extension_policy.permits(&policy, &cert, &exts).is_err()); + assert!(extension_validator.permits(&policy, &cert, None).is_err()); } fn maybe_extension_validator( @@ -556,7 +630,7 @@ mod tests { } #[test] - fn test_extension_policy_maybe() { + fn test_extension_validator_maybe() { // The certificate doesn't get used for this validator, so the certificate we use isn't important. let cert_pem = v1_cert_pem(); let cert = cert(&cert_pem); @@ -568,9 +642,8 @@ mod tests { None, ); - // Test a policy that stipulates that a given extension CAN be present. - let extension_policy = ExtensionPolicy::maybe_present( - BASIC_CONSTRAINTS_OID, + // Test a validator that stipulates that a given extension CAN be present. + let extension_validator = ExtensionValidator::maybe_present( Criticality::Critical, Some(maybe_extension_validator), ); @@ -580,24 +653,18 @@ mod tests { ca: false, path_length: Some(3), }; - let der_exts = create_encoded_extensions(BASIC_CONSTRAINTS_OID, true, &bc); - let raw_exts = asn1::parse_single(&der_exts).unwrap(); - let exts = Extensions::from_raw_extensions(Some(&raw_exts)) - .ok() - .unwrap(); - assert!(extension_policy.permits(&policy, &cert, &exts).is_ok()); + let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, true, &bc); + let raw_ext = asn1::parse_single(&der_ext).unwrap(); + assert!(extension_validator + .permits(&policy, &cert, Some(&raw_ext)) + .is_ok()); // Check the case where the extension isn't present. - let der_exts: Vec = create_empty_encoded_extensions(); - let raw_exts = asn1::parse_single(&der_exts).unwrap(); - let exts = Extensions::from_raw_extensions(Some(&raw_exts)) - .ok() - .unwrap(); - assert!(extension_policy.permits(&policy, &cert, &exts).is_ok()); + assert!(extension_validator.permits(&policy, &cert, None).is_ok()); } #[test] - fn test_extension_policy_not_present() { + fn test_extension_validator_not_present() { // The certificate doesn't get used for this validator, so the certificate we use isn't important. let cert_pem = v1_cert_pem(); let cert = cert(&cert_pem); @@ -609,32 +676,26 @@ mod tests { None, ); - // Test a policy that stipulates that a given extension MUST NOT be present. - let extension_policy = ExtensionPolicy::not_present(BASIC_CONSTRAINTS_OID); + // Test a validator that stipulates that a given extension MUST NOT be present. + let extension_validator = ExtensionValidator::not_present(); // Check the case where the extension is present. let bc = BasicConstraints { ca: false, path_length: Some(3), }; - let der_exts = create_encoded_extensions(BASIC_CONSTRAINTS_OID, true, &bc); - let raw_exts = asn1::parse_single(&der_exts).unwrap(); - let exts = Extensions::from_raw_extensions(Some(&raw_exts)) - .ok() - .unwrap(); - assert!(extension_policy.permits(&policy, &cert, &exts).is_err()); + let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, true, &bc); + let raw_ext = asn1::parse_single(&der_ext).unwrap(); + assert!(extension_validator + .permits(&policy, &cert, Some(&raw_ext)) + .is_err()); // Check the case where the extension isn't present. - let der_exts: Vec = create_empty_encoded_extensions(); - let raw_exts = asn1::parse_single(&der_exts).unwrap(); - let exts = Extensions::from_raw_extensions(Some(&raw_exts)) - .ok() - .unwrap(); - assert!(extension_policy.permits(&policy, &cert, &exts).is_ok()); + assert!(extension_validator.permits(&policy, &cert, None).is_ok()); } #[test] - fn test_extension_policy_present_incorrect_criticality() { + fn test_extension_validator_present_incorrect_criticality() { // The certificate doesn't get used for this validator, so the certificate we use isn't important. let cert_pem = v1_cert_pem(); let cert = cert(&cert_pem); @@ -647,27 +708,23 @@ mod tests { ); // Test a present policy that stipulates that a given extension MUST be critical. - let extension_policy = ExtensionPolicy::present( - BASIC_CONSTRAINTS_OID, - Criticality::Critical, - Some(present_extension_validator), - ); + let extension_validator = + ExtensionValidator::present(Criticality::Critical, Some(present_extension_validator)); // Mark the extension as non-critical despite our policy stipulating that it must be critical. let bc = BasicConstraints { ca: true, path_length: Some(3), }; - let der_exts = create_encoded_extensions(BASIC_CONSTRAINTS_OID, false, &bc); - let raw_exts = asn1::parse_single(&der_exts).unwrap(); - let exts = Extensions::from_raw_extensions(Some(&raw_exts)) - .ok() - .unwrap(); - assert!(extension_policy.permits(&policy, &cert, &exts).is_err()); + let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, false, &bc); + let raw_ext = asn1::parse_single(&der_ext).unwrap(); + assert!(extension_validator + .permits(&policy, &cert, Some(&raw_ext)) + .is_err()); } #[test] - fn test_extension_policy_maybe_present_incorrect_criticality() { + fn test_extension_validator_maybe_present_incorrect_criticality() { // The certificate doesn't get used for this validator, so the certificate we use isn't important. let cert_pem = v1_cert_pem(); let cert = cert(&cert_pem); @@ -679,9 +736,8 @@ mod tests { None, ); - // Test a maybe present policy that stipulates that a given extension MUST be critical. - let extension_policy = ExtensionPolicy::maybe_present( - BASIC_CONSTRAINTS_OID, + // Test a maybe present validator that stipulates that a given extension MUST be critical. + let extension_validator = ExtensionValidator::maybe_present( Criticality::Critical, Some(maybe_extension_validator), ); @@ -691,11 +747,10 @@ mod tests { ca: true, path_length: Some(3), }; - let der_exts = create_encoded_extensions(BASIC_CONSTRAINTS_OID, false, &bc); - let raw_exts = asn1::parse_single(&der_exts).unwrap(); - let exts = Extensions::from_raw_extensions(Some(&raw_exts)) - .ok() - .unwrap(); - assert!(extension_policy.permits(&policy, &cert, &exts).is_err()); + let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, false, &bc); + let raw_ext = asn1::parse_single(&der_ext).unwrap(); + assert!(extension_validator + .permits(&policy, &cert, Some(&raw_ext)) + .is_err()); } } diff --git a/src/rust/cryptography-x509-verification/src/policy/mod.rs b/src/rust/cryptography-x509-verification/src/policy/mod.rs index e67cf2fb0da6..e51f0a1c413c 100644 --- a/src/rust/cryptography-x509-verification/src/policy/mod.rs +++ b/src/rust/cryptography-x509-verification/src/policy/mod.rs @@ -17,15 +17,12 @@ use cryptography_x509::common::{ use cryptography_x509::extensions::{BasicConstraints, Extensions, SubjectAlternativeName}; use cryptography_x509::name::GeneralName; use cryptography_x509::oid::{ - AUTHORITY_INFORMATION_ACCESS_OID, AUTHORITY_KEY_IDENTIFIER_OID, BASIC_CONSTRAINTS_OID, - EC_SECP256R1, EC_SECP384R1, EC_SECP521R1, EKU_SERVER_AUTH_OID, EXTENDED_KEY_USAGE_OID, - KEY_USAGE_OID, NAME_CONSTRAINTS_OID, POLICY_CONSTRAINTS_OID, SUBJECT_ALTERNATIVE_NAME_OID, - SUBJECT_DIRECTORY_ATTRIBUTES_OID, SUBJECT_KEY_IDENTIFIER_OID, + BASIC_CONSTRAINTS_OID, EC_SECP256R1, EC_SECP384R1, EC_SECP521R1, EKU_SERVER_AUTH_OID, }; use once_cell::sync::Lazy; use crate::ops::CryptoOps; -use crate::policy::extension::{ca, common, ee, Criticality, ExtensionPolicy}; +use crate::policy::extension::{ca, common, ee, Criticality, ExtensionPolicy, ExtensionValidator}; use crate::types::{DNSName, DNSPattern, IPAddress}; use crate::{ValidationError, VerificationCertificate}; @@ -216,9 +213,8 @@ pub struct Policy<'a, B: CryptoOps> { /// algorithm identifiers. pub permitted_signature_algorithms: HashSet>, - common_extension_policies: Vec>, - ca_extension_policies: Vec>, - ee_extension_policies: Vec>, + ca_extension_policy: ExtensionPolicy, + ee_extension_policy: ExtensionPolicy, } impl<'a, B: CryptoOps> Policy<'a, B> { @@ -246,105 +242,93 @@ impl<'a, B: CryptoOps> Policy<'a, B> { .into_iter() .cloned() .collect(), - common_extension_policies: Vec::from([ - // 5280 4.2.1.8: Subject Directory Attributes - ExtensionPolicy::maybe_present( - SUBJECT_DIRECTORY_ATTRIBUTES_OID, - Criticality::NonCritical, - None, - ), + ca_extension_policy: ExtensionPolicy { // 5280 4.2.2.1: Authority Information Access - ExtensionPolicy::maybe_present( - AUTHORITY_INFORMATION_ACCESS_OID, + authority_information_access: ExtensionValidator::maybe_present( Criticality::NonCritical, Some(common::authority_information_access), ), - ]), - ca_extension_policies: Vec::from([ // 5280 4.2.1.1: Authority Key Identifier - ExtensionPolicy::maybe_present( - AUTHORITY_KEY_IDENTIFIER_OID, + authority_key_identifier: ExtensionValidator::maybe_present( Criticality::NonCritical, Some(ca::authority_key_identifier), ), // 5280 4.2.1.2: Subject Key Identifier // NOTE: CABF requires SKI in CA certificates, but many older CAs lack it. // We choose to be permissive here. - ExtensionPolicy::maybe_present( - SUBJECT_KEY_IDENTIFIER_OID, + subject_key_identifier: ExtensionValidator::maybe_present( Criticality::NonCritical, None, ), // 5280 4.2.1.3: Key Usage - ExtensionPolicy::present(KEY_USAGE_OID, Criticality::Agnostic, Some(ca::key_usage)), + key_usage: ExtensionValidator::present(Criticality::Agnostic, Some(ca::key_usage)), + subject_alternative_name: ExtensionValidator::maybe_present( + Criticality::Agnostic, + None, + ), // 5280 4.2.1.9: Basic Constraints - ExtensionPolicy::present( - BASIC_CONSTRAINTS_OID, + basic_constraints: ExtensionValidator::present( Criticality::Critical, Some(ca::basic_constraints), ), // 5280 4.2.1.10: Name Constraints // NOTE: MUST be critical in 5280, but CABF relaxes to MAY. - ExtensionPolicy::maybe_present( - NAME_CONSTRAINTS_OID, + name_constraints: ExtensionValidator::maybe_present( Criticality::Agnostic, Some(ca::name_constraints), ), - // 5280 4.2.1.11: Policy Constraints - ExtensionPolicy::maybe_present(POLICY_CONSTRAINTS_OID, Criticality::Critical, None), // 5280: 4.2.1.12: Extended Key Usage // NOTE: CABF requires EKUs in many non-root CA certs, but validators widely // ignore this requirement and treat a missing EKU as "any EKU". // We choose to be permissive here. - ExtensionPolicy::maybe_present( - EXTENDED_KEY_USAGE_OID, + extended_key_usage: ExtensionValidator::maybe_present( Criticality::NonCritical, Some(ca::extended_key_usage), ), - ]), - ee_extension_policies: Vec::from([ + }, + ee_extension_policy: ExtensionPolicy { + // 5280 4.2.2.1: Authority Information Access + authority_information_access: ExtensionValidator::maybe_present( + Criticality::NonCritical, + Some(common::authority_information_access), + ), // 5280 4.2.1.1.: Authority Key Identifier - ExtensionPolicy::present( - AUTHORITY_KEY_IDENTIFIER_OID, + authority_key_identifier: ExtensionValidator::present( Criticality::NonCritical, None, ), + subject_key_identifier: ExtensionValidator::maybe_present( + Criticality::Agnostic, + None, + ), // 5280 4.2.1.3: Key Usage - ExtensionPolicy::maybe_present( - KEY_USAGE_OID, + key_usage: ExtensionValidator::maybe_present( Criticality::Agnostic, Some(ee::key_usage), ), // CA/B 7.1.2.7.12 Subscriber Certificate Subject Alternative Name - ExtensionPolicy::present( - SUBJECT_ALTERNATIVE_NAME_OID, + subject_alternative_name: ExtensionValidator::present( Criticality::Agnostic, Some(ee::subject_alternative_name), ), // 5280 4.2.1.9: Basic Constraints - ExtensionPolicy::maybe_present( - BASIC_CONSTRAINTS_OID, + basic_constraints: ExtensionValidator::maybe_present( Criticality::Agnostic, Some(ee::basic_constraints), ), // 5280 4.2.1.10: Name Constraints - ExtensionPolicy::not_present(NAME_CONSTRAINTS_OID), + name_constraints: ExtensionValidator::not_present(), // CA/B: 7.1.2.7.10: Subscriber Certificate Extended Key Usage // NOTE: CABF requires EKUs in EE certs, while RFC 5280 does not. - ExtensionPolicy::maybe_present( - EXTENDED_KEY_USAGE_OID, + extended_key_usage: ExtensionValidator::maybe_present( Criticality::NonCritical, Some(ee::extended_key_usage), ), - ]), + }, } } - fn permits_basic( - &self, - cert: &Certificate<'_>, - extensions: &Extensions<'_>, - ) -> Result<(), ValidationError> { + fn permits_basic(&self, cert: &Certificate<'_>) -> Result<(), ValidationError> { // CA/B 7.1.1: // Certificates MUST be of type X.509 v3. if cert.tbs_cert.version != 2 { @@ -404,36 +388,6 @@ impl<'a, B: CryptoOps> Policy<'a, B> { )); } - // Extension policy checks. - for ext_policy in self.common_extension_policies.iter() { - ext_policy.permits(self, cert, extensions)?; - } - - // Check that all critical extensions in this certificate are accounted for. - let critical_extensions = extensions - .iter() - .filter(|e| e.critical) - .map(|e| e.extn_id) - .collect::>(); - let checked_extensions = self - .common_extension_policies - .iter() - .chain(self.ca_extension_policies.iter()) - .chain(self.ee_extension_policies.iter()) - .map(|p| p.oid.clone()) - .collect::>(); - - if critical_extensions - .difference(&checked_extensions) - .next() - .is_some() - { - // TODO: Render the OIDs here. - return Err(ValidationError::Other( - "certificate contains unaccounted-for critical extensions".to_string(), - )); - } - Ok(()) } @@ -444,7 +398,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> { current_depth: u8, extensions: &Extensions<'_>, ) -> Result<(), ValidationError> { - self.permits_basic(cert, extensions)?; + self.permits_basic(cert)?; // 5280 4.1.2.6: Subject // CA certificates MUST have a subject populated with a non-empty distinguished name. @@ -472,9 +426,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> { } } - for ext_policy in self.ca_extension_policies.iter() { - ext_policy.permits(self, cert, extensions)?; - } + self.ca_extension_policy.permits(self, cert, extensions)?; Ok(()) } @@ -485,11 +437,9 @@ impl<'a, B: CryptoOps> Policy<'a, B> { cert: &Certificate<'_>, extensions: &Extensions<'_>, ) -> Result<(), ValidationError> { - self.permits_basic(cert, extensions)?; + self.permits_basic(cert)?; - for ext_policy in self.ee_extension_policies.iter() { - ext_policy.permits(self, cert, extensions)?; - } + self.ee_extension_policy.permits(self, cert, extensions)?; Ok(()) } diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py index 54aafe33c061..194b64f1f0bd 100644 --- a/tests/x509/verification/test_limbo.py +++ b/tests/x509/verification/test_limbo.py @@ -34,6 +34,8 @@ # incompatible ways. Our validator always tries (by default) to comply # closer to CABF, so we skip these. "rfc5280-incompatible-with-webpki", + # We do not support policy constraints. + "has-policy-constraints", } LIMBO_SKIP_TESTCASES = {