From 926db7b735d315292731e1cbb302972f4101662a Mon Sep 17 00:00:00 2001 From: John Howard Date: Fri, 17 Jan 2025 15:52:36 -0800 Subject: [PATCH 1/2] Fix: mark SAN as critical when subject is empty Fixes https://github.com/rustls/rcgen/issues/310 --- rcgen/src/certificate.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 83560713..8fd6b138 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -499,7 +499,10 @@ impl CertificateParams { return; } - write_x509_extension(writer, oid::SUBJECT_ALT_NAME, false, |writer| { + // Per https://tools.ietf.org/html/rfc5280#section-4.1.2.6, SAN must be marked + // as critical if subject is empty. + let critical = self.distinguished_name.entries.is_empty(); + write_x509_extension(writer, oid::SUBJECT_ALT_NAME, critical, |writer| { writer.write_sequence(|writer| { for san in self.subject_alt_names.iter() { writer.next().write_tagged_implicit( From 5e1cc35675bc7c8565bc6201e2f723cdbd024a55 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sun, 19 Jan 2025 11:24:11 -0500 Subject: [PATCH 2/2] tests: verify SAN ext. criticality --- rcgen/tests/generic.rs | 51 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/rcgen/tests/generic.rs b/rcgen/tests/generic.rs index 73324a42..f141fb4d 100644 --- a/rcgen/tests/generic.rs +++ b/rcgen/tests/generic.rs @@ -518,3 +518,54 @@ mod test_csr { assert_eq!(*params, csrp.params); } } + +#[cfg(feature = "x509-parser")] +mod test_subject_alternative_name_criticality { + use x509_parser::certificate::X509Certificate; + use x509_parser::extensions::X509Extension; + use x509_parser::{oid_registry, parse_x509_certificate}; + + use crate::util::default_params; + + #[test] + fn with_subject_sans_not_critical() { + let (params, keypair) = default_params(); + assert!( + !params + .distinguished_name + .iter() + .collect::>() + .is_empty(), + "non-empty subject required for test" + ); + + let cert = params.self_signed(&keypair).unwrap(); + let cert = cert.der(); + let (_, parsed) = parse_x509_certificate(cert).unwrap(); + assert!( + !san_ext(&parsed).critical, + "with subject, SAN ext should not be critical" + ); + } + + #[test] + fn without_subject_sans_critical() { + let (mut params, keypair) = default_params(); + params.distinguished_name = Default::default(); + + let cert = params.self_signed(&keypair).unwrap(); + let cert = cert.der(); + let (_, parsed) = parse_x509_certificate(cert).unwrap(); + assert!( + san_ext(&parsed).critical, + "without subject, SAN ext should be critical" + ); + } + + fn san_ext<'cert>(cert: &'cert X509Certificate) -> &'cert X509Extension<'cert> { + cert.extensions() + .iter() + .find(|ext| ext.oid == oid_registry::OID_X509_EXT_SUBJECT_ALT_NAME) + .expect("missing SAN extension") + } +}