diff --git a/src/collateral.rs b/src/collateral.rs index ee579ba..cc3231f 100644 --- a/src/collateral.rs +++ b/src/collateral.rs @@ -239,12 +239,9 @@ fn extract_fmspc_and_ca_with(pem_chain: &str) -> Result<(String, &'st let fmspc_hex = hex::encode_upper(fmspc); // Extract CA type from issuer (reuses parsed cert above) - let issuer = parsed - .issuer_dn() - .context("Failed to extract certificate issuer")?; - let ca = if issuer.contains(crate::constants::PROCESSOR_ISSUER) { + let ca = if parsed.issuer_contains(crate::constants::PROCESSOR_ISSUER.as_bytes()) { crate::constants::PROCESSOR_ISSUER_ID - } else if issuer.contains(crate::constants::PLATFORM_ISSUER) { + } else if parsed.issuer_contains(crate::constants::PLATFORM_ISSUER.as_bytes()) { crate::constants::PLATFORM_ISSUER_ID } else { crate::constants::PROCESSOR_ISSUER_ID diff --git a/src/config.rs b/src/config.rs index 9e1f1f1..b6afa29 100644 --- a/src/config.rs +++ b/src/config.rs @@ -46,7 +46,6 @@ //! that owns or borrows the parsed data, so multiple field accesses share a //! single parse. -use alloc::string::String; use alloc::vec::Vec; use anyhow::Result; @@ -74,19 +73,33 @@ pub trait X509Codec { /// Read-only accessors over a parsed X.509 certificate. pub trait ParsedCert { - /// Returns the issuer Distinguished Name as a human-readable string in - /// RFC 4514 form (e.g. `"CN=Foo,O=Bar"`). + /// Returns `true` if any RDN component of the issuer DN whose tag is + /// `PrintableString`, `UTF8String`, `IA5String`, or `TeletexString` + /// contains `needle` as a byte substring. /// - /// The string is consumed by `intel::quote_ca` for substring matching, so - /// a stable, conventional representation is required. - fn issuer_dn(&self) -> Result; + /// Wide-char `BMPString` / `UniversalString` and non-string ATVs are + /// skipped — an ASCII needle cannot meaningfully match those encodings, + /// which is also what the former `issuer_dn().to_string().contains(...)` + /// produced (`x509-cert`'s `Display` hex-encodes non-byte-string ATVs). + /// + /// Used by [`crate::intel::pck_ca_with`] to classify an Intel PCK leaf. + /// Intel PCK issuer CNs are `UTF8String`, so the skipped tags are not + /// reachable on the production path. + fn issuer_contains(&self, needle: &[u8]) -> bool; /// Returns the OCTET STRING contents of the unique extension whose /// `extnID` equals `oid`, where `oid` is the DER-encoded OID body - /// (no tag/length). + /// (no tag/length). Callers SHOULD construct `oid` from a + /// [`const_oid::ObjectIdentifier`] and pass `.as_bytes()`. /// /// * `Ok(Some(value))` — exactly one matching extension was found. - /// * `Ok(None)` — no matching extension. + /// * `Ok(None)` — no matching extension, *including* when `oid` is not a + /// well-formed OID body: a malformed needle cannot equal any cert's + /// `extnID` (which was DER-validated during [`X509Codec::from_der`]), + /// so "not found" is vacuously correct. Implementations are not + /// required to reject malformed `oid` inputs — runtime validation + /// would be pure overhead for callers that supply + /// `const_oid`-constructed OIDs. /// * `Err(_)` — the extension was found more than once, or the certificate /// is malformed. fn extension(&self, oid: &[u8]) -> Result>>; diff --git a/src/intel.rs b/src/intel.rs index b9b82b5..cb91d4f 100644 --- a/src/intel.rs +++ b/src/intel.rs @@ -126,13 +126,10 @@ pub fn parse_pck_extension_from_pem(pem_data: &[u8]) -> Result { /// Generic over [`Config`]; the issuer DN extraction goes through the /// configured [`X509Codec`]. pub fn pck_ca_with(cert_der: &[u8]) -> Result<&'static str> { - let issuer = C::X509::from_der(cert_der) - .context("Failed to decode certificate")? - .issuer_dn() - .context("Failed to extract certificate issuer")?; - if issuer.contains(constants::PROCESSOR_ISSUER) { + let parsed = C::X509::from_der(cert_der).context("Failed to decode certificate")?; + if parsed.issuer_contains(constants::PROCESSOR_ISSUER.as_bytes()) { Ok(constants::PROCESSOR_ISSUER_ID) - } else if issuer.contains(constants::PLATFORM_ISSUER) { + } else if parsed.issuer_contains(constants::PLATFORM_ISSUER.as_bytes()) { Ok(constants::PLATFORM_ISSUER_ID) } else { // Preserve legacy fallback behavior: unknown issuer is treated as processor. diff --git a/src/x509.rs b/src/x509.rs index 57af64c..c8c00c7 100644 --- a/src/x509.rs +++ b/src/x509.rs @@ -3,9 +3,9 @@ //! Selected by [`crate::configs::RingConfig`] / [`crate::configs::RustCryptoConfig`] //! / [`crate::configs::DefaultConfig`]. -use alloc::string::{String, ToString}; use alloc::vec::Vec; -use anyhow::{anyhow, bail, Context, Result}; +use anyhow::{bail, Context, Result}; +use der::{Tag, Tagged}; use crate::config::{ParsedCert, X509Codec}; @@ -33,30 +33,54 @@ impl X509Codec for X509CertBackend { } impl ParsedCert for X509CertParsed { - fn issuer_dn(&self) -> Result { - Ok(self.cert.tbs_certificate.issuer.to_string()) + fn issuer_contains(&self, needle: &[u8]) -> bool { + if needle.is_empty() { + return true; + } + for rdn in self.cert.tbs_certificate.issuer.0.iter() { + for atv in rdn.0.iter() { + if matches!( + atv.value.tag(), + Tag::PrintableString | Tag::Utf8String | Tag::Ia5String | Tag::TeletexString + ) && atv.value.value().windows(needle.len()).any(|w| w == needle) + { + return true; + } + } + } + false } fn extension(&self, oid: &[u8]) -> Result>> { - let oid = const_oid::ObjectIdentifier::from_bytes(oid) - .map_err(|_| anyhow!("Invalid OID encoding"))?; - let mut iter = self + // Compare raw DER-encoded OID bodies byte-for-byte. Callers pass the + // body of a `const_oid`-constructed OID, so there is nothing left to + // validate at runtime. (The parse itself would not add footprint — + // `der::Decode` is already reachable via cert + // parsing — but it is pure runtime cost for no benefit.) + let mut found: Option<&[u8]> = None; + for ext in self .cert .tbs_certificate .extensions .as_deref() .unwrap_or(&[]) - .iter() - .filter(|e| e.extn_id == oid) - .map(|e| e.extn_value.clone()); - - let extension = match iter.next() { - Some(ext) => ext, - None => return Ok(None), - }; - if iter.next().is_some() { - bail!("extension {} appears more than once", oid); + { + if ext.extn_id.as_bytes() == oid { + if found.is_some() { + // NOTE: would be friendlier to include the offending OID here + // (e.g. `"extension {} appears more than once", ext.extn_id`), + // but using `` from this call + // site adds ~60–100 B of format-args setup to the stripped + // contract wasm (measured on wasm32 `opt-level=z` + `lto=fat` + // + `wasm-opt -O -Oz`). The duplicate-extension path is only + // reachable on a malformed cert, which Intel-signed chains + // never produce, so the ergonomics aren't worth the binary + // cost. Revisit if a maintainer pushes back. + bail!("extension appears more than once"); + } + found = Some(ext.extn_value.as_bytes()); + } } - Ok(Some(extension.into_bytes())) + Ok(found.map(<[u8]>::to_vec)) } } diff --git a/tests/config_conformance.rs b/tests/config_conformance.rs index bbca164..cf5bf59 100644 --- a/tests/config_conformance.rs +++ b/tests/config_conformance.rs @@ -47,11 +47,19 @@ pub fn assert_config_conforms() { let default = ::X509::from_der(&cert_der).expect("default from_der"); - assert_eq!( - custom.issuer_dn().expect("custom issuer_dn"), - default.issuer_dn().expect("default issuer_dn"), - "issuer_dn output mismatch" - ); + for needle in [ + b"Processor".as_slice(), + b"Platform", + b"Intel SGX", + b"NotPresent", + ] { + assert_eq!( + custom.issuer_contains(needle), + default.issuer_contains(needle), + "issuer_contains output mismatch for needle {:?}", + core::str::from_utf8(needle).unwrap_or(""), + ); + } let custom_ext = custom.extension(SGX_EXTENSION_OID).expect("custom ext"); let default_ext = default.extension(SGX_EXTENSION_OID).expect("default ext");