From 573f82ed3df330a99127cf85e5d84dce78f44809 Mon Sep 17 00:00:00 2001 From: Eirik Botten Nicolaysen Date: Mon, 31 Aug 2026 14:13:56 +0200 Subject: [PATCH 1/5] Reject present-but-empty name constraint subtrees GeneralSubtrees is SEQUENCE SIZE (1..MAX), so a subtree field that is present must not be empty. An empty permittedSubtrees was accepted and then had no effect, because the loop over the subtrees never runs, so every name passed the permitted side instead of none. --- .../src/policy/extension.rs | 124 +++++++++++++++++- 1 file changed, 122 insertions(+), 2 deletions(-) diff --git a/src/rust/cryptography-x509-verification/src/policy/extension.rs b/src/rust/cryptography-x509-verification/src/policy/extension.rs index 17d694bd094c..aa5dc4452e79 100644 --- a/src/rust/cryptography-x509-verification/src/policy/extension.rs +++ b/src/rust/cryptography-x509-verification/src/policy/extension.rs @@ -739,6 +739,32 @@ mod ca { ))); } + // `GeneralSubtrees ::= SEQUENCE SIZE (1..MAX) OF GeneralSubtree`, + // so a subtree field that is present must not be empty. This + // matters most for permittedSubtrees: RFC 5280 6.1.4 (g)(1) + // intersects it with its previous value, and 6.1.3 (b) requires + // each name to lie within it, so an empty permittedSubtrees admits + // no name at all. Accepting it here would instead permit every + // name, because the loop over the subtrees never runs. + if name_constraints + .permitted_subtrees + .as_ref() + .is_some_and(|pst| pst.is_empty()) + { + return Err(ValidationError::new(ValidationErrorKind::Other( + "nameConstraints permittedSubtrees must not be empty".to_string(), + ))); + } + if name_constraints + .excluded_subtrees + .as_ref() + .is_some_and(|est| est.is_empty()) + { + return Err(ValidationError::new(ValidationErrorKind::Other( + "nameConstraints excludedSubtrees must not be empty".to_string(), + ))); + } + // NOTE: Both RFC 5280 and CABF require each `GeneralSubtree` // to have `minimum=0` and `maximum=NULL`, but experimentally // not many validators check for this. @@ -798,9 +824,9 @@ mod tests { use asn1::{ObjectIdentifier, SimpleAsn1Writable}; use cryptography_x509::extensions::{BasicConstraints, Extension}; - use cryptography_x509::oid::BASIC_CONSTRAINTS_OID; + use cryptography_x509::oid::{BASIC_CONSTRAINTS_OID, NAME_CONSTRAINTS_OID}; - use super::{Criticality, ExtensionValidator}; + use super::{ca, Criticality, ExtensionValidator}; use crate::certificate::tests::PublicKeyErrorOps; use crate::ops::tests::{cert, epoch, v1_cert_pem}; use crate::ops::{CryptoOps, VerificationCertificate}; @@ -1056,4 +1082,98 @@ mod tests { ) .is_err()); } + + fn name_constraints_policy() -> PolicyDefinition<'static, PublicKeyErrorOps> { + PolicyDefinition::server( + PublicKeyErrorOps {}, + Subject::DNS(DNSName::new("example.com").unwrap()), + epoch(), + None, + None, + None, + ) + .expect("failed to create policy definition") + } + + #[test] + fn test_ca_name_constraints_empty_permitted_subtrees() { + // The certificate is not used by this validator, so which one we use + // does not matter. + let cert_pem = v1_cert_pem(); + let cert = cert(&cert_pem); + let verification_cert = VerificationCertificate::new(&cert, ()); + let policy_def = name_constraints_policy(); + let policy = Policy::new(&policy_def, ()); + + // NameConstraints with a present-but-empty permittedSubtrees and a + // non-empty excludedSubtrees, written as raw DER because the empty + // sequence cannot be built through the writing API. + // + // SEQUENCE { + // [0] {} -- permittedSubtrees + // [1] { SEQUENCE { [2] "bad.example" }} -- excludedSubtrees + // } + let extn_value: &[u8] = &[ + 0x30, 0x13, 0xa0, 0x00, 0xa1, 0x0f, 0x30, 0x0d, 0x82, 0x0b, b'b', b'a', b'd', b'.', + b'e', b'x', b'a', b'm', b'p', b'l', b'e', + ]; + let extn = Extension { + extn_id: NAME_CONSTRAINTS_OID, + critical: true, + extn_value, + }; + assert!(ca::name_constraints(&policy, &verification_cert, Some(&extn)).is_err()); + } + + #[test] + fn test_ca_name_constraints_empty_excluded_subtrees() { + let cert_pem = v1_cert_pem(); + let cert = cert(&cert_pem); + let verification_cert = VerificationCertificate::new(&cert, ()); + let policy_def = name_constraints_policy(); + let policy = Policy::new(&policy_def, ()); + + // SEQUENCE { + // [0] { SEQUENCE { [2] "ok.example" }} -- permittedSubtrees + // [1] {} -- excludedSubtrees + // } + let extn_value: &[u8] = &[ + 0x30, 0x12, 0xa0, 0x0e, 0x30, 0x0c, 0x82, 0x0a, b'o', b'k', b'.', b'e', b'x', b'a', + b'm', b'p', b'l', b'e', 0xa1, 0x00, + ]; + let extn = Extension { + extn_id: NAME_CONSTRAINTS_OID, + critical: true, + extn_value, + }; + assert!(ca::name_constraints(&policy, &verification_cert, Some(&extn)).is_err()); + } + + #[test] + fn test_ca_name_constraints_non_empty_permitted_subtrees() { + let cert_pem = v1_cert_pem(); + let cert = cert(&cert_pem); + let verification_cert = VerificationCertificate::new(&cert, ()); + let policy_def = name_constraints_policy(); + let policy = Policy::new(&policy_def, ()); + + // Control: the same shape with a non-empty permittedSubtrees is still + // accepted. + // + // SEQUENCE { + // [0] { SEQUENCE { [2] "ok.example" }} -- permittedSubtrees + // [1] { SEQUENCE { [2] "bad.example" }} -- excludedSubtrees + // } + let extn_value: &[u8] = &[ + 0x30, 0x21, 0xa0, 0x0e, 0x30, 0x0c, 0x82, 0x0a, b'o', b'k', b'.', b'e', b'x', b'a', + b'm', b'p', b'l', b'e', 0xa1, 0x0f, 0x30, 0x0d, 0x82, 0x0b, b'b', b'a', b'd', b'.', + b'e', b'x', b'a', b'm', b'p', b'l', b'e', + ]; + let extn = Extension { + extn_id: NAME_CONSTRAINTS_OID, + critical: true, + extn_value, + }; + assert!(ca::name_constraints(&policy, &verification_cert, Some(&extn)).is_ok()); + } } From 7aa3e8d396917ed9bbfc8bed53b83651467b477f Mon Sep 17 00:00:00 2001 From: Eirik Botten Nicolaysen Date: Wed, 2 Sep 2026 09:33:03 +0200 Subject: [PATCH 2/5] Drop two name constraint tests now covered by x509-limbo C2SP/x509-limbo#658 landed, so rfc5280::nc::permitted-empty-sequence-excluded-nonempty and rfc5280::nc::permitted-nonempty-excluded-nonempty cover these two cases. Co-Authored-By: Claude Opus 5 --- .../src/policy/extension.rs | 58 ------------------- 1 file changed, 58 deletions(-) diff --git a/src/rust/cryptography-x509-verification/src/policy/extension.rs b/src/rust/cryptography-x509-verification/src/policy/extension.rs index aa5dc4452e79..fc5a142419d6 100644 --- a/src/rust/cryptography-x509-verification/src/policy/extension.rs +++ b/src/rust/cryptography-x509-verification/src/policy/extension.rs @@ -1095,36 +1095,6 @@ mod tests { .expect("failed to create policy definition") } - #[test] - fn test_ca_name_constraints_empty_permitted_subtrees() { - // The certificate is not used by this validator, so which one we use - // does not matter. - let cert_pem = v1_cert_pem(); - let cert = cert(&cert_pem); - let verification_cert = VerificationCertificate::new(&cert, ()); - let policy_def = name_constraints_policy(); - let policy = Policy::new(&policy_def, ()); - - // NameConstraints with a present-but-empty permittedSubtrees and a - // non-empty excludedSubtrees, written as raw DER because the empty - // sequence cannot be built through the writing API. - // - // SEQUENCE { - // [0] {} -- permittedSubtrees - // [1] { SEQUENCE { [2] "bad.example" }} -- excludedSubtrees - // } - let extn_value: &[u8] = &[ - 0x30, 0x13, 0xa0, 0x00, 0xa1, 0x0f, 0x30, 0x0d, 0x82, 0x0b, b'b', b'a', b'd', b'.', - b'e', b'x', b'a', b'm', b'p', b'l', b'e', - ]; - let extn = Extension { - extn_id: NAME_CONSTRAINTS_OID, - critical: true, - extn_value, - }; - assert!(ca::name_constraints(&policy, &verification_cert, Some(&extn)).is_err()); - } - #[test] fn test_ca_name_constraints_empty_excluded_subtrees() { let cert_pem = v1_cert_pem(); @@ -1148,32 +1118,4 @@ mod tests { }; assert!(ca::name_constraints(&policy, &verification_cert, Some(&extn)).is_err()); } - - #[test] - fn test_ca_name_constraints_non_empty_permitted_subtrees() { - let cert_pem = v1_cert_pem(); - let cert = cert(&cert_pem); - let verification_cert = VerificationCertificate::new(&cert, ()); - let policy_def = name_constraints_policy(); - let policy = Policy::new(&policy_def, ()); - - // Control: the same shape with a non-empty permittedSubtrees is still - // accepted. - // - // SEQUENCE { - // [0] { SEQUENCE { [2] "ok.example" }} -- permittedSubtrees - // [1] { SEQUENCE { [2] "bad.example" }} -- excludedSubtrees - // } - let extn_value: &[u8] = &[ - 0x30, 0x21, 0xa0, 0x0e, 0x30, 0x0c, 0x82, 0x0a, b'o', b'k', b'.', b'e', b'x', b'a', - b'm', b'p', b'l', b'e', 0xa1, 0x0f, 0x30, 0x0d, 0x82, 0x0b, b'b', b'a', b'd', b'.', - b'e', b'x', b'a', b'm', b'p', b'l', b'e', - ]; - let extn = Extension { - extn_id: NAME_CONSTRAINTS_OID, - critical: true, - extn_value, - }; - assert!(ca::name_constraints(&policy, &verification_cert, Some(&extn)).is_ok()); - } } From 1d9a61c098b44b25c934cd9aa2ca138df5926321 Mon Sep 17 00:00:00 2001 From: Eirik Botten Nicolaysen Date: Wed, 2 Sep 2026 09:51:56 +0200 Subject: [PATCH 3/5] Restore test_ca_name_constraints_empty_permitted_subtrees CI pins x509-limbo to 972626160c26b45426bbd8c935a605219bd93207 (2026-08-27), which predates C2SP/x509-limbo#658, so the new cases are not in the vectors CI runs against yet. Without this test extension.rs:754-756 is uncovered and the coverage gate fails. Co-Authored-By: Claude Opus 5 --- .../src/policy/extension.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/rust/cryptography-x509-verification/src/policy/extension.rs b/src/rust/cryptography-x509-verification/src/policy/extension.rs index fc5a142419d6..27c55466aaf9 100644 --- a/src/rust/cryptography-x509-verification/src/policy/extension.rs +++ b/src/rust/cryptography-x509-verification/src/policy/extension.rs @@ -1095,6 +1095,36 @@ mod tests { .expect("failed to create policy definition") } + #[test] + fn test_ca_name_constraints_empty_permitted_subtrees() { + // The certificate is not used by this validator, so which one we use + // does not matter. + let cert_pem = v1_cert_pem(); + let cert = cert(&cert_pem); + let verification_cert = VerificationCertificate::new(&cert, ()); + let policy_def = name_constraints_policy(); + let policy = Policy::new(&policy_def, ()); + + // NameConstraints with a present-but-empty permittedSubtrees and a + // non-empty excludedSubtrees, written as raw DER because the empty + // sequence cannot be built through the writing API. + // + // SEQUENCE { + // [0] {} -- permittedSubtrees + // [1] { SEQUENCE { [2] "bad.example" }} -- excludedSubtrees + // } + let extn_value: &[u8] = &[ + 0x30, 0x13, 0xa0, 0x00, 0xa1, 0x0f, 0x30, 0x0d, 0x82, 0x0b, b'b', b'a', b'd', b'.', + b'e', b'x', b'a', b'm', b'p', b'l', b'e', + ]; + let extn = Extension { + extn_id: NAME_CONSTRAINTS_OID, + critical: true, + extn_value, + }; + assert!(ca::name_constraints(&policy, &verification_cert, Some(&extn)).is_err()); + } + #[test] fn test_ca_name_constraints_empty_excluded_subtrees() { let cert_pem = v1_cert_pem(); From 293c359c55ce3b4a00a58230838b12013d3200fd Mon Sep 17 00:00:00 2001 From: "pyca-boringbot[bot]" Date: Wed, 2 Sep 2026 10:13:09 +0000 Subject: [PATCH 4/5] Bump x509-limbo and/or wycheproof in CI Cherry-picked from pyca/cryptography#15580 at Alex's suggestion on #15560: the new pin 21cc053 is the merge commit for C2SP/x509-limbo#658, so the two new name-constraint testcases are in the vectors CI runs. (cherry picked from commit 319a8a4681d4e855faf53624d78793f963bf3006) Co-Authored-By: Claude Opus 5 --- .github/actions/fetch-vectors/action.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/fetch-vectors/action.yml b/.github/actions/fetch-vectors/action.yml index bd7923531ead..f5f9771c301c 100644 --- a/.github/actions/fetch-vectors/action.yml +++ b/.github/actions/fetch-vectors/action.yml @@ -30,5 +30,5 @@ runs: env: # Latest commit on the wycheproof main branch, as of Aug 19, 2026. WYCHEPROOF_REF: "dac1dd4729fd1f8dd9e1e9f3dce51d783da6c166" # wycheproof-ref - # Latest commit on the x509-limbo main branch, as of Aug 28, 2026. - X509_LIMBO_REF: "972626160c26b45426bbd8c935a605219bd93207" # x509-limbo-ref + # Latest commit on the x509-limbo main branch, as of Sep 02, 2026. + X509_LIMBO_REF: "21cc053f7edbd22e0e8d8a98a7fe13918912af9d" # x509-limbo-ref From f31e97fefa0ba2edeafc21c1be755c2654e0ebb0 Mon Sep 17 00:00:00 2001 From: Eirik Botten Nicolaysen Date: Wed, 2 Sep 2026 14:32:49 +0200 Subject: [PATCH 5/5] Drop test_ca_name_constraints_empty_permitted_subtrees The pin now points at 21cc053, which carries rfc5280::nc::permitted-empty-sequence-excluded-nonempty, so the branch this test guarded is exercised by the limbo vectors CI runs. test_ca_name_constraints_empty_excluded_subtrees stays: no limbo case covers a non-empty permittedSubtrees with an empty excludedSubtrees. Co-Authored-By: Claude Opus 5 --- .../src/policy/extension.rs | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/src/rust/cryptography-x509-verification/src/policy/extension.rs b/src/rust/cryptography-x509-verification/src/policy/extension.rs index 27c55466aaf9..fc5a142419d6 100644 --- a/src/rust/cryptography-x509-verification/src/policy/extension.rs +++ b/src/rust/cryptography-x509-verification/src/policy/extension.rs @@ -1095,36 +1095,6 @@ mod tests { .expect("failed to create policy definition") } - #[test] - fn test_ca_name_constraints_empty_permitted_subtrees() { - // The certificate is not used by this validator, so which one we use - // does not matter. - let cert_pem = v1_cert_pem(); - let cert = cert(&cert_pem); - let verification_cert = VerificationCertificate::new(&cert, ()); - let policy_def = name_constraints_policy(); - let policy = Policy::new(&policy_def, ()); - - // NameConstraints with a present-but-empty permittedSubtrees and a - // non-empty excludedSubtrees, written as raw DER because the empty - // sequence cannot be built through the writing API. - // - // SEQUENCE { - // [0] {} -- permittedSubtrees - // [1] { SEQUENCE { [2] "bad.example" }} -- excludedSubtrees - // } - let extn_value: &[u8] = &[ - 0x30, 0x13, 0xa0, 0x00, 0xa1, 0x0f, 0x30, 0x0d, 0x82, 0x0b, b'b', b'a', b'd', b'.', - b'e', b'x', b'a', b'm', b'p', b'l', b'e', - ]; - let extn = Extension { - extn_id: NAME_CONSTRAINTS_OID, - critical: true, - extn_value, - }; - assert!(ca::name_constraints(&policy, &verification_cert, Some(&extn)).is_err()); - } - #[test] fn test_ca_name_constraints_empty_excluded_subtrees() { let cert_pem = v1_cert_pem();