From c14602993fdc696e90b5efc0d5efbe8a60a97691 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Thu, 15 Oct 2026 14:27:33 +0800 Subject: [PATCH] fix(sign): an unreadable extendedKeyUsage allows no purpose (#1807) (#1866) A malformed EKU left extended_key_usage at None, which reads as "no restriction", so an issuer-signed certificate with a broken EKU could reach a Valid verdict; EKU being on the processed-critical list let a critical one through as well. Like keyUsage, a present EKU now restricts the key even when it doesn't parse: it must be a SEQUENCE whose members are all OIDs, otherwise it allows no purpose. Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz Co-authored-by: Claude Opus 5.5 --- crates/sign/src/x509.rs | 12 ++++++++---- crates/sign/tests/chain.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/sign/src/x509.rs b/crates/sign/src/x509.rs index e3e784bae..1a0ef58ad 100644 --- a/crates/sign/src/x509.rs +++ b/crates/sign/src/x509.rs @@ -231,11 +231,15 @@ impl Extensions { self.authority_key_id = aki.children().ok().and_then(|c| c.into_iter().find(|t| t.tag == tag::ctx(0))).map(|t| t.value.to_vec()); } } - // ExtendedKeyUsage: a SEQUENCE OF OID. + // ExtendedKeyUsage: a SEQUENCE OF OID. Like keyUsage, one that is present restricts + // the key even when it doesn't parse: an unreadable one allows no purpose at all. "2.5.29.37" => { - if let Ok(eku) = Tlv::parse_all(value.value) { - self.extended_key_usage = eku.children().ok().map(|c| c.into_iter().filter_map(|t| t.oid().ok()).collect::>()); - } + let purposes = Tlv::parse_all(value.value) + .ok() + .filter(|eku| eku.tag == tag::SEQUENCE) + .and_then(|eku| eku.children().ok()) + .and_then(|c| c.iter().map(|t| t.oid().ok()).collect::>>()); + self.extended_key_usage = Some(purposes.unwrap_or_default()); } // Authority Information Access: OCSP and CA-issuer locations. "1.3.6.1.5.5.7.1.1" => { diff --git a/crates/sign/tests/chain.rs b/crates/sign/tests/chain.rs index e8fd01274..ece529765 100644 --- a/crates/sign/tests/chain.rs +++ b/crates/sign/tests/chain.rs @@ -298,3 +298,30 @@ fn unreadable_constraint_extensions_fail_closed() { let c = issue("Garbled Constraints", None, FOREVER, &[garbage]); assert!(c.cert.has_basic_constraints && !c.cert.is_ca && !c.cert.may_issue()); } + +#[test] +fn an_unreadable_extended_key_usage_allows_no_purpose() { + // An extendedKeyUsage that is present restricts the key even when it doesn't parse (#1807): + // truncated, critical or not; a SET instead of a SEQUENCE; or a member that isn't an OID. + // Each would otherwise read as "no restriction", or as only its readable members. + let r = root(); + let any = der::oid("2.5.29.37.0"); + let truncated = [0x30, 0x05, 0x06]; + let malformed = [ + ext("2.5.29.37", &truncated), + der::seq(&[&der::oid("2.5.29.37"), &der::octets(&truncated)]), + ext("2.5.29.37", &der::tlv(tag::SET, &any)), + ext("2.5.29.37", &der::seq(&[&der::int(1), &any])), + ]; + for (i, eku) in malformed.into_iter().enumerate() { + let signer = signer_with("Malformed EKU", &r, &[key_usage(DIGITAL_SIGNATURE), eku]); + assert_eq!(signer.cert.extended_key_usage.as_deref(), Some(&[][..]), "case {i}"); + let (status, details) = verdict(signer, vec![], &r.cert); + assert_eq!(status, Status::Unknown, "case {i}: {details:?}"); + assert!(details.iter().any(|d| d.contains("not issued for signing documents")), "case {i}: {details:?}"); + } + // The same document-signing purpose, well formed, may sign. + let (status, details) = + verdict(signer_with("Good EKU", &r, &[key_usage(DIGITAL_SIGNATURE), extended_key_usage(&["2.5.29.37.0"])]), vec![], &r.cert); + assert_eq!(status, Status::Valid, "{details:?}"); +}