diff --git a/Cargo.lock b/Cargo.lock index 3f7b1c50..68fa0621 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -533,7 +533,7 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.10" +version = "0.103.11" dependencies = [ "aws-lc-rs", "base64", diff --git a/Cargo.toml b/Cargo.toml index ddce1830..0d9da8ab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ license = "ISC" name = "rustls-webpki" readme = "README.md" repository = "https://github.com/rustls/webpki" -version = "0.103.10" +version = "0.103.11" include = [ "Cargo.toml", diff --git a/src/cert.rs b/src/cert.rs index d3ee733a..1998a14c 100644 --- a/src/cert.rs +++ b/src/cert.rs @@ -21,7 +21,10 @@ use crate::error::{DerTypeId, Error}; use crate::public_values_eq; use crate::signed_data::SignedData; use crate::subject_name::{GeneralName, NameIterator, WildcardDnsNameRef}; -use crate::x509::{DistributionPointName, Extension, remember_extension, set_extension_once}; +use crate::x509::{ + DistributionPointName, Extension, UnknownExtensionPolicy, remember_extension, + set_extension_once, +}; /// A parsed X509 certificate. pub struct Cert<'a> { @@ -47,7 +50,18 @@ pub struct Cert<'a> { } impl<'a> Cert<'a> { + pub(crate) fn for_trust_anchor(cert_der: untrusted::Input<'a>) -> Result { + Self::from_input(cert_der, UnknownExtensionPolicy::IgnoreCritical) + } + pub(crate) fn from_der(cert_der: untrusted::Input<'a>) -> Result { + Self::from_input(cert_der, UnknownExtensionPolicy::default()) + } + + fn from_input( + cert_der: untrusted::Input<'a>, + ext_policy: UnknownExtensionPolicy, + ) -> Result { let (tbs, signed_data) = cert_der.read_all(Error::TrailingData(DerTypeId::Certificate), |cert_der| { der::nested( @@ -130,6 +144,7 @@ impl<'a> Cert<'a> { remember_cert_extension( &mut cert, &Extension::from_der(extension)?, + ext_policy, ) }, ) @@ -275,12 +290,13 @@ pub(crate) fn lenient_certificate_serial_number<'a>( fn remember_cert_extension<'a>( cert: &mut Cert<'a>, extension: &Extension<'a>, + 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 // are not marked critical. - remember_extension(extension, |id| { + remember_extension(extension, ext_policy, |id| { let out = match id { // id-ce-keyUsage 2.5.29.15. 15 => &mut cert.key_usage, @@ -301,7 +317,7 @@ fn remember_cert_extension<'a>( 37 => &mut cert.eku, // Unsupported extension - _ => return extension.unsupported(), + _ => return extension.unsupported(ext_policy), }; set_extension_once(out, || { diff --git a/src/crl/types.rs b/src/crl/types.rs index d303bb7b..3e316768 100644 --- a/src/crl/types.rs +++ b/src/crl/types.rs @@ -14,7 +14,10 @@ use crate::public_values_eq; use crate::signed_data::{self, SignedData}; use crate::subject_name::GeneralName; use crate::verify_cert::{Budget, PathNode, Role}; -use crate::x509::{DistributionPointName, Extension, remember_extension, set_extension_once}; +use crate::x509::{ + DistributionPointName, Extension, UnknownExtensionPolicy, remember_extension, + set_extension_once, +}; /// A RFC 5280[^1] profile Certificate Revocation List (CRL). /// @@ -266,7 +269,7 @@ impl<'a> BorrowedCertRevocationList<'a> { } fn remember_extension(&mut self, extension: &Extension<'a>) -> Result<(), Error> { - remember_extension(extension, |id| { + remember_extension(extension, UnknownExtensionPolicy::default(), |id| { match id { // id-ce-cRLNumber 2.5.29.20 - RFC 5280 §5.2.3 20 => { @@ -304,7 +307,7 @@ impl<'a> BorrowedCertRevocationList<'a> { 35 => Ok(()), // Unsupported extension - _ => extension.unsupported(), + _ => extension.unsupported(UnknownExtensionPolicy::default()), } }) } @@ -729,7 +732,7 @@ impl<'a> BorrowedRevokedCert<'a> { } fn remember_extension(&mut self, extension: &Extension<'a>) -> Result<(), Error> { - remember_extension(extension, |id| { + remember_extension(extension, UnknownExtensionPolicy::default(), |id| { match id { // id-ce-cRLReasons 2.5.29.21 - RFC 5280 §5.3.1. 21 => set_extension_once(&mut self.reason_code, || der::read_all(extension.value)), @@ -749,7 +752,7 @@ impl<'a> BorrowedRevokedCert<'a> { 29 => Err(Error::UnsupportedIndirectCrl), // Unsupported extension - _ => extension.unsupported(), + _ => extension.unsupported(UnknownExtensionPolicy::default()), } }) } diff --git a/src/trust_anchor.rs b/src/trust_anchor.rs index 6a5763a4..1489c90c 100644 --- a/src/trust_anchor.rs +++ b/src/trust_anchor.rs @@ -34,7 +34,7 @@ pub fn anchor_from_trusted_cert<'a>( // certificate using a special parser for v1 certificates. Notably, that // parser doesn't allow extensions, so there's no need to worry about // embedded name constraints in a v1 certificate. - match Cert::from_der(cert_der) { + match Cert::for_trust_anchor(cert_der) { Ok(cert) => Ok(TrustAnchor::from(cert)), Err(Error::UnsupportedCertVersion) => { extract_trust_anchor_from_v1_cert_der(cert_der).or(Err(Error::BadDer)) @@ -101,3 +101,41 @@ impl<'a> From> for TrustAnchor<'a> { fn skip(input: &mut untrusted::Reader<'_>, tag: der::Tag) -> Result<(), Error> { der::expect_tag(input, tag).map(|_| ()) } + +#[cfg(test)] +mod tests { + use rcgen::{CertificateParams, CustomExtension, KeyPair}; + + use super::*; + + // OID 1.2.3.4 -- not under the id-ce arc, so `ExtensionOid::lookup` returns `None`. + // This exercises the unknown-OID path in `remember_extension`. + #[test] + fn anchor_ignores_critical_extension_with_unknown_oid() { + let der = cert_with_critical_extension(&[1, 2, 3, 4]); + anchor_from_trusted_cert(&der) + .expect("critical extension with unknown OID should be ignored for trust anchors"); + } + + // OID 2.5.29.99 -- under the id-ce arc, so `ExtensionOid::lookup` returns + // `Some(Standard(99))`, but 99 isn't handled in `remember_cert_extension`. + // This exercises the unsupported-extension path in `remember_cert_extension`. + #[test] + fn anchor_ignores_critical_extension_with_unknown_id_ce_oid() { + let der = cert_with_critical_extension(&[2, 5, 29, 99]); + anchor_from_trusted_cert(&der).expect( + "critical id-ce extension with unknown OID should be ignored for trust anchors", + ); + } + + fn cert_with_critical_extension(oid: &[u64]) -> CertificateDer<'static> { + let mut params = CertificateParams::new(vec!["example.com".into()]).unwrap(); + let mut ext = CustomExtension::from_oid_content(oid, vec![1, 2]); + ext.set_criticality(true); + params.custom_extensions.push(ext); + + let key = KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).unwrap(); + let cert = params.self_signed(&key).unwrap(); + CertificateDer::from(cert.der().to_vec()) + } +} diff --git a/src/x509.rs b/src/x509.rs index 87b219ac..b0f9e13e 100644 --- a/src/x509.rs +++ b/src/x509.rs @@ -23,10 +23,10 @@ pub(crate) struct Extension<'a> { } impl Extension<'_> { - pub(crate) fn unsupported(&self) -> Result<(), Error> { - match self.critical { - true => Err(Error::UnsupportedCriticalExtension), - false => Ok(()), + pub(crate) fn unsupported(&self, policy: UnknownExtensionPolicy) -> Result<(), Error> { + match (policy, self.critical) { + (UnknownExtensionPolicy::Strict, true) => Err(Error::UnsupportedCriticalExtension), + _ => Ok(()), } } } @@ -63,6 +63,7 @@ pub(crate) fn set_extension_once( pub(crate) fn remember_extension( extension: &Extension<'_>, + ext_policy: UnknownExtensionPolicy, mut handler: impl FnMut(u8) -> Result<(), Error>, ) -> Result<(), Error> { // ISO arc for standard certificate and CRL extensions. @@ -72,7 +73,7 @@ pub(crate) fn remember_extension( if extension.id.len() != ID_CE.len() + 1 || !extension.id.as_slice_less_safe().starts_with(&ID_CE) { - return extension.unsupported(); + return extension.unsupported(ext_policy); } // safety: we verify len is non-zero and has the correct prefix above. @@ -80,6 +81,13 @@ pub(crate) fn remember_extension( handler(last_octet) } +#[derive(Clone, Copy, Debug, Default)] +pub(crate) enum UnknownExtensionPolicy { + #[default] + Strict, + IgnoreCritical, +} + /// A certificate revocation list (CRL) distribution point name, describing a source of /// CRL information for a given certificate as described in RFC 5280 section 4.2.3.13[^1]. ///