Skip to content

Commit 39c91d2

Browse files
djcctz
authored andcommitted
Actually fail closed for URI matching against excluded subtrees
1 parent 27131d4 commit 39c91d2

2 files changed

Lines changed: 42 additions & 6 deletions

File tree

src/subject_name/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,16 @@ fn check_presented_id_conforms_to_constraints(
164164
(GeneralName::IpAddress(_), _) => continue,
165165

166166
// We currently don't support URI constraints -- fail closed for now.
167+
//
168+
// Rejection is achieved by not matching any PermittedSubtrees, and matching all
169+
// ExcludedSubtrees.
167170
(
168171
GeneralName::UniformResourceIdentifier(_),
169172
GeneralName::UniformResourceIdentifier(_),
170-
) => Ok(false),
173+
) => Ok(match subtrees {
174+
Subtrees::Permitted => false,
175+
Subtrees::Excluded => true,
176+
}),
171177
(GeneralName::UniformResourceIdentifier(_), _) => continue,
172178

173179
// RFC 4280 says "If a name constraints extension that is marked as

tests/tls_server_certs.rs

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,23 +47,53 @@ fn uri_san_rejected_against_uri_permitted_subtree() {
4747
);
4848
}
4949

50+
/// Since we don't have real constraint matching implemented for URI names, fail closed.
51+
#[test]
52+
fn uri_san_rejected_against_uri_excluded_subtree() {
53+
let ca_key = KeyPair::generate().unwrap();
54+
let mut ca_params = issuer_params("issuer.example.com").unwrap();
55+
ca_params
56+
.custom_extensions
57+
.push(uri_excluded_name_constraints(b"https://evil.example.com"));
58+
let issuer = CertifiedIssuer::self_signed(ca_params, ca_key).expect("failed to generate CA");
59+
60+
let ee = generate_cert(
61+
vec![SanType::URI("https://evil.example.com".try_into().unwrap())],
62+
&issuer,
63+
);
64+
assert_eq!(
65+
check_cert(ee.der(), issuer.der(), &[], &[], &[]),
66+
Err(webpki::Error::NameConstraintViolation),
67+
);
68+
}
69+
5070
// Hand-encode a NameConstraints extension (OID 2.5.29.30) with a single
5171
// permittedSubtree containing a URI GeneralName. rcgen's GeneralSubtree enum
5272
// doesn't expose a URI variant, so we emit the DER directly.
5373
fn uri_permitted_name_constraints(uri: &[u8]) -> CustomExtension {
74+
uri_name_constraints(uri, 0xa0) // permittedSubtrees [0] IMPLICIT
75+
}
76+
77+
// Hand-encode a NameConstraints extension (OID 2.5.29.30) with a single
78+
// excludedSubtree containing a URI GeneralName.
79+
fn uri_excluded_name_constraints(uri: &[u8]) -> CustomExtension {
80+
uri_name_constraints(uri, 0xa1) // excludedSubtrees [1] IMPLICIT
81+
}
82+
83+
fn uri_name_constraints(uri: &[u8], subtrees_tag: u8) -> CustomExtension {
5484
assert!(uri.len() < 128);
5585
// URI GeneralName: [6] IMPLICIT IA5String
5686
let mut uri_gn = vec![0x86, uri.len() as u8];
5787
uri_gn.extend_from_slice(uri);
5888
// GeneralSubtree SEQUENCE { base GeneralName, ... }
5989
let mut subtree = vec![0x30, uri_gn.len() as u8];
6090
subtree.extend_from_slice(&uri_gn);
61-
// permittedSubtrees [0] IMPLICIT GeneralSubtrees
62-
let mut permitted = vec![0xa0, subtree.len() as u8];
63-
permitted.extend_from_slice(&subtree);
91+
// permittedSubtrees [0] or excludedSubtrees [1] IMPLICIT GeneralSubtrees
92+
let mut subtrees = vec![subtrees_tag, subtree.len() as u8];
93+
subtrees.extend_from_slice(&subtree);
6494
// NameConstraints SEQUENCE
65-
let mut nc = vec![0x30, permitted.len() as u8];
66-
nc.extend_from_slice(&permitted);
95+
let mut nc = vec![0x30, subtrees.len() as u8];
96+
nc.extend_from_slice(&subtrees);
6797

6898
let mut ext = CustomExtension::from_oid_content(&[2, 5, 29, 30], nc);
6999
ext.set_criticality(true);

0 commit comments

Comments
 (0)