From 4c9677b81fd392203da0b6ac893024ba535e56a8 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 11:16:59 -0400 Subject: [PATCH 01/28] ext: introduce module for X.509 extension writing Add an `Extension` trait (OID, criticality, DER value) and a single `write_extension()` serializer so each extension's encoding lives beside its presence logic as types are migrated. Extensions whose OID and criticality are fixed by the profile defining them implement the `StaticExtension` trait instead, receiving `Extension` through a blanket impl. Move authority key identifier writing into the module as the first static extension, replacing `write_x509_authority_key_identifier()` in the certificate and CRL paths. --- rcgen/src/certificate.rs | 18 +-- rcgen/src/crl.rs | 12 +- rcgen/src/ext.rs | 249 +++++++++++++++++++++++++++++++++++++++ rcgen/src/lib.rs | 22 +--- 4 files changed, 262 insertions(+), 39 deletions(-) create mode 100644 rcgen/src/ext.rs diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 6c518018..2bf312ac 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -10,15 +10,16 @@ use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; +use crate::ext::{write_extension, AuthorityKeyIdentifier}; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] use crate::ring_like::digest; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ - oid, write_distinguished_name, write_dt_utc_or_generalized, - write_x509_authority_key_identifier, write_x509_extension, DistinguishedName, Error, Issuer, - KeyIdMethod, KeyUsagePurpose, SanType, SerialNumber, SigningKey, + oid, write_distinguished_name, write_dt_utc_or_generalized, write_x509_extension, + DistinguishedName, Error, Issuer, KeyIdMethod, KeyUsagePurpose, SanType, SerialNumber, + SigningKey, }; /// An issued certificate @@ -521,16 +522,7 @@ impl CertificateParams { issuer: &Issuer<'_, impl SigningKey>, ) -> Result<(), Error> { if self.use_authority_key_identifier_extension { - write_x509_authority_key_identifier( - writer.next(), - match issuer.key_identifier_method.as_ref() { - KeyIdMethod::PreSpecified(aki) => aki.clone(), - #[cfg(feature = "crypto")] - _ => issuer - .key_identifier_method - .derive(issuer.signing_key.subject_public_key_info()), - }, - ); + write_extension(writer.next(), &AuthorityKeyIdentifier::from(issuer)); } self.write_subject_alt_names(writer.next()); diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index 3addf637..b79e2a6a 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -4,13 +4,13 @@ use pki_types::CertificateRevocationListDer; use time::OffsetDateTime; use yasna::{DERWriter, Tag}; +use crate::ext::{write_extension, AuthorityKeyIdentifier}; use crate::key_pair::sign_der; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ dt_to_generalized, oid, write_distinguished_name, write_dt_utc_or_generalized, - write_x509_authority_key_identifier, write_x509_extension, Error, Issuer, KeyIdMethod, - KeyUsagePurpose, SerialNumber, SigningKey, + write_x509_extension, Error, Issuer, KeyIdMethod, KeyUsagePurpose, SerialNumber, SigningKey, }; /// A certificate revocation list (CRL) @@ -273,10 +273,12 @@ impl CertificateRevocationListParams { writer.next().write_tagged(Tag::context(0), |writer| { writer.write_sequence(|writer| { // Write authority key identifier. - write_x509_authority_key_identifier( + write_extension( writer.next(), - self.key_identifier_method - .derive(issuer.signing_key.subject_public_key_info()), + &AuthorityKeyIdentifier( + self.key_identifier_method + .derive(issuer.signing_key.subject_public_key_info()), + ), ); // Write CRL number. diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs new file mode 100644 index 00000000..747bca17 --- /dev/null +++ b/rcgen/src/ext.rs @@ -0,0 +1,249 @@ +use std::fmt::Debug; + +use yasna::models::ObjectIdentifier; +use yasna::{DERWriter, Tag}; + +use crate::{oid, Issuer, KeyIdMethod, SigningKey}; + +/// An X.509 extension whose OID and criticality are fixed by the profile +/// defining it. +/// +/// Implementors receive [`Extension`] through a blanket impl. Extensions that +/// decide criticality (or OID) at runtime implement [`Extension`] directly +/// instead. +pub(crate) trait StaticExtension: Debug { + /// The OID components of the extension. + const OID: &'static [u64]; + + /// The criticality of the extension. + const CRITICALITY: Criticality; + + /// Write the extension's value (the content of the extnValue OCTET STRING). + fn write_value(&self, writer: DERWriter); +} + +impl Extension for T { + fn oid(&self) -> &[u64] { + T::OID + } + + fn criticality(&self) -> Criticality { + T::CRITICALITY + } + + fn write_value(&self, writer: DERWriter) { + // Calling with fully qualified syntax to disambiguate. + StaticExtension::write_value(self, writer) + } +} + +/// An X.509 extension. +/// +/// All extensions have an OID, a criticality, and a DER encoded value for inclusion in +/// an X.509 extension SEQUENCE. +pub(crate) trait Extension: Debug { + /// Return the OID components of the extension. + fn oid(&self) -> &[u64]; + + /// Return the criticality of the extension. + fn criticality(&self) -> Criticality; + + /// Write the extension's value (the content of the extnValue OCTET STRING). + fn write_value(&self, writer: DERWriter); +} + +/// The criticality of an X.509 extension. +/// +/// This controls how consumers should handle an unrecognized extension. +/// +/// See [RFC 5280 §4.2] for more information. +/// +/// [RFC 5280 §4.2]: +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum Criticality { + /// The extension MUST be recognized and parsed correctly. + Critical, + + /// The extension MAY be ignored if it is not recognized. + NonCritical, +} + +impl From for Criticality { + fn from(critical: bool) -> Self { + match critical { + true => Self::Critical, + false => Self::NonCritical, + } + } +} + +/// Serializes an X.509v3 extension according to RFC 5280. +pub(crate) fn write_extension(writer: DERWriter, extension: &dyn Extension) { + /* + Extension ::= SEQUENCE { + extnID OBJECT IDENTIFIER, + critical BOOLEAN DEFAULT FALSE, + extnValue OCTET STRING + -- contains the DER encoding of an ASN.1 value + -- corresponding to the extension type identified + -- by extnID + } + */ + writer.write_sequence(|writer| { + writer + .next() + .write_oid(&ObjectIdentifier::from_slice(extension.oid())); + // DER requires that DEFAULT values be omitted (X.690 §11.5): the critical + // flag may only be encoded when it is TRUE. + if extension.criticality() == Criticality::Critical { + writer.next().write_bool(true); + } + writer.next().write_bytes(&yasna::construct_der(|writer| { + extension.write_value(writer) + })); + }) +} + +/// An X.509v3 authority key identifier extension according to [RFC 5280 §4.2.1.1]. +/// +/// RFC 5280 states: +/// 'The keyIdentifier field of the authorityKeyIdentifier extension MUST +/// be included in all certificates generated by conforming CAs to +/// facilitate certification path construction. There is one exception; +/// where a CA distributes its public key in the form of a "self-signed" +/// certificate, the authority key identifier MAY be omitted.' +/// In addition, for CRLs: +/// 'Conforming CRL issuers MUST use the key identifier method, and MUST +/// include this extension in all CRLs issued.' +/// +/// [RFC 5280 §4.2.1.1]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct AuthorityKeyIdentifier(pub(crate) Vec); + +impl From<&Issuer<'_, S>> for AuthorityKeyIdentifier { + fn from(issuer: &Issuer<'_, S>) -> Self { + Self(match issuer.key_identifier_method.as_ref() { + KeyIdMethod::PreSpecified(aki) => aki.clone(), + #[cfg(feature = "crypto")] + _ => issuer + .key_identifier_method + .derive(issuer.signing_key.subject_public_key_info()), + }) + } +} + +impl StaticExtension for AuthorityKeyIdentifier { + const OID: &'static [u64] = oid::AUTHORITY_KEY_IDENTIFIER; + + // RFC 5280 §4.2.1.1: "Conforming CAs MUST mark this extension as non-critical." + const CRITICALITY: Criticality = Criticality::NonCritical; + + fn write_value(&self, writer: DERWriter) { + /* + AuthorityKeyIdentifier ::= SEQUENCE { + keyIdentifier [0] KeyIdentifier OPTIONAL, + authorityCertIssuer [1] GeneralNames OPTIONAL, + authorityCertSerialNumber [2] CertificateSerialNumber OPTIONAL } + KeyIdentifier ::= OCTET STRING + */ + writer.write_sequence(|writer| { + writer + .next() + .write_tagged_implicit(Tag::context(0), |writer| writer.write_bytes(&self.0)) + }); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn critical_flag_omitted_when_false() { + // The critical flag is DEFAULT FALSE, so DER (X.690 §11.5) requires that a + // non-critical extension omit it entirely rather than encode FALSE. + // See https://github.com/rustls/rcgen/pull/444 for a past instance of this + // bug class. + let ext = DummyExt(Criticality::NonCritical); + let der = yasna::construct_der(|writer| write_extension(writer, &ext)); + assert_eq!( + der, + yasna::construct_der(|writer| { + writer.write_sequence(|writer| { + writer + .next() + .write_oid(&ObjectIdentifier::from_slice(ext.oid())); + // No BOOLEAN between the OID and the value: the critical + // flag must be absent, not encoded as FALSE. + writer + .next() + .write_bytes(&yasna::construct_der(|writer| ext.write_value(writer))); + }) + }) + ); + } + + #[test] + fn critical_flag_written_when_true() { + let ext = DummyExt(Criticality::Critical); + let der = yasna::construct_der(|writer| write_extension(writer, &ext)); + assert_eq!( + der, + yasna::construct_der(|writer| { + writer.write_sequence(|writer| { + writer + .next() + .write_oid(&ObjectIdentifier::from_slice(ext.oid())); + writer.next().write_bool(true); // critical TRUE + writer + .next() + .write_bytes(&yasna::construct_der(|writer| ext.write_value(writer))); + }) + }) + ); + } + + #[test] + fn aki_encoding() { + let ext = AuthorityKeyIdentifier(vec![0xDE, 0xAD]); + let der = yasna::construct_der(|writer| write_extension(writer, &ext)); + assert_eq!( + der, + yasna::construct_der(|writer| { + writer.write_sequence(|writer| { + writer + .next() + .write_oid(&ObjectIdentifier::from_slice(oid::AUTHORITY_KEY_IDENTIFIER)); + // Non-critical: the critical flag must be absent. + writer.next().write_bytes(&yasna::construct_der(|writer| { + // AuthorityKeyIdentifier ::= SEQUENCE { keyIdentifier [0] OCTET STRING } + writer.write_sequence(|writer| { + writer + .next() + .write_tagged_implicit(Tag::context(0), |writer| { + writer.write_bytes(&[0xDE, 0xAD]) + }) + }) + })); + }) + }) + ); + } + + #[derive(Debug)] + struct DummyExt(Criticality); + + impl Extension for DummyExt { + fn oid(&self) -> &[u64] { + &[1, 3, 6, 1, 4, 1, 99] + } + + fn criticality(&self) -> Criticality { + self.0 + } + + fn write_value(&self, writer: DERWriter) { + writer.write_null() + } + } +} diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 83816182..4cb5b256 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -74,6 +74,7 @@ mod certificate; mod crl; mod csr; mod error; +mod ext; mod key_pair; mod oid; mod ring_like; @@ -838,27 +839,6 @@ fn write_x509_extension( }) } -/// Serializes an X.509v3 authority key identifier extension according to RFC 5280. -fn write_x509_authority_key_identifier(writer: DERWriter, aki: Vec) { - // Write Authority Key Identifier - // RFC 5280 states: - // 'The keyIdentifier field of the authorityKeyIdentifier extension MUST - // be included in all certificates generated by conforming CAs to - // facilitate certification path construction. There is one exception; - // where a CA distributes its public key in the form of a "self-signed" - // certificate, the authority key identifier MAY be omitted.' - // In addition, for CRLs: - // 'Conforming CRL issuers MUST use the key identifier method, and MUST - // include this extension in all CRLs issued.' - write_x509_extension(writer, oid::AUTHORITY_KEY_IDENTIFIER, false, |writer| { - writer.write_sequence(|writer| { - writer - .next() - .write_tagged_implicit(Tag::context(0), |writer| writer.write_bytes(&aki)) - }); - }); -} - #[cfg(feature = "zeroize")] impl zeroize::Zeroize for KeyPair { fn zeroize(&mut self) { From 650126f4e0e509412fb1bd53eefef402caa20100 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 11:28:41 -0400 Subject: [PATCH 02/28] ext: move subject alternative name writing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port the SAN extension into an `ext::SubjectAlternativeName` type whose `from_params` constructor owns the presence decision, replacing `write_subject_alt_names()` in the certificate and CSR paths. The RFC 5280 §4.1.2.6 criticality rule (critical if the subject DN is empty) is now unit tested. --- rcgen/src/certificate.rs | 48 +++----------------- rcgen/src/ext.rs | 97 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 42 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 2bf312ac..f80b3879 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -10,7 +10,7 @@ use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; -use crate::ext::{write_extension, AuthorityKeyIdentifier}; +use crate::ext::{write_extension, AuthorityKeyIdentifier, SubjectAlternativeName}; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] use crate::ring_like::digest; @@ -197,7 +197,9 @@ impl CertificateParams { writer.next().write_set(|writer| { writer.next().write_sequence(|writer| { self.write_key_usage(writer.next()); - self.write_subject_alt_names(writer.next()); + if let Some(san) = SubjectAlternativeName::from_params(self) { + write_extension(writer.next(), &san); + } self.write_extended_key_usage(writer.next()); self.write_ca_extensions(writer, None); for ext in &self.custom_extensions { @@ -286,44 +288,6 @@ impl CertificateParams { }); } - fn write_subject_alt_names(&self, writer: DERWriter) { - if self.subject_alt_names.is_empty() { - return; - } - - // 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( - Tag::context(san.tag()), - |writer| match san { - SanType::Rfc822Name(name) - | SanType::DnsName(name) - | SanType::URI(name) => writer.write_ia5_string(name.as_str()), - SanType::IpAddress(IpAddr::V4(addr)) => { - writer.write_bytes(&addr.octets()) - }, - SanType::IpAddress(IpAddr::V6(addr)) => { - writer.write_bytes(&addr.octets()) - }, - SanType::OtherName((oid, value)) => { - // otherName SEQUENCE { OID, [0] explicit any defined by oid } - // https://datatracker.ietf.org/doc/html/rfc5280#page-38 - writer.write_sequence(|writer| { - writer.next().write_oid(&ObjectIdentifier::from_slice(oid)); - value.write_der(writer.next()); - }); - }, - }, - ); - } - }); - }); - } - /// Generate and serialize a certificate signing request (CSR). /// /// The constructed CSR will contain attributes based on the certificate parameters, @@ -525,7 +489,9 @@ impl CertificateParams { write_extension(writer.next(), &AuthorityKeyIdentifier::from(issuer)); } - self.write_subject_alt_names(writer.next()); + if let Some(san) = SubjectAlternativeName::from_params(self) { + write_extension(writer.next(), &san); + } self.write_key_usage(writer.next()); self.write_extended_key_usage(writer.next()); diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 747bca17..ba5bb7c8 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -1,9 +1,10 @@ use std::fmt::Debug; +use std::net::IpAddr; use yasna::models::ObjectIdentifier; use yasna::{DERWriter, Tag}; -use crate::{oid, Issuer, KeyIdMethod, SigningKey}; +use crate::{oid, CertificateParams, Issuer, KeyIdMethod, SanType, SigningKey}; /// An X.509 extension whose OID and criticality are fixed by the profile /// defining it. @@ -154,6 +155,72 @@ impl StaticExtension for AuthorityKeyIdentifier { } } +/// An X.509v3 subject alternative name extension according to [RFC 5280 §4.2.1.6]. +/// +/// [RFC 5280 §4.2.1.6]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct SubjectAlternativeName<'params> { + criticality: Criticality, + names: &'params [SanType], +} + +impl<'params> SubjectAlternativeName<'params> { + pub(crate) fn from_params(params: &'params CertificateParams) -> Option { + // GeneralNames ::= SEQUENCE SIZE (1..MAX): an empty SAN can't be encoded, + // so the extension is omitted (RFC 5280 §4.2.1.6). + if params.subject_alt_names.is_empty() { + return None; + } + + Some(Self { + // Per RFC 5280 §4.1.2.6, SAN must be marked critical if the subject + // is an empty sequence, and SHOULD be non-critical otherwise. + criticality: params.distinguished_name.entries.is_empty().into(), + names: ¶ms.subject_alt_names, + }) + } + + fn write_name(writer: DERWriter, san: &SanType) { + writer.write_tagged_implicit(Tag::context(san.tag()), |writer| match san { + SanType::Rfc822Name(name) | SanType::DnsName(name) | SanType::URI(name) => { + writer.write_ia5_string(name.as_str()) + }, + SanType::IpAddress(IpAddr::V4(addr)) => writer.write_bytes(&addr.octets()), + SanType::IpAddress(IpAddr::V6(addr)) => writer.write_bytes(&addr.octets()), + SanType::OtherName((oid, value)) => { + // otherName SEQUENCE { OID, [0] explicit any defined by oid } + // https://datatracker.ietf.org/doc/html/rfc5280#page-38 + writer.write_sequence(|writer| { + writer.next().write_oid(&ObjectIdentifier::from_slice(oid)); + value.write_der(writer.next()); + }); + }, + }) + } +} + +impl Extension for SubjectAlternativeName<'_> { + fn oid(&self) -> &[u64] { + oid::SUBJECT_ALT_NAME + } + + fn criticality(&self) -> Criticality { + self.criticality + } + + fn write_value(&self, writer: DERWriter) { + /* + SubjectAltName ::= GeneralNames + GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName + */ + writer.write_sequence(|writer| { + for san in self.names.iter() { + Self::write_name(writer.next(), san); + } + }); + } +} + #[cfg(test)] mod tests { use super::*; @@ -230,6 +297,34 @@ mod tests { ); } + #[test] + fn san_absent_when_no_names() { + assert!(SubjectAlternativeName::from_params(&CertificateParams::default()).is_none()); + } + + #[test] + fn san_critical_when_subject_empty() { + // RFC 5280 §4.1.2.6: SAN must be critical if the subject is an empty sequence. + let mut params = CertificateParams { + subject_alt_names: vec![SanType::DnsName("example.com".try_into().unwrap())], + ..CertificateParams::default() + }; + assert_eq!( + SubjectAlternativeName::from_params(¶ms) + .unwrap() + .criticality(), + Criticality::NonCritical + ); + + params.distinguished_name = crate::DistinguishedName::new(); + assert_eq!( + SubjectAlternativeName::from_params(¶ms) + .unwrap() + .criticality(), + Criticality::Critical + ); + } + #[derive(Debug)] struct DummyExt(Criticality); From e9886b78c95a1f550ede33678075c6ad822fb144 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 11:31:08 -0400 Subject: [PATCH 03/28] ext: move key usage writing Port the KeyUsage extension into an `ext::KeyUsage` static extension whose `from_params` constructor owns the presence decision, replacing `write_key_usage()` in the certificate and CSR paths. The minimal-length BIT STRING encoding (including the 9-bit `decipherOnly` case) remains covered by the existing certificate tests. --- rcgen/src/certificate.rs | 35 ++++++-------------------- rcgen/src/ext.rs | 54 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 29 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index f80b3879..0456a67c 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -10,7 +10,7 @@ use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; -use crate::ext::{write_extension, AuthorityKeyIdentifier, SubjectAlternativeName}; +use crate::ext::{write_extension, AuthorityKeyIdentifier, KeyUsage, SubjectAlternativeName}; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] use crate::ring_like::digest; @@ -196,7 +196,9 @@ impl CertificateParams { )); writer.next().write_set(|writer| { writer.next().write_sequence(|writer| { - self.write_key_usage(writer.next()); + if let Some(ku) = KeyUsage::from_params(self) { + write_extension(writer.next(), &ku); + } if let Some(san) = SubjectAlternativeName::from_params(self) { write_extension(writer.next(), &san); } @@ -212,31 +214,6 @@ impl CertificateParams { }); } - /// Write a certificate's KeyUsage as defined in RFC 5280. - fn write_key_usage(&self, writer: DERWriter) { - if self.key_usages.is_empty() { - return; - } - - // "When present, conforming CAs SHOULD mark this extension as critical." - write_x509_extension(writer, oid::KEY_USAGE, true, |writer| { - // u16 is large enough to encode the largest possible key usage (two-bytes) - let bit_string = self.key_usages.iter().fold(0u16, |bit_string, key_usage| { - bit_string | key_usage.to_u16() - }); - - match u16::BITS - bit_string.trailing_zeros() { - bits @ 0..=8 => { - writer.write_bitvec_bytes(&bit_string.to_be_bytes()[..1], bits as usize) - }, - bits @ 9..=16 => { - writer.write_bitvec_bytes(&bit_string.to_be_bytes(), bits as usize) - }, - _ => unreachable!(), - } - }); - } - fn write_extended_key_usage(&self, writer: DERWriter) { if !self.extended_key_usages.is_empty() { write_x509_extension(writer, oid::EXT_KEY_USAGE, false, |writer| { @@ -492,7 +469,9 @@ impl CertificateParams { if let Some(san) = SubjectAlternativeName::from_params(self) { write_extension(writer.next(), &san); } - self.write_key_usage(writer.next()); + if let Some(ku) = KeyUsage::from_params(self) { + write_extension(writer.next(), &ku); + } self.write_extended_key_usage(writer.next()); if let Some(name_constraints) = &self.name_constraints { diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index ba5bb7c8..d92bb2a6 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -4,7 +4,7 @@ use std::net::IpAddr; use yasna::models::ObjectIdentifier; use yasna::{DERWriter, Tag}; -use crate::{oid, CertificateParams, Issuer, KeyIdMethod, SanType, SigningKey}; +use crate::{oid, CertificateParams, Issuer, KeyIdMethod, KeyUsagePurpose, SanType, SigningKey}; /// An X.509 extension whose OID and criticality are fixed by the profile /// defining it. @@ -221,6 +221,58 @@ impl Extension for SubjectAlternativeName<'_> { } } +/// An X.509v3 key usage extension according to [RFC 5280 §4.2.1.3]. +/// +/// [RFC 5280 §4.2.1.3]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct KeyUsage<'params>(&'params [KeyUsagePurpose]); + +impl<'params> KeyUsage<'params> { + pub(crate) fn from_params(params: &'params CertificateParams) -> Option { + if params.key_usages.is_empty() { + return None; + } + + Some(Self(¶ms.key_usages)) + } +} + +impl StaticExtension for KeyUsage<'_> { + const OID: &'static [u64] = oid::KEY_USAGE; + + // RFC 5280 §4.2.1.3: "When present, conforming CAs SHOULD mark this extension + // as critical." + const CRITICALITY: Criticality = Criticality::Critical; + + fn write_value(&self, writer: DERWriter) { + /* + KeyUsage ::= BIT STRING { + digitalSignature (0), + nonRepudiation (1), -- recent editions of X.509 have + -- renamed this bit to contentCommitment + keyEncipherment (2), + dataEncipherment (3), + keyAgreement (4), + keyCertSign (5), + cRLSign (6), + encipherOnly (7), + decipherOnly (8) } + */ + // u16 is large enough to encode the largest possible key usage (two-bytes) + let bit_string = self.0.iter().fold(0u16, |bit_string, key_usage| { + bit_string | key_usage.to_u16() + }); + + match u16::BITS - bit_string.trailing_zeros() { + bits @ 0..=8 => { + writer.write_bitvec_bytes(&bit_string.to_be_bytes()[..1], bits as usize) + }, + bits @ 9..=16 => writer.write_bitvec_bytes(&bit_string.to_be_bytes(), bits as usize), + _ => unreachable!(), + } + } +} + #[cfg(test)] mod tests { use super::*; From c63bdfa41db4871598f24ae798866919d45c3789 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 11:35:42 -0400 Subject: [PATCH 04/28] ext: move extended key usage writing Port the EKU extension into an `ext::ExtendedKeyUsage` static extension whose `from_params` constructor owns the presence decision, replacing `write_extended_key_usage()` in the certificate and CSR paths. --- rcgen/src/certificate.rs | 28 +++++++++---------------- rcgen/src/ext.rs | 44 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 0456a67c..d212b50b 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -10,7 +10,9 @@ use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; -use crate::ext::{write_extension, AuthorityKeyIdentifier, KeyUsage, SubjectAlternativeName}; +use crate::ext::{ + write_extension, AuthorityKeyIdentifier, ExtendedKeyUsage, KeyUsage, SubjectAlternativeName, +}; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] use crate::ring_like::digest; @@ -202,7 +204,9 @@ impl CertificateParams { if let Some(san) = SubjectAlternativeName::from_params(self) { write_extension(writer.next(), &san); } - self.write_extended_key_usage(writer.next()); + if let Some(eku) = ExtendedKeyUsage::from_params(self) { + write_extension(writer.next(), &eku); + } self.write_ca_extensions(writer, None); for ext in &self.custom_extensions { write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { @@ -214,20 +218,6 @@ impl CertificateParams { }); } - fn write_extended_key_usage(&self, writer: DERWriter) { - if !self.extended_key_usages.is_empty() { - write_x509_extension(writer, oid::EXT_KEY_USAGE, false, |writer| { - writer.write_sequence(|writer| { - for usage in &self.extended_key_usages { - writer - .next() - .write_oid(&ObjectIdentifier::from_slice(usage.oid())); - } - }); - }); - } - } - /// Write a certificate's BasicConstraints as defined in RFC 5280. fn write_ca_extensions(&self, writer: &mut DERWriterSeq, pub_key_spki: Option<&[u8]>) { let is_ca = match &self.is_ca { @@ -472,7 +462,9 @@ impl CertificateParams { if let Some(ku) = KeyUsage::from_params(self) { write_extension(writer.next(), &ku); } - self.write_extended_key_usage(writer.next()); + if let Some(eku) = ExtendedKeyUsage::from_params(self) { + write_extension(writer.next(), &eku); + } if let Some(name_constraints) = &self.name_constraints { // If both trees are empty, the extension must be omitted. @@ -740,7 +732,7 @@ impl ExtendedKeyUsagePurpose { Ok(extended_key_usages) } - fn oid(&self) -> &[u64] { + pub(crate) fn oid(&self) -> &[u64] { use ExtendedKeyUsagePurpose::*; match self { // anyExtendedKeyUsage diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index d92bb2a6..1b0c7209 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -4,7 +4,10 @@ use std::net::IpAddr; use yasna::models::ObjectIdentifier; use yasna::{DERWriter, Tag}; -use crate::{oid, CertificateParams, Issuer, KeyIdMethod, KeyUsagePurpose, SanType, SigningKey}; +use crate::{ + oid, CertificateParams, ExtendedKeyUsagePurpose, Issuer, KeyIdMethod, KeyUsagePurpose, SanType, + SigningKey, +}; /// An X.509 extension whose OID and criticality are fixed by the profile /// defining it. @@ -273,6 +276,45 @@ impl StaticExtension for KeyUsage<'_> { } } +/// An X.509v3 extended key usage extension according to [RFC 5280 §4.2.1.12]. +/// +/// [RFC 5280 §4.2.1.12]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ExtendedKeyUsage<'params>(&'params [ExtendedKeyUsagePurpose]); + +impl<'params> ExtendedKeyUsage<'params> { + pub(crate) fn from_params(params: &'params CertificateParams) -> Option { + if params.extended_key_usages.is_empty() { + return None; + } + + Some(Self(¶ms.extended_key_usages)) + } +} + +impl StaticExtension for ExtendedKeyUsage<'_> { + const OID: &'static [u64] = oid::EXT_KEY_USAGE; + + // RFC 5280 §4.2.1.12: "This extension MAY, at the option of the certificate + // issuer, be either critical or non-critical." + // TODO(XXX): make this configurable? + const CRITICALITY: Criticality = Criticality::NonCritical; + + fn write_value(&self, writer: DERWriter) { + /* + ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId + KeyPurposeId ::= OBJECT IDENTIFIER + */ + writer.write_sequence(|writer| { + for usage in self.0.iter() { + writer + .next() + .write_oid(&ObjectIdentifier::from_slice(usage.oid())); + } + }); + } +} + #[cfg(test)] mod tests { use super::*; From 45a2e2b40c9d17a26af2c4346120a565e7c754d3 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 11:38:40 -0400 Subject: [PATCH 05/28] ext: move name constraints writing Port the NameConstraints extension into an `ext::NameConstraints` static extension whose `from_params` constructor owns the presence decision (including omitting the extension when both subtrees are empty), replacing the inline writer and `write_general_subtrees()` in the certificate path. --- rcgen/src/certificate.rs | 58 +++-------------------- rcgen/src/ext.rs | 99 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 53 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index d212b50b..649742a6 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -11,7 +11,8 @@ use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; use crate::ext::{ - write_extension, AuthorityKeyIdentifier, ExtendedKeyUsage, KeyUsage, SubjectAlternativeName, + write_extension, AuthorityKeyIdentifier, ExtendedKeyUsage, KeyUsage, + NameConstraints as NameConstraintsExt, SubjectAlternativeName, }; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] @@ -466,28 +467,8 @@ impl CertificateParams { write_extension(writer.next(), &eku); } - if let Some(name_constraints) = &self.name_constraints { - // If both trees are empty, the extension must be omitted. - if !name_constraints.is_empty() { - write_x509_extension(writer.next(), oid::NAME_CONSTRAINTS, true, |writer| { - writer.write_sequence(|writer| { - if !name_constraints.permitted_subtrees.is_empty() { - write_general_subtrees( - writer.next(), - 0, - &name_constraints.permitted_subtrees, - ); - } - if !name_constraints.excluded_subtrees.is_empty() { - write_general_subtrees( - writer.next(), - 1, - &name_constraints.excluded_subtrees, - ); - } - }); - }); - } + if let Some(nc) = NameConstraintsExt::from_params(self) { + write_extension(writer.next(), &nc); } if !self.crl_distribution_points.is_empty() { @@ -530,31 +511,6 @@ impl AsRef for CertificateParams { } } -fn write_general_subtrees(writer: DERWriter, tag: u64, general_subtrees: &[GeneralSubtree]) { - writer.write_tagged_implicit(Tag::context(tag), |writer| { - writer.write_sequence(|writer| { - for subtree in general_subtrees.iter() { - writer.next().write_sequence(|writer| { - let writer = writer.next(); - let tag = Tag::context(subtree.tag()); - match subtree { - GeneralSubtree::Rfc822Name(name) | GeneralSubtree::DnsName(name) => writer - .write_tagged_implicit(tag, |writer| writer.write_ia5_string(name)), - // `Name` is a CHOICE, so X.680 §31.2.7 requires explicit tagging. - GeneralSubtree::DirectoryName(name) => writer - .write_tagged(tag, |writer| write_distinguished_name(writer, name)), - GeneralSubtree::IpAddress(subnet) => writer - .write_tagged_implicit(tag, |writer| { - writer.write_bytes(&subnet.to_bytes()) - }), - } - // minimum must be 0 (the default) and maximum must be absent - }); - } - }); - }); -} - /// A PKCS #10 CSR attribute, as defined in [RFC 5280] and constrained /// by [RFC 2986]. /// @@ -793,7 +749,7 @@ impl NameConstraints { })) } - fn is_empty(&self) -> bool { + pub(crate) fn is_empty(&self) -> bool { self.permitted_subtrees.is_empty() && self.excluded_subtrees.is_empty() } } @@ -847,7 +803,7 @@ impl GeneralSubtree { Ok(result) } - fn tag(&self) -> u64 { + pub(crate) fn tag(&self) -> u64 { // Defined in the GeneralName list in // https://tools.ietf.org/html/rfc5280#page-38 const TAG_RFC822_NAME: u64 = 1; @@ -917,7 +873,7 @@ impl CidrSubnet { pub fn from_v6_prefix(addr: [u8; 16], prefix: u8) -> Self { CidrSubnet::V6(addr, mask!(u128, prefix)) } - fn to_bytes(self) -> Vec { + pub(crate) fn to_bytes(self) -> Vec { let mut res = Vec::new(); match self { CidrSubnet::V4(addr, mask) => { diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 1b0c7209..f0812d8d 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -5,8 +5,8 @@ use yasna::models::ObjectIdentifier; use yasna::{DERWriter, Tag}; use crate::{ - oid, CertificateParams, ExtendedKeyUsagePurpose, Issuer, KeyIdMethod, KeyUsagePurpose, SanType, - SigningKey, + oid, write_distinguished_name, CertificateParams, ExtendedKeyUsagePurpose, GeneralSubtree, + Issuer, KeyIdMethod, KeyUsagePurpose, SanType, SigningKey, }; /// An X.509 extension whose OID and criticality are fixed by the profile @@ -315,6 +315,87 @@ impl StaticExtension for ExtendedKeyUsage<'_> { } } +/// An X.509v3 name constraints extension according to [RFC 5280 §4.2.1.10]. +/// +/// [RFC 5280 §4.2.1.10]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct NameConstraints<'params> { + permitted_subtrees: &'params [GeneralSubtree], + excluded_subtrees: &'params [GeneralSubtree], +} + +impl<'params> NameConstraints<'params> { + pub(crate) fn from_params(params: &'params CertificateParams) -> Option { + match ¶ms.name_constraints { + // If both subtrees are empty, the extension must be omitted. + Some(nc) if !nc.is_empty() => Some(Self { + permitted_subtrees: &nc.permitted_subtrees, + excluded_subtrees: &nc.excluded_subtrees, + }), + _ => None, + } + } + + fn write_general_subtrees(writer: DERWriter, tag: u64, general_subtrees: &[GeneralSubtree]) { + /* + GeneralSubtrees ::= SEQUENCE SIZE (1..MAX) OF GeneralSubtree + GeneralSubtree ::= SEQUENCE { + base GeneralName, + minimum [0] BaseDistance DEFAULT 0, + maximum [1] BaseDistance OPTIONAL } + BaseDistance ::= INTEGER (0..MAX) + */ + writer.write_tagged_implicit(Tag::context(tag), |writer| { + writer.write_sequence(|writer| { + for subtree in general_subtrees.iter() { + writer.next().write_sequence(|writer| { + let writer = writer.next(); + let tag = Tag::context(subtree.tag()); + match subtree { + GeneralSubtree::Rfc822Name(name) | GeneralSubtree::DnsName(name) => { + writer.write_tagged_implicit(tag, |writer| { + writer.write_ia5_string(name) + }) + }, + // `Name` is a CHOICE, so X.680 §31.2.7 requires explicit tagging. + GeneralSubtree::DirectoryName(name) => writer + .write_tagged(tag, |writer| write_distinguished_name(writer, name)), + GeneralSubtree::IpAddress(subnet) => writer + .write_tagged_implicit(tag, |writer| { + writer.write_bytes(&subnet.to_bytes()) + }), + } + // minimum must be 0 (the default) and maximum must be absent + }); + } + }); + }); + } +} + +impl StaticExtension for NameConstraints<'_> { + const OID: &'static [u64] = oid::NAME_CONSTRAINTS; + + // RFC 5280 §4.2.1.10: "Conforming CAs MUST mark this extension as critical." + const CRITICALITY: Criticality = Criticality::Critical; + + fn write_value(&self, writer: DERWriter) { + /* + NameConstraints ::= SEQUENCE { + permittedSubtrees [0] GeneralSubtrees OPTIONAL, + excludedSubtrees [1] GeneralSubtrees OPTIONAL } + */ + writer.write_sequence(|writer| { + if !self.permitted_subtrees.is_empty() { + Self::write_general_subtrees(writer.next(), 0, self.permitted_subtrees); + } + if !self.excluded_subtrees.is_empty() { + Self::write_general_subtrees(writer.next(), 1, self.excluded_subtrees); + } + }); + } +} + #[cfg(test)] mod tests { use super::*; @@ -391,6 +472,20 @@ mod tests { ); } + #[test] + fn name_constraints_absent_when_subtrees_empty() { + // A name constraints extension with no permitted or excluded subtrees + // would violate SEQUENCE SIZE (1..MAX) and must be omitted. + let params = CertificateParams { + name_constraints: Some(crate::NameConstraints { + permitted_subtrees: Vec::new(), + excluded_subtrees: Vec::new(), + }), + ..CertificateParams::default() + }; + assert!(NameConstraints::from_params(¶ms).is_none()); + } + #[test] fn san_absent_when_no_names() { assert!(SubjectAlternativeName::from_params(&CertificateParams::default()).is_none()); From 1eeb9be8881dffe311512a344bb49e0ca6db743f Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 11:40:21 -0400 Subject: [PATCH 06/28] ext: move CRL distribution points writing Port the certificate CRLDistributionPoints extension into an `ext::CrlDistributionPoints` static extension whose `from_params` constructor owns the presence decision, replacing the inline writer in the certificate path. The empty-URIs rejection in `serialize_der_with_signer` is unchanged. --- rcgen/src/certificate.rs | 17 +++-------------- rcgen/src/ext.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 649742a6..4ae46402 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -11,7 +11,7 @@ use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; use crate::ext::{ - write_extension, AuthorityKeyIdentifier, ExtendedKeyUsage, KeyUsage, + write_extension, AuthorityKeyIdentifier, CrlDistributionPoints, ExtendedKeyUsage, KeyUsage, NameConstraints as NameConstraintsExt, SubjectAlternativeName, }; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; @@ -471,19 +471,8 @@ impl CertificateParams { write_extension(writer.next(), &nc); } - if !self.crl_distribution_points.is_empty() { - write_x509_extension( - writer.next(), - oid::CRL_DISTRIBUTION_POINTS, - false, - |writer| { - writer.write_sequence(|writer| { - for distribution_point in &self.crl_distribution_points { - distribution_point.write_der(writer.next()); - } - }) - }, - ); + if let Some(crl_dps) = CrlDistributionPoints::from_params(self) { + write_extension(writer.next(), &crl_dps); } self.write_ca_extensions(writer, Some(pub_key_spki)); diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index f0812d8d..9ab0c290 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -4,6 +4,7 @@ use std::net::IpAddr; use yasna::models::ObjectIdentifier; use yasna::{DERWriter, Tag}; +use crate::crl::CrlDistributionPoint; use crate::{ oid, write_distinguished_name, CertificateParams, ExtendedKeyUsagePurpose, GeneralSubtree, Issuer, KeyIdMethod, KeyUsagePurpose, SanType, SigningKey, @@ -396,6 +397,38 @@ impl StaticExtension for NameConstraints<'_> { } } +/// An X.509v3 CRL distribution points extension according to [RFC 5280 §4.2.1.13]. +/// +/// [RFC 5280 §4.2.1.13]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct CrlDistributionPoints<'params>(&'params [CrlDistributionPoint]); + +impl<'params> CrlDistributionPoints<'params> { + pub(crate) fn from_params(params: &'params CertificateParams) -> Option { + if params.crl_distribution_points.is_empty() { + return None; + } + + Some(Self(¶ms.crl_distribution_points)) + } +} + +impl StaticExtension for CrlDistributionPoints<'_> { + const OID: &'static [u64] = oid::CRL_DISTRIBUTION_POINTS; + + // RFC 5280 §4.2.1.13: "The extension SHOULD be non-critical". + const CRITICALITY: Criticality = Criticality::NonCritical; + + fn write_value(&self, writer: DERWriter) { + // CRLDistributionPoints ::= SEQUENCE SIZE (1..MAX) OF DistributionPoint + writer.write_sequence(|writer| { + for distribution_point in self.0 { + distribution_point.write_der(writer.next()); + } + }) + } +} + #[cfg(test)] mod tests { use super::*; From 413e30ec6b8b141c041d4ec3458e28b929ae918f Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 11:43:04 -0400 Subject: [PATCH 07/28] ext: move subject key identifier writing Port the SKI extension into an `ext::SubjectKeyIdentifier` static extension constructed from a `KeyIdMethod` and the subject public key info. The current behavior of only emitting SKI for `IsCa::Ca`/`ExplicitNoCa` certificates (and never for CSRs) is preserved. --- rcgen/src/certificate.rs | 28 +++++++++++++--------------- rcgen/src/ext.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 4ae46402..9ee54a08 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -12,7 +12,7 @@ use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; use crate::ext::{ write_extension, AuthorityKeyIdentifier, CrlDistributionPoints, ExtendedKeyUsage, KeyUsage, - NameConstraints as NameConstraintsExt, SubjectAlternativeName, + NameConstraints as NameConstraintsExt, SubjectAlternativeName, SubjectKeyIdentifier, }; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] @@ -208,7 +208,7 @@ impl CertificateParams { if let Some(eku) = ExtendedKeyUsage::from_params(self) { write_extension(writer.next(), &eku); } - self.write_ca_extensions(writer, None); + self.write_ca_extensions(writer); for ext in &self.custom_extensions { write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { writer.write_der(ext.content()) @@ -220,24 +220,13 @@ impl CertificateParams { } /// Write a certificate's BasicConstraints as defined in RFC 5280. - fn write_ca_extensions(&self, writer: &mut DERWriterSeq, pub_key_spki: Option<&[u8]>) { + fn write_ca_extensions(&self, writer: &mut DERWriterSeq) { let is_ca = match &self.is_ca { IsCa::Ca(bc) => Some(bc), IsCa::ExplicitNoCa => None, IsCa::NoCa => return, }; - if let Some(pub_key_spki) = pub_key_spki { - write_x509_extension( - writer.next(), - oid::SUBJECT_KEY_IDENTIFIER, - false, - |writer| { - writer.write_bytes(&self.key_identifier_method.derive(pub_key_spki)); - }, - ); - } - // Write basic_constraints write_x509_extension(writer.next(), oid::BASIC_CONSTRAINTS, true, |writer| { writer.write_sequence(|writer| { @@ -475,7 +464,16 @@ impl CertificateParams { write_extension(writer.next(), &crl_dps); } - self.write_ca_extensions(writer, Some(pub_key_spki)); + // SKI is currently only written for CA certificates (IsCa::Ca or + // IsCa::ExplicitNoCa). + if self.is_ca != IsCa::NoCa { + write_extension( + writer.next(), + &SubjectKeyIdentifier::new(&self.key_identifier_method, pub_key_spki), + ); + } + + self.write_ca_extensions(writer); for ext in &self.custom_extensions { write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 9ab0c290..56647b51 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -429,6 +429,33 @@ impl StaticExtension for CrlDistributionPoints<'_> { } } +/// An X.509v3 subject key identifier extension according to [RFC 5280 §4.2.1.2]. +/// +/// [RFC 5280 §4.2.1.2]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct SubjectKeyIdentifier(Vec); + +impl SubjectKeyIdentifier { + pub(crate) fn new(key_identifier_method: &KeyIdMethod, pub_key_spki: &[u8]) -> Self { + Self(key_identifier_method.derive(pub_key_spki)) + } +} + +impl StaticExtension for SubjectKeyIdentifier { + const OID: &'static [u64] = oid::SUBJECT_KEY_IDENTIFIER; + + // RFC 5280 §4.2.1.2: "Conforming CAs MUST mark this extension as non-critical." + const CRITICALITY: Criticality = Criticality::NonCritical; + + fn write_value(&self, writer: DERWriter) { + /* + SubjectKeyIdentifier ::= KeyIdentifier + KeyIdentifier ::= OCTET STRING + */ + writer.write_bytes(&self.0) + } +} + #[cfg(test)] mod tests { use super::*; From cab08e146c6c0841e0789c58e39eb42c04e0e715 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 12:36:26 -0400 Subject: [PATCH 08/28] rcgen: bump version to 0.15.0 The following commits make semver-incompatible changes to the public extension API. --- Cargo.lock | 2 +- rcgen/Cargo.toml | 2 +- rustls-cert-gen/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2faca654..d8bac6e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -822,7 +822,7 @@ checksum = "76afc826de14238e6e8c374ddcc1fa19e374fd8dd986b0d2af0d02377261d83c" [[package]] name = "rcgen" -version = "0.14.10" +version = "0.15.0" dependencies = [ "aws-lc-rs", "openssl", diff --git a/rcgen/Cargo.toml b/rcgen/Cargo.toml index 68ad52d7..580aae3d 100644 --- a/rcgen/Cargo.toml +++ b/rcgen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rcgen" -version = "0.14.10" +version = "0.15.0" documentation = "https://docs.rs/rcgen" description.workspace = true repository.workspace = true diff --git a/rustls-cert-gen/Cargo.toml b/rustls-cert-gen/Cargo.toml index 28f7c300..1fa60399 100644 --- a/rustls-cert-gen/Cargo.toml +++ b/rustls-cert-gen/Cargo.toml @@ -23,7 +23,7 @@ aws-lc-rs = { workspace = true, optional = true } bpaf = { workspace = true } pem = { workspace = true } pki-types = { workspace = true } -rcgen = { version = "0.14.2", path = "../rcgen", default-features = false, features = ["pem"] } +rcgen = { version = "0.15.0", path = "../rcgen", default-features = false, features = ["pem"] } ring = { workspace = true, optional = true } [dev-dependencies] From dcb1dab8f7cee4b2e6113e5cf3fa0a3123d26445 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 12:52:36 -0400 Subject: [PATCH 09/28] lib: rename `BasicConstraints` to `PathLenConstraint` The enum represents the optional pathLenConstraint field of the basic constraints extension, not the extension itself. Renaming it frees the `BasicConstraints` name for the extension type introduced next, without needing to disambiguate between the two. --- rcgen/examples/sign-leaf-with-ca.rs | 6 +++--- rcgen/src/certificate.rs | 18 +++++++++--------- rcgen/src/crl.rs | 6 +++--- rcgen/src/csr.rs | 8 ++++---- rcgen/src/lib.rs | 4 ++-- rustls-cert-gen/src/cert.rs | 12 ++++++++---- verify-tests/src/lib.rs | 9 ++++----- verify-tests/tests/botan.rs | 13 ++++++------- verify-tests/tests/generic.rs | 6 +++--- verify-tests/tests/openssl.rs | 14 +++++++------- verify-tests/tests/webpki.rs | 20 ++++++++++---------- 11 files changed, 59 insertions(+), 57 deletions(-) diff --git a/rcgen/examples/sign-leaf-with-ca.rs b/rcgen/examples/sign-leaf-with-ca.rs index bfa08eeb..7373e181 100644 --- a/rcgen/examples/sign-leaf-with-ca.rs +++ b/rcgen/examples/sign-leaf-with-ca.rs @@ -1,7 +1,7 @@ use rcgen::DnValue::PrintableString; use rcgen::{ - BasicConstraints, Certificate, CertificateParams, DnType, ExtendedKeyUsagePurpose, IsCa, - Issuer, KeyPair, KeyUsagePurpose, + Certificate, CertificateParams, DnType, ExtendedKeyUsagePurpose, IsCa, Issuer, KeyPair, + KeyUsagePurpose, PathLenConstraint, }; use time::{Duration, OffsetDateTime}; @@ -21,7 +21,7 @@ fn new_ca() -> (Certificate, Issuer<'static, KeyPair>) { let mut params = CertificateParams::new(Vec::default()).expect("empty subject alt name can't produce error"); let (yesterday, tomorrow) = validity_period(); - params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); params.distinguished_name.push( DnType::CountryName, PrintableString("BR".try_into().unwrap()), diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 9ee54a08..0a58306f 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -236,8 +236,8 @@ impl CertificateParams { writer.next().write_bool(true); // cA flag match constraints { - BasicConstraints::Unconstrained => {}, - BasicConstraints::Constrained(path_len_constraint) => { + PathLenConstraint::Unconstrained => {}, + PathLenConstraint::Constrained(path_len_constraint) => { writer.next().write_u8(*path_len_constraint); // pathLenConstraint integer }, } @@ -926,7 +926,7 @@ pub enum IsCa { /// The certificate can only sign itself, adding the extension and `CA:FALSE` ExplicitNoCa, /// The certificate may be used to sign other certificates - Ca(BasicConstraints), + Ca(PathLenConstraint), } impl IsCa { @@ -953,7 +953,7 @@ impl IsCa { B { ca: true, path_len_constraint: Some(n), - } if *n <= u8::MAX as u32 => Self::Ca(BasicConstraints::Constrained(*n as u8)), + } if *n <= u8::MAX as u32 => Self::Ca(PathLenConstraint::Constrained(*n as u8)), B { ca: true, path_len_constraint: Some(_), @@ -961,7 +961,7 @@ impl IsCa { B { ca: true, path_len_constraint: None, - } => Self::Ca(BasicConstraints::Unconstrained), + } => Self::Ca(PathLenConstraint::Unconstrained), B { ca: false, .. } => Self::ExplicitNoCa, }) } @@ -972,7 +972,7 @@ impl IsCa { /// Sets an optional upper limit on the length of the intermediate certificate chain /// length allowed for this CA certificate (not including the end entity certificate). #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] -pub enum BasicConstraints { +pub enum PathLenConstraint { /// No constraint Unconstrained, /// Constrain to the contained number of intermediate certificates @@ -1007,7 +1007,7 @@ mod tests { KeyUsagePurpose::ContentCommitment, ], // This can sign things! - is_ca: IsCa::Ca(BasicConstraints::Constrained(0)), + is_ca: IsCa::Ca(PathLenConstraint::Constrained(0)), ..CertificateParams::default() }; @@ -1142,7 +1142,7 @@ mod tests { // Set key usages key_usages: vec![KeyUsagePurpose::DecipherOnly], // This can sign things! - is_ca: IsCa::Ca(BasicConstraints::Constrained(0)), + is_ca: IsCa::Ca(PathLenConstraint::Constrained(0)), ..CertificateParams::default() }; @@ -1313,7 +1313,7 @@ mod tests { params.subject_alt_names.push(ip_san.clone()); // Because we're using a function for CA certificates - params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); // Serialize our cert that has our chosen san, so we can testing parsing/deserializing it. let cert = params.self_signed(&ca_key).unwrap(); diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index b79e2a6a..63ee8bbb 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -36,7 +36,7 @@ use crate::{ /// // Generate a CRL issuer. /// let mut issuer_params = CertificateParams::new(vec!["crl.issuer.example.com".to_string()]).unwrap(); /// issuer_params.serial_number = Some(SerialNumber::from(9999)); -/// issuer_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); +/// issuer_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); /// issuer_params.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::DigitalSignature, KeyUsagePurpose::CrlSign]; /// #[cfg(feature = "crypto")] /// let key_pair = KeyPair::generate().unwrap(); @@ -430,7 +430,7 @@ mod tests { use x509_parser::{oid_registry, parse_x509_crl}; use super::*; - use crate::{date_time_ymd, BasicConstraints, CertificateParams, IsCa, KeyPair}; + use crate::{date_time_ymd, CertificateParams, IsCa, KeyPair, PathLenConstraint}; #[test] fn test_empty_issuing_distribution_point_uris_rejected() { @@ -515,7 +515,7 @@ mod tests { let mut issuer_params = CertificateParams::new(vec!["crl.issuer.example.com".to_string()]).unwrap(); issuer_params.serial_number = Some(SerialNumber::from(9999u64)); - issuer_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + issuer_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); issuer_params.key_usages = vec![ KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::DigitalSignature, diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index 28bd5c34..f3f8051d 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -95,7 +95,7 @@ impl CertificateSigningRequestParams { /// - `Subject Alternative Name` (see [`SanType`]) /// - `Key Usage` (see [`KeyUsagePurpose`]) /// - `Extended Key Usage` (see [`ExtendedKeyUsagePurpose`]) - /// - `Basic Constraints` (see [`crate::BasicConstraints`]) + /// - `Basic Constraints` (see [`crate::PathLenConstraint`]) /// /// On encountering other extensions, this function will return [`Error::UnsupportedExtension`]. /// If the request's signature is invalid, it will return @@ -218,8 +218,8 @@ mod tests { use x509_parser::prelude::{FromDer, ParsedExtension}; use crate::{ - BasicConstraints, CertificateParams, CertificateSigningRequestParams, - ExtendedKeyUsagePurpose, IsCa, KeyPair, KeyUsagePurpose, + CertificateParams, CertificateSigningRequestParams, ExtendedKeyUsagePurpose, IsCa, KeyPair, + KeyUsagePurpose, PathLenConstraint, }; #[test] @@ -282,7 +282,7 @@ mod tests { #[test] fn serialize_and_deserialize_eq_basic_constraints() { let params = CertificateParams { - is_ca: IsCa::Ca(BasicConstraints::Constrained(10)), + is_ca: IsCa::Ca(PathLenConstraint::Constrained(10)), ..Default::default() }; let key_pair = KeyPair::generate().unwrap(); diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 4cb5b256..2d0f5915 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -42,8 +42,8 @@ use std::net::{Ipv4Addr, Ipv6Addr}; use std::ops::Deref; pub use certificate::{ - date_time_ymd, Attribute, BasicConstraints, Certificate, CertificateParams, CidrSubnet, - CustomExtension, DnType, ExtendedKeyUsagePurpose, GeneralSubtree, IsCa, NameConstraints, + date_time_ymd, Attribute, Certificate, CertificateParams, CidrSubnet, CustomExtension, DnType, + ExtendedKeyUsagePurpose, GeneralSubtree, IsCa, NameConstraints, PathLenConstraint, }; pub use crl::{ CertificateRevocationList, CertificateRevocationListParams, CrlDistributionPoint, diff --git a/rustls-cert-gen/src/cert.rs b/rustls-cert-gen/src/cert.rs index 3b3625d2..59a2f0c9 100644 --- a/rustls-cert-gen/src/cert.rs +++ b/rustls-cert-gen/src/cert.rs @@ -6,8 +6,9 @@ use std::{fmt, io}; use bpaf::Bpaf; use rcgen::DnValue::PrintableString; use rcgen::{ - BasicConstraints, Certificate, CertificateParams, CertifiedIssuer, DistinguishedName, DnType, - ExtendedKeyUsagePurpose, IsCa, KeyPair, KeyUsagePurpose, SanType, SignatureAlgorithm, + Certificate, CertificateParams, CertifiedIssuer, DistinguishedName, DnType, + ExtendedKeyUsagePurpose, IsCa, KeyPair, KeyUsagePurpose, PathLenConstraint, SanType, + SignatureAlgorithm, }; /// Builder to configure TLS [CertificateParams] to be finalized @@ -64,7 +65,7 @@ pub struct CaBuilder { impl CaBuilder { /// Initialize `CaBuilder` pub fn new(mut params: CertificateParams, alg: KeyPairAlgorithm) -> Self { - params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); params.key_usages.push(KeyUsagePurpose::DigitalSignature); params.key_usages.push(KeyUsagePurpose::KeyCertSign); params.key_usages.push(KeyUsagePurpose::CrlSign); @@ -319,7 +320,10 @@ mod tests { #[test] fn init_ca() { let cert = CertificateBuilder::new().certificate_authority(); - assert_eq!(cert.params.is_ca, IsCa::Ca(BasicConstraints::Unconstrained)) + assert_eq!( + cert.params.is_ca, + IsCa::Ca(PathLenConstraint::Unconstrained) + ) } #[test] fn with_sig_algo_default() -> anyhow::Result<()> { diff --git a/verify-tests/src/lib.rs b/verify-tests/src/lib.rs index 466f3105..54997a10 100644 --- a/verify-tests/src/lib.rs +++ b/verify-tests/src/lib.rs @@ -1,8 +1,7 @@ use rcgen::{ - BasicConstraints, Certificate, CertificateParams, CertificateRevocationList, - CertificateRevocationListParams, CrlDistributionPoint, CrlIssuingDistributionPoint, CrlScope, - DnType, IsCa, Issuer, KeyIdMethod, KeyPair, KeyUsagePurpose, RevocationReason, - RevokedCertParams, SerialNumber, + Certificate, CertificateParams, CertificateRevocationList, CertificateRevocationListParams, + CrlDistributionPoint, CrlIssuingDistributionPoint, CrlScope, DnType, IsCa, Issuer, KeyIdMethod, + KeyPair, KeyUsagePurpose, PathLenConstraint, RevocationReason, RevokedCertParams, SerialNumber, }; use time::{Duration, OffsetDateTime}; @@ -82,7 +81,7 @@ pub fn test_crl() -> ( Certificate, ) { let (mut issuer, key_pair) = default_params(); - issuer.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + issuer.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); issuer.key_usages = vec![ KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::DigitalSignature, diff --git a/verify-tests/tests/botan.rs b/verify-tests/tests/botan.rs index 76c48a60..5ba394ca 100644 --- a/verify-tests/tests/botan.rs +++ b/verify-tests/tests/botan.rs @@ -1,9 +1,8 @@ #![cfg(feature = "x509-parser")] use rcgen::{ - BasicConstraints, Certificate, CertificateParams, CertificateRevocationListParams, DnType, - DnValue, IsCa, Issuer, KeyPair, KeyUsagePurpose, RevocationReason, RevokedCertParams, - SerialNumber, + Certificate, CertificateParams, CertificateRevocationListParams, DnType, DnValue, IsCa, Issuer, + KeyPair, KeyUsagePurpose, PathLenConstraint, RevocationReason, RevokedCertParams, SerialNumber, }; use time::{Duration, OffsetDateTime}; use verify_tests as util; @@ -128,7 +127,7 @@ fn test_botan_rsa_given() { #[test] fn test_botan_separate_ca() { let (mut ca_params, ca_key) = default_params(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_cert = ca_params.self_signed(&ca_key).unwrap(); let mut params = CertificateParams::new(vec!["crabs.crabs".to_string()]).unwrap(); @@ -151,7 +150,7 @@ fn test_botan_separate_ca() { #[test] fn test_botan_imported_ca() { let (mut params, ca_key) = default_params(); - params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_cert = params.self_signed(&ca_key).unwrap(); let ca_cert_der = ca_cert.der(); let ca = Issuer::from_ca_cert_der(ca_cert.der(), ca_key).unwrap(); @@ -179,7 +178,7 @@ fn test_botan_imported_ca_with_printable_string() { DnType::CountryName, DnValue::PrintableString("US".try_into().unwrap()), ); - params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_cert = params.self_signed(&imported_ca_key).unwrap(); let ca = Issuer::from_ca_cert_der(ca_cert.der(), imported_ca_key).unwrap(); @@ -203,7 +202,7 @@ fn test_botan_crl_parse() { // Create an issuer CA. let alg = &rcgen::PKCS_ECDSA_P256_SHA256; let (mut issuer, _) = util::default_params(); - issuer.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + issuer.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); issuer.key_usages = vec![ KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::DigitalSignature, diff --git a/verify-tests/tests/generic.rs b/verify-tests/tests/generic.rs index 0837d527..1ddd4549 100644 --- a/verify-tests/tests/generic.rs +++ b/verify-tests/tests/generic.rs @@ -172,7 +172,7 @@ mod test_csr_custom_attributes { #[cfg(feature = "x509-parser")] mod test_csr_basic_constraints { - use rcgen::{BasicConstraints, CertificateSigningRequestParams, Error, IsCa}; + use rcgen::{CertificateSigningRequestParams, Error, IsCa, PathLenConstraint}; /// Tests deserializing a csr with a basic constraint of CA:TRUE,pathlen:5 /// @@ -185,7 +185,7 @@ mod test_csr_basic_constraints { assert_eq!( csr_params.params.is_ca, - IsCa::Ca(BasicConstraints::Constrained(5)) + IsCa::Ca(PathLenConstraint::Constrained(5)) ); } @@ -258,7 +258,7 @@ RioOvAyCH6bFMvSJxZm7FYM= assert_eq!( csr_params.params.is_ca, - IsCa::Ca(BasicConstraints::Unconstrained) + IsCa::Ca(PathLenConstraint::Unconstrained) ); } diff --git a/verify-tests/tests/openssl.rs b/verify-tests/tests/openssl.rs index c19d22f2..4366135d 100644 --- a/verify-tests/tests/openssl.rs +++ b/verify-tests/tests/openssl.rs @@ -12,8 +12,8 @@ use openssl::stack::Stack; use openssl::x509::store::{X509Store, X509StoreBuilder}; use openssl::x509::{CrlStatus, X509Crl, X509Req, X509StoreContext, X509}; use rcgen::{ - BasicConstraints, Certificate, CertificateParams, DistinguishedName, DnType, DnValue, - GeneralSubtree, IsCa, Issuer, KeyPair, NameConstraints, + Certificate, CertificateParams, DistinguishedName, DnType, DnValue, GeneralSubtree, IsCa, + Issuer, KeyPair, NameConstraints, PathLenConstraint, }; use verify_tests as util; @@ -306,7 +306,7 @@ fn test_openssl_rsa_combinations_given() { #[test] fn test_openssl_separate_ca() { let (mut ca_params, ca_key) = util::default_params(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_cert = ca_params.self_signed(&ca_key).unwrap(); let ca_cert_pem = ca_cert.pem(); let ca = Issuer::new(ca_params, ca_key); @@ -332,7 +332,7 @@ fn test_openssl_separate_ca_with_printable_string() { DnType::CountryName, DnValue::PrintableString("US".try_into().unwrap()), ); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_cert = ca_params.self_signed(&ca_key).unwrap(); let mut params = CertificateParams::new(vec!["crabs.crabs".to_string()]).unwrap(); @@ -353,7 +353,7 @@ fn test_openssl_separate_ca_with_printable_string() { #[test] fn test_openssl_separate_ca_with_other_signing_alg() { let (mut ca_params, _) = util::default_params(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_key = KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).unwrap(); let ca_cert = ca_params.self_signed(&ca_key).unwrap(); let ca = Issuer::new(ca_params, ca_key); @@ -375,7 +375,7 @@ fn test_openssl_separate_ca_with_other_signing_alg() { #[test] fn test_openssl_separate_ca_name_constraints() { let (mut ca_params, ca_key) = util::default_params(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); println!("openssl version: {:x}", openssl::version::number()); @@ -406,7 +406,7 @@ fn test_openssl_separate_ca_name_constraints() { #[test] fn test_openssl_separate_ca_name_constraints_directory_name() { let (mut ca_params, ca_key) = util::default_params(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let mut permitted = DistinguishedName::new(); permitted.push(DnType::OrganizationName, "Crab widgits SE"); diff --git a/verify-tests/tests/webpki.rs b/verify-tests/tests/webpki.rs index e03cb10c..89627492 100644 --- a/verify-tests/tests/webpki.rs +++ b/verify-tests/tests/webpki.rs @@ -6,9 +6,9 @@ use aws_lc_rs::signature::{ }; use pki_types::{CertificateDer, ServerName, SignatureVerificationAlgorithm, UnixTime}; use rcgen::{ - BasicConstraints, Certificate, CertificateParams, CertificateRevocationListParams, DnType, - Error, ExtendedKeyUsagePurpose, IsCa, Issuer, KeyPair, KeyUsagePurpose, PublicKeyData, - RevocationReason, RevokedCertParams, SerialNumber, SigningKey, + Certificate, CertificateParams, CertificateRevocationListParams, DnType, Error, + ExtendedKeyUsagePurpose, IsCa, Issuer, KeyPair, KeyUsagePurpose, PathLenConstraint, + PublicKeyData, RevocationReason, RevokedCertParams, SerialNumber, SigningKey, }; #[cfg(feature = "x509-parser")] use rcgen::{CertificateSigningRequestParams, DnValue}; @@ -308,7 +308,7 @@ fn test_webpki_rsa_combinations_given() { #[test] fn test_webpki_separate_ca() { let (mut ca_params, ca_key) = util::default_params(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_cert = ca_params.self_signed(&ca_key).unwrap(); let mut params = CertificateParams::new(vec!["crabs.crabs".to_string()]).unwrap(); @@ -336,7 +336,7 @@ fn test_webpki_separate_ca() { #[test] fn test_webpki_separate_ca_with_other_signing_alg() { let (mut ca_params, _) = util::default_params(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_key = KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).unwrap(); let ca_cert = ca_params.self_signed(&ca_key).unwrap(); @@ -425,7 +425,7 @@ fn from_remote() { #[test] fn test_webpki_separate_ca_name_constraints() { let mut params = util::default_params(); - params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); params.name_constraints = Some(NameConstraints { // TODO also add a test with non-empty permitted_subtrees that // doesn't contain a DirectoryName entry. This isn't possible @@ -461,7 +461,7 @@ fn test_webpki_separate_ca_name_constraints() { #[test] fn test_webpki_imported_ca() { let (mut params, ca_key) = util::default_params(); - params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); params.key_usages.push(KeyUsagePurpose::KeyCertSign); let ca_cert = params.self_signed(&ca_key).unwrap(); @@ -497,7 +497,7 @@ fn test_webpki_imported_ca_with_printable_string() { DnType::CountryName, DnValue::PrintableString("US".try_into().unwrap()), ); - params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); let ca_cert = params.self_signed(&ca_key).unwrap(); let ca = Issuer::from_ca_cert_der(ca_cert.der(), ca_key).unwrap(); @@ -556,7 +556,7 @@ fn test_certificate_from_csr() { } let (mut ca_params, ca_key) = util::default_params(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); for eku in &eku_test { ca_params.insert_extended_key_usage(eku.clone()); } @@ -651,7 +651,7 @@ fn test_webpki_crl_revoke() { // Create an issuer CA. let alg = &rcgen::PKCS_ECDSA_P256_SHA256; let (mut issuer, _) = util::default_params(); - issuer.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + issuer.is_ca = IsCa::Ca(PathLenConstraint::Unconstrained); issuer.key_usages = vec![ KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::DigitalSignature, From bc6f98dd17da4f12c55fe8e1c9d9945847376199 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 12:54:34 -0400 Subject: [PATCH 10/28] ext: move basic constraints writing Port the BasicConstraints extension into an `ext::BasicConstraints` static extension whose `from_params` constructor owns the presence decision (extension omitted entirely for `IsCa::NoCa`), removing `write_ca_extensions()`. --- rcgen/src/certificate.rs | 39 ++++------------- rcgen/src/ext.rs | 95 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 31 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 0a58306f..3b2d0007 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -11,8 +11,9 @@ use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; use crate::ext::{ - write_extension, AuthorityKeyIdentifier, CrlDistributionPoints, ExtendedKeyUsage, KeyUsage, - NameConstraints as NameConstraintsExt, SubjectAlternativeName, SubjectKeyIdentifier, + write_extension, AuthorityKeyIdentifier, BasicConstraints, CrlDistributionPoints, + ExtendedKeyUsage, KeyUsage, NameConstraints as NameConstraintsExt, SubjectAlternativeName, + SubjectKeyIdentifier, }; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] @@ -208,7 +209,9 @@ impl CertificateParams { if let Some(eku) = ExtendedKeyUsage::from_params(self) { write_extension(writer.next(), &eku); } - self.write_ca_extensions(writer); + if let Some(bc) = BasicConstraints::from_params(self) { + write_extension(writer.next(), &bc); + } for ext in &self.custom_extensions { write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { writer.write_der(ext.content()) @@ -219,32 +222,6 @@ impl CertificateParams { }); } - /// Write a certificate's BasicConstraints as defined in RFC 5280. - fn write_ca_extensions(&self, writer: &mut DERWriterSeq) { - let is_ca = match &self.is_ca { - IsCa::Ca(bc) => Some(bc), - IsCa::ExplicitNoCa => None, - IsCa::NoCa => return, - }; - - // Write basic_constraints - write_x509_extension(writer.next(), oid::BASIC_CONSTRAINTS, true, |writer| { - writer.write_sequence(|writer| { - let Some(constraints) = is_ca else { - return; - }; - - writer.next().write_bool(true); // cA flag - match constraints { - PathLenConstraint::Unconstrained => {}, - PathLenConstraint::Constrained(path_len_constraint) => { - writer.next().write_u8(*path_len_constraint); // pathLenConstraint integer - }, - } - }); - }); - } - /// Generate and serialize a certificate signing request (CSR). /// /// The constructed CSR will contain attributes based on the certificate parameters, @@ -473,7 +450,9 @@ impl CertificateParams { ); } - self.write_ca_extensions(writer); + if let Some(bc) = BasicConstraints::from_params(self) { + write_extension(writer.next(), &bc); + } for ext in &self.custom_extensions { write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 56647b51..49f0dda1 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -7,7 +7,7 @@ use yasna::{DERWriter, Tag}; use crate::crl::CrlDistributionPoint; use crate::{ oid, write_distinguished_name, CertificateParams, ExtendedKeyUsagePurpose, GeneralSubtree, - Issuer, KeyIdMethod, KeyUsagePurpose, SanType, SigningKey, + IsCa, Issuer, KeyIdMethod, KeyUsagePurpose, PathLenConstraint, SanType, SigningKey, }; /// An X.509 extension whose OID and criticality are fixed by the profile @@ -456,6 +456,56 @@ impl StaticExtension for SubjectKeyIdentifier { } } +/// An X.509v3 basic constraints extension according to [RFC 5280 §4.2.1.9]. +/// +/// [RFC 5280 §4.2.1.9]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct BasicConstraints(IsCa); + +impl BasicConstraints { + pub(crate) fn from_params(params: &CertificateParams) -> Option { + // For IsCa::NoCa the extension is omitted entirely: absence implies the + // certificate is not a CA. Use IsCa::ExplicitNoCa to emit the extension + // with cA absent (FALSE). + if params.is_ca == IsCa::NoCa { + return None; + } + + Some(Self(params.is_ca)) + } +} + +impl StaticExtension for BasicConstraints { + const OID: &'static [u64] = oid::BASIC_CONSTRAINTS; + + // RFC 5280 §4.2.1.9: "Conforming CAs MUST include this extension in all CA + // certificates that contain public keys used to validate digital signatures + // on certificates and MUST mark the extension as critical in such + // certificates." + const CRITICALITY: Criticality = Criticality::Critical; + + fn write_value(&self, writer: DERWriter) { + /* + BasicConstraints ::= SEQUENCE { + cA BOOLEAN DEFAULT FALSE, + pathLenConstraint INTEGER (0..MAX) OPTIONAL } + */ + writer.write_sequence(|writer| { + let IsCa::Ca(constraints) = &self.0 else { + // The cA flag is DEFAULT FALSE, so DER (X.690 §11.5) requires it + // to be omitted when false: the extension value is an empty + // SEQUENCE. + return; + }; + + writer.next().write_bool(true); // cA flag + if let PathLenConstraint::Constrained(path_len_constraint) = constraints { + writer.next().write_u8(*path_len_constraint); // pathLenConstraint integer + } + }); + } +} + #[cfg(test)] mod tests { use super::*; @@ -532,6 +582,49 @@ mod tests { ); } + #[test] + fn basic_constraints_absent_for_no_ca() { + // IsCa::NoCa means no BasicConstraints extension at all. + assert!(BasicConstraints::from_params(&CertificateParams::default()).is_none()); + } + + #[test] + fn basic_constraints_encoding() { + // The cA flag is DEFAULT FALSE, so DER (X.690 §11.5) requires that + // ExplicitNoCa encode as an empty SEQUENCE with the flag omitted. + // See https://github.com/rustls/rcgen/pull/444. + for (is_ca, expected) in [ + ( + // cA absent (FALSE): an empty SEQUENCE. + IsCa::ExplicitNoCa, + yasna::construct_der(|writer| writer.write_sequence(|_writer| {})), + ), + ( + IsCa::Ca(PathLenConstraint::Unconstrained), + yasna::construct_der(|writer| { + writer.write_sequence(|writer| writer.next().write_bool(true)) + }), + ), + ( + IsCa::Ca(PathLenConstraint::Constrained(5)), + yasna::construct_der(|writer| { + writer.write_sequence(|writer| { + writer.next().write_bool(true); + writer.next().write_u8(5); + }) + }), + ), + ] { + let params = CertificateParams { + is_ca, + ..CertificateParams::default() + }; + let bc = BasicConstraints::from_params(¶ms).unwrap(); + let value = yasna::construct_der(|writer| StaticExtension::write_value(&bc, writer)); + assert_eq!(value, expected, "unexpected encoding for {is_ca:?}"); + } + } + #[test] fn name_constraints_absent_when_subtrees_empty() { // A name constraints extension with no permitted or excluded subtrees From bdecd361f5e7f1f377fa1c39b5ad2a091fe77ee4 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 12:55:07 -0400 Subject: [PATCH 11/28] ext: write custom extensions through the Extension trait Implement `Extension` for `&CustomExtension` so the certificate and CSR paths write user-supplied extensions through `ext::write_extension()` like the built-in ones, borrowing them from the params rather than cloning. `CustomExtension` now stores a `Criticality` instead of a `bool`, converting at the public accessors. --- rcgen/src/certificate.rs | 33 ++++++++++++++------------------- rcgen/src/ext.rs | 21 ++++++++++++++++++--- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 3b2d0007..4766e5b6 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -11,7 +11,7 @@ use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; use crate::ext::{ - write_extension, AuthorityKeyIdentifier, BasicConstraints, CrlDistributionPoints, + write_extension, AuthorityKeyIdentifier, BasicConstraints, Criticality, CrlDistributionPoints, ExtendedKeyUsage, KeyUsage, NameConstraints as NameConstraintsExt, SubjectAlternativeName, SubjectKeyIdentifier, }; @@ -21,9 +21,8 @@ use crate::ring_like::digest; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ - oid, write_distinguished_name, write_dt_utc_or_generalized, write_x509_extension, - DistinguishedName, Error, Issuer, KeyIdMethod, KeyUsagePurpose, SanType, SerialNumber, - SigningKey, + oid, write_distinguished_name, write_dt_utc_or_generalized, DistinguishedName, Error, Issuer, + KeyIdMethod, KeyUsagePurpose, SanType, SerialNumber, SigningKey, }; /// An issued certificate @@ -212,10 +211,8 @@ impl CertificateParams { if let Some(bc) = BasicConstraints::from_params(self) { write_extension(writer.next(), &bc); } - for ext in &self.custom_extensions { - write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { - writer.write_der(ext.content()) - }); + for custom_ext in &self.custom_extensions { + write_extension(writer.next(), &custom_ext); } }); }); @@ -454,10 +451,8 @@ impl CertificateParams { write_extension(writer.next(), &bc); } - for ext in &self.custom_extensions { - write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { - writer.write_der(ext.content()) - }); + for custom_ext in &self.custom_extensions { + write_extension(writer.next(), &custom_ext); } Ok(()) @@ -500,11 +495,11 @@ pub struct Attribute { /// [RFC 5280](https://tools.ietf.org/html/rfc5280#section-4.2) #[derive(Debug, PartialEq, Eq, Hash, Clone)] pub struct CustomExtension { - oid: Vec, - critical: bool, + pub(crate) oid: Vec, + pub(crate) criticality: Criticality, /// The content must be DER-encoded - content: Vec, + pub(crate) content: Vec, } impl CustomExtension { @@ -519,7 +514,7 @@ impl CustomExtension { }); Self { oid: oid::PE_ACME.to_owned(), - critical: true, + criticality: Criticality::Critical, content, } } @@ -527,17 +522,17 @@ impl CustomExtension { pub fn from_oid_content(oid: &[u64], content: Vec) -> Self { Self { oid: oid.to_owned(), - critical: false, + criticality: Criticality::NonCritical, content, } } /// Sets the criticality flag of the extension. pub fn set_criticality(&mut self, criticality: bool) { - self.critical = criticality; + self.criticality = criticality.into(); } /// Obtains the criticality flag of the extension. pub fn criticality(&self) -> bool { - self.critical + self.criticality == Criticality::Critical } /// Obtains the content of the extension. pub fn content(&self) -> &[u8] { diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 49f0dda1..2a9717cc 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -6,8 +6,9 @@ use yasna::{DERWriter, Tag}; use crate::crl::CrlDistributionPoint; use crate::{ - oid, write_distinguished_name, CertificateParams, ExtendedKeyUsagePurpose, GeneralSubtree, - IsCa, Issuer, KeyIdMethod, KeyUsagePurpose, PathLenConstraint, SanType, SigningKey, + oid, write_distinguished_name, CertificateParams, CustomExtension, ExtendedKeyUsagePurpose, + GeneralSubtree, IsCa, Issuer, KeyIdMethod, KeyUsagePurpose, PathLenConstraint, SanType, + SigningKey, }; /// An X.509 extension whose OID and criticality are fixed by the profile @@ -64,7 +65,7 @@ pub(crate) trait Extension: Debug { /// See [RFC 5280 §4.2] for more information. /// /// [RFC 5280 §4.2]: -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub(crate) enum Criticality { /// The extension MUST be recognized and parsed correctly. Critical, @@ -506,6 +507,20 @@ impl StaticExtension for BasicConstraints { } } +impl Extension for &CustomExtension { + fn oid(&self) -> &[u64] { + &self.oid + } + + fn criticality(&self) -> Criticality { + self.criticality + } + + fn write_value(&self, writer: DERWriter) { + writer.write_der(&self.content) + } +} + #[cfg(test)] mod tests { use super::*; From 55ae28b9e4a83d91d91a1d68ae83113105e40592 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 12:56:14 -0400 Subject: [PATCH 12/28] ext: move CRL number and issuing distribution point writing Port both CRL-level extensions to static extensions: a `CrlNumber` type in the ext module built via `From<&SerialNumber>`, and a `StaticExtension` impl directly on the existing `CrlIssuingDistributionPoint` params type, replacing the inline writers in the CRL serialization path. --- rcgen/src/crl.rs | 31 ++++++++++++++++--------------- rcgen/src/ext.rs | 27 ++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index 63ee8bbb..5c0356e0 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -4,7 +4,9 @@ use pki_types::CertificateRevocationListDer; use time::OffsetDateTime; use yasna::{DERWriter, Tag}; -use crate::ext::{write_extension, AuthorityKeyIdentifier}; +use crate::ext::{ + write_extension, AuthorityKeyIdentifier, Criticality, CrlNumber, StaticExtension, +}; use crate::key_pair::sign_der; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; @@ -282,20 +284,11 @@ impl CertificateRevocationListParams { ); // Write CRL number. - write_x509_extension(writer.next(), oid::CRL_NUMBER, false, |writer| { - writer.write_bigint_bytes(self.crl_number.as_ref(), true); - }); + write_extension(writer.next(), &CrlNumber::from(&self.crl_number)); // Write issuing distribution point (if present). - if let Some(issuing_distribution_point) = &self.issuing_distribution_point { - write_x509_extension( - writer.next(), - oid::CRL_ISSUING_DISTRIBUTION_POINT, - true, - |writer| { - issuing_distribution_point.write_der(writer); - }, - ); + if let Some(idp) = &self.issuing_distribution_point { + write_extension(writer.next(), &idp); } }); }); @@ -316,8 +309,16 @@ pub struct CrlIssuingDistributionPoint { pub scope: Option, } -impl CrlIssuingDistributionPoint { - fn write_der(&self, writer: DERWriter) { +// An X.509v3 issuing distribution point extension according to RFC 5280 §5.2.5 +// (). +impl StaticExtension for &CrlIssuingDistributionPoint { + const OID: &'static [u64] = oid::CRL_ISSUING_DISTRIBUTION_POINT; + + // RFC 5280 §5.2.5: "Although the extension is critical, conforming + // implementations are not required to support this extension." + const CRITICALITY: Criticality = Criticality::Critical; + + fn write_value(&self, writer: DERWriter) { // IssuingDistributionPoint SEQUENCE writer.write_sequence(|writer| { // distributionPoint [0] DistributionPointName OPTIONAL diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 2a9717cc..076c6d63 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -8,7 +8,7 @@ use crate::crl::CrlDistributionPoint; use crate::{ oid, write_distinguished_name, CertificateParams, CustomExtension, ExtendedKeyUsagePurpose, GeneralSubtree, IsCa, Issuer, KeyIdMethod, KeyUsagePurpose, PathLenConstraint, SanType, - SigningKey, + SerialNumber, SigningKey, }; /// An X.509 extension whose OID and criticality are fixed by the profile @@ -521,6 +521,31 @@ impl Extension for &CustomExtension { } } +/// An X.509v3 CRL number extension according to [RFC 5280 §5.2.3]. +/// +/// [RFC 5280 §5.2.3]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct CrlNumber<'params>(&'params SerialNumber); + +impl<'params> From<&'params SerialNumber> for CrlNumber<'params> { + fn from(number: &'params SerialNumber) -> Self { + Self(number) + } +} + +impl StaticExtension for CrlNumber<'_> { + const OID: &'static [u64] = oid::CRL_NUMBER; + + // RFC 5280 §5.2.3: "CRL issuers conforming to this profile MUST include this + // extension in all CRLs and MUST mark this extension as non-critical." + const CRITICALITY: Criticality = Criticality::NonCritical; + + fn write_value(&self, writer: DERWriter) { + // CRLNumber ::= INTEGER (0..MAX) + writer.write_bigint_bytes(self.0.as_ref(), true); + } +} + #[cfg(test)] mod tests { use super::*; From 8816572a0b9e665d4d4c8218b11b34a1dd77aff9 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 12:57:11 -0400 Subject: [PATCH 13/28] ext: move CRL entry extension writing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port the reasonCode and invalidityDate CRL entry extensions into `ext::ReasonCode` and `ext::InvalidityDate` static extensions. The `from_params` constructors own the presence decisions (filtering `unspecified(0)` per RFC 5280 §5.3.1), so the crlEntryExtensions presence gate and the writers now share a single source of truth. The unconditional GeneralizedTime encoding for invalidityDate (RFC 5280 §5.3.2) is preserved and remains covered by the existing CRL tests. With no callers left, the `write_x509_extension()` helper is deleted. --- rcgen/src/crl.rs | 38 +++++++----------------- rcgen/src/ext.rs | 77 +++++++++++++++++++++++++++++++++++++++++++++--- rcgen/src/lib.rs | 30 +------------------ 3 files changed, 85 insertions(+), 60 deletions(-) diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index 5c0356e0..320d1bc7 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -5,14 +5,15 @@ use time::OffsetDateTime; use yasna::{DERWriter, Tag}; use crate::ext::{ - write_extension, AuthorityKeyIdentifier, Criticality, CrlNumber, StaticExtension, + write_extension, AuthorityKeyIdentifier, Criticality, CrlNumber, InvalidityDate, ReasonCode, + StaticExtension, }; use crate::key_pair::sign_der; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ - dt_to_generalized, oid, write_distinguished_name, write_dt_utc_or_generalized, - write_x509_extension, Error, Issuer, KeyIdMethod, KeyUsagePurpose, SerialNumber, SigningKey, + oid, write_distinguished_name, write_dt_utc_or_generalized, Error, Issuer, KeyIdMethod, + KeyUsagePurpose, SerialNumber, SigningKey, }; /// A certificate revocation list (CRL) @@ -389,35 +390,18 @@ impl RevokedCertParams { // optional for conforming CRL issuers and applications. However, CRL // issuers SHOULD include reason codes (Section 5.3.1) and invalidity // dates (Section 5.3.2) whenever this information is available. - // RFC 5280 §5.3.1: "The reason code CRL entry extension SHOULD be - // absent instead of using the unspecified (0) reasonCode value." - let reason_code = self - .reason_code - .filter(|reason| *reason != RevocationReason::Unspecified); - let has_invalidity_date = self.invalidity_date.is_some(); - if reason_code.is_some() || has_invalidity_date { + let reason_code = ReasonCode::from_params(self); + let invalidity_date = InvalidityDate::from_params(self); + if reason_code.is_some() || invalidity_date.is_some() { writer.next().write_sequence(|writer| { // Write reason code if present. - if let Some(reason_code) = reason_code { - write_x509_extension(writer.next(), oid::CRL_REASONS, false, |writer| { - writer.write_enum(reason_code as i64); - }); + if let Some(reason_code) = &reason_code { + write_extension(writer.next(), reason_code); } // Write invalidity date if present. - // RFC 5280 §5.3.2: InvalidityDate ::= GeneralizedTime. - // Unlike the Time CHOICE used elsewhere, dates in the - // UTCTime range (1950-2049) must still be encoded as - // GeneralizedTime. - if let Some(invalidity_date) = self.invalidity_date { - write_x509_extension( - writer.next(), - oid::CRL_INVALIDITY_DATE, - false, - |writer| { - writer.write_generalized_time(&dt_to_generalized(invalidity_date)); - }, - ) + if let Some(invalidity_date) = &invalidity_date { + write_extension(writer.next(), invalidity_date); } }); } diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 076c6d63..43c6146e 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -1,14 +1,15 @@ use std::fmt::Debug; use std::net::IpAddr; +use time::OffsetDateTime; use yasna::models::ObjectIdentifier; use yasna::{DERWriter, Tag}; -use crate::crl::CrlDistributionPoint; +use crate::crl::{CrlDistributionPoint, RevocationReason, RevokedCertParams}; use crate::{ - oid, write_distinguished_name, CertificateParams, CustomExtension, ExtendedKeyUsagePurpose, - GeneralSubtree, IsCa, Issuer, KeyIdMethod, KeyUsagePurpose, PathLenConstraint, SanType, - SerialNumber, SigningKey, + dt_to_generalized, oid, write_distinguished_name, CertificateParams, CustomExtension, + ExtendedKeyUsagePurpose, GeneralSubtree, IsCa, Issuer, KeyIdMethod, KeyUsagePurpose, + PathLenConstraint, SanType, SerialNumber, SigningKey, }; /// An X.509 extension whose OID and criticality are fixed by the profile @@ -546,6 +547,74 @@ impl StaticExtension for CrlNumber<'_> { } } +/// An X.509v3 CRL reason code entry extension according to [RFC 5280 §5.3.1]. +/// +/// [RFC 5280 §5.3.1]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ReasonCode(RevocationReason); + +impl ReasonCode { + pub(crate) fn from_params(params: &RevokedCertParams) -> Option { + // RFC 5280 §5.3.1: "The reason code CRL entry extension SHOULD be absent + // instead of using the unspecified (0) reasonCode value." + params + .reason_code + .filter(|reason| *reason != RevocationReason::Unspecified) + .map(Self) + } +} + +impl StaticExtension for ReasonCode { + const OID: &'static [u64] = oid::CRL_REASONS; + + // RFC 5280 §5.3.1: "The reasonCode is a non-critical CRL entry extension". + const CRITICALITY: Criticality = Criticality::NonCritical; + + fn write_value(&self, writer: DERWriter) { + /* + CRLReason ::= ENUMERATED { + unspecified (0), + keyCompromise (1), + cACompromise (2), + affiliationChanged (3), + superseded (4), + cessationOfOperation (5), + certificateHold (6), + -- value 7 is not used + removeFromCRL (8), + privilegeWithdrawn (9), + aACompromise (10) } + */ + writer.write_enum(self.0 as i64); + } +} + +/// An X.509v3 CRL invalidity date entry extension according to [RFC 5280 §5.3.2]. +/// +/// [RFC 5280 §5.3.2]: +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct InvalidityDate(OffsetDateTime); + +impl InvalidityDate { + pub(crate) fn from_params(params: &RevokedCertParams) -> Option { + params.invalidity_date.map(Self) + } +} + +impl StaticExtension for InvalidityDate { + const OID: &'static [u64] = oid::CRL_INVALIDITY_DATE; + + // RFC 5280 §5.3.2: "The invalidity date is a non-critical CRL entry extension". + const CRITICALITY: Criticality = Criticality::NonCritical; + + fn write_value(&self, writer: DERWriter) { + // RFC 5280 §5.3.2: InvalidityDate ::= GeneralizedTime. Unlike the Time + // CHOICE used elsewhere, dates in the UTCTime range (1950-2049) must still + // be encoded as GeneralizedTime. + writer.write_generalized_time(&dt_to_generalized(self.0)); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 2d0f5915..685e9018 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -64,7 +64,7 @@ use ring_like::digest; pub use sign_algo::algo::*; pub use sign_algo::SignatureAlgorithm; use time::{OffsetDateTime, Time}; -use yasna::models::{GeneralizedTime, ObjectIdentifier, UTCTime}; +use yasna::models::{GeneralizedTime, UTCTime}; use yasna::tags::{TAG_BMPSTRING, TAG_TELETEXSTRING, TAG_UNIVERSALSTRING}; use yasna::{DERWriter, Tag}; @@ -811,34 +811,6 @@ fn write_distinguished_name(writer: DERWriter, dn: &DistinguishedName) { }); } -/// Serializes an X.509v3 extension according to RFC 5280 -fn write_x509_extension( - writer: DERWriter, - extension_oid: &[u64], - is_critical: bool, - value_serializer: impl FnOnce(DERWriter), -) { - // Extension specification: - // Extension ::= SEQUENCE { - // extnID OBJECT IDENTIFIER, - // critical BOOLEAN DEFAULT FALSE, - // extnValue OCTET STRING - // -- contains the DER encoding of an ASN.1 value - // -- corresponding to the extension type identified - // -- by extnID - // } - - writer.write_sequence(|writer| { - let oid = ObjectIdentifier::from_slice(extension_oid); - writer.next().write_oid(&oid); - if is_critical { - writer.next().write_bool(true); - } - let bytes = yasna::construct_der(value_serializer); - writer.next().write_bytes(&bytes); - }) -} - #[cfg(feature = "zeroize")] impl zeroize::Zeroize for KeyPair { fn zeroize(&mut self) { From 3799391382087f05701729fbc3f763cfcdff11d6 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 16:21:36 -0400 Subject: [PATCH 14/28] ext: add Extensions collection, build certificate extensions before writing Add an `ext::Extensions` collection that preserves insertion order and rejects duplicate OIDs with a new `Error::DuplicateExtension` variant. The collection borrows the extensions (and through them, the params payloads) for the duration of one serialization. The certificate path now builds the full collection via `CertificateParams::extensions()` and only emits the extensions field of the certificate when the built collection is non-empty. This deletes the `should_write_exts` predicate whose drift from the writers previously caused requested extensions to be silently dropped. Presence is now observed from what was built rather than predicted from the params, removing that bug class structurally. Params that request two extensions with the same OID (previously serialized as an invalid duplicate) now fail with `Error::DuplicateExtension`. --- rcgen/src/certificate.rs | 64 +++++++---------- rcgen/src/error.rs | 5 ++ rcgen/src/ext.rs | 147 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 173 insertions(+), 43 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 4766e5b6..4bc4d610 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -6,14 +6,14 @@ use pem::Pem; use pki_types::{CertificateDer, CertificateSigningRequestDer}; use time::{Date, Month, OffsetDateTime, PrimitiveDateTime, Time}; use yasna::models::ObjectIdentifier; -use yasna::{DERWriter, DERWriterSeq, Tag}; +use yasna::{DERWriter, Tag}; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; use crate::ext::{ write_extension, AuthorityKeyIdentifier, BasicConstraints, Criticality, CrlDistributionPoints, - ExtendedKeyUsage, KeyUsage, NameConstraints as NameConstraintsExt, SubjectAlternativeName, - SubjectKeyIdentifier, + ExtendedKeyUsage, Extensions, KeyUsage, NameConstraints as NameConstraintsExt, + SubjectAlternativeName, SubjectKeyIdentifier, }; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] @@ -386,23 +386,10 @@ impl CertificateParams { write_distinguished_name(writer.next(), &self.distinguished_name); // Write subjectPublicKeyInfo serialize_public_key_der(pub_key, writer.next()); - // write extensions - let should_write_exts = self.use_authority_key_identifier_extension - || !self.subject_alt_names.is_empty() - || !self.key_usages.is_empty() - || !self.extended_key_usages.is_empty() - || self.name_constraints.iter().any(|c| !c.is_empty()) - || !self.crl_distribution_points.is_empty() - || matches!(self.is_ca, IsCa::ExplicitNoCa) - || matches!(self.is_ca, IsCa::Ca(_)) - || !self.custom_extensions.is_empty(); - if !should_write_exts { - return Ok(()); - } - - writer.next().write_tagged(Tag::context(3), |writer| { - writer.write_sequence(|writer| self.write_extensions(writer, &pub_key_spki, issuer)) - })?; + // Write extensions. The field is omitted entirely when the built + // collection is empty. + self.extensions(&pub_key_spki, issuer)? + .write_exts_der(writer.next()); Ok(()) })?; @@ -410,52 +397,55 @@ impl CertificateParams { Ok(der.into()) } - fn write_extensions( + /// Returns the X.509 extensions that the [`CertificateParams`] describe. + /// + /// Returns an [`Error`] if the described extensions are invalid. + fn extensions( &self, - writer: &mut DERWriterSeq, pub_key_spki: &[u8], issuer: &Issuer<'_, impl SigningKey>, - ) -> Result<(), Error> { + ) -> Result, Error> { + let mut exts = Extensions::default(); + if self.use_authority_key_identifier_extension { - write_extension(writer.next(), &AuthorityKeyIdentifier::from(issuer)); + exts.add_extension(Box::new(AuthorityKeyIdentifier::from(issuer)))?; } if let Some(san) = SubjectAlternativeName::from_params(self) { - write_extension(writer.next(), &san); + exts.add_extension(Box::new(san))?; } if let Some(ku) = KeyUsage::from_params(self) { - write_extension(writer.next(), &ku); + exts.add_extension(Box::new(ku))?; } if let Some(eku) = ExtendedKeyUsage::from_params(self) { - write_extension(writer.next(), &eku); + exts.add_extension(Box::new(eku))?; } if let Some(nc) = NameConstraintsExt::from_params(self) { - write_extension(writer.next(), &nc); + exts.add_extension(Box::new(nc))?; } if let Some(crl_dps) = CrlDistributionPoints::from_params(self) { - write_extension(writer.next(), &crl_dps); + exts.add_extension(Box::new(crl_dps))?; } // SKI is currently only written for CA certificates (IsCa::Ca or // IsCa::ExplicitNoCa). if self.is_ca != IsCa::NoCa { - write_extension( - writer.next(), - &SubjectKeyIdentifier::new(&self.key_identifier_method, pub_key_spki), - ); + exts.add_extension(Box::new(SubjectKeyIdentifier::new( + &self.key_identifier_method, + pub_key_spki, + )))?; } - if let Some(bc) = BasicConstraints::from_params(self) { - write_extension(writer.next(), &bc); + exts.add_extension(Box::new(bc))?; } for custom_ext in &self.custom_extensions { - write_extension(writer.next(), &custom_ext); + exts.add_extension(Box::new(custom_ext))?; } - Ok(()) + Ok(exts) } /// Insert an extended key usage (EKU) into the parameters if it does not already exist diff --git a/rcgen/src/error.rs b/rcgen/src/error.rs index 9ba0b30e..e18ef85a 100644 --- a/rcgen/src/error.rs +++ b/rcgen/src/error.rs @@ -47,6 +47,8 @@ pub enum Error { IssuerNotCrlSigner, /// A CRL distribution point was specified without any URIs. EmptyCrlDistributionPointUris, + /// Two extensions with the same OID were requested. + DuplicateExtension(String), #[cfg(not(feature = "crypto"))] /// Missing serial number MissingSerialNumber, @@ -102,6 +104,9 @@ impl fmt::Display for Error { EmptyCrlDistributionPointUris => { write!(f, "CRL distribution points must include at least one URI")? }, + DuplicateExtension(oid) => { + write!(f, "Only one extension with the OID {oid} may be written")? + }, #[cfg(not(feature = "crypto"))] MissingSerialNumber => write!(f, "A serial number must be specified")?, #[cfg(feature = "x509-parser")] diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 43c6146e..3628159b 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -7,11 +7,65 @@ use yasna::{DERWriter, Tag}; use crate::crl::{CrlDistributionPoint, RevocationReason, RevokedCertParams}; use crate::{ - dt_to_generalized, oid, write_distinguished_name, CertificateParams, CustomExtension, + dt_to_generalized, oid, write_distinguished_name, CertificateParams, CustomExtension, Error, ExtendedKeyUsagePurpose, GeneralSubtree, IsCa, Issuer, KeyIdMethod, KeyUsagePurpose, PathLenConstraint, SanType, SerialNumber, SigningKey, }; +/// A collection of X.509 extensions. +/// +/// Preserves the order that extensions were added and maintains the invariant that +/// there are no duplicate extension OIDs. The extensions borrow from the params +/// they were built from for the duration of one serialization. +#[derive(Debug, Default)] +pub(crate) struct Extensions<'params> { + exts: Vec>, +} + +impl<'params> Extensions<'params> { + /// Add an extension to the collection. + /// + /// Returns [`Error::DuplicateExtension`] if the extension's OID is already present + /// in the collection. + pub(crate) fn add_extension( + &mut self, + extension: Box, + ) -> Result<(), Error> { + let oid = extension.oid(); + // A linear scan is plenty: no profile puts more than a handful of + // extensions in one certificate. + if self.exts.iter().any(|existing| existing.oid() == oid) { + return Err(Error::DuplicateExtension( + ObjectIdentifier::from_slice(oid).to_string(), + )); + } + + self.exts.push(extension); + Ok(()) + } + + /// Write the certificate's optional extensions field. + /// + /// Nothing is written when the collection is empty: presence is decided by the + /// built collection, not predicted from the params, so an empty extensions + /// field is never emitted and requested extensions can never be silently + /// dropped. + pub(crate) fn write_exts_der(&self, writer: DERWriter) { + if self.exts.is_empty() { + return; + } + + writer.write_tagged(Tag::context(3), |writer| { + // Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension + writer.write_sequence(|writer| { + for extension in &self.exts { + write_extension(writer.next(), extension.as_ref()); + } + }) + }); + } +} + /// An X.509 extension whose OID and criticality are fixed by the profile /// defining it. /// @@ -619,13 +673,86 @@ impl StaticExtension for InvalidityDate { mod tests { use super::*; + #[test] + fn extensions_reject_duplicate_oids() { + let mut exts = Extensions::default(); + exts.add_extension(Box::new(DummyExt { + oid: TEST_OID, + criticality: Criticality::NonCritical, + })) + .unwrap(); + assert_eq!( + exts.add_extension(Box::new(DummyExt { + oid: TEST_OID, + criticality: Criticality::Critical, + })), + Err(Error::DuplicateExtension( + ObjectIdentifier::from_slice(TEST_OID).to_string() + )), + ); + } + + #[test] + fn extensions_preserve_insertion_order() { + let mut exts = Extensions::default(); + // Add an extension with a lexicographically larger OID first: the encoded + // SEQUENCE must preserve insertion order, not sort. + exts.add_extension(Box::new(DummyExt { + oid: &[1, 3, 6, 1, 4, 1, 98], + criticality: Criticality::NonCritical, + })) + .unwrap(); + exts.add_extension(Box::new(DummyExt { + oid: &[1, 3, 6, 1, 4, 1, 97], + criticality: Criticality::NonCritical, + })) + .unwrap(); + + let der = yasna::construct_der(|writer| exts.write_exts_der(writer)); + assert_eq!( + der, + yasna::construct_der(|writer| { + writer.write_tagged(Tag::context(3), |writer| { + writer.write_sequence(|writer| { + // Insertion order, not OID order: 98 first, then 97. + for oid in [&[1, 3, 6, 1, 4, 1, 98], &[1, 3, 6, 1, 4, 1, 97]] { + writer.next().write_sequence(|writer| { + writer.next().write_oid(&ObjectIdentifier::from_slice(oid)); + writer.next().write_bytes(&yasna::construct_der(|writer| { + writer.write_null() + })); + }); + } + }) + }) + }) + ); + } + + #[test] + fn extensions_elided_when_empty() { + // An empty collection writes nothing at all: no extensions field, no + // empty SEQUENCE. + let exts = Extensions::default(); + let der = yasna::construct_der(|writer| { + writer.write_sequence(|writer| exts.write_exts_der(writer.next())) + }); + assert_eq!( + der, + yasna::construct_der(|writer| writer.write_sequence(|_writer| {})) + ); + } + #[test] fn critical_flag_omitted_when_false() { // The critical flag is DEFAULT FALSE, so DER (X.690 §11.5) requires that a // non-critical extension omit it entirely rather than encode FALSE. // See https://github.com/rustls/rcgen/pull/444 for a past instance of this // bug class. - let ext = DummyExt(Criticality::NonCritical); + let ext = DummyExt { + oid: TEST_OID, + criticality: Criticality::NonCritical, + }; let der = yasna::construct_der(|writer| write_extension(writer, &ext)); assert_eq!( der, @@ -646,7 +773,10 @@ mod tests { #[test] fn critical_flag_written_when_true() { - let ext = DummyExt(Criticality::Critical); + let ext = DummyExt { + oid: TEST_OID, + criticality: Criticality::Critical, + }; let der = yasna::construct_der(|writer| write_extension(writer, &ext)); assert_eq!( der, @@ -777,19 +907,24 @@ mod tests { } #[derive(Debug)] - struct DummyExt(Criticality); + struct DummyExt { + oid: &'static [u64], + criticality: Criticality, + } impl Extension for DummyExt { fn oid(&self) -> &[u64] { - &[1, 3, 6, 1, 4, 1, 99] + self.oid } fn criticality(&self) -> Criticality { - self.0 + self.criticality } fn write_value(&self, writer: DERWriter) { writer.write_null() } } + + const TEST_OID: &[u64] = &[1, 3, 6, 1, 4, 1, 99]; } From a7c5d4a0e66bba33becdf4901791d0921767dbcf Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 12:59:30 -0400 Subject: [PATCH 15/28] csr: build extension request via Extensions collection Replace `write_extension_request_attribute()` and the `write_extension_request` predicate with a `csr_extensions()` collection builder. Like the certificate extensions field, the PKCS #9 extensionRequest attribute elides itself when the built collection is empty, claiming a slot in the attributes SET only when there is something to write (yasna rejects set elements that produce no output). --- rcgen/src/certificate.rs | 80 ++++++++++++++++++++-------------------- rcgen/src/ext.rs | 69 ++++++++++++++++++++++++++++++---- 2 files changed, 101 insertions(+), 48 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 4bc4d610..5e658b34 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -6,14 +6,14 @@ use pem::Pem; use pki_types::{CertificateDer, CertificateSigningRequestDer}; use time::{Date, Month, OffsetDateTime, PrimitiveDateTime, Time}; use yasna::models::ObjectIdentifier; -use yasna::{DERWriter, Tag}; +use yasna::Tag; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; use crate::ext::{ - write_extension, AuthorityKeyIdentifier, BasicConstraints, Criticality, CrlDistributionPoints, - ExtendedKeyUsage, Extensions, KeyUsage, NameConstraints as NameConstraintsExt, - SubjectAlternativeName, SubjectKeyIdentifier, + AuthorityKeyIdentifier, BasicConstraints, Criticality, CrlDistributionPoints, ExtendedKeyUsage, + Extensions, KeyUsage, NameConstraints as NameConstraintsExt, SubjectAlternativeName, + SubjectKeyIdentifier, }; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] @@ -189,34 +189,33 @@ impl CertificateParams { }) } - /// Write a CSR extension request attribute as defined in [RFC 2985]. + /// Returns the X.509 extensions for a CSR extension request attribute as defined + /// in [RFC 2985]. + /// + /// Returns an [`Error`] if the described extensions are invalid. /// /// [RFC 2985]: - fn write_extension_request_attribute(&self, writer: DERWriter) { - writer.write_sequence(|writer| { - writer.next().write_oid(&ObjectIdentifier::from_slice( - oid::PKCS_9_AT_EXTENSION_REQUEST, - )); - writer.next().write_set(|writer| { - writer.next().write_sequence(|writer| { - if let Some(ku) = KeyUsage::from_params(self) { - write_extension(writer.next(), &ku); - } - if let Some(san) = SubjectAlternativeName::from_params(self) { - write_extension(writer.next(), &san); - } - if let Some(eku) = ExtendedKeyUsage::from_params(self) { - write_extension(writer.next(), &eku); - } - if let Some(bc) = BasicConstraints::from_params(self) { - write_extension(writer.next(), &bc); - } - for custom_ext in &self.custom_extensions { - write_extension(writer.next(), &custom_ext); - } - }); - }); - }); + fn csr_extensions(&self) -> Result, Error> { + let mut exts = Extensions::default(); + + if let Some(ku) = KeyUsage::from_params(self) { + exts.add_extension(Box::new(ku))?; + } + if let Some(san) = SubjectAlternativeName::from_params(self) { + exts.add_extension(Box::new(san))?; + } + if let Some(eku) = ExtendedKeyUsage::from_params(self) { + exts.add_extension(Box::new(eku))?; + } + if let Some(bc) = BasicConstraints::from_params(self) { + exts.add_extension(Box::new(bc))?; + } + + for custom_ext in &self.custom_extensions { + exts.add_extension(Box::new(custom_ext))?; + } + + Ok(exts) } /// Generate and serialize a certificate signing request (CSR). @@ -269,7 +268,9 @@ impl CertificateParams { } = self; // - subject_key will be used by the caller // - not_before and not_after cannot be put in a CSR - // - key_identifier_method is here because self.write_extended_key_usage uses it + // - The extension request fields (subject_alt_names, key_usages, + // extended_key_usages, is_ca, custom_extensions) are handled by + // self.csr_extensions() // - There might be a use case for specifying the key identifier // in the CSR, but in the current API it can't be distinguished // from the defaults so this is left for a later version if @@ -278,7 +279,11 @@ impl CertificateParams { not_before, not_after, key_identifier_method, + subject_alt_names, + key_usages, extended_key_usages, + is_ca, + custom_extensions, ); if serial_number.is_some() || name_constraints.is_some() @@ -288,12 +293,9 @@ impl CertificateParams { return Err(Error::UnsupportedInCsr); } - // Whether or not to write an extension request attribute - let write_extension_request = !key_usages.is_empty() - || !subject_alt_names.is_empty() - || !extended_key_usages.is_empty() - || !custom_extensions.is_empty() - || matches!(is_ca, IsCa::ExplicitNoCa | IsCa::Ca(_)); + // The extension request attribute is elided entirely when the built + // collection is empty. + let extension_request = self.csr_extensions()?; let der = sign_der(subject_key, |writer| { // Write version @@ -307,9 +309,7 @@ impl CertificateParams { .write_tagged_implicit(Tag::context(0), |writer| { // RFC 2986 specifies that attributes are a SET OF Attribute writer.write_set_of(|writer| { - if write_extension_request { - self.write_extension_request_attribute(writer.next()); - } + extension_request.write_csr_attribute(writer); for Attribute { oid, values } in attrs { writer.next().write_sequence(|writer| { diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 3628159b..7ab37f67 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -3,7 +3,7 @@ use std::net::IpAddr; use time::OffsetDateTime; use yasna::models::ObjectIdentifier; -use yasna::{DERWriter, Tag}; +use yasna::{DERWriter, DERWriterSet, Tag}; use crate::crl::{CrlDistributionPoint, RevocationReason, RevokedCertParams}; use crate::{ @@ -55,15 +55,54 @@ impl<'params> Extensions<'params> { return; } - writer.write_tagged(Tag::context(3), |writer| { - // Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension - writer.write_sequence(|writer| { - for extension in &self.exts { - write_extension(writer.next(), extension.as_ref()); - } - }) + writer.write_tagged(Tag::context(3), |writer| self.write_der(writer)); + } + + /// Write the PKCS #9 extensionRequest attribute for a CSR into the + /// attributes SET, containing the collection as its single `Extensions` + /// value. + /// + /// Nothing is written when the collection is empty: attribute values are a + /// SET SIZE(1..MAX), so an empty extension request can't be encoded and the + /// attribute is elided entirely. The attribute's slot in the SET is only + /// claimed when there is something to write: yasna rejects set elements + /// that produce no output. + pub(crate) fn write_csr_attribute(&self, writer: &mut DERWriterSet<'_>) { + if self.exts.is_empty() { + return; + } + + /* + Attribute { ATTRIBUTE:IOSet } ::= SEQUENCE { + type ATTRIBUTE.&id({IOSet}), + values SET SIZE(1..MAX) OF ATTRIBUTE.&Type({IOSet}{@type}) + } + ExtensionRequest ::= Extensions + */ + writer.next().write_sequence(|writer| { + writer.next().write_oid(&ObjectIdentifier::from_slice( + oid::PKCS_9_AT_EXTENSION_REQUEST, + )); + writer.next().write_set(|writer| { + self.write_der(writer.next()); + }); }); } + + /// Write `Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension`. + /// + /// Nothing is written when the collection is empty. + fn write_der(&self, writer: DERWriter) { + if self.exts.is_empty() { + return; + } + + writer.write_sequence(|writer| { + for extension in &self.exts { + write_extension(writer.next(), extension.as_ref()); + } + }) + } } /// An X.509 extension whose OID and criticality are fixed by the profile @@ -743,6 +782,20 @@ mod tests { ); } + #[test] + fn csr_attribute_elided_when_empty() { + // An empty collection must not claim a slot in the attributes SET at + // all: yasna rejects set elements that produce no output. + let exts = Extensions::default(); + let der = yasna::construct_der(|writer| { + writer.write_set_of(|writer| exts.write_csr_attribute(writer)) + }); + assert_eq!( + der, + yasna::construct_der(|writer| writer.write_set_of(|_writer| {})) + ); + } + #[test] fn critical_flag_omitted_when_false() { // The critical flag is DEFAULT FALSE, so DER (X.690 §11.5) requires that a From 8ecac9951c6681d240a5617dd4e4ef446f93f056 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 13:00:30 -0400 Subject: [PATCH 16/28] crl: build CRL and entry extensions via Extensions collection Build the crlExtensions field (AKI, CRL number, optional IDP) and each entry's crlEntryExtensions through `ext::Extensions`, eliding the fields when the built collections are empty. The reasonCode/invalidityDate presence gate is gone: the collection's emptiness is the single source of truth. With all writers converted, `write_extension()` becomes private to the ext module. --- rcgen/src/crl.rs | 85 ++++++++++++++++++++++++------------------------ rcgen/src/ext.rs | 18 ++++++++-- 2 files changed, 57 insertions(+), 46 deletions(-) diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index 320d1bc7..1faf5c5e 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -5,7 +5,7 @@ use time::OffsetDateTime; use yasna::{DERWriter, Tag}; use crate::ext::{ - write_extension, AuthorityKeyIdentifier, Criticality, CrlNumber, InvalidityDate, ReasonCode, + AuthorityKeyIdentifier, Criticality, CrlNumber, Extensions, InvalidityDate, ReasonCode, StaticExtension, }; use crate::key_pair::sign_der; @@ -260,43 +260,46 @@ impl CertificateRevocationListParams { if !self.revoked_certs.is_empty() { writer.next().write_sequence(|writer| { for revoked_cert in &self.revoked_certs { - revoked_cert.write_der(writer.next()); + revoked_cert.write_der(writer.next())?; } - }); + Ok::<(), Error>(()) + })?; } // Write crlExtensions. // RFC 5280 §5.1.2.7: // This field may only appear if the version is 2 (Section 5.1.2.1). If // present, this field is a sequence of one or more CRL extensions. - // RFC 5280 §5.2: - // Conforming CRL issuers are REQUIRED to include the authority key - // identifier (Section 5.2.1) and the CRL number (Section 5.2.3) - // extensions in all CRLs issued. - writer.next().write_tagged(Tag::context(0), |writer| { - writer.write_sequence(|writer| { - // Write authority key identifier. - write_extension( - writer.next(), - &AuthorityKeyIdentifier( - self.key_identifier_method - .derive(issuer.signing_key.subject_public_key_info()), - ), - ); - - // Write CRL number. - write_extension(writer.next(), &CrlNumber::from(&self.crl_number)); - - // Write issuing distribution point (if present). - if let Some(idp) = &self.issuing_distribution_point { - write_extension(writer.next(), &idp); - } - }); - }); + // The field is elided entirely when the built collection is empty. + self.extensions(issuer)?.write_crl_der(writer.next()); Ok(()) }) } + + /// Returns the X.509 extensions that the [`CertificateRevocationListParams`] + /// describe. + /// + /// Returns an [`Error`] if the described extensions are invalid. + fn extensions(&self, issuer: &Issuer<'_, impl SigningKey>) -> Result, Error> { + let mut exts = Extensions::default(); + + // RFC 5280 §5.2: + // Conforming CRL issuers are REQUIRED to include the authority key + // identifier (Section 5.2.1) and the CRL number (Section 5.2.3) + // extensions in all CRLs issued. + exts.add_extension(Box::new(AuthorityKeyIdentifier( + self.key_identifier_method + .derive(issuer.signing_key.subject_public_key_info()), + )))?; + exts.add_extension(Box::new(CrlNumber::from(&self.crl_number)))?; + + if let Some(idp) = &self.issuing_distribution_point { + exts.add_extension(Box::new(idp))?; + } + + Ok(exts) + } } /// A certificate revocation list (CRL) issuing distribution point, to be included in a CRL's @@ -367,7 +370,7 @@ pub struct RevokedCertParams { } impl RevokedCertParams { - fn write_der(&self, writer: DERWriter) { + fn write_der(&self, writer: DERWriter) -> Result<(), Error> { writer.write_sequence(|writer| { // Write serial number. // RFC 5280 §4.1.2.2: @@ -384,27 +387,23 @@ impl RevokedCertParams { // Write revocation date. write_dt_utc_or_generalized(writer.next(), self.revocation_time); - // Write extensions if applicable. + // Write crlEntryExtensions. // RFC 5280 §5.3: // Support for the CRL entry extensions defined in this specification is // optional for conforming CRL issuers and applications. However, CRL // issuers SHOULD include reason codes (Section 5.3.1) and invalidity // dates (Section 5.3.2) whenever this information is available. - let reason_code = ReasonCode::from_params(self); - let invalidity_date = InvalidityDate::from_params(self); - if reason_code.is_some() || invalidity_date.is_some() { - writer.next().write_sequence(|writer| { - // Write reason code if present. - if let Some(reason_code) = &reason_code { - write_extension(writer.next(), reason_code); - } - - // Write invalidity date if present. - if let Some(invalidity_date) = &invalidity_date { - write_extension(writer.next(), invalidity_date); - } - }); + // The field is elided entirely when the built collection is empty. + let mut exts = Extensions::default(); + if let Some(reason_code) = ReasonCode::from_params(self) { + exts.add_extension(Box::new(reason_code))?; + } + if let Some(invalidity_date) = InvalidityDate::from_params(self) { + exts.add_extension(Box::new(invalidity_date))?; } + exts.write_der(writer.next()); + + Ok(()) }) } } diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 7ab37f67..bb84d0d6 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -89,10 +89,22 @@ impl<'params> Extensions<'params> { }); } - /// Write `Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension`. + /// Write the `crlExtensions [0] EXPLICIT Extensions OPTIONAL` field of a CRL. /// /// Nothing is written when the collection is empty. - fn write_der(&self, writer: DERWriter) { + pub(crate) fn write_crl_der(&self, writer: DERWriter) { + if self.exts.is_empty() { + return; + } + + writer.write_tagged(Tag::context(0), |writer| self.write_der(writer)); + } + + /// Write `Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension`, e.g. for the + /// untagged `crlEntryExtensions` field of a CRL entry. + /// + /// Nothing is written when the collection is empty. + pub(crate) fn write_der(&self, writer: DERWriter) { if self.exts.is_empty() { return; } @@ -178,7 +190,7 @@ impl From for Criticality { } /// Serializes an X.509v3 extension according to RFC 5280. -pub(crate) fn write_extension(writer: DERWriter, extension: &dyn Extension) { +fn write_extension(writer: DERWriter, extension: &dyn Extension) { /* Extension ::= SEQUENCE { extnID OBJECT IDENTIFIER, From ce8f70fe59f7455f2cade9f7c088a4ab89149dad Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 13:01:02 -0400 Subject: [PATCH 17/28] ext: emit subject key identifier for all certificates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously SKI was only written for `IsCa::Ca`/`ExplicitNoCa` certificates. RFC 5280 §4.2.1.2 describes the SKI as a MUST for CA certificates and a SHOULD for end entity certificates, so emit it unconditionally. CSRs are unchanged (no SKI is requested). --- rcgen/src/certificate.rs | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 5e658b34..30bae736 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -429,14 +429,12 @@ impl CertificateParams { exts.add_extension(Box::new(crl_dps))?; } - // SKI is currently only written for CA certificates (IsCa::Ca or - // IsCa::ExplicitNoCa). - if self.is_ca != IsCa::NoCa { - exts.add_extension(Box::new(SubjectKeyIdentifier::new( - &self.key_identifier_method, - pub_key_spki, - )))?; - } + // RFC 5280 §4.2.1.2 describes the SKI as a MUST for CA certificates and a + // SHOULD for end entity certificates, so it is emitted for all certificates. + exts.add_extension(Box::new(SubjectKeyIdentifier::new( + &self.key_identifier_method, + pub_key_spki, + )))?; if let Some(bc) = BasicConstraints::from_params(self) { exts.add_extension(Box::new(bc))?; } @@ -1057,6 +1055,28 @@ mod tests { ); } + #[cfg(feature = "crypto")] + #[test] + fn test_end_entity_subject_key_identifier() { + // RFC 5280 §4.2.1.2 describes the SKI as a SHOULD for end entity + // certificates, so we expect it to be present for end entity certs too. + let params = CertificateParams::default(); + let key_pair = KeyPair::generate().unwrap(); + let cert = params.self_signed(&key_pair).unwrap(); + + let (_rem, cert) = x509_parser::parse_x509_certificate(cert.der()).unwrap(); + let ski = cert + .iter_extensions() + .find_map(|ext| match ext.parsed_extension() { + x509_parser::extensions::ParsedExtension::SubjectKeyIdentifier(ski) => { + Some(ski.0.to_vec()) + }, + _ => None, + }) + .unwrap(); + assert_eq!(ski, params.key_identifier(&key_pair)); + } + #[cfg(feature = "crypto")] #[test] fn test_with_key_usages_only() { From e24ce9f7d8ff5d7c0c3082a6cdfea86988e378f4 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 12:28:39 -0400 Subject: [PATCH 18/28] csr: align extension request order with certificate extensions CSRs previously requested extensions as KU, SAN, EKU while certificates emit SAN, KU, EKU. Use the certificate order for the CSR extension request attribute so both paths serialize extensions consistently. --- rcgen/src/certificate.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 30bae736..2e7652e0 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -198,12 +198,12 @@ impl CertificateParams { fn csr_extensions(&self) -> Result, Error> { let mut exts = Extensions::default(); - if let Some(ku) = KeyUsage::from_params(self) { - exts.add_extension(Box::new(ku))?; - } if let Some(san) = SubjectAlternativeName::from_params(self) { exts.add_extension(Box::new(san))?; } + if let Some(ku) = KeyUsage::from_params(self) { + exts.add_extension(Box::new(ku))?; + } if let Some(eku) = ExtendedKeyUsage::from_params(self) { exts.add_extension(Box::new(eku))?; } From 6200093beb28de0ac52b76718113a15020b1cfea Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 13:01:49 -0400 Subject: [PATCH 19/28] ext: move CustomExtension into the ext module Move the type next to its `Extension` impl, adding whitespace between the impl's methods; the crate root re-export path is unchanged. --- rcgen/src/certificate.rs | 62 +++------------------------------------- rcgen/src/ext.rs | 60 +++++++++++++++++++++++++++++++++++++- rcgen/src/lib.rs | 3 +- 3 files changed, 65 insertions(+), 60 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 2e7652e0..fe744637 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -11,9 +11,8 @@ use yasna::Tag; use crate::crl::CrlDistributionPoint; use crate::csr::CertificateSigningRequest; use crate::ext::{ - AuthorityKeyIdentifier, BasicConstraints, Criticality, CrlDistributionPoints, ExtendedKeyUsage, - Extensions, KeyUsage, NameConstraints as NameConstraintsExt, SubjectAlternativeName, - SubjectKeyIdentifier, + AuthorityKeyIdentifier, BasicConstraints, CrlDistributionPoints, ExtendedKeyUsage, Extensions, + KeyUsage, NameConstraints as NameConstraintsExt, SubjectAlternativeName, SubjectKeyIdentifier, }; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] @@ -21,8 +20,8 @@ use crate::ring_like::digest; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ - oid, write_distinguished_name, write_dt_utc_or_generalized, DistinguishedName, Error, Issuer, - KeyIdMethod, KeyUsagePurpose, SanType, SerialNumber, SigningKey, + oid, write_distinguished_name, write_dt_utc_or_generalized, CustomExtension, DistinguishedName, + Error, Issuer, KeyIdMethod, KeyUsagePurpose, SanType, SerialNumber, SigningKey, }; /// An issued certificate @@ -479,59 +478,6 @@ pub struct Attribute { pub values: Vec, } -/// A custom extension of a certificate, as specified in -/// [RFC 5280](https://tools.ietf.org/html/rfc5280#section-4.2) -#[derive(Debug, PartialEq, Eq, Hash, Clone)] -pub struct CustomExtension { - pub(crate) oid: Vec, - pub(crate) criticality: Criticality, - - /// The content must be DER-encoded - pub(crate) content: Vec, -} - -impl CustomExtension { - /// Creates a new acmeIdentifier extension for ACME TLS-ALPN-01 - /// as specified in [RFC 8737](https://tools.ietf.org/html/rfc8737#section-3) - /// - /// Panics if the passed `sha_digest` parameter doesn't hold 32 bytes (256 bits). - pub fn new_acme_identifier(sha_digest: &[u8]) -> Self { - assert_eq!(sha_digest.len(), 32, "wrong size of sha_digest"); - let content = yasna::construct_der(|writer| { - writer.write_bytes(sha_digest); - }); - Self { - oid: oid::PE_ACME.to_owned(), - criticality: Criticality::Critical, - content, - } - } - /// Create a new custom extension with the specified content - pub fn from_oid_content(oid: &[u64], content: Vec) -> Self { - Self { - oid: oid.to_owned(), - criticality: Criticality::NonCritical, - content, - } - } - /// Sets the criticality flag of the extension. - pub fn set_criticality(&mut self, criticality: bool) { - self.criticality = criticality.into(); - } - /// Obtains the criticality flag of the extension. - pub fn criticality(&self) -> bool { - self.criticality == Criticality::Critical - } - /// Obtains the content of the extension. - pub fn content(&self) -> &[u8] { - &self.content - } - /// Obtains the OID components of the extensions, as u64 pieces - pub fn oid_components(&self) -> impl Iterator + '_ { - self.oid.iter().copied() - } -} - #[derive(Debug, PartialEq, Eq, Hash, Clone)] #[non_exhaustive] /// The attribute type of a distinguished name entry diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index bb84d0d6..3826d29f 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -7,7 +7,7 @@ use yasna::{DERWriter, DERWriterSet, Tag}; use crate::crl::{CrlDistributionPoint, RevocationReason, RevokedCertParams}; use crate::{ - dt_to_generalized, oid, write_distinguished_name, CertificateParams, CustomExtension, Error, + dt_to_generalized, oid, write_distinguished_name, CertificateParams, Error, ExtendedKeyUsagePurpose, GeneralSubtree, IsCa, Issuer, KeyIdMethod, KeyUsagePurpose, PathLenConstraint, SanType, SerialNumber, SigningKey, }; @@ -613,6 +613,64 @@ impl StaticExtension for BasicConstraints { } } +/// A custom extension of a certificate, as specified in +/// [RFC 5280](https://tools.ietf.org/html/rfc5280#section-4.2) +#[derive(Debug, PartialEq, Eq, Hash, Clone)] +pub struct CustomExtension { + pub(crate) oid: Vec, + pub(crate) criticality: Criticality, + + /// The content must be DER-encoded + pub(crate) content: Vec, +} + +impl CustomExtension { + /// Creates a new acmeIdentifier extension for ACME TLS-ALPN-01 + /// as specified in [RFC 8737](https://tools.ietf.org/html/rfc8737#section-3) + /// + /// Panics if the passed `sha_digest` parameter doesn't hold 32 bytes (256 bits). + pub fn new_acme_identifier(sha_digest: &[u8]) -> Self { + assert_eq!(sha_digest.len(), 32, "wrong size of sha_digest"); + let content = yasna::construct_der(|writer| { + writer.write_bytes(sha_digest); + }); + Self { + oid: oid::PE_ACME.to_owned(), + criticality: Criticality::Critical, + content, + } + } + + /// Create a new custom extension with the specified content + pub fn from_oid_content(oid: &[u64], content: Vec) -> Self { + Self { + oid: oid.to_owned(), + criticality: Criticality::NonCritical, + content, + } + } + + /// Sets the criticality flag of the extension. + pub fn set_criticality(&mut self, criticality: bool) { + self.criticality = criticality.into(); + } + + /// Obtains the criticality flag of the extension. + pub fn criticality(&self) -> bool { + self.criticality == Criticality::Critical + } + + /// Obtains the content of the extension. + pub fn content(&self) -> &[u8] { + &self.content + } + + /// Obtains the OID components of the extensions, as u64 pieces + pub fn oid_components(&self) -> impl Iterator + '_ { + self.oid.iter().copied() + } +} + impl Extension for &CustomExtension { fn oid(&self) -> &[u64] { &self.oid diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 685e9018..2ae15336 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -42,7 +42,7 @@ use std::net::{Ipv4Addr, Ipv6Addr}; use std::ops::Deref; pub use certificate::{ - date_time_ymd, Attribute, Certificate, CertificateParams, CidrSubnet, CustomExtension, DnType, + date_time_ymd, Attribute, Certificate, CertificateParams, CidrSubnet, DnType, ExtendedKeyUsagePurpose, GeneralSubtree, IsCa, NameConstraints, PathLenConstraint, }; pub use crl::{ @@ -51,6 +51,7 @@ pub use crl::{ }; pub use csr::{CertificateSigningRequest, CertificateSigningRequestParams, PublicKey}; pub use error::{Error, InvalidAsn1String}; +pub use ext::CustomExtension; #[cfg(feature = "crypto")] pub use key_pair::KeyPair; #[cfg(all(feature = "crypto", feature = "aws_lc_rs"))] From 416161dbdd259a3372277b4e5a699b7696781550 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 13:02:13 -0400 Subject: [PATCH 20/28] ext: rework CustomExtension public API Make `Criticality` public and restructure `CustomExtension` with public `oid`, `criticality` and `der_value` fields, dropping the `set_criticality()`/`criticality()`/`content()` accessors. `from_oid_content()` now takes the criticality directly instead of defaulting to non-critical with later mutation. --- rcgen/src/ext.rs | 50 ++++++++++++++++------------------- rcgen/src/lib.rs | 2 +- verify-tests/tests/generic.rs | 6 ++--- 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 3826d29f..2c4df57c 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -172,7 +172,7 @@ pub(crate) trait Extension: Debug { /// /// [RFC 5280 §4.2]: #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub(crate) enum Criticality { +pub enum Criticality { /// The extension MUST be recognized and parsed correctly. Critical, @@ -617,11 +617,22 @@ impl StaticExtension for BasicConstraints { /// [RFC 5280](https://tools.ietf.org/html/rfc5280#section-4.2) #[derive(Debug, PartialEq, Eq, Hash, Clone)] pub struct CustomExtension { - pub(crate) oid: Vec, - pub(crate) criticality: Criticality, + /// OID identifying the extension. + /// + /// Only one extension with a given OID may appear within a certificate. + pub oid: Vec, + + /// Criticality of the extension. + /// + /// See [`Criticality`] for more information. + pub criticality: Criticality, - /// The content must be DER-encoded - pub(crate) content: Vec, + /// The raw DER encoded value of the extension. + /// + /// This should not contain the OID, criticality, OCTET STRING, or the outer + /// extension SEQUENCE of the extension itself: it should only be the DER encoded + /// bytes that will be found within the extension's OCTET STRING value. + pub der_value: Vec, } impl CustomExtension { @@ -631,40 +642,25 @@ impl CustomExtension { /// Panics if the passed `sha_digest` parameter doesn't hold 32 bytes (256 bits). pub fn new_acme_identifier(sha_digest: &[u8]) -> Self { assert_eq!(sha_digest.len(), 32, "wrong size of sha_digest"); - let content = yasna::construct_der(|writer| { + let der_value = yasna::construct_der(|writer| { writer.write_bytes(sha_digest); }); Self { oid: oid::PE_ACME.to_owned(), criticality: Criticality::Critical, - content, + der_value, } } /// Create a new custom extension with the specified content - pub fn from_oid_content(oid: &[u64], content: Vec) -> Self { + pub fn from_oid_content(oid: &[u64], criticality: Criticality, der_value: Vec) -> Self { Self { - oid: oid.to_owned(), - criticality: Criticality::NonCritical, - content, + oid: oid.to_vec(), + criticality, + der_value, } } - /// Sets the criticality flag of the extension. - pub fn set_criticality(&mut self, criticality: bool) { - self.criticality = criticality.into(); - } - - /// Obtains the criticality flag of the extension. - pub fn criticality(&self) -> bool { - self.criticality == Criticality::Critical - } - - /// Obtains the content of the extension. - pub fn content(&self) -> &[u8] { - &self.content - } - /// Obtains the OID components of the extensions, as u64 pieces pub fn oid_components(&self) -> impl Iterator + '_ { self.oid.iter().copied() @@ -681,7 +677,7 @@ impl Extension for &CustomExtension { } fn write_value(&self, writer: DERWriter) { - writer.write_der(&self.content) + writer.write_der(&self.der_value) } } diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 2ae15336..694c5daa 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -51,7 +51,7 @@ pub use crl::{ }; pub use csr::{CertificateSigningRequest, CertificateSigningRequestParams, PublicKey}; pub use error::{Error, InvalidAsn1String}; -pub use ext::CustomExtension; +pub use ext::{Criticality, CustomExtension}; #[cfg(feature = "crypto")] pub use key_pair::KeyPair; #[cfg(all(feature = "crypto", feature = "aws_lc_rs"))] diff --git a/verify-tests/tests/generic.rs b/verify-tests/tests/generic.rs index 1ddd4549..03657449 100644 --- a/verify-tests/tests/generic.rs +++ b/verify-tests/tests/generic.rs @@ -49,7 +49,7 @@ mod test_key_params_mismatch { #[cfg(feature = "x509-parser")] mod test_x509_custom_ext { - use rcgen::CustomExtension; + use rcgen::{Criticality, CustomExtension}; use verify_tests as util; use x509_parser::oid_registry::asn1_rs; use x509_parser::prelude::{ @@ -63,11 +63,11 @@ mod test_x509_custom_ext { let test_ext = yasna::construct_der(|writer| { writer.write_utf8_string("🦀 greetz to ferris 🦀"); }); - let mut custom_ext = CustomExtension::from_oid_content( + let custom_ext = CustomExtension::from_oid_content( test_oid.iter().unwrap().collect::>().as_slice(), + Criticality::Critical, test_ext.clone(), ); - custom_ext.set_criticality(true); // Generate a certificate with the custom extension, parse it with x509-parser. let (mut params, test_key) = util::default_params(); From 434cda8a6b4426ddfb71cc390b37e87fe36774be Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 12:44:57 -0400 Subject: [PATCH 21/28] ext: add AcmeIdentifier, replacing CustomExtension::new_acme_identifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a dedicated `AcmeIdentifier` type for RFC 8737 TLS-ALPN-01 challenge response extensions. It converts into a `CustomExtension` for use in `CertificateParams::custom_extensions`, always critical per RFC 8737 §3. It stays a `CustomExtension` conversion rather than becoming a first-class params field: the extension never appears in CSRs, and nearly every `CertificateParams` would carry a `None` for it. The `TryFrom<&[u8]>` constructor returns the new `Error::InvalidAcmeIdentifierLength` instead of panicking on wrong-length digests like `new_acme_identifier` did. --- rcgen/src/error.rs | 6 ++++ rcgen/src/ext.rs | 82 +++++++++++++++++++++++++++++++++++++--------- rcgen/src/lib.rs | 2 +- 3 files changed, 73 insertions(+), 17 deletions(-) diff --git a/rcgen/src/error.rs b/rcgen/src/error.rs index e18ef85a..7d3946c3 100644 --- a/rcgen/src/error.rs +++ b/rcgen/src/error.rs @@ -49,6 +49,8 @@ pub enum Error { EmptyCrlDistributionPointUris, /// Two extensions with the same OID were requested. DuplicateExtension(String), + /// An ACME TLS-ALPN-01 key authorization digest was not 32 bytes long. + InvalidAcmeIdentifierLength, #[cfg(not(feature = "crypto"))] /// Missing serial number MissingSerialNumber, @@ -107,6 +109,10 @@ impl fmt::Display for Error { DuplicateExtension(oid) => { write!(f, "Only one extension with the OID {oid} may be written")? }, + InvalidAcmeIdentifierLength => write!( + f, + "An ACME TLS-ALPN-01 key authorization digest must be 32 bytes" + )?, #[cfg(not(feature = "crypto"))] MissingSerialNumber => write!(f, "A serial number must be specified")?, #[cfg(feature = "x509-parser")] diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 2c4df57c..40680702 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -636,22 +636,6 @@ pub struct CustomExtension { } impl CustomExtension { - /// Creates a new acmeIdentifier extension for ACME TLS-ALPN-01 - /// as specified in [RFC 8737](https://tools.ietf.org/html/rfc8737#section-3) - /// - /// Panics if the passed `sha_digest` parameter doesn't hold 32 bytes (256 bits). - pub fn new_acme_identifier(sha_digest: &[u8]) -> Self { - assert_eq!(sha_digest.len(), 32, "wrong size of sha_digest"); - let der_value = yasna::construct_der(|writer| { - writer.write_bytes(sha_digest); - }); - Self { - oid: oid::PE_ACME.to_owned(), - criticality: Criticality::Critical, - der_value, - } - } - /// Create a new custom extension with the specified content pub fn from_oid_content(oid: &[u64], criticality: Criticality, der_value: Vec) -> Self { Self { @@ -681,6 +665,51 @@ impl Extension for &CustomExtension { } } +/// An ACME TLS-ALPN-01 challenge response certificate extension. +/// +/// Add it to [`CertificateParams::custom_extensions`] by converting it into a +/// [`CustomExtension`]. See [RFC 8737 §3] for more information. +/// +/// If you have a `Vec` or `&[u8]` digest, use `try_from` and handle the +/// potential error if the input length is not 32 bytes. +/// +/// [RFC 8737 §3]: +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AcmeIdentifier( + /// The SHA-256 digest of the RFC 8555 key authorization for a TLS-ALPN-01 + /// challenge issued by the CA. + pub [u8; 32], +); + +impl TryFrom<&[u8]> for AcmeIdentifier { + type Error = Error; + + fn try_from(key_auth_digest: &[u8]) -> Result { + // All TLS-ALPN-01 challenge response digests are 32 bytes long, + // matching the output of the SHA-256 digest algorithm. + Ok(Self( + key_auth_digest + .try_into() + .map_err(|_| Error::InvalidAcmeIdentifierLength)?, + )) + } +} + +impl From for CustomExtension { + fn from(identifier: AcmeIdentifier) -> Self { + Self { + oid: oid::PE_ACME.to_owned(), + // RFC 8737 §3: "The acmeIdentifier extension MUST be critical so that + // the certificate isn't inadvertently used by non-ACME software." + criticality: Criticality::Critical, + der_value: yasna::construct_der(|writer| { + // Authorization ::= OCTET STRING (SIZE (32)) + writer.write_bytes(&identifier.0) + }), + } + } +} + /// An X.509v3 CRL number extension according to [RFC 5280 §5.2.3]. /// /// [RFC 5280 §5.2.3]: @@ -940,6 +969,27 @@ mod tests { ); } + #[test] + fn acme_identifier_to_custom_extension() { + let identifier = AcmeIdentifier::try_from([0xAB; 32].as_slice()).unwrap(); + let custom_ext = CustomExtension::from(identifier); + assert_eq!(custom_ext.oid, oid::PE_ACME); + // RFC 8737 §3: the acmeIdentifier extension MUST be critical. + assert_eq!(custom_ext.criticality, Criticality::Critical); + // Authorization ::= OCTET STRING (SIZE (32)) + let mut expected = vec![0x04, 0x20]; + expected.extend([0xAB; 32]); + assert_eq!(custom_ext.der_value, expected); + } + + #[test] + fn acme_identifier_rejects_wrong_digest_length() { + assert_eq!( + AcmeIdentifier::try_from([0u8; 31].as_slice()).unwrap_err(), + Error::InvalidAcmeIdentifierLength, + ); + } + #[test] fn basic_constraints_absent_for_no_ca() { // IsCa::NoCa means no BasicConstraints extension at all. diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 694c5daa..3fe9676d 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -51,7 +51,7 @@ pub use crl::{ }; pub use csr::{CertificateSigningRequest, CertificateSigningRequestParams, PublicKey}; pub use error::{Error, InvalidAsn1String}; -pub use ext::{Criticality, CustomExtension}; +pub use ext::{AcmeIdentifier, Criticality, CustomExtension}; #[cfg(feature = "crypto")] pub use key_pair::KeyPair; #[cfg(all(feature = "crypto", feature = "aws_lc_rs"))] From 030a7aa4c3cd744b26bd8baee8508b492c0918d5 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 13:05:12 -0400 Subject: [PATCH 22/28] csr: parse requested extensions via ext type constructors Move the CSR extension parsing into `from_parsed` constructors on the ext types (`KeyUsage`, `SubjectAlternativeName`, `ExtendedKeyUsage`, `BasicConstraints`), symmetric with their `from_params` serializing counterparts. Unknown requested extensions still yield `Error::UnsupportedExtension`. Custom EKU purpose OIDs now parse into `ExtendedKeyUsagePurpose::Other` and round-trip, where `from_der` previously rejected them with `Error::UnsupportedExtension`. --- rcgen/src/csr.rs | 85 ++++++++++++++++---------------------- rcgen/src/ext.rs | 103 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 51 deletions(-) diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index f3f8051d..6ac4898b 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -4,13 +4,15 @@ use std::hash::Hash; use pem::Pem; use pki_types::CertificateSigningRequestDer; +#[cfg(feature = "x509-parser")] +use crate::ext::{BasicConstraints, ExtendedKeyUsage, KeyUsage, SubjectAlternativeName}; +#[cfg(feature = "x509-parser")] +use crate::DistinguishedName; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ Certificate, CertificateParams, Error, Issuer, PublicKeyData, SignatureAlgorithm, SigningKey, }; -#[cfg(feature = "x509-parser")] -use crate::{DistinguishedName, ExtendedKeyUsagePurpose, IsCa, KeyUsagePurpose, SanType}; /// A public key, extracted from a CSR #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -92,9 +94,9 @@ impl CertificateSigningRequestParams { /// Parse and verify a certificate signing request from DER-encoded bytes /// /// Currently, this supports the following extensions: - /// - `Subject Alternative Name` (see [`SanType`]) - /// - `Key Usage` (see [`KeyUsagePurpose`]) - /// - `Extended Key Usage` (see [`ExtendedKeyUsagePurpose`]) + /// - `Subject Alternative Name` (see [`crate::SanType`]) + /// - `Key Usage` (see [`crate::KeyUsagePurpose`]) + /// - `Extended Key Usage` (see [`crate::ExtendedKeyUsagePurpose`]) /// - `Basic Constraints` (see [`crate::PathLenConstraint`]) /// /// On encountering other extensions, this function will return [`Error::UnsupportedExtension`]. @@ -131,52 +133,13 @@ impl CertificateSigningRequestParams { let raw = info.subject_pki.subject_public_key.data.to_vec(); if let Some(extensions) = csr.requested_extensions() { - for ext in extensions { - match ext { - x509_parser::extensions::ParsedExtension::KeyUsage(key_usage) => { - // This x509 parser stores flags in reversed bit BIT STRING order - params.key_usages = - KeyUsagePurpose::from_u16(key_usage.flags.reverse_bits()); - }, - x509_parser::extensions::ParsedExtension::SubjectAlternativeName(san) => { - for name in &san.general_names { - params - .subject_alt_names - .push(SanType::try_from_general(name)?); - } - }, - x509_parser::extensions::ParsedExtension::ExtendedKeyUsage(eku) => { - if eku.any { - params.insert_extended_key_usage(ExtendedKeyUsagePurpose::Any); - } - if eku.server_auth { - params.insert_extended_key_usage(ExtendedKeyUsagePurpose::ServerAuth); - } - if eku.client_auth { - params.insert_extended_key_usage(ExtendedKeyUsagePurpose::ClientAuth); - } - if eku.code_signing { - params.insert_extended_key_usage(ExtendedKeyUsagePurpose::CodeSigning); - } - if eku.email_protection { - params.insert_extended_key_usage( - ExtendedKeyUsagePurpose::EmailProtection, - ); - } - if eku.time_stamping { - params.insert_extended_key_usage(ExtendedKeyUsagePurpose::TimeStamping); - } - if eku.ocsp_signing { - params.insert_extended_key_usage(ExtendedKeyUsagePurpose::OcspSigning); - } - if !eku.other.is_empty() { - return Err(Error::UnsupportedExtension); - } - }, - x509_parser::extensions::ParsedExtension::BasicConstraints(bc) => { - params.is_ca = IsCa::from_basic_constraints(bc)?; - }, - _ => return Err(Error::UnsupportedExtension), + for parsed in extensions { + let handled = KeyUsage::from_parsed(&mut params, parsed)? + || SubjectAlternativeName::from_parsed(&mut params, parsed)? + || ExtendedKeyUsage::from_parsed(&mut params, parsed)? + || BasicConstraints::from_parsed(&mut params, parsed)?; + if !handled { + return Err(Error::UnsupportedExtension); } } } @@ -279,6 +242,26 @@ mod tests { )); } + #[test] + fn serialize_and_deserialize_eq_other_eku() { + // Custom EKU purpose OIDs must survive a serialize/parse round trip. + let params = CertificateParams { + extended_key_usages: vec![ + ExtendedKeyUsagePurpose::ServerAuth, + ExtendedKeyUsagePurpose::Other(vec![1, 3, 6, 1, 4, 1, 99, 7]), + ], + ..Default::default() + }; + let key_pair = KeyPair::generate().unwrap(); + let csr = params.serialize_request(&key_pair).unwrap(); + let csr_de = CertificateSigningRequestParams::from_der(csr.der()).unwrap(); + + assert_eq!( + csr_de.params.extended_key_usages, + params.extended_key_usages + ); + } + #[test] fn serialize_and_deserialize_eq_basic_constraints() { let params = CertificateParams { diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 40680702..ffc92841 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -291,6 +291,27 @@ impl<'params> SubjectAlternativeName<'params> { }) } + /// Recover [`CertificateParams`] state from a parsed SAN extension. + /// + /// Returns true if the parsed extension was a SAN and `params` were updated. + #[cfg(feature = "x509-parser")] + pub(crate) fn from_parsed( + params: &mut CertificateParams, + parsed: &x509_parser::extensions::ParsedExtension<'_>, + ) -> Result { + Ok(match parsed { + x509_parser::extensions::ParsedExtension::SubjectAlternativeName(san) => { + for name in &san.general_names { + params + .subject_alt_names + .push(SanType::try_from_general(name)?); + } + true + }, + _ => false, + }) + } + fn write_name(writer: DERWriter, san: &SanType) { writer.write_tagged_implicit(Tag::context(san.tag()), |writer| match san { SanType::Rfc822Name(name) | SanType::DnsName(name) | SanType::URI(name) => { @@ -346,6 +367,24 @@ impl<'params> KeyUsage<'params> { Some(Self(¶ms.key_usages)) } + + /// Recover [`CertificateParams`] state from a parsed KeyUsage extension. + /// + /// Returns true if the parsed extension was a KeyUsage and `params` were updated. + #[cfg(feature = "x509-parser")] + pub(crate) fn from_parsed( + params: &mut CertificateParams, + parsed: &x509_parser::extensions::ParsedExtension<'_>, + ) -> Result { + Ok(match parsed { + x509_parser::extensions::ParsedExtension::KeyUsage(ku) => { + // x509-parser stores BIT STRING flags in reversed bit order + params.key_usages = KeyUsagePurpose::from_u16(ku.flags.reverse_bits()); + true + }, + _ => false, + }) + } } impl StaticExtension for KeyUsage<'_> { @@ -398,6 +437,53 @@ impl<'params> ExtendedKeyUsage<'params> { Some(Self(¶ms.extended_key_usages)) } + + /// Recover [`CertificateParams`] state from a parsed EKU extension. + /// + /// Returns true if the parsed extension was an EKU and `params` were updated. + #[cfg(feature = "x509-parser")] + pub(crate) fn from_parsed( + params: &mut CertificateParams, + parsed: &x509_parser::extensions::ParsedExtension<'_>, + ) -> Result { + use ExtendedKeyUsagePurpose::*; + + Ok(match parsed { + x509_parser::extensions::ParsedExtension::ExtendedKeyUsage(eku) => { + if eku.any { + params.insert_extended_key_usage(Any); + } + if eku.server_auth { + params.insert_extended_key_usage(ServerAuth); + } + if eku.client_auth { + params.insert_extended_key_usage(ClientAuth); + } + if eku.code_signing { + params.insert_extended_key_usage(CodeSigning); + } + if eku.email_protection { + params.insert_extended_key_usage(EmailProtection); + } + if eku.time_stamping { + params.insert_extended_key_usage(TimeStamping); + } + if eku.ocsp_signing { + params.insert_extended_key_usage(OcspSigning); + } + for other in &eku.other { + params.insert_extended_key_usage(Other( + other + .iter() + .ok_or(Error::UnsupportedExtension)? + .collect::>(), + )); + } + true + }, + _ => false, + }) + } } impl StaticExtension for ExtendedKeyUsage<'_> { @@ -580,6 +666,23 @@ impl BasicConstraints { Some(Self(params.is_ca)) } + + /// Recover [`CertificateParams`] state from a parsed BasicConstraints extension. + /// + /// Returns true if the parsed extension was a BasicConstraints and `params` were updated. + #[cfg(feature = "x509-parser")] + pub(crate) fn from_parsed( + params: &mut CertificateParams, + parsed: &x509_parser::extensions::ParsedExtension<'_>, + ) -> Result { + Ok(match parsed { + x509_parser::extensions::ParsedExtension::BasicConstraints(bc) => { + params.is_ca = IsCa::from_basic_constraints(bc)?; + true + }, + _ => false, + }) + } } impl StaticExtension for BasicConstraints { From 91325bc0f66bf919927249cc17d256c746ad4d50 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 13:13:25 -0400 Subject: [PATCH 23/28] certificate: parse CA cert extensions via ext type constructors Rewrite the test-only `CertificateParams::from_ca_cert_der()` as a single pass over the parsed extensions using the shared `from_parsed` constructors, adding test-only `from_parsed` for `NameConstraints` and `SubjectKeyIdentifier`. This deletes the per-field `from_x509` helpers that each re-scanned the certificate (`SanType`, `ExtendedKeyUsagePurpose`, `NameConstraints`, `IsCa`). `KeyUsagePurpose::from_x509` and `KeyIdMethod::from_x509` remain for `Issuer::from_ca_cert_der`. --- rcgen/src/certificate.rs | 103 ++++++--------------------------------- rcgen/src/ext.rs | 45 +++++++++++++++++ rcgen/src/lib.rs | 20 -------- 3 files changed, 60 insertions(+), 108 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index fe744637..3aa6705d 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -173,19 +173,25 @@ impl CertificateParams { let (_remainder, x509) = x509_parser::parse_x509_certificate(ca_cert) .map_err(|_| Error::CouldNotParseCertificate)?; - Ok(CertificateParams { - is_ca: IsCa::from_x509(&x509)?, - subject_alt_names: SanType::from_x509(&x509)?, - key_usages: KeyUsagePurpose::from_x509(&x509)?, - extended_key_usages: ExtendedKeyUsagePurpose::from_x509(&x509)?, - name_constraints: NameConstraints::from_x509(&x509)?, + let mut params = CertificateParams { serial_number: Some(x509.serial.to_bytes_be().into()), - key_identifier_method: KeyIdMethod::from_x509(&x509)?, distinguished_name: DistinguishedName::from_name(&x509.tbs_certificate.subject)?, not_before: x509.validity().not_before.to_datetime(), not_after: x509.validity().not_after.to_datetime(), ..Default::default() - }) + }; + + for parsed in x509.iter_extensions().map(|ext| ext.parsed_extension()) { + // Extensions that can't be represented in params are ignored. + let _ = BasicConstraints::from_parsed(&mut params, parsed)? + || SubjectAlternativeName::from_parsed(&mut params, parsed)? + || KeyUsage::from_parsed(&mut params, parsed)? + || ExtendedKeyUsage::from_parsed(&mut params, parsed)? + || NameConstraintsExt::from_parsed(&mut params, parsed)? + || SubjectKeyIdentifier::from_parsed(&mut params, parsed)?; + } + + Ok(params) } /// Returns the X.509 extensions for a CSR extension request attribute as defined @@ -548,41 +554,6 @@ pub enum ExtendedKeyUsagePurpose { } impl ExtendedKeyUsagePurpose { - #[cfg(all(test, feature = "x509-parser"))] - fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result, Error> { - let extended_key_usage = x509 - .extended_key_usage() - .map_err(|_| Error::CouldNotParseCertificate)? - .map(|ext| ext.value); - - let mut extended_key_usages = Vec::new(); - if let Some(extended_key_usage) = extended_key_usage { - if extended_key_usage.any { - extended_key_usages.push(Self::Any); - } - if extended_key_usage.server_auth { - extended_key_usages.push(Self::ServerAuth); - } - if extended_key_usage.client_auth { - extended_key_usages.push(Self::ClientAuth); - } - if extended_key_usage.code_signing { - extended_key_usages.push(Self::CodeSigning); - } - if extended_key_usage.email_protection { - extended_key_usages.push(Self::EmailProtection); - } - if extended_key_usage.time_stamping { - extended_key_usages.push(Self::TimeStamping); - } - if extended_key_usage.ocsp_signing { - extended_key_usages.push(Self::OcspSigning); - } - } - - Ok(extended_key_usages) - } - pub(crate) fn oid(&self) -> &[u64] { use ExtendedKeyUsagePurpose::*; match self { @@ -613,37 +584,6 @@ pub struct NameConstraints { } impl NameConstraints { - #[cfg(all(test, feature = "x509-parser"))] - fn from_x509( - x509: &x509_parser::certificate::X509Certificate<'_>, - ) -> Result, Error> { - let constraints = x509 - .name_constraints() - .map_err(|_| Error::CouldNotParseCertificate)? - .map(|ext| ext.value); - - let Some(constraints) = constraints else { - return Ok(None); - }; - - let permitted_subtrees = if let Some(permitted) = &constraints.permitted_subtrees { - GeneralSubtree::from_x509(permitted)? - } else { - Vec::new() - }; - - let excluded_subtrees = if let Some(excluded) = &constraints.excluded_subtrees { - GeneralSubtree::from_x509(excluded)? - } else { - Vec::new() - }; - - Ok(Some(Self { - permitted_subtrees, - excluded_subtrees, - })) - } - pub(crate) fn is_empty(&self) -> bool { self.permitted_subtrees.is_empty() && self.excluded_subtrees.is_empty() } @@ -667,7 +607,7 @@ pub enum GeneralSubtree { impl GeneralSubtree { #[cfg(all(test, feature = "x509-parser"))] - fn from_x509( + pub(crate) fn from_x509( subtrees: &[x509_parser::extensions::GeneralSubtree<'_>], ) -> Result, Error> { use x509_parser::extensions::GeneralName; @@ -838,19 +778,6 @@ pub enum IsCa { } impl IsCa { - #[cfg(all(test, feature = "x509-parser"))] - fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result { - let basic_constraints = x509 - .basic_constraints() - .map_err(|_| Error::CouldNotParseCertificate)? - .map(|ext| ext.value); - - match basic_constraints { - Some(bc) => Self::from_basic_constraints(bc), - None => Ok(Self::NoCa), - } - } - #[cfg(feature = "x509-parser")] pub(crate) fn from_basic_constraints( basic_constraints: &x509_parser::extensions::BasicConstraints, diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index ffc92841..8a805317 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -530,6 +530,34 @@ impl<'params> NameConstraints<'params> { } } + /// Recover [`CertificateParams`] state from a parsed NameConstraints extension. + /// + /// Returns true if the parsed extension was a NameConstraints and `params` were updated. + #[cfg(all(test, feature = "x509-parser"))] + pub(crate) fn from_parsed( + params: &mut CertificateParams, + parsed: &x509_parser::extensions::ParsedExtension<'_>, + ) -> Result { + Ok(match parsed { + x509_parser::extensions::ParsedExtension::NameConstraints(nc) => { + let permitted_subtrees = match &nc.permitted_subtrees { + Some(permitted) => GeneralSubtree::from_x509(permitted)?, + None => Vec::new(), + }; + let excluded_subtrees = match &nc.excluded_subtrees { + Some(excluded) => GeneralSubtree::from_x509(excluded)?, + None => Vec::new(), + }; + params.name_constraints = Some(crate::NameConstraints { + permitted_subtrees, + excluded_subtrees, + }); + true + }, + _ => false, + }) + } + fn write_general_subtrees(writer: DERWriter, tag: u64, general_subtrees: &[GeneralSubtree]) { /* GeneralSubtrees ::= SEQUENCE SIZE (1..MAX) OF GeneralSubtree @@ -632,6 +660,23 @@ impl SubjectKeyIdentifier { pub(crate) fn new(key_identifier_method: &KeyIdMethod, pub_key_spki: &[u8]) -> Self { Self(key_identifier_method.derive(pub_key_spki)) } + + /// Recover [`CertificateParams`] state from a parsed SKI extension. + /// + /// Returns true if the parsed extension was a SKI and `params` were updated. + #[cfg(all(test, feature = "x509-parser"))] + pub(crate) fn from_parsed( + params: &mut CertificateParams, + parsed: &x509_parser::extensions::ParsedExtension<'_>, + ) -> Result { + Ok(match parsed { + x509_parser::extensions::ParsedExtension::SubjectKeyIdentifier(ski) => { + params.key_identifier_method = KeyIdMethod::PreSpecified(ski.0.to_vec()); + true + }, + _ => false, + }) + } } impl StaticExtension for SubjectKeyIdentifier { diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 3fe9676d..7e9a1db4 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -316,26 +316,6 @@ pub enum SanType { OtherName((Vec, OtherNameValue)), } -impl SanType { - #[cfg(all(test, feature = "x509-parser"))] - fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result, Error> { - let sans = x509 - .subject_alternative_name() - .map_err(|_| Error::CouldNotParseCertificate)? - .map(|ext| &ext.value.general_names); - - let Some(sans) = sans else { - return Ok(Vec::new()); - }; - - let mut subject_alt_names = Vec::with_capacity(sans.len()); - for san in sans { - subject_alt_names.push(Self::try_from_general(san)?); - } - Ok(subject_alt_names) - } -} - /// An `OtherName` value, defined in [RFC 5280§4.1.2.4]. /// /// While the standard specifies this could be any ASN.1 type rcgen limits From 6f7fbff43a34eb976054700ac044f553e0ee25d5 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 11 Aug 2026 13:18:42 -0400 Subject: [PATCH 24/28] certificate: add kitchen-sink extension round-trip test Serialize params exercising every certificate extension writer, then assert the exact extension count and that parsing recovers what was requested. A certificate missing a requested extension is still well-formed, so presence regressions can only be caught by comparing the output against the requested params. --- rcgen/src/certificate.rs | 71 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 3aa6705d..2a086de8 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -1102,6 +1102,77 @@ mod tests { } } + #[cfg(feature = "x509-parser")] + #[test] + fn test_kitchen_sink_params_round_trip() { + // Every requested extension must appear in the serialized certificate: + // presence is derived from the built extension collection, and this test + // guards against a params field being silently dropped from the output + // (see rustls/rcgen#446). + let params = CertificateParams { + subject_alt_names: vec![ + SanType::DnsName("kitchen.example.com".try_into().unwrap()), + SanType::Rfc822Name("mail@example.com".try_into().unwrap()), + ], + key_usages: vec![ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::KeyCertSign, + ], + extended_key_usages: vec![ + ExtendedKeyUsagePurpose::ServerAuth, + ExtendedKeyUsagePurpose::Other(vec![1, 3, 6, 1, 4, 1, 99, 7]), + ], + name_constraints: Some(NameConstraints { + permitted_subtrees: vec![GeneralSubtree::DnsName("example.com".into())], + excluded_subtrees: Vec::new(), + }), + crl_distribution_points: vec![CrlDistributionPoint { + uris: vec!["http://crl.example.com/kitchen.crl".into()], + }], + is_ca: IsCa::Ca(PathLenConstraint::Constrained(1)), + custom_extensions: vec![CustomExtension::from_oid_content( + &[1, 3, 6, 1, 4, 1, 99, 8], + crate::Criticality::NonCritical, + vec![0x05, 0x00], + )], + serial_number: Some(SerialNumber::from_slice(&[0x0A, 0x0B])), + ..CertificateParams::default() + }; + + let key_pair = KeyPair::generate().unwrap(); + let cert = params.self_signed(&key_pair).unwrap(); + let (_rem, x509) = x509_parser::parse_x509_certificate(cert.der()).unwrap(); + + // SAN, KU, EKU, NC, CRLDP, SKI, BC and the custom extension: nothing + // requested may be missing, and nothing extra may appear. + assert_eq!(x509.iter_extensions().count(), 8); + + // Fields recoverable through parsing must match what was requested. + let recovered = CertificateParams::from_ca_cert_der(cert.der()).unwrap(); + assert_eq!(recovered.subject_alt_names, params.subject_alt_names); + assert_eq!(recovered.key_usages, params.key_usages); + assert_eq!(recovered.extended_key_usages, params.extended_key_usages); + assert_eq!(recovered.name_constraints, params.name_constraints); + assert_eq!(recovered.is_ca, params.is_ca); + assert_eq!(recovered.serial_number, params.serial_number); + assert_eq!( + recovered.key_identifier_method, + KeyIdMethod::PreSpecified(params.key_identifier(&key_pair)), + ); + + // The CRL distribution points and custom extension are not recovered into + // params, so check them against the parsed certificate directly. + assert!(x509.iter_extensions().any(|ext| matches!( + ext.parsed_extension(), + x509_parser::extensions::ParsedExtension::CRLDistributionPoints(_) + ))); + let custom = x509 + .iter_extensions() + .find(|ext| ext.oid.to_id_string() == "1.3.6.1.4.1.99.8") + .unwrap(); + assert_eq!(custom.value, &[0x05, 0x00]); + } + #[cfg(feature = "x509-parser")] #[test] fn parse_other_name_alt_name() { From 128483159e941f90635127b87a2185670ea25c59 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 12 Aug 2026 16:26:23 -0400 Subject: [PATCH 25/28] csr: preserve unhandled requested extensions as custom extensions `CertificateSigningRequestParams::from_der` previously returned `Error::UnsupportedExtension` for any requested extension other than SAN/KU/EKU/BasicConstraints - including custom extensions rcgen itself wrote via `serialize_request`. Iterate the raw extension request attribute (rather than x509-parser's pre-parsed view, which discards OIDs) and recover anything unhandled into `CertificateParams::custom_extensions` via a new `CustomExtension::from_parsed` constructor, preserving OID, criticality and value so that serializing the recovered params reproduces the request. --- rcgen/src/csr.rs | 68 ++++++++++++++++++++++++++++++++++++++---------- rcgen/src/ext.rs | 17 ++++++++++++ 2 files changed, 71 insertions(+), 14 deletions(-) diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index 6ac4898b..ea4f8c15 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -6,13 +6,13 @@ use pki_types::CertificateSigningRequestDer; #[cfg(feature = "x509-parser")] use crate::ext::{BasicConstraints, ExtendedKeyUsage, KeyUsage, SubjectAlternativeName}; -#[cfg(feature = "x509-parser")] -use crate::DistinguishedName; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ Certificate, CertificateParams, Error, Issuer, PublicKeyData, SignatureAlgorithm, SigningKey, }; +#[cfg(feature = "x509-parser")] +use crate::{CustomExtension, DistinguishedName}; /// A public key, extracted from a CSR #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -93,14 +93,15 @@ impl CertificateSigningRequestParams { /// Parse and verify a certificate signing request from DER-encoded bytes /// - /// Currently, this supports the following extensions: + /// The following requested extensions are parsed natively into params: /// - `Subject Alternative Name` (see [`crate::SanType`]) /// - `Key Usage` (see [`crate::KeyUsagePurpose`]) /// - `Extended Key Usage` (see [`crate::ExtendedKeyUsagePurpose`]) /// - `Basic Constraints` (see [`crate::PathLenConstraint`]) /// - /// On encountering other extensions, this function will return [`Error::UnsupportedExtension`]. - /// If the request's signature is invalid, it will return + /// Any other requested extensions are preserved verbatim in + /// [`CertificateParams::custom_extensions`] as [`CustomExtension`]s. + /// If the request's signature is invalid, this function will return /// [`Error::InvalidCertificationRequestSignature`]. /// /// The [`PemObject`] trait is often used to obtain a [`CertificateSigningRequestDer`] from @@ -132,22 +133,34 @@ impl CertificateSigningRequestParams { }; let raw = info.subject_pki.subject_public_key.data.to_vec(); - if let Some(extensions) = csr.requested_extensions() { - for parsed in extensions { + let requested_extensions = + info.iter_attributes() + .find_map(|attr| match attr.parsed_attribute() { + x509_parser::prelude::ParsedCriAttribute::ExtensionRequest(requested) => { + Some(&requested.extensions) + }, + _ => None, + }); + + if let Some(requested_extensions) = requested_extensions { + for extension in requested_extensions { + let parsed = extension.parsed_extension(); let handled = KeyUsage::from_parsed(&mut params, parsed)? || SubjectAlternativeName::from_parsed(&mut params, parsed)? || ExtendedKeyUsage::from_parsed(&mut params, parsed)? || BasicConstraints::from_parsed(&mut params, parsed)?; + + // Extensions that params can't represent natively are preserved + // verbatim, so serializing the recovered params reproduces the + // requested extensions. if !handled { - return Err(Error::UnsupportedExtension); + params + .custom_extensions + .push(CustomExtension::from_parsed(extension)?); } } } - // Not yet handled: - // * name_constraints - // and any other extensions. - Ok(Self { params, public_key: PublicKey { alg, raw }, @@ -181,8 +194,8 @@ mod tests { use x509_parser::prelude::{FromDer, ParsedExtension}; use crate::{ - CertificateParams, CertificateSigningRequestParams, ExtendedKeyUsagePurpose, IsCa, KeyPair, - KeyUsagePurpose, PathLenConstraint, + CertificateParams, CertificateSigningRequestParams, Criticality, CustomExtension, + ExtendedKeyUsagePurpose, IsCa, KeyPair, KeyUsagePurpose, PathLenConstraint, }; #[test] @@ -242,6 +255,33 @@ mod tests { )); } + #[test] + fn serialize_and_deserialize_eq_custom_extensions() { + // Custom extensions must survive a serialize/parse round trip, preserving + // OID, criticality and value. See rustls/rcgen#446 for context: rcgen + // previously rejected CSRs containing extensions it wrote itself. + let params = CertificateParams { + custom_extensions: vec![ + CustomExtension::from_oid_content( + &[1, 3, 6, 1, 4, 1, 99, 9], + Criticality::Critical, + vec![0x0C, 0x02, 0x68, 0x69], + ), + CustomExtension::from_oid_content( + &[1, 3, 6, 1, 4, 1, 99, 10], + Criticality::NonCritical, + vec![0x05, 0x00], + ), + ], + ..Default::default() + }; + let key_pair = KeyPair::generate().unwrap(); + let csr = params.serialize_request(&key_pair).unwrap(); + let csr_de = CertificateSigningRequestParams::from_der(csr.der()).unwrap(); + + assert_eq!(csr_de.params.custom_extensions, params.custom_extensions); + } + #[test] fn serialize_and_deserialize_eq_other_eku() { // Custom EKU purpose OIDs must survive a serialize/parse round trip. diff --git a/rcgen/src/ext.rs b/rcgen/src/ext.rs index 8a805317..8367ba2a 100644 --- a/rcgen/src/ext.rs +++ b/rcgen/src/ext.rs @@ -793,6 +793,23 @@ impl CustomExtension { } } + /// Recover a custom extension from a parsed X.509 extension that rcgen does not + /// represent natively in [`CertificateParams`]. + #[cfg(feature = "x509-parser")] + pub(crate) fn from_parsed( + parsed: &x509_parser::extensions::X509Extension<'_>, + ) -> Result { + Ok(Self { + oid: parsed + .oid + .iter() + .ok_or(Error::UnsupportedExtension)? + .collect::>(), + criticality: parsed.critical.into(), + der_value: parsed.value.to_vec(), + }) + } + /// Obtains the OID components of the extensions, as u64 pieces pub fn oid_components(&self) -> impl Iterator + '_ { self.oid.iter().copied() From f1c5c84c113397bfa55359fef7f8d8665f7dd0bf Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 16:41:41 -0400 Subject: [PATCH 26/28] csr: reject duplicate requested extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 5280 §4.2 forbids multiple instances of the same extension. A CSR requesting an extension twice previously either merged both instances into the recovered `CertificateParams` (for natively handled types) or preserved both as custom extensions, deferring the failure to re-serialization. Reject the duplicate up front with `Error::DuplicateExtension`, mirroring the write-side collection invariant. --- rcgen/src/csr.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index ea4f8c15..00dede4d 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -143,7 +143,17 @@ impl CertificateSigningRequestParams { }); if let Some(requested_extensions) = requested_extensions { + let mut seen_oids = Vec::new(); for extension in requested_extensions { + // RFC 5280 §4.2: "A certificate MUST NOT include more than one + // instance of a particular extension." Reject duplicates up front + // instead of merging them, or deferring the failure to + // re-serialization of the recovered params. + if seen_oids.contains(&&extension.oid) { + return Err(Error::DuplicateExtension(extension.oid.to_string())); + } + seen_oids.push(&extension.oid); + let parsed = extension.parsed_extension(); let handled = KeyUsage::from_parsed(&mut params, parsed)? || SubjectAlternativeName::from_parsed(&mut params, parsed)? @@ -198,6 +208,65 @@ mod tests { ExtendedKeyUsagePurpose, IsCa, KeyPair, KeyUsagePurpose, PathLenConstraint, }; + #[test] + fn rejects_duplicate_requested_extensions() { + use yasna::models::ObjectIdentifier; + use yasna::{DERWriter, Tag}; + + use crate::key_pair::{serialize_public_key_der, sign_der}; + use crate::{oid, write_distinguished_name, DistinguishedName, Error}; + + // Hand-build a CSR requesting the same extension twice: rcgen itself + // refuses to serialize duplicates, so the DER is written directly. + let key_pair = KeyPair::generate().unwrap(); + let csr = sign_der(&key_pair, |writer| { + writer.next().write_u8(0); // version + write_distinguished_name(writer.next(), &DistinguishedName::new()); + serialize_public_key_der(&key_pair, writer.next()); + // attributes [0] IMPLICIT SET OF Attribute + writer + .next() + .write_tagged_implicit(Tag::context(0), |writer| { + writer.write_set_of(|writer| write_extension_request(writer.next())); + }); + Ok(()) + }) + .unwrap(); + + assert_eq!( + CertificateSigningRequestParams::from_der(&csr.into()).unwrap_err(), + Error::DuplicateExtension("1.3.6.1.4.1.99".into()), + ); + + // The PKCS #9 extensionRequest attribute, requesting the same extension + // twice. + fn write_extension_request(writer: DERWriter) { + writer.write_sequence(|writer| { + writer.next().write_oid(&ObjectIdentifier::from_slice( + oid::PKCS_9_AT_EXTENSION_REQUEST, + )); + writer.next().write_set(|writer| { + writer.next().write_sequence(|writer| { + write_test_extension(writer.next()); + write_test_extension(writer.next()); + }); + }); + }); + } + + // A minimal extension with a fixed private OID and a NULL value. + fn write_test_extension(writer: DERWriter) { + writer.write_sequence(|writer| { + writer + .next() + .write_oid(&ObjectIdentifier::from_slice(&[1, 3, 6, 1, 4, 1, 99])); + writer + .next() + .write_bytes(&yasna::construct_der(|writer| writer.write_null())); + }); + } + } + #[test] fn dont_write_sans_extension_if_no_sans_are_present() { let mut params = CertificateParams::default(); From 4c1633edf85a146bb6d7323d8d41211f77662da5 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 16:43:01 -0400 Subject: [PATCH 27/28] certificate: reject duplicate extensions when parsing CA certs Like the CSR extension request path, `from_ca_cert_der()` previously merged duplicate instances of natively handled extensions into params silently. Reject them with `Error::DuplicateExtension` instead, mirroring the write-side collection invariant. --- rcgen/src/certificate.rs | 73 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 2a086de8..8eab956b 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -181,8 +181,18 @@ impl CertificateParams { ..Default::default() }; - for parsed in x509.iter_extensions().map(|ext| ext.parsed_extension()) { + let mut seen_oids = Vec::new(); + for ext in x509.iter_extensions() { + // RFC 5280 §4.2: "A certificate MUST NOT include more than one + // instance of a particular extension." Reject duplicates up front + // instead of merging them. + if seen_oids.contains(&&ext.oid) { + return Err(Error::DuplicateExtension(ext.oid.to_string())); + } + seen_oids.push(&ext.oid); + // Extensions that can't be represented in params are ignored. + let parsed = ext.parsed_extension(); let _ = BasicConstraints::from_parsed(&mut params, parsed)? || SubjectAlternativeName::from_parsed(&mut params, parsed)? || KeyUsage::from_parsed(&mut params, parsed)? @@ -1173,6 +1183,67 @@ mod tests { assert_eq!(custom.value, &[0x05, 0x00]); } + #[cfg(feature = "x509-parser")] + #[test] + fn from_ca_cert_der_rejects_duplicate_extensions() { + use yasna::DERWriter; + + use crate::key_pair::sign_der; + + // Hand-build a v3 certificate carrying the same extension twice: rcgen + // itself refuses to serialize duplicates, so the DER is written directly. + let key_pair = KeyPair::generate().unwrap(); + let der = sign_der(&key_pair, |writer| { + // Write version + writer.next().write_tagged(Tag::context(0), |writer| { + writer.write_u8(2); + }); + writer.next().write_u8(1); // serialNumber + key_pair.algorithm().write_alg_ident(writer.next()); + write_distinguished_name(writer.next(), &DistinguishedName::new()); // issuer + write_validity(writer.next()); + write_distinguished_name(writer.next(), &DistinguishedName::new()); // subject + serialize_public_key_der(&key_pair, writer.next()); + write_duplicate_extensions(writer.next()); + Ok(()) + }) + .unwrap(); + + assert_eq!( + CertificateParams::from_ca_cert_der(&der.into()).unwrap_err(), + Error::DuplicateExtension("1.3.6.1.4.1.99".into()), + ); + + fn write_validity(writer: DERWriter) { + writer.write_sequence(|writer| { + write_dt_utc_or_generalized(writer.next(), date_time_ymd(1975, 1, 1)); + write_dt_utc_or_generalized(writer.next(), date_time_ymd(4096, 1, 1)); + }); + } + + // The X.509v3 extensions field, holding a minimal private-OID extension + // twice. + fn write_duplicate_extensions(writer: DERWriter) { + writer.write_tagged(Tag::context(3), |writer| { + writer.write_sequence(|writer| { + write_test_extension(writer.next()); + write_test_extension(writer.next()); + }) + }); + } + + fn write_test_extension(writer: DERWriter) { + writer.write_sequence(|writer| { + writer + .next() + .write_oid(&ObjectIdentifier::from_slice(&[1, 3, 6, 1, 4, 1, 99])); + writer + .next() + .write_bytes(&yasna::construct_der(|writer| writer.write_null())); + }); + } + } + #[cfg(feature = "x509-parser")] #[test] fn parse_other_name_alt_name() { From e9c60c6c8a9212aa2d69c6a4ab837fd2fb180ea9 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 29 Aug 2026 17:09:19 -0400 Subject: [PATCH 28/28] string: replace chunks_exact with as_chunks Clippy's new `chunks_exact_to_as_chunks` lint (denied in CI) flags `chunks_exact` calls with a constant chunk size. `slice::as_chunks` stabilized in Rust 1.88, the crate MSRV, and yields `[u8; N]` chunks directly, resolving the FIXMEs that waited on `array_chunks`. --- rcgen/src/string.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/rcgen/src/string.rs b/rcgen/src/string.rs index 759cd0f4..81e5972e 100644 --- a/rcgen/src/string.rs +++ b/rcgen/src/string.rs @@ -425,10 +425,11 @@ impl BmpString { ))); } - // FIXME: Update this when `array_chunks` is stabilized. for maybe_char in char::decode_utf16( - vec.chunks_exact(2) - .map(|chunk| u16::from_be_bytes([chunk[0], chunk[1]])), + vec.as_chunks::<2>() + .0 + .iter() + .map(|chunk| u16::from_be_bytes(*chunk)), ) { // We check we only use the BMP subset of Unicode (the first 65 536 code points) match maybe_char { @@ -544,10 +545,11 @@ impl UniversalString { )); } - // FIXME: Update this when `array_chunks` is stabilized. for maybe_char in vec - .chunks_exact(4) - .map(|chunk| u32::from_be_bytes([chunk[0], chunk[1], chunk[2], chunk[3]])) + .as_chunks::<4>() + .0 + .iter() + .map(|chunk| u32::from_be_bytes(*chunk)) { if core::char::from_u32(maybe_char).is_none() { return Err(Error::InvalidAsn1String(