From 8c4489d043845403badf6d08614630ab4833513e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 22:39:04 +0200 Subject: [PATCH 1/9] Require passing in CSR attributes --- rcgen/src/certificate.rs | 17 +---------------- rcgen/tests/generic.rs | 10 +++++----- rcgen/tests/openssl.rs | 2 +- rcgen/tests/webpki.rs | 2 +- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 143dfaf1..2f248199 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -513,21 +513,6 @@ impl CertificateParams { }); } - /// Generate and serialize a certificate signing request (CSR). - /// - /// The constructed CSR will contain attributes based on the certificate parameters, - /// and include the subject public key information from `subject_key`. Additionally, - /// the CSR will be signed using the subject key. - /// - /// Note that subsequent invocations of `serialize_request()` will not produce the exact - /// same output. - pub fn serialize_request( - &self, - subject_key: &impl SigningKey, - ) -> Result { - self.serialize_request_with_attributes(subject_key, Vec::new()) - } - /// Generate and serialize a certificate signing request (CSR) with custom PKCS #10 attributes. /// as defined in [RFC 2986]. /// @@ -539,7 +524,7 @@ impl CertificateParams { /// same output. /// /// [RFC 2986]: - pub fn serialize_request_with_attributes( + pub fn serialize_request( &self, subject_key: &impl SigningKey, attrs: Vec, diff --git a/rcgen/tests/generic.rs b/rcgen/tests/generic.rs index ed1a8abc..1f6f997d 100644 --- a/rcgen/tests/generic.rs +++ b/rcgen/tests/generic.rs @@ -107,7 +107,7 @@ mod test_x509_custom_ext { assert_eq!(favorite_drink_ext.value, test_ext); // Generate a CSR with the custom extension, parse it with x509-parser. - let test_cert_csr = params.serialize_request(&test_key).unwrap(); + let test_cert_csr = params.serialize_request(&test_key, Vec::new()).unwrap(); let (_, x509_csr) = X509CertificationRequest::from_der(test_cert_csr.der()).unwrap(); // We should find that the CSR contains requested extensions. @@ -176,7 +176,7 @@ mod test_csr_custom_attributes { let params = CertificateParams::default(); let key_pair = KeyPair::generate().unwrap(); let csr = params - .serialize_request_with_attributes(&key_pair, vec![challenge_password_attribute]) + .serialize_request(&key_pair, vec![challenge_password_attribute]) .unwrap(); // Parse the CSR @@ -425,7 +425,7 @@ mod test_csr_extension_request { let mut params = CertificateParams::default(); params.key_usages.push(KeyUsagePurpose::DigitalSignature); let key_pair = KeyPair::generate().unwrap(); - let csr = params.serialize_request(&key_pair).unwrap(); + let csr = params.serialize_request(&key_pair, Vec::new()).unwrap(); let (_, parsed_csr) = X509CertificationRequest::from_der(csr.der()).unwrap(); assert!(!parsed_csr .requested_extensions() @@ -440,7 +440,7 @@ mod test_csr_extension_request { .extended_key_usages .push(ExtendedKeyUsagePurpose::ClientAuth); let key_pair = KeyPair::generate().unwrap(); - let csr = params.serialize_request(&key_pair).unwrap(); + let csr = params.serialize_request(&key_pair, Vec::new()).unwrap(); let (_, parsed_csr) = X509CertificationRequest::from_der(csr.der()).unwrap(); let requested_extensions = parsed_csr .requested_extensions() @@ -508,7 +508,7 @@ mod test_csr { // Generate a key pair for the CSR let key_pair = KeyPair::generate().unwrap(); // Serialize the CSR into DER from the given parameters - let csr = params.serialize_request(&key_pair).unwrap(); + let csr = params.serialize_request(&key_pair, Vec::new()).unwrap(); // Parse the CSR we just serialized let csrp = CertificateSigningRequestParams::from_der(csr.der()).unwrap(); diff --git a/rcgen/tests/openssl.rs b/rcgen/tests/openssl.rs index 19912a3e..ea36298c 100644 --- a/rcgen/tests/openssl.rs +++ b/rcgen/tests/openssl.rs @@ -165,7 +165,7 @@ fn verify_cert_ca(cert_pem: &str, key: &[u8], ca_cert_pem: &str) { fn verify_csr(params: &CertificateParams, key_pair: &KeyPair) { let csr = params - .serialize_request(key_pair) + .serialize_request(key_pair, Vec::new()) .and_then(|csr| csr.pem()) .unwrap(); println!("{csr}"); diff --git a/rcgen/tests/webpki.rs b/rcgen/tests/webpki.rs index 82d7f630..b57b8e75 100644 --- a/rcgen/tests/webpki.rs +++ b/rcgen/tests/webpki.rs @@ -511,7 +511,7 @@ fn test_certificate_from_csr() { } let cert_key = KeyPair::generate().unwrap(); - let csr = params.serialize_request(&cert_key).unwrap(); + let csr = params.serialize_request(&cert_key, Vec::new()).unwrap(); let csr = CertificateSigningRequestParams::from_der(csr.der()).unwrap(); let ekus_contained = &csr.params.extended_key_usages; From 696dd88856e8c349ba0be71e3a5bb4082bba032c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 22:48:27 +0200 Subject: [PATCH 2/9] Rename CertificateParams::serialize_request() to CertificateSigningRequest::new() --- rcgen/src/certificate.rs | 136 ++------------------------------------- rcgen/src/csr.rs | 126 +++++++++++++++++++++++++++++++++++- rcgen/tests/generic.rs | 27 ++++---- rcgen/tests/openssl.rs | 7 +- rcgen/tests/webpki.rs | 4 +- 5 files changed, 152 insertions(+), 148 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 2f248199..34912790 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -3,13 +3,12 @@ use std::str::FromStr; #[cfg(feature = "pem")] use pem::Pem; -use pki_types::{CertificateDer, CertificateSigningRequestDer}; +use pki_types::CertificateDer; use time::{Date, Month, OffsetDateTime, PrimitiveDateTime, Time}; use yasna::models::ObjectIdentifier; use yasna::{DERWriter, Tag}; use crate::crl::CrlDistributionPoint; -use crate::csr::CertificateSigningRequest; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; #[cfg(feature = "crypto")] use crate::ring_like::digest; @@ -415,35 +414,8 @@ impl CertificateParams { Ok(result) } - /// Write a CSR extension request attribute as defined in [RFC 2985]. - /// - /// [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| { - // Write key_usage - self.write_key_usage(writer.next()); - // Write subject_alt_names - self.write_subject_alt_names(writer.next()); - self.write_extended_key_usage(writer.next()); - - // Write custom extensions - for ext in &self.custom_extensions { - write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { - writer.write_der(ext.content()) - }); - } - }); - }); - }); - } - /// Write a certificate's KeyUsage as defined in RFC 5280. - fn write_key_usage(&self, writer: DERWriter) { + pub(crate) fn write_key_usage(&self, writer: DERWriter) { // RFC 5280 defines 9 key usages, which we detail in our key usage enum // We could use std::mem::variant_count here, but it's experimental const KEY_USAGE_BITS: usize = 9; @@ -461,7 +433,7 @@ impl CertificateParams { }); } - fn write_extended_key_usage(&self, writer: DERWriter) { + pub(crate) 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| { @@ -475,7 +447,7 @@ impl CertificateParams { } } - fn write_subject_alt_names(&self, writer: DERWriter) { + pub(crate) fn write_subject_alt_names(&self, writer: DERWriter) { if self.subject_alt_names.is_empty() { return; } @@ -513,100 +485,6 @@ impl CertificateParams { }); } - /// Generate and serialize a certificate signing request (CSR) with custom PKCS #10 attributes. - /// as defined in [RFC 2986]. - /// - /// The constructed CSR will contain attributes based on the certificate parameters, - /// and include the subject public key information from `subject_key`. Additionally, - /// the CSR will be self-signed using the subject key. - /// - /// Note that subsequent invocations of `serialize_request_with_attributes()` will not produce the exact - /// same output. - /// - /// [RFC 2986]: - pub fn serialize_request( - &self, - subject_key: &impl SigningKey, - attrs: Vec, - ) -> Result { - // No .. pattern, we use this to ensure every field is used - #[deny(unused)] - let Self { - not_before, - not_after, - serial_number, - subject_alt_names, - distinguished_name, - is_ca, - key_usages, - extended_key_usages, - name_constraints, - crl_distribution_points, - custom_extensions, - use_authority_key_identifier_extension, - key_identifier_method, - } = self; - // - alg and key_pair 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 - // - 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 - // needed. - let _ = ( - not_before, - not_after, - key_identifier_method, - extended_key_usages, - ); - if serial_number.is_some() - || *is_ca != IsCa::NoCa - || name_constraints.is_some() - || !crl_distribution_points.is_empty() - || *use_authority_key_identifier_extension - { - 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(); - - let der = sign_der(subject_key, |writer| { - // Write version - writer.next().write_u8(0); - write_distinguished_name(writer.next(), distinguished_name); - serialize_public_key_der(subject_key, writer.next()); - - // According to the spec in RFC 2986, even if attributes are empty we need the empty attribute tag - writer - .next() - .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()); - } - - for Attribute { oid, values } in attrs { - writer.next().write_sequence(|writer| { - writer.next().write_oid(&ObjectIdentifier::from_slice(&oid)); - writer.next().write_der(&values); - }); - } - }); - }); - - Ok(()) - })?; - - Ok(CertificateSigningRequest { - der: CertificateSigningRequestDer::from(der), - }) - } - pub(crate) fn serialize_der_with_signer( &self, pub_key: &K, @@ -874,11 +752,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) critical: bool, /// The content must be DER-encoded - content: Vec, + pub(crate) content: Vec, } impl CustomExtension { diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index 1833794c..1bbf2ad9 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -3,11 +3,14 @@ use std::hash::Hash; #[cfg(feature = "pem")] use pem::Pem; use pki_types::CertificateSigningRequestDer; +use yasna::{models::ObjectIdentifier, DERWriter, Tag}; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ - Certificate, CertificateParams, Error, Issuer, PublicKeyData, SignatureAlgorithm, SigningKey, + key_pair::{serialize_public_key_der, sign_der}, + oid, write_distinguished_name, write_x509_extension, Attribute, Certificate, CertificateParams, + Error, IsCa, Issuer, PublicKeyData, SignatureAlgorithm, SigningKey, }; #[cfg(feature = "x509-parser")] use crate::{DistinguishedName, SanType}; @@ -43,6 +46,100 @@ pub struct CertificateSigningRequest { } impl CertificateSigningRequest { + /// Generate and serialize a certificate signing request (CSR) with custom PKCS #10 attributes. + /// as defined in [RFC 2986]. + /// + /// The constructed CSR will contain attributes based on the certificate parameters, + /// and include the subject public key information from `subject_key`. Additionally, + /// the CSR will be self-signed using the subject key. + /// + /// Note that subsequent invocations of `serialize_request_with_attributes()` will not produce the exact + /// same output. + /// + /// [RFC 2986]: + pub fn new( + params: &CertificateParams, + subject_key: &impl SigningKey, + attrs: Vec, + ) -> Result { + // No .. pattern, we use this to ensure every field is used + #[deny(unused)] + let CertificateParams { + not_before, + not_after, + serial_number, + subject_alt_names, + distinguished_name, + is_ca, + key_usages, + extended_key_usages, + name_constraints, + crl_distribution_points, + custom_extensions, + use_authority_key_identifier_extension, + key_identifier_method, + } = params; + // - alg and key_pair 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 + // - 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 + // needed. + let _ = ( + not_before, + not_after, + key_identifier_method, + extended_key_usages, + ); + if serial_number.is_some() + || *is_ca != IsCa::NoCa + || name_constraints.is_some() + || !crl_distribution_points.is_empty() + || *use_authority_key_identifier_extension + { + 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(); + + let der = sign_der(subject_key, |writer| { + // Write version + writer.next().write_u8(0); + write_distinguished_name(writer.next(), distinguished_name); + serialize_public_key_der(subject_key, writer.next()); + + // According to the spec in RFC 2986, even if attributes are empty we need the empty attribute tag + writer + .next() + .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 { + write_extension_request_attribute(params, writer.next()); + } + + for Attribute { oid, values } in attrs { + writer.next().write_sequence(|writer| { + writer.next().write_oid(&ObjectIdentifier::from_slice(&oid)); + writer.next().write_der(&values); + }); + } + }); + }); + + Ok(()) + })?; + + Ok(Self { + der: CertificateSigningRequestDer::from(der), + }) + } + /// Get the PEM-encoded bytes of the certificate signing request. #[cfg(feature = "pem")] pub fn pem(&self) -> Result { @@ -65,6 +162,33 @@ impl From for CertificateSigningRequestDer<'static> { } } +/// Write a CSR extension request attribute as defined in [RFC 2985]. +/// +/// [RFC 2985]: +pub(crate) fn write_extension_request_attribute(params: &CertificateParams, 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 key_usage + params.write_key_usage(writer.next()); + // Write subject_alt_names + params.write_subject_alt_names(writer.next()); + params.write_extended_key_usage(writer.next()); + + // Write custom extensions + for ext in ¶ms.custom_extensions { + write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { + writer.write_der(ext.content()) + }); + } + }); + }); + }); +} + /// Parameters for a certificate signing request #[derive(Debug)] pub struct CertificateSigningRequestParams { diff --git a/rcgen/tests/generic.rs b/rcgen/tests/generic.rs index 1f6f997d..f3bcae01 100644 --- a/rcgen/tests/generic.rs +++ b/rcgen/tests/generic.rs @@ -70,7 +70,7 @@ mod test_convert_x509_subject_alternative_name { mod test_x509_custom_ext { use crate::util; - use rcgen::CustomExtension; + use rcgen::{CertificateSigningRequest, CustomExtension}; use x509_parser::oid_registry::asn1_rs; use x509_parser::prelude::{ FromDer, ParsedCriAttribute, X509Certificate, X509CertificationRequest, @@ -107,7 +107,7 @@ mod test_x509_custom_ext { assert_eq!(favorite_drink_ext.value, test_ext); // Generate a CSR with the custom extension, parse it with x509-parser. - let test_cert_csr = params.serialize_request(&test_key, Vec::new()).unwrap(); + let test_cert_csr = CertificateSigningRequest::new(¶ms, &test_key, Vec::new()).unwrap(); let (_, x509_csr) = X509CertificationRequest::from_der(test_cert_csr.der()).unwrap(); // We should find that the CSR contains requested extensions. @@ -137,7 +137,7 @@ mod test_x509_custom_ext { #[cfg(feature = "x509-parser")] mod test_csr_custom_attributes { - use rcgen::{Attribute, CertificateParams, KeyPair}; + use rcgen::{Attribute, CertificateParams, CertificateSigningRequest, KeyPair}; use x509_parser::{ der_parser::Oid, prelude::{FromDer, X509CertificationRequest}, @@ -175,9 +175,9 @@ mod test_csr_custom_attributes { // Serialize a DER-encoded CSR let params = CertificateParams::default(); let key_pair = KeyPair::generate().unwrap(); - let csr = params - .serialize_request(&key_pair, vec![challenge_password_attribute]) - .unwrap(); + let csr = + CertificateSigningRequest::new(¶ms, &key_pair, vec![challenge_password_attribute]) + .unwrap(); // Parse the CSR let (_, x509_csr) = X509CertificationRequest::from_der(csr.der()).unwrap(); @@ -417,7 +417,10 @@ mod test_parse_other_name_alt_name { #[cfg(feature = "x509-parser")] mod test_csr_extension_request { - use rcgen::{CertificateParams, ExtendedKeyUsagePurpose, KeyPair, KeyUsagePurpose}; + use rcgen::{ + CertificateParams, CertificateSigningRequest, ExtendedKeyUsagePurpose, KeyPair, + KeyUsagePurpose, + }; use x509_parser::prelude::{FromDer, ParsedExtension, X509CertificationRequest}; #[test] @@ -425,7 +428,7 @@ mod test_csr_extension_request { let mut params = CertificateParams::default(); params.key_usages.push(KeyUsagePurpose::DigitalSignature); let key_pair = KeyPair::generate().unwrap(); - let csr = params.serialize_request(&key_pair, Vec::new()).unwrap(); + let csr = CertificateSigningRequest::new(¶ms, &key_pair, Vec::new()).unwrap(); let (_, parsed_csr) = X509CertificationRequest::from_der(csr.der()).unwrap(); assert!(!parsed_csr .requested_extensions() @@ -440,7 +443,7 @@ mod test_csr_extension_request { .extended_key_usages .push(ExtendedKeyUsagePurpose::ClientAuth); let key_pair = KeyPair::generate().unwrap(); - let csr = params.serialize_request(&key_pair, Vec::new()).unwrap(); + let csr = CertificateSigningRequest::new(¶ms, &key_pair, Vec::new()).unwrap(); let (_, parsed_csr) = X509CertificationRequest::from_der(csr.der()).unwrap(); let requested_extensions = parsed_csr .requested_extensions() @@ -456,8 +459,8 @@ mod test_csr_extension_request { #[cfg(feature = "x509-parser")] mod test_csr { use rcgen::{ - CertificateParams, CertificateSigningRequestParams, ExtendedKeyUsagePurpose, KeyPair, - KeyUsagePurpose, + CertificateParams, CertificateSigningRequest, CertificateSigningRequestParams, + ExtendedKeyUsagePurpose, KeyPair, KeyUsagePurpose, }; #[test] @@ -508,7 +511,7 @@ mod test_csr { // Generate a key pair for the CSR let key_pair = KeyPair::generate().unwrap(); // Serialize the CSR into DER from the given parameters - let csr = params.serialize_request(&key_pair, Vec::new()).unwrap(); + let csr = CertificateSigningRequest::new(params, &key_pair, Vec::new()).unwrap(); // Parse the CSR we just serialized let csrp = CertificateSigningRequestParams::from_der(csr.der()).unwrap(); diff --git a/rcgen/tests/openssl.rs b/rcgen/tests/openssl.rs index ea36298c..c851eae5 100644 --- a/rcgen/tests/openssl.rs +++ b/rcgen/tests/openssl.rs @@ -13,8 +13,8 @@ use openssl::x509::store::{X509Store, X509StoreBuilder}; use openssl::x509::{CrlStatus, X509Crl, X509Req, X509StoreContext, X509}; use rcgen::{ - BasicConstraints, Certificate, CertificateParams, DnType, DnValue, GeneralSubtree, IsCa, - KeyPair, NameConstraints, + BasicConstraints, Certificate, CertificateParams, CertificateSigningRequest, DnType, DnValue, + GeneralSubtree, IsCa, KeyPair, NameConstraints, }; mod util; @@ -164,8 +164,7 @@ fn verify_cert_ca(cert_pem: &str, key: &[u8], ca_cert_pem: &str) { } fn verify_csr(params: &CertificateParams, key_pair: &KeyPair) { - let csr = params - .serialize_request(key_pair, Vec::new()) + let csr = CertificateSigningRequest::new(params, key_pair, Vec::new()) .and_then(|csr| csr.pem()) .unwrap(); println!("{csr}"); diff --git a/rcgen/tests/webpki.rs b/rcgen/tests/webpki.rs index b57b8e75..78be8b99 100644 --- a/rcgen/tests/webpki.rs +++ b/rcgen/tests/webpki.rs @@ -19,7 +19,7 @@ use rcgen::{ }; use rcgen::{CertificateRevocationListParams, RevocationReason, RevokedCertParams}; #[cfg(feature = "x509-parser")] -use rcgen::{CertificateSigningRequestParams, DnValue}; +use rcgen::{CertificateSigningRequest, CertificateSigningRequestParams, DnValue}; use rcgen::{ExtendedKeyUsagePurpose, KeyUsagePurpose, SerialNumber}; mod util; @@ -511,7 +511,7 @@ fn test_certificate_from_csr() { } let cert_key = KeyPair::generate().unwrap(); - let csr = params.serialize_request(&cert_key, Vec::new()).unwrap(); + let csr = CertificateSigningRequest::new(¶ms, &cert_key, Vec::new()).unwrap(); let csr = CertificateSigningRequestParams::from_der(csr.der()).unwrap(); let ekus_contained = &csr.params.extended_key_usages; From 5267b39e420f977db7113f4b164303031426f28e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 22:53:43 +0200 Subject: [PATCH 3/9] Hoist sign_der() calls up one level --- rcgen/src/certificate.rs | 361 ++++++++++++++++++++------------------- rcgen/src/crl.rs | 163 +++++++++--------- rcgen/src/csr.rs | 8 +- 3 files changed, 269 insertions(+), 263 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 34912790..95402c96 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -6,7 +6,7 @@ use pem::Pem; use pki_types::CertificateDer; use time::{Date, Month, OffsetDateTime, PrimitiveDateTime, Time}; use yasna::models::ObjectIdentifier; -use yasna::{DERWriter, Tag}; +use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; @@ -150,7 +150,10 @@ impl CertificateParams { }; Ok(Certificate { - der: self.serialize_der_with_signer(public_key, issuer)?, + der: sign_der(issuer.key_pair, |writer| { + self.serialize_der_with_signer(writer, public_key, issuer) + })? + .into(), }) } @@ -167,7 +170,10 @@ impl CertificateParams { }; Ok(Certificate { - der: self.serialize_der_with_signer(key_pair, issuer)?, +der: sign_der(issuer.key_pair, |writer| { + self.serialize_der_with_signer(writer, key_pair, issuer) +})? +.into(), }) } @@ -487,205 +493,200 @@ impl CertificateParams { pub(crate) fn serialize_der_with_signer( &self, + writer: &mut DERWriterSeq, pub_key: &K, issuer: Issuer<'_, impl SigningKey>, - ) -> Result, Error> { - let der = sign_der(issuer.key_pair, |writer| { - let pub_key_spki = pub_key.subject_public_key_info(); - // Write version - writer.next().write_tagged(Tag::context(0), |writer| { - writer.write_u8(2); - }); - // Write serialNumber - if let Some(ref serial) = self.serial_number { - writer.next().write_bigint_bytes(serial.as_ref(), true); - } else { - #[cfg(feature = "crypto")] - { - let hash = digest::digest(&digest::SHA256, pub_key.der_bytes()); - // RFC 5280 specifies at most 20 bytes for a serial number - let mut sl = hash.as_ref()[0..20].to_vec(); - sl[0] &= 0x7f; // MSB must be 0 to ensure encoding bignum in 20 bytes - writer.next().write_bigint_bytes(&sl, true); + ) -> Result<(), Error> { + let pub_key_spki = pub_key.subject_public_key_info(); + // Write version + writer.next().write_tagged(Tag::context(0), |writer| { + writer.write_u8(2); + }); + // Write serialNumber + if let Some(ref serial) = self.serial_number { + writer.next().write_bigint_bytes(serial.as_ref(), true); + } else { + #[cfg(feature = "crypto")] + { + let hash = digest::digest(&digest::SHA256, pub_key.der_bytes()); + // RFC 5280 specifies at most 20 bytes for a serial number + let mut sl = hash.as_ref()[0..20].to_vec(); + sl[0] &= 0x7f; // MSB must be 0 to ensure encoding bignum in 20 bytes + writer.next().write_bigint_bytes(&sl, true); + } + #[cfg(not(feature = "crypto"))] + if self.serial_number.is_none() { + return Err(Error::MissingSerialNumber); + } + }; + // Write signature algorithm + issuer.key_pair.algorithm().write_alg_ident(writer.next()); + // Write issuer name + write_distinguished_name(writer.next(), &issuer.distinguished_name); + // Write validity + writer.next().write_sequence(|writer| { + // Not before + write_dt_utc_or_generalized(writer.next(), self.not_before); + // Not after + write_dt_utc_or_generalized(writer.next(), self.not_after); + Ok::<(), Error>(()) + })?; + // Write subject + 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.extended_key_usages.is_empty() + || self.name_constraints.iter().any(|c| !c.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| { + if self.use_authority_key_identifier_extension { + write_x509_authority_key_identifier( + writer.next(), + match issuer.key_identifier_method { + KeyIdMethod::PreSpecified(aki) => aki.clone(), + #[cfg(feature = "crypto")] + _ => issuer + .key_identifier_method + .derive(issuer.key_pair.subject_public_key_info()), + }, + ); } - #[cfg(not(feature = "crypto"))] - if self.serial_number.is_none() { - return Err(Error::MissingSerialNumber); + // Write subject_alt_names + if !self.subject_alt_names.is_empty() { + self.write_subject_alt_names(writer.next()); } - }; - // Write signature algorithm - issuer.key_pair.algorithm().write_alg_ident(writer.next()); - // Write issuer name - write_distinguished_name(writer.next(), &issuer.distinguished_name); - // Write validity - writer.next().write_sequence(|writer| { - // Not before - write_dt_utc_or_generalized(writer.next(), self.not_before); - // Not after - write_dt_utc_or_generalized(writer.next(), self.not_after); - Ok::<(), Error>(()) - })?; - // Write subject - 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.extended_key_usages.is_empty() - || self.name_constraints.iter().any(|c| !c.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| { - if self.use_authority_key_identifier_extension { - write_x509_authority_key_identifier( + // Write standard key usage + self.write_key_usage(writer.next()); + + // Write extended key usage + if !self.extended_key_usages.is_empty() { + write_x509_extension(writer.next(), oid::EXT_KEY_USAGE, false, |writer| { + writer.write_sequence(|writer| { + for usage in self.extended_key_usages.iter() { + let oid = ObjectIdentifier::from_slice(usage.oid()); + writer.next().write_oid(&oid); + } + }); + }); + } + 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(), - match issuer.key_identifier_method { - KeyIdMethod::PreSpecified(aki) => aki.clone(), - #[cfg(feature = "crypto")] - _ => issuer - .key_identifier_method - .derive(issuer.key_pair.subject_public_key_info()), + 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, + ); + } + }); }, ); } - // Write subject_alt_names - if !self.subject_alt_names.is_empty() { - self.write_subject_alt_names(writer.next()); - } - - // Write standard key usage - self.write_key_usage(writer.next()); - - // Write extended key usage - if !self.extended_key_usages.is_empty() { - write_x509_extension(writer.next(), oid::EXT_KEY_USAGE, false, |writer| { + } + if !self.crl_distribution_points.is_empty() { + write_x509_extension( + writer.next(), + oid::CRL_DISTRIBUTION_POINTS, + false, + |writer| { writer.write_sequence(|writer| { - for usage in self.extended_key_usages.iter() { - let oid = ObjectIdentifier::from_slice(usage.oid()); - writer.next().write_oid(&oid); + for distribution_point in &self.crl_distribution_points { + distribution_point.write_der(writer.next()); } - }); - }); - } - 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 !self.crl_distribution_points.is_empty() { + }) + }, + ); + } + match self.is_ca { + IsCa::Ca(ref constraint) => { + // Write subject_key_identifier write_x509_extension( writer.next(), - oid::CRL_DISTRIBUTION_POINTS, + 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| { - for distribution_point in &self.crl_distribution_points { - distribution_point.write_der(writer.next()); + writer.next().write_bool(true); // cA flag + if let BasicConstraints::Constrained(path_len_constraint) = + constraint + { + writer.next().write_u8(*path_len_constraint); } - }) + }); }, ); - } - match self.is_ca { - IsCa::Ca(ref constraint) => { - // Write subject_key_identifier - 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| { - writer.next().write_bool(true); // cA flag - if let BasicConstraints::Constrained(path_len_constraint) = - constraint - { - writer.next().write_u8(*path_len_constraint); - } - }); - }, - ); - }, - IsCa::ExplicitNoCa => { - // Write subject_key_identifier - 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| { - writer.next().write_bool(false); // cA flag - }); - }, - ); - }, - IsCa::NoCa => {}, - } + }, + IsCa::ExplicitNoCa => { + // Write subject_key_identifier + 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| { + writer.next().write_bool(false); // cA flag + }); + }, + ); + }, + IsCa::NoCa => {}, + } - // Write the custom extensions - for ext in &self.custom_extensions { - write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { - writer.write_der(ext.content()) - }); - } - }); + // Write the custom extensions + for ext in &self.custom_extensions { + write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { + writer.write_der(ext.content()) + }); + } }); + }); - Ok(()) - })?; - - Ok(der.into()) + Ok(()) } /// Insert an extended key usage (EKU) into the parameters if it does not already exist diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index b23f1e90..f8b5381d 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -3,6 +3,7 @@ use pem::Pem; use pki_types::CertificateRevocationListDer; use time::OffsetDateTime; use yasna::DERWriter; +use yasna::DERWriterSeq; use yasna::Tag; use crate::key_pair::sign_der; @@ -205,95 +206,97 @@ impl CertificateRevocationListParams { } Ok(CertificateRevocationList { - der: self.serialize_der(issuer)?.into(), + der: sign_der(issuer.key_pair, |writer| self.serialize_der(writer, issuer))?.into(), }) } - fn serialize_der(&self, issuer: Issuer<'_, impl SigningKey>) -> Result, Error> { - sign_der(issuer.key_pair, |writer| { - // Write CRL version. - // RFC 5280 §5.1.2.1: - // This optional field describes the version of the encoded CRL. When - // extensions are used, as required by this profile, this field MUST be - // present and MUST specify version 2 (the integer value is 1). - // 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_u8(1); - - // Write algorithm identifier. - // RFC 5280 §5.1.2.2: - // This field MUST contain the same algorithm identifier as the - // signatureAlgorithm field in the sequence CertificateList - issuer.key_pair.algorithm().write_alg_ident(writer.next()); - - // Write issuer. - // RFC 5280 §5.1.2.3: - // The issuer field MUST contain a non-empty X.500 distinguished name (DN). - write_distinguished_name(writer.next(), &issuer.distinguished_name); - - // Write thisUpdate date. - // RFC 5280 §5.1.2.4: - // This field indicates the issue date of this CRL. thisUpdate may be - // encoded as UTCTime or GeneralizedTime. - write_dt_utc_or_generalized(writer.next(), self.this_update); - - // Write nextUpdate date. - // While OPTIONAL in the ASN.1 module, RFC 5280 §5.1.2.5 says: - // Conforming CRL issuers MUST include the nextUpdate field in all CRLs. - write_dt_utc_or_generalized(writer.next(), self.next_update); - - // Write revokedCertificates. - // RFC 5280 §5.1.2.6: - // When there are no revoked certificates, the revoked certificates list - // MUST be absent - if !self.revoked_certs.is_empty() { - writer.next().write_sequence(|writer| { - for revoked_cert in &self.revoked_certs { - revoked_cert.write_der(writer.next()); - } + fn serialize_der( + &self, + writer: &mut DERWriterSeq, + issuer: Issuer<'_, impl SigningKey>, + ) -> Result<(), Error> { + // Write CRL version. + // RFC 5280 §5.1.2.1: + // This optional field describes the version of the encoded CRL. When + // extensions are used, as required by this profile, this field MUST be + // present and MUST specify version 2 (the integer value is 1). + // 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_u8(1); + + // Write algorithm identifier. + // RFC 5280 §5.1.2.2: + // This field MUST contain the same algorithm identifier as the + // signatureAlgorithm field in the sequence CertificateList + issuer.key_pair.algorithm().write_alg_ident(writer.next()); + + // Write issuer. + // RFC 5280 §5.1.2.3: + // The issuer field MUST contain a non-empty X.500 distinguished name (DN). + write_distinguished_name(writer.next(), &issuer.distinguished_name); + + // Write thisUpdate date. + // RFC 5280 §5.1.2.4: + // This field indicates the issue date of this CRL. thisUpdate may be + // encoded as UTCTime or GeneralizedTime. + write_dt_utc_or_generalized(writer.next(), self.this_update); + + // Write nextUpdate date. + // While OPTIONAL in the ASN.1 module, RFC 5280 §5.1.2.5 says: + // Conforming CRL issuers MUST include the nextUpdate field in all CRLs. + write_dt_utc_or_generalized(writer.next(), self.next_update); + + // Write revokedCertificates. + // RFC 5280 §5.1.2.6: + // When there are no revoked certificates, the revoked certificates list + // MUST be absent + if !self.revoked_certs.is_empty() { + writer.next().write_sequence(|writer| { + for revoked_cert in &self.revoked_certs { + revoked_cert.write_der(writer.next()); + } + }); + } + + // 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_x509_authority_key_identifier( + writer.next(), + self.key_identifier_method + .derive(issuer.key_pair.der_bytes()), + ); + + // Write CRL number. + write_x509_extension(writer.next(), oid::CRL_NUMBER, false, |writer| { + writer.write_bigint_bytes(self.crl_number.as_ref(), true); }); - } - // 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_x509_authority_key_identifier( + // Write issuing distribution point (if present). + if let Some(issuing_distribution_point) = &self.issuing_distribution_point { + write_x509_extension( writer.next(), - self.key_identifier_method - .derive(issuer.key_pair.subject_public_key_info()), + oid::CRL_ISSUING_DISTRIBUTION_POINT, + true, + |writer| { + issuing_distribution_point.write_der(writer); + }, ); - - // Write CRL number. - write_x509_extension(writer.next(), oid::CRL_NUMBER, false, |writer| { - writer.write_bigint_bytes(self.crl_number.as_ref(), true); - }); - - // 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); - }, - ); - } - }); + } }); + }); - Ok(()) - }) + Ok(()) } } diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index 1bbf2ad9..2720eef0 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -337,9 +337,11 @@ impl CertificateSigningRequestParams { }; Ok(Certificate { - der: self - .params - .serialize_der_with_signer(&self.public_key, issuer)?, + der: sign_der(issuer.key_pair, |writer| { + self.params + .serialize_der_with_signer(writer, &self.public_key, issuer) + })? + .into(), }) } } From 6e0a0836daba487f17e2bba129705a5ff6e05812 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 22:53:43 +0200 Subject: [PATCH 4/9] Hoist sign_der() calls up one level --- rcgen/src/certificate.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 95402c96..6ca45b6a 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -170,10 +170,10 @@ impl CertificateParams { }; Ok(Certificate { -der: sign_der(issuer.key_pair, |writer| { - self.serialize_der_with_signer(writer, key_pair, issuer) -})? -.into(), + der: sign_der(issuer.key_pair, |writer| { + self.serialize_der_with_signer(writer, key_pair, issuer) + })? + .into(), }) } From de1a2f2d39dbb8549dc9b2901e927377d14fa61a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 23:08:56 +0200 Subject: [PATCH 5/9] Extract write_extensions() method, reducing rightward drift --- rcgen/src/certificate.rs | 257 +++++++++++++++++++-------------------- 1 file changed, 125 insertions(+), 132 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 6ca45b6a..6c978aa0 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -497,7 +497,6 @@ impl CertificateParams { pub_key: &K, issuer: Issuer<'_, impl SigningKey>, ) -> Result<(), Error> { - let pub_key_spki = pub_key.subject_public_key_info(); // Write version writer.next().write_tagged(Tag::context(0), |writer| { writer.write_u8(2); @@ -547,144 +546,138 @@ impl CertificateParams { return Ok(()); } + let pub_key_spki = yasna::construct_der(|writer| serialize_public_key_der(pub_key, writer)); writer.next().write_tagged(Tag::context(3), |writer| { - writer.write_sequence(|writer| { - if self.use_authority_key_identifier_extension { - write_x509_authority_key_identifier( - writer.next(), - match issuer.key_identifier_method { - KeyIdMethod::PreSpecified(aki) => aki.clone(), - #[cfg(feature = "crypto")] - _ => issuer - .key_identifier_method - .derive(issuer.key_pair.subject_public_key_info()), - }, - ); - } - // Write subject_alt_names - if !self.subject_alt_names.is_empty() { - self.write_subject_alt_names(writer.next()); - } + writer.write_sequence(|writer| self.write_extensions(writer, &pub_key_spki, &issuer)) + })?; - // Write standard key usage - self.write_key_usage(writer.next()); - - // Write extended key usage - if !self.extended_key_usages.is_empty() { - write_x509_extension(writer.next(), oid::EXT_KEY_USAGE, false, |writer| { - writer.write_sequence(|writer| { - for usage in self.extended_key_usages.iter() { - let oid = ObjectIdentifier::from_slice(usage.oid()); - writer.next().write_oid(&oid); - } - }); - }); - } - 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, - ); - } - }); - }, - ); + Ok(()) + } + + fn write_extensions( + &self, + writer: &mut DERWriterSeq, + pub_key_spki: &[u8], + 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 { + KeyIdMethod::PreSpecified(aki) => aki.clone(), + #[cfg(feature = "crypto")] + _ => issuer + .key_identifier_method + .derive(issuer.key_pair.subject_public_key_info()), + }, + ); + } + // Write subject_alt_names + if !self.subject_alt_names.is_empty() { + self.write_subject_alt_names(writer.next()); + } + + // Write standard key usage + self.write_key_usage(writer.next()); + + // Write extended key usage + if !self.extended_key_usages.is_empty() { + write_x509_extension(writer.next(), oid::EXT_KEY_USAGE, false, |writer| { + writer.write_sequence(|writer| { + for usage in self.extended_key_usages.iter() { + let oid = ObjectIdentifier::from_slice(usage.oid()); + writer.next().write_oid(&oid); } - } - 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()); - } - }) - }, - ); - } - match self.is_ca { - IsCa::Ca(ref constraint) => { - // Write subject_key_identifier - 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| { - writer.next().write_bool(true); // cA flag - if let BasicConstraints::Constrained(path_len_constraint) = - constraint - { - writer.next().write_u8(*path_len_constraint); - } - }); - }, - ); + }); + }); + } + + 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 !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()); + } + }) + }, + ); + } + + match self.is_ca { + IsCa::Ca(ref constraint) => { + // Write subject_key_identifier + write_x509_extension( + writer.next(), + oid::SUBJECT_KEY_IDENTIFIER, + false, + |writer| { + writer.write_bytes(&self.key_identifier_method.derive(pub_key_spki)); }, - IsCa::ExplicitNoCa => { - // Write subject_key_identifier - 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| { - writer.next().write_bool(false); // cA flag - }); - }, - ); + ); + // Write basic_constraints + write_x509_extension(writer.next(), oid::BASIC_CONSTRAINTS, true, |writer| { + writer.write_sequence(|writer| { + writer.next().write_bool(true); // cA flag + if let BasicConstraints::Constrained(path_len_constraint) = constraint { + writer.next().write_u8(*path_len_constraint); + } + }); + }); + }, + IsCa::ExplicitNoCa => { + // Write subject_key_identifier + write_x509_extension( + writer.next(), + oid::SUBJECT_KEY_IDENTIFIER, + false, + |writer| { + writer.write_bytes(&self.key_identifier_method.derive(pub_key_spki)); }, - IsCa::NoCa => {}, - } - - // Write the custom extensions - for ext in &self.custom_extensions { - write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { - writer.write_der(ext.content()) + ); + // Write basic_constraints + write_x509_extension(writer.next(), oid::BASIC_CONSTRAINTS, true, |writer| { + writer.write_sequence(|writer| { + writer.next().write_bool(false); // cA flag }); - } + }); + }, + IsCa::NoCa => {}, + } + + // Write the custom extensions + for ext in &self.custom_extensions { + write_x509_extension(writer.next(), &ext.oid, ext.critical, |writer| { + writer.write_der(ext.content()) }); - }); + } Ok(()) } From 2672d13de3ac6082451c428ef0cf3750f1416469 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 23:28:26 +0200 Subject: [PATCH 6/9] Introduce ToDer trait --- rcgen/src/certificate.rs | 160 +++++++++++++++++++++------------------ rcgen/src/crl.rs | 46 ++++++----- rcgen/src/csr.rs | 129 +++++++++++++++++-------------- rcgen/src/lib.rs | 6 +- 4 files changed, 195 insertions(+), 146 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 6c978aa0..ae6725b3 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -17,7 +17,7 @@ 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, + KeyIdMethod, KeyUsagePurpose, SanType, SerialNumber, SigningKey, ToDer, }; /// An issued certificate @@ -149,11 +149,14 @@ impl CertificateParams { key_pair: issuer_key, }; + let signable = SignableCertificateParams { + params: self, + pub_key: public_key, + issuer: &issuer, + }; + Ok(Certificate { - der: sign_der(issuer.key_pair, |writer| { - self.serialize_der_with_signer(writer, public_key, issuer) - })? - .into(), + der: sign_der(issuer.key_pair, |writer| signable.write_der(writer))?.into(), }) } @@ -169,11 +172,14 @@ impl CertificateParams { key_pair, }; + let signable = SignableCertificateParams { + params: self, + pub_key: &*key_pair, + issuer: &issuer, + }; + Ok(Certificate { - der: sign_der(issuer.key_pair, |writer| { - self.serialize_der_with_signer(writer, key_pair, issuer) - })? - .into(), + der: sign_der(issuer.key_pair, |writer| signable.write_der(writer))?.into(), }) } @@ -491,69 +497,6 @@ impl CertificateParams { }); } - pub(crate) fn serialize_der_with_signer( - &self, - writer: &mut DERWriterSeq, - pub_key: &K, - issuer: Issuer<'_, impl SigningKey>, - ) -> Result<(), Error> { - // Write version - writer.next().write_tagged(Tag::context(0), |writer| { - writer.write_u8(2); - }); - // Write serialNumber - if let Some(ref serial) = self.serial_number { - writer.next().write_bigint_bytes(serial.as_ref(), true); - } else { - #[cfg(feature = "crypto")] - { - let hash = digest::digest(&digest::SHA256, pub_key.der_bytes()); - // RFC 5280 specifies at most 20 bytes for a serial number - let mut sl = hash.as_ref()[0..20].to_vec(); - sl[0] &= 0x7f; // MSB must be 0 to ensure encoding bignum in 20 bytes - writer.next().write_bigint_bytes(&sl, true); - } - #[cfg(not(feature = "crypto"))] - if self.serial_number.is_none() { - return Err(Error::MissingSerialNumber); - } - }; - // Write signature algorithm - issuer.key_pair.algorithm().write_alg_ident(writer.next()); - // Write issuer name - write_distinguished_name(writer.next(), &issuer.distinguished_name); - // Write validity - writer.next().write_sequence(|writer| { - // Not before - write_dt_utc_or_generalized(writer.next(), self.not_before); - // Not after - write_dt_utc_or_generalized(writer.next(), self.not_after); - Ok::<(), Error>(()) - })?; - // Write subject - 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.extended_key_usages.is_empty() - || self.name_constraints.iter().any(|c| !c.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(()); - } - - let pub_key_spki = yasna::construct_der(|writer| serialize_public_key_der(pub_key, writer)); - writer.next().write_tagged(Tag::context(3), |writer| { - writer.write_sequence(|writer| self.write_extensions(writer, &pub_key_spki, &issuer)) - })?; - - Ok(()) - } - fn write_extensions( &self, writer: &mut DERWriterSeq, @@ -696,6 +639,79 @@ impl AsRef for CertificateParams { } } +pub(crate) struct SignableCertificateParams<'a, P, S> { + pub(crate) params: &'a CertificateParams, + pub(crate) pub_key: &'a P, + pub(crate) issuer: &'a Issuer<'a, S>, +} + +impl<'a, P: PublicKeyData, S: SigningKey> ToDer for SignableCertificateParams<'_, P, S> { + fn write_der(&self, writer: &mut DERWriterSeq) -> Result<(), Error> { + // Write version + writer.next().write_tagged(Tag::context(0), |writer| { + writer.write_u8(2); + }); + // Write serialNumber + if let Some(ref serial) = self.params.serial_number { + writer.next().write_bigint_bytes(serial.as_ref(), true); + } else { + #[cfg(feature = "crypto")] + { + let hash = digest::digest(&digest::SHA256, self.pub_key.der_bytes()); + // RFC 5280 specifies at most 20 bytes for a serial number + let mut sl = hash.as_ref()[0..20].to_vec(); + sl[0] &= 0x7f; // MSB must be 0 to ensure encoding bignum in 20 bytes + writer.next().write_bigint_bytes(&sl, true); + } + #[cfg(not(feature = "crypto"))] + if self.serial_number.is_none() { + return Err(Error::MissingSerialNumber); + } + }; + // Write signature algorithm + self.issuer + .key_pair + .algorithm() + .write_alg_ident(writer.next()); + // Write issuer name + write_distinguished_name(writer.next(), &self.issuer.distinguished_name); + // Write validity + writer.next().write_sequence(|writer| { + // Not before + write_dt_utc_or_generalized(writer.next(), self.params.not_before); + // Not after + write_dt_utc_or_generalized(writer.next(), self.params.not_after); + Ok::<(), Error>(()) + })?; + // Write subject + write_distinguished_name(writer.next(), &self.params.distinguished_name); + // Write subjectPublicKeyInfo + serialize_public_key_der(self.pub_key, writer.next()); + // write extensions + let should_write_exts = self.params.use_authority_key_identifier_extension + || !self.params.subject_alt_names.is_empty() + || !self.params.extended_key_usages.is_empty() + || self.params.name_constraints.iter().any(|c| !c.is_empty()) + || matches!(self.params.is_ca, IsCa::ExplicitNoCa) + || matches!(self.params.is_ca, IsCa::Ca(_)) + || !self.params.custom_extensions.is_empty(); + if !should_write_exts { + return Ok(()); + } + + let pub_key_spki = + yasna::construct_der(|writer| serialize_public_key_der(self.pub_key, writer)); + writer.next().write_tagged(Tag::context(3), |writer| { + writer.write_sequence(|writer| { + self.params + .write_extensions(writer, &pub_key_spki, &self.issuer) + }) + })?; + + Ok(()) + } +} + fn write_general_subtrees(writer: DERWriter, tag: u64, general_subtrees: &[GeneralSubtree]) { writer.write_tagged_implicit(Tag::context(tag), |writer| { writer.write_sequence(|writer| { diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index f8b5381d..9e2be1d7 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -12,7 +12,7 @@ use crate::ENCODE_CONFIG; use crate::{ oid, write_distinguished_name, write_dt_utc_or_generalized, write_x509_authority_key_identifier, write_x509_extension, CertificateParams, Error, Issuer, - KeyIdMethod, KeyUsagePurpose, SerialNumber, SigningKey, + KeyIdMethod, KeyUsagePurpose, SerialNumber, SigningKey, ToDer, }; /// A certificate revocation list (CRL) @@ -205,16 +205,24 @@ impl CertificateRevocationListParams { return Err(Error::IssuerNotCrlSigner); } + let signable = SignableCrl { + params: self, + issuer: &issuer, + }; + Ok(CertificateRevocationList { - der: sign_der(issuer.key_pair, |writer| self.serialize_der(writer, issuer))?.into(), + der: sign_der(issuer.key_pair, |writer| signable.write_der(writer))?.into(), }) } +} - fn serialize_der( - &self, - writer: &mut DERWriterSeq, - issuer: Issuer<'_, impl SigningKey>, - ) -> Result<(), Error> { +struct SignableCrl<'a, S> { + params: &'a CertificateRevocationListParams, + issuer: &'a Issuer<'a, S>, +} + +impl<'a, S: SigningKey> ToDer for SignableCrl<'_, S> { + fn write_der(&self, writer: &mut DERWriterSeq) -> Result<(), Error> { // Write CRL version. // RFC 5280 §5.1.2.1: // This optional field describes the version of the encoded CRL. When @@ -230,31 +238,34 @@ impl CertificateRevocationListParams { // RFC 5280 §5.1.2.2: // This field MUST contain the same algorithm identifier as the // signatureAlgorithm field in the sequence CertificateList - issuer.key_pair.algorithm().write_alg_ident(writer.next()); + self.issuer + .key_pair + .algorithm() + .write_alg_ident(writer.next()); // Write issuer. // RFC 5280 §5.1.2.3: // The issuer field MUST contain a non-empty X.500 distinguished name (DN). - write_distinguished_name(writer.next(), &issuer.distinguished_name); + write_distinguished_name(writer.next(), &self.issuer.distinguished_name); // Write thisUpdate date. // RFC 5280 §5.1.2.4: // This field indicates the issue date of this CRL. thisUpdate may be // encoded as UTCTime or GeneralizedTime. - write_dt_utc_or_generalized(writer.next(), self.this_update); + write_dt_utc_or_generalized(writer.next(), self.params.this_update); // Write nextUpdate date. // While OPTIONAL in the ASN.1 module, RFC 5280 §5.1.2.5 says: // Conforming CRL issuers MUST include the nextUpdate field in all CRLs. - write_dt_utc_or_generalized(writer.next(), self.next_update); + write_dt_utc_or_generalized(writer.next(), self.params.next_update); // Write revokedCertificates. // RFC 5280 §5.1.2.6: // When there are no revoked certificates, the revoked certificates list // MUST be absent - if !self.revoked_certs.is_empty() { + if !self.params.revoked_certs.is_empty() { writer.next().write_sequence(|writer| { - for revoked_cert in &self.revoked_certs { + for revoked_cert in &self.params.revoked_certs { revoked_cert.write_der(writer.next()); } }); @@ -273,17 +284,18 @@ impl CertificateRevocationListParams { // Write authority key identifier. write_x509_authority_key_identifier( writer.next(), - self.key_identifier_method - .derive(issuer.key_pair.der_bytes()), + self.params + .key_identifier_method + .derive(self.issuer.key_pair.der_bytes()), ); // Write CRL number. write_x509_extension(writer.next(), oid::CRL_NUMBER, false, |writer| { - writer.write_bigint_bytes(self.crl_number.as_ref(), true); + writer.write_bigint_bytes(self.params.crl_number.as_ref(), true); }); // Write issuing distribution point (if present). - if let Some(issuing_distribution_point) = &self.issuing_distribution_point { + if let Some(issuing_distribution_point) = &self.params.issuing_distribution_point { write_x509_extension( writer.next(), oid::CRL_ISSUING_DISTRIBUTION_POINT, diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index 2720eef0..0a5a2b87 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -8,9 +8,10 @@ use yasna::{models::ObjectIdentifier, DERWriter, Tag}; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ + certificate::SignableCertificateParams, key_pair::{serialize_public_key_der, sign_der}, oid, write_distinguished_name, write_x509_extension, Attribute, Certificate, CertificateParams, - Error, IsCa, Issuer, PublicKeyData, SignatureAlgorithm, SigningKey, + Error, IsCa, Issuer, PublicKeyData, SignatureAlgorithm, SigningKey, ToDer, }; #[cfg(feature = "x509-parser")] use crate::{DistinguishedName, SanType}; @@ -62,6 +63,47 @@ impl CertificateSigningRequest { subject_key: &impl SigningKey, attrs: Vec, ) -> Result { + let signable = SignableRequest { + params, + subject_key, + attrs, + }; + + Ok(Self { + der: sign_der(subject_key, |writer| signable.write_der(writer))?.into(), + }) + } + + /// Get the PEM-encoded bytes of the certificate signing request. + #[cfg(feature = "pem")] + pub fn pem(&self) -> Result { + let p = Pem::new("CERTIFICATE REQUEST", &*self.der); + Ok(pem::encode_config(&p, ENCODE_CONFIG)) + } + + /// Get the DER-encoded bytes of the certificate signing request. + /// + /// [`CertificateSigningRequestDer`] implements `Deref` and `AsRef<[u8]>`, + /// so you can easily extract the DER bytes from the return value. + pub fn der(&self) -> &CertificateSigningRequestDer<'static> { + &self.der + } +} + +impl From for CertificateSigningRequestDer<'static> { + fn from(csr: CertificateSigningRequest) -> Self { + csr.der + } +} + +struct SignableRequest<'a, S> { + params: &'a CertificateParams, + subject_key: &'a S, + attrs: Vec, +} + +impl<'a, S: SigningKey> ToDer for SignableRequest<'a, S> { + fn write_der(&self, writer: &mut yasna::DERWriterSeq) -> Result<(), Error> { // No .. pattern, we use this to ensure every field is used #[deny(unused)] let CertificateParams { @@ -78,7 +120,7 @@ impl CertificateSigningRequest { custom_extensions, use_authority_key_identifier_extension, key_identifier_method, - } = params; + } = &self.params; // - alg and key_pair 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 @@ -107,58 +149,31 @@ impl CertificateSigningRequest { || !extended_key_usages.is_empty() || !custom_extensions.is_empty(); - let der = sign_der(subject_key, |writer| { - // Write version - writer.next().write_u8(0); - write_distinguished_name(writer.next(), distinguished_name); - serialize_public_key_der(subject_key, writer.next()); - - // According to the spec in RFC 2986, even if attributes are empty we need the empty attribute tag - writer - .next() - .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 { - write_extension_request_attribute(params, writer.next()); - } - - for Attribute { oid, values } in attrs { - writer.next().write_sequence(|writer| { - writer.next().write_oid(&ObjectIdentifier::from_slice(&oid)); - writer.next().write_der(&values); - }); - } - }); - }); - - Ok(()) - })?; - - Ok(Self { - der: CertificateSigningRequestDer::from(der), - }) - } + // Write version + writer.next().write_u8(0); + write_distinguished_name(writer.next(), distinguished_name); + serialize_public_key_der(self.subject_key, writer.next()); - /// Get the PEM-encoded bytes of the certificate signing request. - #[cfg(feature = "pem")] - pub fn pem(&self) -> Result { - let p = Pem::new("CERTIFICATE REQUEST", &*self.der); - Ok(pem::encode_config(&p, ENCODE_CONFIG)) - } + // According to the spec in RFC 2986, even if attributes are empty we need the empty attribute tag + writer + .next() + .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 { + write_extension_request_attribute(&self.params, writer.next()); + } - /// Get the DER-encoded bytes of the certificate signing request. - /// - /// [`CertificateSigningRequestDer`] implements `Deref` and `AsRef<[u8]>`, - /// so you can easily extract the DER bytes from the return value. - pub fn der(&self) -> &CertificateSigningRequestDer<'static> { - &self.der - } -} + for Attribute { oid, values } in &self.attrs { + writer.next().write_sequence(|writer| { + writer.next().write_oid(&ObjectIdentifier::from_slice(&oid)); + writer.next().write_der(&values); + }); + } + }); + }); -impl From for CertificateSigningRequestDer<'static> { - fn from(csr: CertificateSigningRequest) -> Self { - csr.der + Ok(()) } } @@ -336,12 +351,14 @@ impl CertificateSigningRequestParams { key_pair: issuer_key, }; + let signable = SignableCertificateParams { + params: &self.params, + pub_key: &self.public_key, + issuer: &issuer, + }; + Ok(Certificate { - der: sign_der(issuer.key_pair, |writer| { - self.params - .serialize_der_with_signer(writer, &self.public_key, issuer) - })? - .into(), + der: sign_der(issuer.key_pair, |writer| signable.write_der(writer))?.into(), }) } } diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 8e20ff4d..b6d5b148 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -44,8 +44,8 @@ use time::{OffsetDateTime, Time}; use yasna::models::ObjectIdentifier; use yasna::models::{GeneralizedTime, UTCTime}; use yasna::tags::{TAG_BMPSTRING, TAG_TELETEXSTRING, TAG_UNIVERSALSTRING}; -use yasna::DERWriter; use yasna::Tag; +use yasna::{DERWriter, DERWriterSeq}; use crate::string::{BmpString, Ia5String, PrintableString, TeletexString, UniversalString}; @@ -139,6 +139,10 @@ struct Issuer<'a, S> { key_pair: &'a S, } +trait ToDer { + fn write_der(&self, writer: &mut DERWriterSeq) -> Result<(), Error>; +} + // https://tools.ietf.org/html/rfc5280#section-4.1.1 // Example certs usable as reference: From f3a0fe3bdf993a5dc85ae55f3f5a56fdafb73f99 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 23:40:01 +0200 Subject: [PATCH 7/9] Rename sign_der() to ToDer::signed() --- rcgen/src/certificate.rs | 30 +++++++++++++++--------------- rcgen/src/crl.rs | 13 ++++++------- rcgen/src/csr.rs | 33 ++++++++++++++++----------------- rcgen/src/key_pair.rs | 24 +----------------------- rcgen/src/lib.rs | 21 +++++++++++++++++++++ 5 files changed, 59 insertions(+), 62 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index ae6725b3..23ff4569 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -9,7 +9,7 @@ use yasna::models::ObjectIdentifier; use yasna::{DERWriter, DERWriterSeq, Tag}; use crate::crl::CrlDistributionPoint; -use crate::key_pair::{serialize_public_key_der, sign_der, PublicKeyData}; +use crate::key_pair::{serialize_public_key_der, PublicKeyData}; #[cfg(feature = "crypto")] use crate::ring_like::digest; #[cfg(feature = "pem")] @@ -149,14 +149,14 @@ impl CertificateParams { key_pair: issuer_key, }; - let signable = SignableCertificateParams { - params: self, - pub_key: public_key, - issuer: &issuer, - }; - Ok(Certificate { - der: sign_der(issuer.key_pair, |writer| signable.write_der(writer))?.into(), + der: SignableCertificateParams { + params: self, + pub_key: public_key, + issuer: &issuer, + } + .signed(issuer.key_pair)? + .into(), }) } @@ -172,14 +172,14 @@ impl CertificateParams { key_pair, }; - let signable = SignableCertificateParams { - params: self, - pub_key: &*key_pair, - issuer: &issuer, - }; - Ok(Certificate { - der: sign_der(issuer.key_pair, |writer| signable.write_der(writer))?.into(), + der: SignableCertificateParams { + params: self, + pub_key: &*key_pair, + issuer: &issuer, + } + .signed(issuer.key_pair)? + .into(), }) } diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index 9e2be1d7..d8f425e6 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -6,7 +6,6 @@ use yasna::DERWriter; use yasna::DERWriterSeq; use yasna::Tag; -use crate::key_pair::sign_der; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ @@ -205,13 +204,13 @@ impl CertificateRevocationListParams { return Err(Error::IssuerNotCrlSigner); } - let signable = SignableCrl { - params: self, - issuer: &issuer, - }; - Ok(CertificateRevocationList { - der: sign_der(issuer.key_pair, |writer| signable.write_der(writer))?.into(), + der: SignableCrl { + params: self, + issuer: &issuer, + } + .signed(issuer_key)? + .into(), }) } } diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index 0a5a2b87..52f8f303 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -8,9 +8,8 @@ use yasna::{models::ObjectIdentifier, DERWriter, Tag}; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ - certificate::SignableCertificateParams, - key_pair::{serialize_public_key_der, sign_der}, - oid, write_distinguished_name, write_x509_extension, Attribute, Certificate, CertificateParams, + certificate::SignableCertificateParams, key_pair::serialize_public_key_der, oid, + write_distinguished_name, write_x509_extension, Attribute, Certificate, CertificateParams, Error, IsCa, Issuer, PublicKeyData, SignatureAlgorithm, SigningKey, ToDer, }; #[cfg(feature = "x509-parser")] @@ -63,14 +62,14 @@ impl CertificateSigningRequest { subject_key: &impl SigningKey, attrs: Vec, ) -> Result { - let signable = SignableRequest { - params, - subject_key, - attrs, - }; - Ok(Self { - der: sign_der(subject_key, |writer| signable.write_der(writer))?.into(), + der: SignableRequest { + params, + subject_key, + attrs, + } + .signed(subject_key)? + .into(), }) } @@ -351,14 +350,14 @@ impl CertificateSigningRequestParams { key_pair: issuer_key, }; - let signable = SignableCertificateParams { - params: &self.params, - pub_key: &self.public_key, - issuer: &issuer, - }; - Ok(Certificate { - der: sign_der(issuer.key_pair, |writer| signable.write_der(writer))?.into(), + der: SignableCertificateParams { + params: &self.params, + pub_key: &self.public_key, + issuer: &issuer, + } + .signed(issuer.key_pair)? + .into(), }) } } diff --git a/rcgen/src/key_pair.rs b/rcgen/src/key_pair.rs index 056360a1..5218e97f 100644 --- a/rcgen/src/key_pair.rs +++ b/rcgen/src/key_pair.rs @@ -5,7 +5,7 @@ use std::fmt; use pem::Pem; #[cfg(feature = "crypto")] use pki_types::{PrivateKeyDer, PrivatePkcs8KeyDer}; -use yasna::{DERWriter, DERWriterSeq}; +use yasna::DERWriter; #[cfg(any(feature = "crypto", feature = "pem"))] use crate::error::ExternalError; @@ -584,28 +584,6 @@ pub enum RsaKeySize { _4096, } -pub(crate) fn sign_der( - key: &impl SigningKey, - f: impl FnOnce(&mut DERWriterSeq<'_>) -> Result<(), Error>, -) -> Result, Error> { - yasna::try_construct_der(|writer| { - writer.write_sequence(|writer| { - let data = yasna::try_construct_der(|writer| writer.write_sequence(f))?; - writer.next().write_der(&data); - - // Write signatureAlgorithm - key.algorithm().write_alg_ident(writer.next()); - - // Write signature - let sig = key.sign(&data)?; - let writer = writer.next(); - writer.write_bitvec_bytes(&sig, sig.len() * 8); - - Ok(()) - }) - }) -} - /// A key that can be used to sign messages pub trait SigningKey: PublicKeyData { /// Signs `msg` using the selected algorithm diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index b6d5b148..6365da21 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -140,6 +140,27 @@ struct Issuer<'a, S> { } trait ToDer { + fn signed(&self, key: &impl SigningKey) -> Result, Error> { + yasna::try_construct_der(|writer| { + writer.write_sequence(|writer| { + let data = yasna::try_construct_der(|writer| { + writer.write_sequence(|writer| self.write_der(writer)) + })?; + writer.next().write_der(&data); + + // Write signatureAlgorithm + key.algorithm().write_alg_ident(writer.next()); + + // Write signature + let sig = key.sign(&data)?; + let writer = writer.next(); + writer.write_bitvec_bytes(&sig, sig.len() * 8); + + Ok(()) + }) + }) + } + fn write_der(&self, writer: &mut DERWriterSeq) -> Result<(), Error>; } From 766f17c7307f51d8652072f73e2a90e713ada776 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 23:42:48 +0200 Subject: [PATCH 8/9] Deduplicate Issuer construction --- rcgen/src/certificate.rs | 16 ++-------------- rcgen/src/crl.rs | 8 +------- rcgen/src/csr.rs | 8 +------- rcgen/src/lib.rs | 11 +++++++++++ 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 23ff4569..1b16d926 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -142,13 +142,7 @@ impl CertificateParams { issuer: &CertificateParams, issuer_key: &impl SigningKey, ) -> Result { - let issuer = Issuer { - distinguished_name: &issuer.distinguished_name, - key_identifier_method: &issuer.key_identifier_method, - key_usages: &issuer.key_usages, - key_pair: issuer_key, - }; - + let issuer = Issuer::new(&issuer, issuer_key); Ok(Certificate { der: SignableCertificateParams { params: self, @@ -165,13 +159,7 @@ impl CertificateParams { /// The returned [`Certificate`] may be serialized using [`Certificate::der`] and /// [`Certificate::pem`]. pub fn self_signed(&self, key_pair: &impl SigningKey) -> Result { - let issuer = Issuer { - distinguished_name: &self.distinguished_name, - key_identifier_method: &self.key_identifier_method, - key_usages: &self.key_usages, - key_pair, - }; - + let issuer = Issuer::new(self, key_pair); Ok(Certificate { der: SignableCertificateParams { params: self, diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index d8f425e6..f2cb104d 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -193,13 +193,7 @@ impl CertificateRevocationListParams { return Err(Error::InvalidCrlNextUpdate); } - let issuer = Issuer { - distinguished_name: &issuer.distinguished_name, - key_identifier_method: &issuer.key_identifier_method, - key_usages: &issuer.key_usages, - key_pair: issuer_key, - }; - + let issuer = Issuer::new(issuer, issuer_key); if !issuer.key_usages.is_empty() && !issuer.key_usages.contains(&KeyUsagePurpose::CrlSign) { return Err(Error::IssuerNotCrlSigner); } diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index 52f8f303..3c03b1ff 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -343,13 +343,7 @@ impl CertificateSigningRequestParams { issuer: &CertificateParams, issuer_key: &impl SigningKey, ) -> Result { - let issuer = Issuer { - distinguished_name: &issuer.distinguished_name, - key_identifier_method: &issuer.key_identifier_method, - key_usages: &issuer.key_usages, - key_pair: issuer_key, - }; - + let issuer = Issuer::new(issuer, issuer_key); Ok(Certificate { der: SignableCertificateParams { params: &self.params, diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 6365da21..3803bad8 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -139,6 +139,17 @@ struct Issuer<'a, S> { key_pair: &'a S, } +impl<'a, S: SigningKey> Issuer<'a, S> { + fn new(params: &'a CertificateParams, key_pair: &'a S) -> Self { + Self { + distinguished_name: ¶ms.distinguished_name, + key_identifier_method: ¶ms.key_identifier_method, + key_usages: ¶ms.key_usages, + key_pair, + } + } +} + trait ToDer { fn signed(&self, key: &impl SigningKey) -> Result, Error> { yasna::try_construct_der(|writer| { From 6bd7c4d7f7d89b3e94017246fde4179b704a1e3a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Apr 2025 23:48:41 +0200 Subject: [PATCH 9/9] Deduplicate SignableCertificateParams construction --- rcgen/src/certificate.rs | 34 ++++++++++++++++++++-------------- rcgen/src/csr.rs | 18 ++++++++---------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 1b16d926..b97eedc1 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -144,13 +144,10 @@ impl CertificateParams { ) -> Result { let issuer = Issuer::new(&issuer, issuer_key); Ok(Certificate { - der: SignableCertificateParams { - params: self, - pub_key: public_key, - issuer: &issuer, - } - .signed(issuer.key_pair)? - .into(), + der: self + .signable(public_key, &issuer) + .signed(issuer.key_pair)? + .into(), }) } @@ -161,13 +158,10 @@ impl CertificateParams { pub fn self_signed(&self, key_pair: &impl SigningKey) -> Result { let issuer = Issuer::new(self, key_pair); Ok(Certificate { - der: SignableCertificateParams { - params: self, - pub_key: &*key_pair, - issuer: &issuer, - } - .signed(issuer.key_pair)? - .into(), + der: self + .signable(&*key_pair, &issuer) + .signed(issuer.key_pair)? + .into(), }) } @@ -178,6 +172,18 @@ impl CertificateParams { .derive(&key.subject_public_key_info()) } + pub(crate) fn signable<'a, P, S>( + &'a self, + pub_key: &'a P, + issuer: &'a Issuer<'a, S>, + ) -> SignableCertificateParams<'a, P, S> { + SignableCertificateParams { + params: self, + pub_key, + issuer, + } + } + /// Parses an existing ca certificate from the ASCII PEM format. /// /// See [`from_ca_cert_der`](Self::from_ca_cert_der) for more details. diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index 3c03b1ff..bcd71f87 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -8,9 +8,9 @@ use yasna::{models::ObjectIdentifier, DERWriter, Tag}; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ - certificate::SignableCertificateParams, key_pair::serialize_public_key_der, oid, - write_distinguished_name, write_x509_extension, Attribute, Certificate, CertificateParams, - Error, IsCa, Issuer, PublicKeyData, SignatureAlgorithm, SigningKey, ToDer, + key_pair::serialize_public_key_der, oid, write_distinguished_name, write_x509_extension, + Attribute, Certificate, CertificateParams, Error, IsCa, Issuer, PublicKeyData, + SignatureAlgorithm, SigningKey, ToDer, }; #[cfg(feature = "x509-parser")] use crate::{DistinguishedName, SanType}; @@ -345,13 +345,11 @@ impl CertificateSigningRequestParams { ) -> Result { let issuer = Issuer::new(issuer, issuer_key); Ok(Certificate { - der: SignableCertificateParams { - params: &self.params, - pub_key: &self.public_key, - issuer: &issuer, - } - .signed(issuer.key_pair)? - .into(), + der: self + .params + .signable(&self.public_key, &issuer) + .signed(issuer.key_pair)? + .into(), }) } }