Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/collateral.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,12 +239,9 @@ fn extract_fmspc_and_ca_with<C: Config>(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
Expand Down
29 changes: 21 additions & 8 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<String>;
/// 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<Option<Vec<u8>>>;
Expand Down
9 changes: 3 additions & 6 deletions src/intel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,10 @@ pub fn parse_pck_extension_from_pem(pem_data: &[u8]) -> Result<PckExtension> {
/// Generic over [`Config`]; the issuer DN extraction goes through the
/// configured [`X509Codec`].
pub fn pck_ca_with<C: Config>(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.
Expand Down
60 changes: 42 additions & 18 deletions src/x509.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -33,30 +33,54 @@ impl X509Codec for X509CertBackend {
}

impl ParsedCert for X509CertParsed {
fn issuer_dn(&self) -> Result<String> {
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<Option<Vec<u8>>> {
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<ObjectIdentifier>` is already reachable via cert
// parsing — but it is pure runtime cost for no benefit.)
let mut found: Option<&[u8]> = None;
Comment thread
pbeza marked this conversation as resolved.
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 `<ObjectIdentifier as Display>` 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");
Comment thread
pbeza marked this conversation as resolved.
}
found = Some(ext.extn_value.as_bytes());
}
}
Ok(Some(extension.into_bytes()))
Ok(found.map(<[u8]>::to_vec))
}
}
18 changes: 13 additions & 5 deletions tests/config_conformance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,19 @@ pub fn assert_config_conforms<C: Config>() {
let default =
<DefaultConfig as Config>::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("<non-utf8>"),
);
}

let custom_ext = custom.extension(SGX_EXTENSION_OID).expect("custom ext");
let default_ext = default.extension(SGX_EXTENSION_OID).expect("default ext");
Expand Down
Loading