diff --git a/src/cert.rs b/src/cert.rs index 89f56545..ac7646e3 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 ff4b702b..0c3fe03e 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::{cert, 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 8615fd17..e7ad2e79 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -85,6 +85,7 @@ pub use { ExtendedKeyUsage, ExtendedKeyUsageValidator, IntermediateIterator, KeyPurposeId, KeyPurposeIdIter, PathBuilder, RequiredEkuNotFoundContext, VerifiedPath, }, + x509::ExtensionId, }; #[cfg(feature = "alloc")] diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 7d0f098c..23d0287b 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::{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> { @@ -65,6 +67,7 @@ impl<'a, 'p: 'a> PathBuilder<'a, 'p> { intermediate_certs: &[], revocation: None, verify_path: None, + extension_policy: UnknownExtensionPolicy::default(), } } @@ -84,6 +87,23 @@ impl<'a, 'p: 'a> PathBuilder<'a, 'p> { self } + /// 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 @@ -170,7 +190,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 26bdc5be..848ecbf4 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 ea7bc62b..3ef11abb 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")] @@ -195,6 +215,96 @@ 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( + &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( + &ExtendedKeyUsage::SERVER_AUTH, + ALL_VERIFICATION_ALGS, + &anchors, + ) + .with_intermediate_certs(slice::from_ref(&intermediate_der)); + + assert!(matches!( + builder.build(&cert, time), + Err(webpki::Error::UnsupportedCriticalExtension) + )); + + let ignored = [webpki::ExtensionId::new(PRIVATE_CRITICAL_EXTENSION_OID)]; + let builder = PathBuilder::new( + &ExtendedKeyUsage::SERVER_AUTH, + ALL_VERIFICATION_ALGS, + &anchors, + ) + .with_intermediate_certs(slice::from_ref(&intermediate_der)) + .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")[..]); @@ -371,6 +481,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() {