From 0174c8af4432e5d83fb8db8eb57aa78b1e5118d1 Mon Sep 17 00:00:00 2001 From: Adam Michalik Date: Tue, 28 Apr 2026 19:48:19 -0700 Subject: [PATCH] Allow selected unsupported critical extensions Add ExtensionId and an opt-in EndEntityCert constructor that accepts a DER OID allowlist for unsupported critical extensions on the leaf certificate. Existing TryFrom parsing remains strict by default. Add PathBuilder::with_ignored_critical_extensions so callers can explicitly apply the same allowlist when path building parses intermediate certificates. PathBuilder remains strict for intermediates unless this builder option is used. Supported extensions are still parsed and validated normally; the allowlist only suppresses UnsupportedCriticalExtension for exact unsupported OID matches. Add integration tests for leaf and intermediate allowlisting, exact-match behavior, and supported-extension parsing. --- src/cert.rs | 11 ++- src/end_entity.rs | 30 ++++++++ src/lib.rs | 1 + src/verify_cert.rs | 25 +++++- src/x509.rs | 76 +++++++++++++++++-- tests/integration.rs | 177 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 312 insertions(+), 8 deletions(-) diff --git a/src/cert.rs b/src/cert.rs index f3a505f0..4bfc64f7 100644 --- a/src/cert.rs +++ b/src/cert.rs @@ -59,9 +59,16 @@ impl<'a> Cert<'a> { Self::from_input(cert_der, UnknownExtensionPolicy::default()) } + pub(crate) fn from_der_with_extension_policy( + cert_der: untrusted::Input<'a>, + ext_policy: UnknownExtensionPolicy<'_>, + ) -> Result { + Self::from_input(cert_der, ext_policy) + } + fn from_input( cert_der: untrusted::Input<'a>, - ext_policy: UnknownExtensionPolicy, + ext_policy: UnknownExtensionPolicy<'_>, ) -> Result { let (tbs, signed_data) = cert_der.read_all(Error::TrailingData(DerTypeId::Certificate), |cert_der| { @@ -308,7 +315,7 @@ pub(crate) fn lenient_certificate_serial_number<'a>( fn remember_cert_extension<'a>( cert: &mut Cert<'a>, extension: &Extension<'a>, - ext_policy: UnknownExtensionPolicy, + ext_policy: UnknownExtensionPolicy<'_>, ) -> Result<(), Error> { // We don't do anything with certificate policies so we can safely ignore // all policy-related stuff. We assume that the policy-related extensions diff --git a/src/end_entity.rs b/src/end_entity.rs index 787f71ff..e1e02169 100644 --- a/src/end_entity.rs +++ b/src/end_entity.rs @@ -18,6 +18,7 @@ use pki_types::{CertificateDer, ServerName, SignatureVerificationAlgorithm}; use crate::error::Error; use crate::subject_name::{verify_dns_names, verify_ip_address_names}; +use crate::x509::{ExtensionId, UnknownExtensionPolicy}; use crate::{DerTypeId, cert, der, sct, signed_data}; /// An end-entity certificate. @@ -68,6 +69,35 @@ impl<'a> TryFrom<&'a CertificateDer<'a>> for EndEntityCert<'a> { } } +impl<'a> EndEntityCert<'a> { + /// Parse the ASN.1 DER-encoded X.509 encoding of the certificate, ignoring the + /// listed unsupported critical extensions. + /// + /// By default, webpki rejects certificates containing unsupported critical extensions, + /// as required by RFC 5280. This constructor is an opt-in escape hatch for applications + /// that understand the listed unsupported extensions and want webpki to accept them when + /// they are marked critical. + /// The `ignored_critical_extensions` values are DER OBJECT IDENTIFIER value bytes, without + /// the OBJECT IDENTIFIER tag or length. + /// + /// Supported extensions are still processed normally. Listing a supported extension here does + /// not disable validation of its value. This constructor only applies the policy when parsing + /// this end-entity certificate. Use + /// [`PathBuilder::with_ignored_critical_extensions`](crate::PathBuilder::with_ignored_critical_extensions) + /// to apply the same policy when parsing intermediate certificates during path building. + pub fn try_from_with_ignored_critical_extensions( + cert: &'a CertificateDer<'a>, + ignored_critical_extensions: &[ExtensionId<'_>], + ) -> Result { + Ok(Self { + inner: cert::Cert::from_der_with_extension_policy( + untrusted::Input::from(cert.as_ref()), + UnknownExtensionPolicy::AllowUnsupportedCritical(ignored_critical_extensions), + )?, + }) + } +} + impl EndEntityCert<'_> { /// Verifies that the certificate is valid for the given Subject Name. pub fn verify_is_valid_for_subject_name( diff --git a/src/lib.rs b/src/lib.rs index 3501218a..53cbd6fa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -88,6 +88,7 @@ pub use verify_cert::{ ExtendedKeyUsage, ExtendedKeyUsageValidator, IntermediateIterator, KeyPurposeId, KeyPurposeIdIter, PathBuilder, RequiredEkuNotFoundContext, VerifiedPath, }; +pub use x509::ExtensionId; fn public_values_eq(a: untrusted::Input<'_>, b: untrusted::Input<'_>) -> bool { a.as_slice_less_safe() == b.as_slice_less_safe() diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 6c18ba70..88107326 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -28,6 +28,7 @@ use crate::end_entity::EndEntityCert; use crate::error::Error; #[cfg(feature = "alloc")] use crate::spki_for_anchor; +use crate::x509::{ExtensionId, UnknownExtensionPolicy}; use crate::{DerTypeId, public_values_eq, subject_name}; /// Build a [`VerifiedPath`] for an end-entity certificate from the given trust anchors. @@ -41,6 +42,7 @@ pub struct PathBuilder<'a, 'p> { pub(crate) revocation: Option>, #[expect(clippy::type_complexity)] pub(crate) verify_path: Option<&'a dyn Fn(&VerifiedPath<'_>) -> Result<(), Error>>, + pub(crate) extension_policy: UnknownExtensionPolicy<'a>, } impl<'a, 'p: 'a> PathBuilder<'a, 'p> { @@ -72,9 +74,27 @@ impl<'a, 'p: 'a> PathBuilder<'a, 'p> { intermediate_certs, revocation, verify_path: None, + extension_policy: UnknownExtensionPolicy::default(), } } + /// Ignore the listed unsupported critical extensions while parsing intermediate + /// certificates for path building. + /// + /// By default, path building rejects intermediate certificates containing unsupported + /// critical extensions. This is an opt-in escape hatch for applications that understand + /// the listed unsupported extensions and want webpki to accept them when they are marked + /// critical. The `ignored_critical_extensions` values identify DER OBJECT IDENTIFIER value + /// bytes through [`ExtensionId`]. Supported extensions are still processed normally. + pub fn with_ignored_critical_extensions( + mut self, + ignored_critical_extensions: &'a [ExtensionId<'a>], + ) -> Self { + self.extension_policy = + UnknownExtensionPolicy::AllowUnsupportedCritical(ignored_critical_extensions); + self + } + /// Set a path verification function to use for path building. /// /// `verify()` will only be called for potentially verified paths, that is, paths that @@ -161,7 +181,10 @@ impl<'a, 'p: 'a> PathBuilder<'a, 'p> { }; loop_while_non_fatal_error(err, self.intermediate_certs, |cert_der| { - let potential_issuer = Cert::from_der(untrusted::Input::from(cert_der))?; + let potential_issuer = Cert::from_der_with_extension_policy( + untrusted::Input::from(cert_der), + self.extension_policy, + )?; if !public_values_eq(potential_issuer.subject, path.head().issuer) { return Err(Error::UnknownIssuer.into()); } diff --git a/src/x509.rs b/src/x509.rs index 9fb07c08..c77a647c 100644 --- a/src/x509.rs +++ b/src/x509.rs @@ -12,9 +12,49 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +use core::fmt; + use crate::der::{self, CONSTRUCTED, CONTEXT_SPECIFIC, DerIterator, FromDer}; use crate::error::{DerTypeId, Error}; +use crate::public_values_eq; use crate::subject_name::GeneralName; +use crate::verify_cert::OidDecoder; + +/// DER OBJECT IDENTIFIER value bytes identifying an X.509 extension. +#[derive(Clone, Copy)] +pub struct ExtensionId<'a> { + oid_value: untrusted::Input<'a>, +} + +impl<'a> ExtensionId<'a> { + /// Construct a new [`ExtensionId`]. + /// + /// `oid` is the DER OBJECT IDENTIFIER value bytes, without the OBJECT IDENTIFIER tag or + /// length. + pub const fn new(oid: &'a [u8]) -> Self { + Self { + oid_value: untrusted::Input::from(oid), + } + } + + fn matches(&self, oid: untrusted::Input<'_>) -> bool { + public_values_eq(self.oid_value, oid) + } +} + +impl fmt::Debug for ExtensionId<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "ExtensionId(")?; + let decoder = OidDecoder::new(self.oid_value.as_slice_less_safe()); + for (i, part) in decoder.enumerate() { + if i > 0 { + write!(f, ".")?; + } + write!(f, "{part}")?; + } + write!(f, ")") + } +} pub(crate) struct Extension<'a> { pub(crate) critical: bool, @@ -23,9 +63,16 @@ pub(crate) struct Extension<'a> { } impl Extension<'_> { - pub(crate) fn unsupported(&self, policy: UnknownExtensionPolicy) -> Result<(), Error> { - match (policy, self.critical) { - (UnknownExtensionPolicy::Strict, true) => Err(Error::UnsupportedCriticalExtension), + pub(crate) fn unsupported(&self, policy: UnknownExtensionPolicy<'_>) -> Result<(), Error> { + match policy { + UnknownExtensionPolicy::Strict if self.critical => { + Err(Error::UnsupportedCriticalExtension) + } + UnknownExtensionPolicy::AllowUnsupportedCritical(ids) + if self.critical && !ids.iter().any(|id| id.matches(self.id)) => + { + Err(Error::UnsupportedCriticalExtension) + } _ => Ok(()), } } @@ -63,7 +110,7 @@ pub(crate) fn set_extension_once( pub(crate) fn remember_extension( extension: &Extension<'_>, - ext_policy: UnknownExtensionPolicy, + ext_policy: UnknownExtensionPolicy<'_>, mut handler: impl FnMut(ExtensionOid) -> Result<(), Error>, ) -> Result<(), Error> { match ExtensionOid::lookup(extension.id) { @@ -73,10 +120,11 @@ pub(crate) fn remember_extension( } #[derive(Clone, Copy, Debug, Default)] -pub(crate) enum UnknownExtensionPolicy { +pub(crate) enum UnknownExtensionPolicy<'a> { #[default] Strict, IgnoreCritical, + AllowUnsupportedCritical(&'a [ExtensionId<'a>]), } /// A certificate revocation list (CRL) distribution point name, describing a source of @@ -151,3 +199,21 @@ const SCT_LIST_OID: [u8; 10] = [40 + 3, 6, 1, 4, 1, 214, 121, 2, 4, 2]; /// /// const ID_CE: [u8; 2] = oid!(2, 5, 29); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unknown_extension_policy_debug() { + let ignored = [ExtensionId::new(&[43, 6, 1, 4, 1, 42, 1])]; + + assert_eq!( + format!( + "{:?}", + UnknownExtensionPolicy::AllowUnsupportedCritical(&ignored) + ), + "AllowUnsupportedCritical([ExtensionId(1.3.6.1.4.1.42.1)])" + ); + } +} diff --git a/tests/integration.rs b/tests/integration.rs index 1fadb90d..b1b09198 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -17,10 +17,30 @@ use core::slice; use core::time::Duration; use pki_types::{CertificateDer, UnixTime}; +#[cfg(feature = "alloc")] +use rcgen::{ + BasicConstraints, CertificateParams, CertifiedIssuer, CustomExtension, DnType, IsCa, KeyPair, + KeyUsagePurpose, +}; use rustls_aws_lc_rs::ALL_VERIFICATION_ALGS; use webpki::sct::LogIdAndTimestamp; use webpki::{ExtendedKeyUsage, PathBuilder, anchor_from_trusted_cert}; +#[cfg(feature = "alloc")] +mod critical_extension_test_oids { + pub(super) const PRIVATE_CRITICAL_EXTENSION_OID_COMPONENTS: &[u64] = &[1, 3, 6, 1, 4, 1, 42, 1]; + pub(super) const PRIVATE_CRITICAL_EXTENSION_OID: &[u8] = &[43, 6, 1, 4, 1, 42, 1]; + pub(super) const OTHER_PRIVATE_CRITICAL_EXTENSION_OID: &[u8] = &[43, 6, 1, 4, 1, 42, 2]; + pub(super) const NAME_CONSTRAINTS_OID_COMPONENTS: &[u64] = &[2, 5, 29, 30]; + pub(super) const NAME_CONSTRAINTS_OID: &[u8] = &[85, 29, 30]; +} + +#[cfg(feature = "alloc")] +use critical_extension_test_oids::{ + NAME_CONSTRAINTS_OID, NAME_CONSTRAINTS_OID_COMPONENTS, OTHER_PRIVATE_CRITICAL_EXTENSION_OID, + PRIVATE_CRITICAL_EXTENSION_OID, PRIVATE_CRITICAL_EXTENSION_OID_COMPONENTS, +}; + /* Checks we can verify netflix's cert chain. This is notable * because they're rooted at a Verisign v1 root. */ #[cfg(feature = "alloc")] @@ -203,6 +223,100 @@ fn critical_extensions() { ); } +#[test] +#[cfg(feature = "alloc")] +fn ignored_critical_extension_on_end_entity() { + let root = make_self_signed_issuer("Root", Vec::new()); + let root_der = root.der().clone(); + let ee = make_end_entity(&root, vec![private_critical_extension()]); + + assert!(matches!( + webpki::EndEntityCert::try_from(&ee), + Err(webpki::Error::UnsupportedCriticalExtension) + )); + + let ignored = [webpki::ExtensionId::new(PRIVATE_CRITICAL_EXTENSION_OID)]; + let cert = + webpki::EndEntityCert::try_from_with_ignored_critical_extensions(&ee, &ignored).unwrap(); + let anchors = [anchor_from_trusted_cert(&root_der).unwrap()]; + let time = UnixTime::since_unix_epoch(Duration::from_secs(0x1fed_f00d)); + let builder = PathBuilder::new( + &[], + None, + &ExtendedKeyUsage::SERVER_AUTH, + ALL_VERIFICATION_ALGS, + &anchors, + ); + + assert!(builder.build(&cert, time).is_ok()); +} + +#[test] +#[cfg(feature = "alloc")] +fn ignored_critical_extension_on_intermediate() { + let root = make_self_signed_issuer("Root", Vec::new()); + let root_der = root.der().clone(); + let intermediate = make_intermediate(&root, vec![private_critical_extension()]); + let intermediate_der = intermediate.der().clone(); + let ee = make_end_entity(&intermediate, Vec::new()); + let anchors = [anchor_from_trusted_cert(&root_der).unwrap()]; + let time = UnixTime::since_unix_epoch(Duration::from_secs(0x1fed_f00d)); + + let cert = webpki::EndEntityCert::try_from(&ee).unwrap(); + let builder = PathBuilder::new( + slice::from_ref(&intermediate_der), + None, + &ExtendedKeyUsage::SERVER_AUTH, + ALL_VERIFICATION_ALGS, + &anchors, + ); + + assert!(matches!( + builder.build(&cert, time), + Err(webpki::Error::UnsupportedCriticalExtension) + )); + + let ignored = [webpki::ExtensionId::new(PRIVATE_CRITICAL_EXTENSION_OID)]; + let builder = PathBuilder::new( + slice::from_ref(&intermediate_der), + None, + &ExtendedKeyUsage::SERVER_AUTH, + ALL_VERIFICATION_ALGS, + &anchors, + ) + .with_ignored_critical_extensions(&ignored); + + assert!(builder.build(&cert, time).is_ok()); +} + +#[test] +#[cfg(feature = "alloc")] +fn ignored_critical_extensions_match_exact_oid() { + let root = make_self_signed_issuer("Root", Vec::new()); + let ee = make_end_entity(&root, vec![private_critical_extension()]); + let ignored = [webpki::ExtensionId::new( + OTHER_PRIVATE_CRITICAL_EXTENSION_OID, + )]; + + assert!(matches!( + webpki::EndEntityCert::try_from_with_ignored_critical_extensions(&ee, &ignored), + Err(webpki::Error::UnsupportedCriticalExtension) + )); +} + +#[test] +#[cfg(feature = "alloc")] +fn ignored_critical_extensions_do_not_disable_supported_extension_parsing() { + let root = make_self_signed_issuer("Root", Vec::new()); + let ee = make_end_entity(&root, vec![invalid_name_constraints_extension()]); + let ignored = [webpki::ExtensionId::new(NAME_CONSTRAINTS_OID)]; + + assert!(matches!( + webpki::EndEntityCert::try_from_with_ignored_critical_extensions(&ee, &ignored), + Err(webpki::Error::BadDer) + )); +} + #[test] fn read_root_with_zero_serial() { let ca = CertificateDer::from(&include_bytes!("misc/serial_zero.der")[..]); @@ -381,6 +495,69 @@ fn expect_cert_uri_names<'name>( assert!(cert.valid_uri_names().eq(expected_uris)) } +#[cfg(feature = "alloc")] +fn make_self_signed_issuer( + org_name: &'static str, + custom_extensions: Vec, +) -> CertifiedIssuer<'static, KeyPair> { + let params = issuer_params(org_name, custom_extensions); + let key = KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).unwrap(); + CertifiedIssuer::self_signed(params, key).unwrap() +} + +#[cfg(feature = "alloc")] +fn make_intermediate( + issuer: &CertifiedIssuer<'_, KeyPair>, + custom_extensions: Vec, +) -> CertifiedIssuer<'static, KeyPair> { + let params = issuer_params("Intermediate", custom_extensions); + let key = KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).unwrap(); + CertifiedIssuer::signed_by(params, key, issuer).unwrap() +} + +#[cfg(feature = "alloc")] +fn issuer_params( + org_name: &'static str, + custom_extensions: Vec, +) -> CertificateParams { + let mut params = CertificateParams::new(Vec::new()).unwrap(); + params + .distinguished_name + .push(DnType::OrganizationName, org_name); + params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.key_usages = vec![KeyUsagePurpose::KeyCertSign]; + params.custom_extensions = custom_extensions; + params +} + +#[cfg(feature = "alloc")] +fn make_end_entity( + issuer: &CertifiedIssuer<'_, KeyPair>, + custom_extensions: Vec, +) -> CertificateDer<'static> { + let mut params = CertificateParams::new(vec!["example.com".into()]).unwrap(); + params.is_ca = IsCa::ExplicitNoCa; + params.custom_extensions = custom_extensions; + + let key = KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).unwrap(); + params.signed_by(&key, issuer).unwrap().der().clone() +} + +#[cfg(feature = "alloc")] +fn private_critical_extension() -> CustomExtension { + let mut extension = + CustomExtension::from_oid_content(PRIVATE_CRITICAL_EXTENSION_OID_COMPONENTS, vec![5, 0]); + extension.set_criticality(true); + extension +} + +#[cfg(feature = "alloc")] +fn invalid_name_constraints_extension() -> CustomExtension { + let mut extension = CustomExtension::from_oid_content(NAME_CONSTRAINTS_OID_COMPONENTS, vec![]); + extension.set_criticality(true); + extension +} + #[cfg(feature = "alloc")] #[test] fn cert_time_validity() {