Support arbitrary extension types in ExtensionPolicy

CHANGELOG.rst

@@ -26,6 +26,12 @@ Changelog
   :class:`~cryptography.hazmat.primitives.hpke.KEM` so callers can split the
   encapsulated key from the ciphertext returned by
   :meth:`~cryptography.hazmat.primitives.hpke.Suite.encrypt`.
+* :meth:`~cryptography.x509.verification.ExtensionPolicy.require_present`,
+  :meth:`~cryptography.x509.verification.ExtensionPolicy.may_be_present`, and
+  :meth:`~cryptography.x509.verification.ExtensionPolicy.require_not_present`
+  now accept any extension type. Previously only a fixed set of extension
+  types was supported, which made it impossible to account for otherwise
+  unrecognized critical extensions during path validation.
 * Added support for using :class:`~cryptography.x509.Certificate`,
   :class:`~cryptography.x509.CertificateSigningRequest`, and
   :class:`~cryptography.x509.CertificateRevocationList` as field types in

docs/x509/verification.rst

@@ -315,15 +315,12 @@ the root of trust:
     .. note:: Calling any of the builder methods (:meth:`require_not_present`, :meth:`may_be_present`, or :meth:`require_present`)
         multiple times with the same extension type will raise an exception.
 
-    .. note:: Currently only the following extension types are supported in the ExtensionPolicy API:
-        :class:`~cryptography.x509.AuthorityInformationAccess`,
-        :class:`~cryptography.x509.AuthorityKeyIdentifier`,
-        :class:`~cryptography.x509.SubjectKeyIdentifier`,
-        :class:`~cryptography.x509.KeyUsage`,
-        :class:`~cryptography.x509.SubjectAlternativeName`,
-        :class:`~cryptography.x509.BasicConstraints`,
-        :class:`~cryptography.x509.NameConstraints`,
-        :class:`~cryptography.x509.ExtendedKeyUsage`.
+    .. versionchanged:: 49.0.0
+        Any extension type may now be used with the builder methods.
+
+    .. note:: If a present extension's value cannot be parsed into a known
+        Python object, it is passed to the validator callback as an
+        :class:`~cryptography.x509.UnrecognizedExtension`.
 
     .. staticmethod:: permit_all()
 

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

@@ -2,6 +2,7 @@
 // 2.0, and the BSD License. See the LICENSE file in the root of this repository
 // for complete details.
 
+use std::collections::HashSet;
 use std::sync::Arc;
 
 use cryptography_x509::extensions::{Extension, Extensions};
@@ -15,7 +16,6 @@ use crate::ops::{CryptoOps, VerificationCertificate};
 use crate::policy::Policy;
 use crate::{ValidationError, ValidationErrorKind, ValidationResult};
 
-#[derive(Clone)]
 pub struct ExtensionPolicy<'cb, B: CryptoOps> {
     pub authority_information_access: ExtensionValidator<'cb, B>,
     pub authority_key_identifier: ExtensionValidator<'cb, B>,
@@ -25,6 +25,33 @@ pub struct ExtensionPolicy<'cb, B: CryptoOps> {
     pub basic_constraints: ExtensionValidator<'cb, B>,
     pub name_constraints: ExtensionValidator<'cb, B>,
     pub extended_key_usage: ExtensionValidator<'cb, B>,
+    // Validators for any other extensions, i.e. those that don't have a
+    // dedicated field above. There is at most one validator per OID.
+    additional_extensions: Vec<ExtensionValidator<'cb, B>>,
+    // OIDs that have been explicitly configured via `with_validator`. Used to
+    // reject reconfiguring the same extension more than once. This is empty for
+    // the policies returned by the constructors, so their built-in validators
+    // can still be overridden once.
+    configured_oids: HashSet<asn1::ObjectIdentifier>,
+}
+
+// NOTE: Derived `Clone` would impose an unnecessary `B: Clone` bound, so we
+// implement it manually (the contents are `Clone` regardless of `B`).
+impl<B: CryptoOps> Clone for ExtensionPolicy<'_, B> {
+    fn clone(&self) -> Self {
+        ExtensionPolicy {
+            authority_information_access: self.authority_information_access.clone(),
+            authority_key_identifier: self.authority_key_identifier.clone(),
+            subject_key_identifier: self.subject_key_identifier.clone(),
+            key_usage: self.key_usage.clone(),
+            subject_alternative_name: self.subject_alternative_name.clone(),
+            basic_constraints: self.basic_constraints.clone(),
+            name_constraints: self.name_constraints.clone(),
+            extended_key_usage: self.extended_key_usage.clone(),
+            additional_extensions: self.additional_extensions.clone(),
+            configured_oids: self.configured_oids.clone(),
+        }
+    }
 }
 
 impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
@@ -50,6 +77,8 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
             basic_constraints: make_permissive_validator(BASIC_CONSTRAINTS_OID),
             name_constraints: make_permissive_validator(NAME_CONSTRAINTS_OID),
             extended_key_usage: make_permissive_validator(EXTENDED_KEY_USAGE_OID),
+            additional_extensions: Vec::new(),
+            configured_oids: HashSet::new(),
         }
     }
 
@@ -112,6 +141,8 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
                 Criticality::NonCritical,
                 Some(Arc::new(ca::extended_key_usage)),
             ),
+            additional_extensions: Vec::new(),
+            configured_oids: HashSet::new(),
         }
     }
 
@@ -168,9 +199,46 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
                 Criticality::NonCritical,
                 Some(Arc::new(ee::extended_key_usage)),
             ),
+            additional_extensions: Vec::new(),
+            configured_oids: HashSet::new(),
         }
     }
 
+    /// Returns a new `ExtensionPolicy` with the given validator assigned to
+    /// the extension identified by the validator's OID.
+    ///
+    /// Extensions with a dedicated field are assigned to that field,
+    /// overwriting the built-in validator (e.g. the one from a default
+    /// policy); all others are stored in `additional_extensions`.
+    ///
+    /// Returns `Err` with the offending OID if this policy has already been
+    /// explicitly configured for that OID via a previous call, so the same
+    /// extension cannot be configured more than once.
+    pub fn with_validator(
+        &self,
+        validator: ExtensionValidator<'cb, B>,
+    ) -> Result<Self, asn1::ObjectIdentifier> {
+        let oid = validator.oid().clone();
+        if self.configured_oids.contains(&oid) {
+            return Err(oid);
+        }
+
+        let mut policy = self.clone();
+        match oid {
+            AUTHORITY_INFORMATION_ACCESS_OID => policy.authority_information_access = validator,
+            AUTHORITY_KEY_IDENTIFIER_OID => policy.authority_key_identifier = validator,
+            SUBJECT_KEY_IDENTIFIER_OID => policy.subject_key_identifier = validator,
+            KEY_USAGE_OID => policy.key_usage = validator,
+            SUBJECT_ALTERNATIVE_NAME_OID => policy.subject_alternative_name = validator,
+            BASIC_CONSTRAINTS_OID => policy.basic_constraints = validator,
+            NAME_CONSTRAINTS_OID => policy.name_constraints = validator,
+            EXTENDED_KEY_USAGE_OID => policy.extended_key_usage = validator,
+            _ => policy.additional_extensions.push(validator),
+        }
+        policy.configured_oids.insert(oid);
+        Ok(policy)
+    }
+
     pub(crate) fn permits<'chain>(
         &self,
         policy: &Policy<'_, B>,
@@ -185,6 +253,7 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
         let mut basic_constraints_seen = false;
         let mut name_constraints_seen = false;
         let mut extended_key_usage_seen = false;
+        let mut additional_extensions_seen = HashSet::new();
 
         // Iterate over each extension and run its policy.
         for ext in extensions.iter() {
@@ -225,13 +294,21 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
                     extended_key_usage_seen = true;
                     self.extended_key_usage.permits(policy, cert, Some(&ext))?;
                 }
-                _ if ext.critical => {
-                    return Err(ValidationError::new(ValidationErrorKind::ExtensionError {
-                        oid: ext.extn_id,
-                        reason: "certificate contains unaccounted-for critical extensions",
-                    }));
+                _ => {
+                    if let Some(validator) = self
+                        .additional_extensions
+                        .iter()
+                        .find(|validator| validator.oid() == &ext.extn_id)
+                    {
+                        additional_extensions_seen.insert(ext.extn_id.clone());
+                        validator.permits(policy, cert, Some(&ext))?;
+                    } else if ext.critical {
+                        return Err(ValidationError::new(ValidationErrorKind::ExtensionError {
+                            oid: ext.extn_id,
+                            reason: "certificate contains unaccounted-for critical extensions",
+                        }));
+                    }
                 }
-                _ => {}
             }
         }
 
@@ -262,6 +339,11 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
         if !extended_key_usage_seen {
             self.extended_key_usage.permits(policy, cert, None)?;
         }
+        for validator in &self.additional_extensions {
+            if !additional_extensions_seen.contains(validator.oid()) {
+                validator.permits(policy, cert, None)?;
+            }
+        }
 
         Ok(())
     }
@@ -313,7 +395,6 @@ pub type MaybeExtensionValidatorCallback<'cb, B> = Arc<
 >;
 
 /// Represents different validation states for an extension.
-#[derive(Clone)]
 pub enum ExtensionValidator<'cb, B: CryptoOps> {
     /// The extension MUST NOT be present.
     NotPresent { oid: asn1::ObjectIdentifier },
@@ -335,7 +416,45 @@ pub enum ExtensionValidator<'cb, B: CryptoOps> {
     },
 }
 
+// NOTE: Derived `Clone` would impose an unnecessary `B: Clone` bound, so we
+// implement it manually (the contents are `Clone` regardless of `B`).
+impl<B: CryptoOps> Clone for ExtensionValidator<'_, B> {
+    fn clone(&self) -> Self {
+        match self {
+            ExtensionValidator::NotPresent { oid } => {
+                ExtensionValidator::NotPresent { oid: oid.clone() }
+            }
+            ExtensionValidator::Present {
+                oid,
+                criticality,
+                validator,
+            } => ExtensionValidator::Present {
+                oid: oid.clone(),
+                criticality: criticality.clone(),
+                validator: validator.clone(),
+            },
+            ExtensionValidator::MaybePresent {
+                oid,
+                criticality,
+                validator,
+            } => ExtensionValidator::MaybePresent {
+                oid: oid.clone(),
+                criticality: criticality.clone(),
+                validator: validator.clone(),
+            },
+        }
+    }
+}
+
 impl<'cb, B: CryptoOps> ExtensionValidator<'cb, B> {
+    pub fn oid(&self) -> &asn1::ObjectIdentifier {
+        match self {
+            ExtensionValidator::NotPresent { oid } => oid,
+            ExtensionValidator::Present { oid, .. } => oid,
+            ExtensionValidator::MaybePresent { oid, .. } => oid,
+        }
+    }
+
     pub(crate) fn not_present(oid: asn1::ObjectIdentifier) -> Self {
         Self::NotPresent { oid }
     }

src/rust/src/x509/verify/extension_policy.rs

@@ -1,12 +1,6 @@
-use std::collections::HashSet;
 use std::sync::Arc;
 
 use cryptography_x509::extensions::Extension;
-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_verification::ops::VerificationCertificate;
 use cryptography_x509_verification::policy::{
     Criticality, ExtensionPolicy, ExtensionValidator, MaybeExtensionValidatorCallback, Policy,
@@ -17,7 +11,8 @@ use pyo3::types::{PyAnyMethods, PyTypeMethods};
 use pyo3::{intern, PyResult};
 
 use super::PyCryptoOps;
-use crate::asn1::py_oid_to_oid;
+use crate::asn1::{oid_to_py_oid, py_oid_to_oid};
+use crate::error::CryptographyError;
 use crate::types;
 use crate::x509::certificate::parse_cert_ext;
 
@@ -55,7 +50,6 @@ impl From<PyCriticality> for Criticality {
 )]
 pub(crate) struct PyExtensionPolicy {
     inner_policy: ExtensionPolicy<'static, PyCryptoOps>,
-    already_set_oids: HashSet<asn1::ObjectIdentifier>,
 }
 
 impl PyExtensionPolicy {
@@ -64,51 +58,19 @@ impl PyExtensionPolicy {
     }
 
     fn new(inner_policy: ExtensionPolicy<'static, PyCryptoOps>) -> Self {
-        PyExtensionPolicy {
-            inner_policy,
-            already_set_oids: HashSet::new(),
-        }
+        PyExtensionPolicy { inner_policy }
     }
 
     fn with_assigned_validator(
         &self,
         validator: ExtensionValidator<'static, PyCryptoOps>,
     ) -> PyResult<PyExtensionPolicy> {
-        let oid = match &validator {
-            ExtensionValidator::NotPresent { oid } => oid,
-            ExtensionValidator::MaybePresent { oid, .. } => oid,
-            ExtensionValidator::Present { oid, .. } => oid,
-        }
-        .clone();
-        if self.already_set_oids.contains(&oid) {
-            return Err(pyo3::exceptions::PyValueError::new_err(format!(
+        let inner_policy = self.inner_policy.with_validator(validator).map_err(|oid| {
+            pyo3::exceptions::PyValueError::new_err(format!(
                 "ExtensionPolicy already configured for extension with OID {oid}"
-            )));
-        }
-
-        let mut policy = self.inner_policy.clone();
-        match oid {
-            AUTHORITY_INFORMATION_ACCESS_OID => policy.authority_information_access = validator,
-            AUTHORITY_KEY_IDENTIFIER_OID => policy.authority_key_identifier = validator,
-            SUBJECT_KEY_IDENTIFIER_OID => policy.subject_key_identifier = validator,
-            KEY_USAGE_OID => policy.key_usage = validator,
-            SUBJECT_ALTERNATIVE_NAME_OID => policy.subject_alternative_name = validator,
-            BASIC_CONSTRAINTS_OID => policy.basic_constraints = validator,
-            NAME_CONSTRAINTS_OID => policy.name_constraints = validator,
-            EXTENDED_KEY_USAGE_OID => policy.extended_key_usage = validator,
-            _ => {
-                return Err(pyo3::exceptions::PyValueError::new_err(format!(
-                    "Unsupported extension OID: {oid}",
-                )))
-            }
-        }
-
-        let mut already_set_oids = self.already_set_oids.clone();
-        already_set_oids.insert(oid);
-        Ok(PyExtensionPolicy {
-            inner_policy: policy,
-            already_set_oids,
-        })
+            ))
+        })?;
+        Ok(PyExtensionPolicy { inner_policy })
     }
 }
 
@@ -232,13 +194,29 @@ fn make_py_extension<'chain, 'p>(
     py: pyo3::Python<'p>,
     ext: Option<&Extension<'p>>,
 ) -> ValidationResult<'chain, Option<pyo3::Bound<'p, pyo3::types::PyAny>>, PyCryptoOps> {
+    let conversion_error = |e: CryptographyError| {
+        ValidationError::new(ValidationErrorKind::Other(format!(
+            "{e} (while converting Extension to Python object)"
+        )))
+    };
+
     Ok(match ext {
         None => None,
-        Some(ext) => parse_cert_ext(py, ext).map_err(|e| {
-            ValidationError::new(ValidationErrorKind::Other(format!(
-                "{e} (while converting Extension to Python object)"
-            )))
-        })?,
+        Some(ext) => match parse_cert_ext(py, ext).map_err(conversion_error)? {
+            Some(parsed) => Some(parsed),
+            // The extension is present but its value can't be parsed into a
+            // known Python object. Mirror `Certificate.extensions` and surface
+            // it as an `UnrecognizedExtension` rather than dropping it to None.
+            None => {
+                let oid =
+                    oid_to_py_oid(py, &ext.extn_id).map_err(|e| conversion_error(e.into()))?;
+                let unrecognized = types::UNRECOGNIZED_EXTENSION
+                    .get(py)
+                    .and_then(|ty| ty.call1((oid, ext.extn_value)))
+                    .map_err(|e| conversion_error(e.into()))?;
+                Some(unrecognized)
+            }
+        },
     })
 }