From 590e939366ea3c6f893c8e9c80bddbfd95d4e9be Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 2 Sep 2026 10:06:06 -0400 Subject: [PATCH 1/2] update x509-limbo to latest This brings in new tests for empty permittedSubtrees name constraints. The non-empty control passes as expected, but webpki treats an empty permittedSubtrees sequence as absent. Add the invalid empty case to exceptions. --- Cargo.lock | 2 +- Cargo.toml | 2 +- third-party/x509-limbo/exceptions.json | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6423ed06..c0061727 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -362,7 +362,7 @@ checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "limbo-harness-support" version = "0.1.0" -source = "git+https://github.com/C2SP/x509-limbo?rev=1252c300df48507fb27e709fb287c01a9caa1e0b#1252c300df48507fb27e709fb287c01a9caa1e0b" +source = "git+https://github.com/C2SP/x509-limbo?rev=21cc053f7edbd22e0e8d8a98a7fe13918912af9d#21cc053f7edbd22e0e8d8a98a7fe13918912af9d" dependencies = [ "chrono", "regress", diff --git a/Cargo.toml b/Cargo.toml index aa09fc75..ad22e4bc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,7 +69,7 @@ untrusted = "0.9" base64 = "0.23" bencher = "0.1.5" chrono = "0.4" -limbo-harness-support = { git = "https://github.com/C2SP/x509-limbo", rev = "1252c300df48507fb27e709fb287c01a9caa1e0b" } +limbo-harness-support = { git = "https://github.com/C2SP/x509-limbo", rev = "21cc053f7edbd22e0e8d8a98a7fe13918912af9d" } once_cell = "1.17.2" rcgen = { version = "0.14.7", default-features = false, features = ["aws_lc_rs"] } rustls-aws-lc-rs = { version = "0.1.0-dev.1" } diff --git a/third-party/x509-limbo/exceptions.json b/third-party/x509-limbo/exceptions.json index 39007c37..0be26650 100644 --- a/third-party/x509-limbo/exceptions.json +++ b/third-party/x509-limbo/exceptions.json @@ -59,6 +59,11 @@ "actual": "SUCCESS", "reason": "webpki does not enforce RFC 5280 requirement that NameConstraints be critical" }, + "rfc5280::nc::permitted-empty-sequence-excluded-nonempty": { + "expected": "FAILURE", + "actual": "SUCCESS", + "reason": "webpki treats an empty permittedSubtrees sequence as absent" + }, "rfc5280::nc::invalid-dnsname-leading-period": { "expected": "FAILURE", "actual": "SUCCESS", From 3e495077221a775169028219dfb1b37f6dfaffdb Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 2 Sep 2026 10:23:04 -0400 Subject: [PATCH 2/2] subject_name: reject empty name constraint sequences RFC 5280 requires NameConstraints to contain at least one of permittedSubtrees or excludedSubtrees. Both fields use GeneralSubtrees, which is defined as a sequence with one or more entries. A present but empty field is therefore invalid. Reject these encodings as malformed name constraints. This allows the x509-limbo cases for an empty extension, two empty subtree fields, and an empty permittedSubtrees alongside excludedSubtrees to pass without exceptions. --- src/error.rs | 4 +- src/subject_name/mod.rs | 13 +++- tests/tls_server_certs.rs | 82 +++++++++++++++++++++----- third-party/x509-limbo/exceptions.json | 15 ----- 4 files changed, 81 insertions(+), 33 deletions(-) diff --git a/src/error.rs b/src/error.rs index 38ca9fbc..a2b7d056 100644 --- a/src/error.rs +++ b/src/error.rs @@ -133,8 +133,8 @@ pub enum Error { /// and as recommended by RFC6125. MalformedExtensions, - /// A name constraint was malformed, potentially containing invalid characters or - /// invalid labels. + /// A name constraint was malformed, potentially containing invalid characters, + /// invalid labels, or an empty sequence where at least one item is required. MalformedNameConstraint, /// The maximum number of name constraint comparisons has been reached. diff --git a/src/subject_name/mod.rs b/src/subject_name/mod.rs index ec361fe9..23bddea4 100644 --- a/src/subject_name/mod.rs +++ b/src/subject_name/mod.rs @@ -31,11 +31,22 @@ pub(crate) fn check_name_constraints( if !inner.peek(subtrees_tag.into()) { return Ok(None); } - der::expect_tag(inner, subtrees_tag).map(Some) + + let subtrees = der::expect_tag(inner, subtrees_tag)?; + // GeneralSubtrees is defined as a SEQUENCE SIZE (1..MAX). + if subtrees.is_empty() { + return Err(Error::MalformedNameConstraint); + } + + Ok(Some(subtrees)) } let permitted_subtrees = parse_subtrees(constraints, der::Tag::ContextSpecificConstructed0)?; let excluded_subtrees = parse_subtrees(constraints, der::Tag::ContextSpecificConstructed1)?; + // At least one of permittedSubtrees or excludedSubtrees must be present. + if permitted_subtrees.is_none() && excluded_subtrees.is_none() { + return Err(Error::MalformedNameConstraint); + } for path in path.iter() { let result = NameIterator::new(path.cert.subject_alt_name).find_map(|result| { diff --git a/tests/tls_server_certs.rs b/tests/tls_server_certs.rs index 0d8f9c59..966a6d5d 100644 --- a/tests/tls_server_certs.rs +++ b/tests/tls_server_certs.rs @@ -641,39 +641,91 @@ fn uri_san_rejected_against_uri_excluded_subtree() { ); } +#[test] +fn empty_name_constraint_sequences_rejected() { + let permitted = name_constraint_subtrees(b"example.com", DNS_NAME_TAG, PERMITTED_SUBTREES_TAG); + let excluded = name_constraint_subtrees(b"example.com", DNS_NAME_TAG, EXCLUDED_SUBTREES_TAG); + let empty_permitted = der_tlv(PERMITTED_SUBTREES_TAG, &[]); + let empty_excluded = der_tlv(EXCLUDED_SUBTREES_TAG, &[]); + + let cases = [ + ("neither subtree field present", vec![]), + ( + "empty permittedSubtrees", + [empty_permitted.as_slice(), excluded.as_slice()].concat(), + ), + ( + "empty excludedSubtrees", + [permitted.as_slice(), empty_excluded.as_slice()].concat(), + ), + ( + "both subtree fields empty", + [empty_permitted.as_slice(), empty_excluded.as_slice()].concat(), + ), + ]; + + for (description, subtrees) in cases { + let ca_key = KeyPair::generate().unwrap(); + let mut ca_params = issuer_params("issuer.example.com").unwrap(); + ca_params + .custom_extensions + .push(name_constraints_extension(&subtrees)); + let issuer = + CertifiedIssuer::self_signed(ca_params, ca_key).expect("failed to generate CA"); + let ee = generate_cert(vec![], &issuer); + + assert_eq!( + check_cert(ee.der(), issuer.der(), &[], &[], &[]), + Err(webpki::Error::MalformedNameConstraint), + "{description}", + ); + } +} + // Hand-encode a NameConstraints extension (OID 2.5.29.30) with a single // permittedSubtree containing a URI GeneralName. rcgen's GeneralSubtree enum // doesn't expose a URI variant, so we emit the DER directly. fn uri_permitted_name_constraints(uri: &[u8]) -> CustomExtension { - uri_name_constraints(uri, 0xa0) // permittedSubtrees [0] IMPLICIT + uri_name_constraints(uri, PERMITTED_SUBTREES_TAG) } // Hand-encode a NameConstraints extension (OID 2.5.29.30) with a single // excludedSubtree containing a URI GeneralName. fn uri_excluded_name_constraints(uri: &[u8]) -> CustomExtension { - uri_name_constraints(uri, 0xa1) // excludedSubtrees [1] IMPLICIT + uri_name_constraints(uri, EXCLUDED_SUBTREES_TAG) } fn uri_name_constraints(uri: &[u8], subtrees_tag: u8) -> CustomExtension { - assert!(uri.len() < 128); - // URI GeneralName: [6] IMPLICIT IA5String - let mut uri_gn = vec![0x86, uri.len() as u8]; - uri_gn.extend_from_slice(uri); - // GeneralSubtree SEQUENCE { base GeneralName, ... } - let mut subtree = vec![0x30, uri_gn.len() as u8]; - subtree.extend_from_slice(&uri_gn); - // permittedSubtrees [0] or excludedSubtrees [1] IMPLICIT GeneralSubtrees - let mut subtrees = vec![subtrees_tag, subtree.len() as u8]; - subtrees.extend_from_slice(&subtree); - // NameConstraints SEQUENCE - let mut nc = vec![0x30, subtrees.len() as u8]; - nc.extend_from_slice(&subtrees); + let subtrees = name_constraint_subtrees(uri, UNIFORM_RESOURCE_IDENTIFIER_TAG, subtrees_tag); + name_constraints_extension(&subtrees) +} + +fn name_constraint_subtrees(name: &[u8], name_tag: u8, subtrees_tag: u8) -> Vec { + let general_name = der_tlv(name_tag, name); + let subtree = der_tlv(SEQUENCE_TAG, &general_name); + der_tlv(subtrees_tag, &subtree) +} +fn name_constraints_extension(subtrees: &[u8]) -> CustomExtension { + let nc = der_tlv(SEQUENCE_TAG, subtrees); let mut ext = CustomExtension::from_oid_content(&[2, 5, 29, 30], nc); ext.set_criticality(true); ext } +fn der_tlv(tag: u8, value: &[u8]) -> Vec { + assert!(value.len() < 128); + let mut encoded = vec![tag, value.len() as u8]; + encoded.extend_from_slice(value); + encoded +} + +const SEQUENCE_TAG: u8 = 0x30; +const DNS_NAME_TAG: u8 = 0x82; // [2] IMPLICIT IA5String +const UNIFORM_RESOURCE_IDENTIFIER_TAG: u8 = 0x86; // [6] IMPLICIT IA5String +const PERMITTED_SUBTREES_TAG: u8 = 0xa0; // [0] IMPLICIT GeneralSubtrees +const EXCLUDED_SUBTREES_TAG: u8 = 0xa1; // [1] IMPLICIT GeneralSubtrees + #[test] fn permit_directory_name_not_implemented() { let mut dn = DistinguishedName::new(); diff --git a/third-party/x509-limbo/exceptions.json b/third-party/x509-limbo/exceptions.json index 0be26650..1a7ba677 100644 --- a/third-party/x509-limbo/exceptions.json +++ b/third-party/x509-limbo/exceptions.json @@ -59,11 +59,6 @@ "actual": "SUCCESS", "reason": "webpki does not enforce RFC 5280 requirement that NameConstraints be critical" }, - "rfc5280::nc::permitted-empty-sequence-excluded-nonempty": { - "expected": "FAILURE", - "actual": "SUCCESS", - "reason": "webpki treats an empty permittedSubtrees sequence as absent" - }, "rfc5280::nc::invalid-dnsname-leading-period": { "expected": "FAILURE", "actual": "SUCCESS", @@ -249,16 +244,6 @@ "actual": "SUCCESS", "reason": "webpki does not enforce CABF prohibition on EKU in root certs" }, - "webpki::nc::intermediate-permitted-excluded-subtrees-both-null": { - "expected": "FAILURE", - "actual": "SUCCESS", - "reason": "webpki does not enforce CABF NameConstraints encoding requirements" - }, - "webpki::nc::intermediate-permitted-excluded-subtrees-both-empty-sequences": { - "expected": "FAILURE", - "actual": "SUCCESS", - "reason": "webpki does not enforce CABF NameConstraints encoding requirements" - }, "webpki::san::san-critical-with-nonempty-subject": { "expected": "FAILURE", "actual": "SUCCESS",