From b89cad78cb628a2c055e6fec6d5eec3a2ec6aa42 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 14:07:13 +0200 Subject: [PATCH 01/10] Move GeneralSubtree processing into method --- rcgen/src/certificate.rs | 66 +++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 8fd4a6c0..018a01cc 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -351,13 +351,13 @@ impl CertificateParams { if let Some(constraints) = constraints { let permitted_subtrees = if let Some(permitted) = &constraints.permitted_subtrees { - Self::convert_x509_general_subtrees(permitted)? + GeneralSubtree::from_x509(permitted)? } else { Vec::new() }; let excluded_subtrees = if let Some(excluded) = &constraints.excluded_subtrees { - Self::convert_x509_general_subtrees(excluded)? + GeneralSubtree::from_x509(excluded)? } else { Vec::new() }; @@ -372,36 +372,6 @@ impl CertificateParams { Ok(None) } } - #[cfg(feature = "x509-parser")] - fn convert_x509_general_subtrees( - subtrees: &[x509_parser::extensions::GeneralSubtree<'_>], - ) -> Result, Error> { - use x509_parser::extensions::GeneralName; - - let mut result = Vec::new(); - for subtree in subtrees { - let subtree = match &subtree.base { - GeneralName::RFC822Name(s) => GeneralSubtree::Rfc822Name(s.to_string()), - GeneralName::DNSName(s) => GeneralSubtree::DnsName(s.to_string()), - GeneralName::DirectoryName(n) => { - GeneralSubtree::DirectoryName(DistinguishedName::from_name(n)?) - }, - GeneralName::IPAddress(bytes) if bytes.len() == 8 => { - let addr: [u8; 4] = bytes[..4].try_into().unwrap(); - let mask: [u8; 4] = bytes[4..].try_into().unwrap(); - GeneralSubtree::IpAddress(CidrSubnet::V4(addr, mask)) - }, - GeneralName::IPAddress(bytes) if bytes.len() == 32 => { - let addr: [u8; 16] = bytes[..16].try_into().unwrap(); - let mask: [u8; 16] = bytes[16..].try_into().unwrap(); - GeneralSubtree::IpAddress(CidrSubnet::V6(addr, mask)) - }, - _ => continue, - }; - result.push(subtree); - } - Ok(result) - } /// Write a CSR extension request attribute as defined in [RFC 2985]. /// @@ -1039,6 +1009,38 @@ pub enum GeneralSubtree { } impl GeneralSubtree { + #[cfg(feature = "x509-parser")] + fn from_x509( + subtrees: &[x509_parser::extensions::GeneralSubtree<'_>], + ) -> Result, Error> { + use x509_parser::extensions::GeneralName; + + let mut result = Vec::new(); + for subtree in subtrees { + let subtree = match &subtree.base { + GeneralName::RFC822Name(s) => Self::Rfc822Name(s.to_string()), + GeneralName::DNSName(s) => Self::DnsName(s.to_string()), + GeneralName::DirectoryName(n) => { + Self::DirectoryName(DistinguishedName::from_name(n)?) + }, + GeneralName::IPAddress(bytes) if bytes.len() == 8 => { + let addr: [u8; 4] = bytes[..4].try_into().unwrap(); + let mask: [u8; 4] = bytes[4..].try_into().unwrap(); + Self::IpAddress(CidrSubnet::V4(addr, mask)) + }, + GeneralName::IPAddress(bytes) if bytes.len() == 32 => { + let addr: [u8; 16] = bytes[..16].try_into().unwrap(); + let mask: [u8; 16] = bytes[16..].try_into().unwrap(); + Self::IpAddress(CidrSubnet::V6(addr, mask)) + }, + _ => continue, + }; + result.push(subtree); + } + + Ok(result) + } + fn tag(&self) -> u64 { // Defined in the GeneralName list in // https://tools.ietf.org/html/rfc5280#page-38 From 9adad592c349aefe845999f72d64ce7da39ebc07 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 14:09:24 +0200 Subject: [PATCH 02/10] Move NameConstraints processing into method --- rcgen/src/certificate.rs | 67 ++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 018a01cc..8bd44f94 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -208,7 +208,7 @@ impl CertificateParams { let subject_alt_names = Self::convert_x509_subject_alternative_name(&x509)?; let key_usages = Self::convert_x509_key_usages(&x509)?; let extended_key_usages = Self::convert_x509_extended_key_usages(&x509)?; - let name_constraints = Self::convert_x509_name_constraints(&x509)?; + let name_constraints = NameConstraints::from_x509(&x509)?; let serial_number = Some(x509.serial.to_bytes_be().into()); let key_identifier_method = @@ -340,38 +340,6 @@ impl CertificateParams { } Ok(extended_key_usages) } - #[cfg(feature = "x509-parser")] - fn convert_x509_name_constraints( - x509: &x509_parser::certificate::X509Certificate<'_>, - ) -> Result, Error> { - let constraints = x509 - .name_constraints() - .or(Err(Error::CouldNotParseCertificate))? - .map(|ext| ext.value); - - if let Some(constraints) = constraints { - 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() - }; - - let name_constraints = NameConstraints { - permitted_subtrees, - excluded_subtrees, - }; - - Ok(Some(name_constraints)) - } else { - Ok(None) - } - } /// Write a CSR extension request attribute as defined in [RFC 2985]. /// @@ -987,6 +955,39 @@ pub struct NameConstraints { } impl NameConstraints { + #[cfg(feature = "x509-parser")] + fn from_x509( + x509: &x509_parser::certificate::X509Certificate<'_>, + ) -> Result, Error> { + let constraints = x509 + .name_constraints() + .or(Err(Error::CouldNotParseCertificate))? + .map(|ext| ext.value); + + if let Some(constraints) = constraints { + 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() + }; + + let name_constraints = Self { + permitted_subtrees, + excluded_subtrees, + }; + + Ok(Some(name_constraints)) + } else { + Ok(None) + } + } + fn is_empty(&self) -> bool { self.permitted_subtrees.is_empty() && self.excluded_subtrees.is_empty() } From 6f3bd42121852f7048c3e6b4565df444243312e9 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 14:12:41 +0200 Subject: [PATCH 03/10] Move IsCa processing into method --- rcgen/src/certificate.rs | 60 ++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 8bd44f94..e805b5eb 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -203,7 +203,7 @@ impl CertificateParams { .or(Err(Error::CouldNotParseCertificate))?; let dn = DistinguishedName::from_name(&x509.tbs_certificate.subject)?; - let is_ca = Self::convert_x509_is_ca(&x509)?; + let is_ca = IsCa::from_x509(&x509)?; let validity = x509.validity(); let subject_alt_names = Self::convert_x509_subject_alternative_name(&x509)?; let key_usages = Self::convert_x509_key_usages(&x509)?; @@ -244,36 +244,7 @@ impl CertificateParams { ..Default::default() }) } - #[cfg(feature = "x509-parser")] - fn convert_x509_is_ca( - x509: &x509_parser::certificate::X509Certificate<'_>, - ) -> Result { - use x509_parser::extensions::BasicConstraints as B; - - let basic_constraints = x509 - .basic_constraints() - .or(Err(Error::CouldNotParseCertificate))? - .map(|ext| ext.value); - - let is_ca = match basic_constraints { - Some(B { - ca: true, - path_len_constraint: Some(n), - }) if *n <= u8::MAX as u32 => IsCa::Ca(BasicConstraints::Constrained(*n as u8)), - Some(B { - ca: true, - path_len_constraint: Some(_), - }) => return Err(Error::CouldNotParseCertificate), - Some(B { - ca: true, - path_len_constraint: None, - }) => IsCa::Ca(BasicConstraints::Unconstrained), - Some(B { ca: false, .. }) => IsCa::ExplicitNoCa, - None => IsCa::NoCa, - }; - Ok(is_ca) - } #[cfg(feature = "x509-parser")] fn convert_x509_subject_alternative_name( x509: &x509_parser::certificate::X509Certificate<'_>, @@ -1181,6 +1152,35 @@ pub enum IsCa { Ca(BasicConstraints), } +impl IsCa { + #[cfg(feature = "x509-parser")] + fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result { + use x509_parser::extensions::BasicConstraints as B; + + let basic_constraints = x509 + .basic_constraints() + .or(Err(Error::CouldNotParseCertificate))? + .map(|ext| ext.value); + + Ok(match basic_constraints { + Some(B { + ca: true, + path_len_constraint: Some(n), + }) if *n <= u8::MAX as u32 => Self::Ca(BasicConstraints::Constrained(*n as u8)), + Some(B { + ca: true, + path_len_constraint: Some(_), + }) => return Err(Error::CouldNotParseCertificate), + Some(B { + ca: true, + path_len_constraint: None, + }) => Self::Ca(BasicConstraints::Unconstrained), + Some(B { ca: false, .. }) => Self::ExplicitNoCa, + None => Self::NoCa, + }) + } +} + /// The path length constraint (only relevant for CA certificates) /// /// Sets an optional upper limit on the length of the intermediate certificate chain From 51163fb3bf5829a5be8673b6dee3e402ad5f6012 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 14:14:23 +0200 Subject: [PATCH 04/10] Move SanType processing into method --- rcgen/src/certificate.rs | 21 +-------------------- rcgen/src/lib.rs | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index e805b5eb..0402598f 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -205,7 +205,7 @@ impl CertificateParams { let dn = DistinguishedName::from_name(&x509.tbs_certificate.subject)?; let is_ca = IsCa::from_x509(&x509)?; let validity = x509.validity(); - let subject_alt_names = Self::convert_x509_subject_alternative_name(&x509)?; + let subject_alt_names = SanType::from_x509(&x509)?; let key_usages = Self::convert_x509_key_usages(&x509)?; let extended_key_usages = Self::convert_x509_extended_key_usages(&x509)?; let name_constraints = NameConstraints::from_x509(&x509)?; @@ -245,25 +245,6 @@ impl CertificateParams { }) } - #[cfg(feature = "x509-parser")] - fn convert_x509_subject_alternative_name( - x509: &x509_parser::certificate::X509Certificate<'_>, - ) -> Result, Error> { - let sans = x509 - .subject_alternative_name() - .or(Err(Error::CouldNotParseCertificate))? - .map(|ext| &ext.value.general_names); - - if let Some(sans) = sans { - let mut subject_alt_names = Vec::with_capacity(sans.len()); - for san in sans { - subject_alt_names.push(SanType::try_from_general(san)?); - } - Ok(subject_alt_names) - } else { - Ok(Vec::new()) - } - } #[cfg(feature = "x509-parser")] fn convert_x509_key_usages( x509: &x509_parser::certificate::X509Certificate<'_>, diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index f0190bdd..05975e2e 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -177,6 +177,26 @@ pub enum SanType { OtherName((Vec, OtherNameValue)), } +impl SanType { + #[cfg(feature = "x509-parser")] + fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result, Error> { + let sans = x509 + .subject_alternative_name() + .or(Err(Error::CouldNotParseCertificate))? + .map(|ext| &ext.value.general_names); + + if let Some(sans) = sans { + 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) + } else { + Ok(Vec::new()) + } + } +} + /// 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 e603bef1af8b06e59c8c4cf76461807450345fd2 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 14:10:23 +0200 Subject: [PATCH 05/10] Use let-else to reduce rightward drift --- rcgen/src/certificate.rs | 36 +++++++++++++++++------------------- rcgen/src/lib.rs | 16 ++++++++-------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 0402598f..04751521 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -916,28 +916,26 @@ impl NameConstraints { .or(Err(Error::CouldNotParseCertificate))? .map(|ext| ext.value); - if let Some(constraints) = constraints { - 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() - }; + let Some(constraints) = constraints else { + return Ok(None); + }; - let name_constraints = Self { - permitted_subtrees, - excluded_subtrees, - }; + let permitted_subtrees = if let Some(permitted) = &constraints.permitted_subtrees { + GeneralSubtree::from_x509(permitted)? + } else { + Vec::new() + }; - Ok(Some(name_constraints)) + let excluded_subtrees = if let Some(excluded) = &constraints.excluded_subtrees { + GeneralSubtree::from_x509(excluded)? } else { - Ok(None) - } + Vec::new() + }; + + Ok(Some(Self { + permitted_subtrees, + excluded_subtrees, + })) } fn is_empty(&self) -> bool { diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 05975e2e..392930b4 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -185,15 +185,15 @@ impl SanType { .or(Err(Error::CouldNotParseCertificate))? .map(|ext| &ext.value.general_names); - if let Some(sans) = sans { - 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) - } else { - Ok(Vec::new()) + 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) } } From cf1942a25a61dbcffb419ce88cffc260335e6082 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 14:18:15 +0200 Subject: [PATCH 06/10] Move KeyUsagePurpose processing into method --- rcgen/src/certificate.rs | 14 +------------- rcgen/src/lib.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 04751521..f7994504 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -206,7 +206,7 @@ impl CertificateParams { let is_ca = IsCa::from_x509(&x509)?; let validity = x509.validity(); let subject_alt_names = SanType::from_x509(&x509)?; - let key_usages = Self::convert_x509_key_usages(&x509)?; + let key_usages = KeyUsagePurpose::from_x509(&x509)?; let extended_key_usages = Self::convert_x509_extended_key_usages(&x509)?; let name_constraints = NameConstraints::from_x509(&x509)?; let serial_number = Some(x509.serial.to_bytes_be().into()); @@ -245,18 +245,6 @@ impl CertificateParams { }) } - #[cfg(feature = "x509-parser")] - fn convert_x509_key_usages( - x509: &x509_parser::certificate::X509Certificate<'_>, - ) -> Result, Error> { - let key_usage = x509 - .key_usage() - .or(Err(Error::CouldNotParseCertificate))? - .map(|ext| ext.value); - // This x509 parser stores flags in reversed bit BIT STRING order - let flags = key_usage.map_or(0u16, |k| k.flags).reverse_bits(); - Ok(KeyUsagePurpose::from_u16(flags)) - } #[cfg(feature = "x509-parser")] fn convert_x509_extended_key_usages( x509: &x509_parser::certificate::X509Certificate<'_>, diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 392930b4..d6824a39 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -469,6 +469,17 @@ pub enum KeyUsagePurpose { } impl KeyUsagePurpose { + #[cfg(feature = "x509-parser")] + fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result, Error> { + let key_usage = x509 + .key_usage() + .or(Err(Error::CouldNotParseCertificate))? + .map(|ext| ext.value); + // This x509 parser stores flags in reversed bit BIT STRING order + let flags = key_usage.map_or(0u16, |k| k.flags).reverse_bits(); + Ok(Self::from_u16(flags)) + } + /// Encode a key usage as the value of a BIT STRING as defined by RFC 5280. /// [`u16`] is sufficient to encode the largest possible key usage value (two bytes). fn to_u16(&self) -> u16 { From b78c2f39bdb76facd0f48198a82937aa3f24ec8e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 15:47:39 +0200 Subject: [PATCH 07/10] Move ExtendedKeyUsagePurpose processing into method --- rcgen/src/certificate.rs | 73 ++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index f7994504..353b5282 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -207,7 +207,7 @@ impl CertificateParams { let validity = x509.validity(); let subject_alt_names = SanType::from_x509(&x509)?; let key_usages = KeyUsagePurpose::from_x509(&x509)?; - let extended_key_usages = Self::convert_x509_extended_key_usages(&x509)?; + let extended_key_usages = ExtendedKeyUsagePurpose::from_x509(&x509)?; let name_constraints = NameConstraints::from_x509(&x509)?; let serial_number = Some(x509.serial.to_bytes_be().into()); @@ -245,42 +245,6 @@ impl CertificateParams { }) } - #[cfg(feature = "x509-parser")] - fn convert_x509_extended_key_usages( - x509: &x509_parser::certificate::X509Certificate<'_>, - ) -> Result, Error> { - let extended_key_usage = x509 - .extended_key_usage() - .or(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(ExtendedKeyUsagePurpose::Any); - } - if extended_key_usage.server_auth { - extended_key_usages.push(ExtendedKeyUsagePurpose::ServerAuth); - } - if extended_key_usage.client_auth { - extended_key_usages.push(ExtendedKeyUsagePurpose::ClientAuth); - } - if extended_key_usage.code_signing { - extended_key_usages.push(ExtendedKeyUsagePurpose::CodeSigning); - } - if extended_key_usage.email_protection { - extended_key_usages.push(ExtendedKeyUsagePurpose::EmailProtection); - } - if extended_key_usage.time_stamping { - extended_key_usages.push(ExtendedKeyUsagePurpose::TimeStamping); - } - if extended_key_usage.ocsp_signing { - extended_key_usages.push(ExtendedKeyUsagePurpose::OcspSigning); - } - } - Ok(extended_key_usages) - } - /// Write a CSR extension request attribute as defined in [RFC 2985]. /// /// [RFC 2985]: @@ -865,6 +829,41 @@ pub enum ExtendedKeyUsagePurpose { } impl ExtendedKeyUsagePurpose { + #[cfg(feature = "x509-parser")] + fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result, Error> { + let extended_key_usage = x509 + .extended_key_usage() + .or(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) + } + fn oid(&self) -> &[u64] { use ExtendedKeyUsagePurpose::*; match self { From a9f7d6dca37f7cd4f1c8c7b41bafcbb9d1b3eaac Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 15:52:07 +0200 Subject: [PATCH 08/10] Inline trivial bindings --- rcgen/src/certificate.rs | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 353b5282..b1d5f639 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -202,15 +202,6 @@ impl CertificateParams { let (_remainder, x509) = x509_parser::parse_x509_certificate(ca_cert) .or(Err(Error::CouldNotParseCertificate))?; - let dn = DistinguishedName::from_name(&x509.tbs_certificate.subject)?; - let is_ca = IsCa::from_x509(&x509)?; - let validity = x509.validity(); - let subject_alt_names = SanType::from_x509(&x509)?; - let key_usages = KeyUsagePurpose::from_x509(&x509)?; - let extended_key_usages = ExtendedKeyUsagePurpose::from_x509(&x509)?; - let name_constraints = NameConstraints::from_x509(&x509)?; - let serial_number = Some(x509.serial.to_bytes_be().into()); - let key_identifier_method = x509.iter_extensions() .find_map(|ext| match ext.parsed_extension() { @@ -231,16 +222,16 @@ impl CertificateParams { }; Ok(CertificateParams { - is_ca, - subject_alt_names, - key_usages, - extended_key_usages, - name_constraints, - serial_number, + 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)?, + serial_number: Some(x509.serial.to_bytes_be().into()), key_identifier_method, - distinguished_name: dn, - not_before: validity.not_before.to_datetime(), - not_after: validity.not_after.to_datetime(), + 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() }) } From ed9b3d915e6bf93286f90a75e1642193a9d7999f Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 15:53:58 +0200 Subject: [PATCH 09/10] Extract method for KeyIdMethod processing --- rcgen/src/certificate.rs | 21 +-------------------- rcgen/src/lib.rs | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index b1d5f639..af724b63 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -202,25 +202,6 @@ impl CertificateParams { let (_remainder, x509) = x509_parser::parse_x509_certificate(ca_cert) .or(Err(Error::CouldNotParseCertificate))?; - let key_identifier_method = - x509.iter_extensions() - .find_map(|ext| match ext.parsed_extension() { - x509_parser::extensions::ParsedExtension::SubjectKeyIdentifier(key_id) => { - Some(KeyIdMethod::PreSpecified(key_id.0.into())) - }, - _ => None, - }); - - let key_identifier_method = match key_identifier_method { - Some(method) => method, - None => { - #[cfg(not(feature = "crypto"))] - return Err(Error::UnsupportedSignatureAlgorithm); - #[cfg(feature = "crypto")] - KeyIdMethod::Sha256 - }, - }; - Ok(CertificateParams { is_ca: IsCa::from_x509(&x509)?, subject_alt_names: SanType::from_x509(&x509)?, @@ -228,7 +209,7 @@ impl CertificateParams { extended_key_usages: ExtendedKeyUsagePurpose::from_x509(&x509)?, name_constraints: NameConstraints::from_x509(&x509)?, serial_number: Some(x509.serial.to_bytes_be().into()), - key_identifier_method, + 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(), diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index d6824a39..34d25631 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -550,6 +550,28 @@ pub enum KeyIdMethod { } impl KeyIdMethod { + #[cfg(feature = "x509-parser")] + fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result { + let key_identifier_method = + x509.iter_extensions() + .find_map(|ext| match ext.parsed_extension() { + x509_parser::extensions::ParsedExtension::SubjectKeyIdentifier(key_id) => { + Some(KeyIdMethod::PreSpecified(key_id.0.into())) + }, + _ => None, + }); + + Ok(match key_identifier_method { + Some(method) => method, + None => { + #[cfg(not(feature = "crypto"))] + return Err(Error::UnsupportedSignatureAlgorithm); + #[cfg(feature = "crypto")] + KeyIdMethod::Sha256 + }, + }) + } + /// Derive a key identifier for the provided subject public key info using the key ID method. /// /// Typically this is a truncated hash over the raw subject public key info, but may From ebdf84cdb436e07ee7a07631cc74ff598b7606b1 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 May 2025 15:59:41 +0200 Subject: [PATCH 10/10] Rewrite unidiomatic use of or() combinator --- rcgen/src/certificate.rs | 10 +++++----- rcgen/src/csr.rs | 2 +- rcgen/src/lib.rs | 4 ++-- rustls-cert-gen/src/cert.rs | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index af724b63..9093952b 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -172,7 +172,7 @@ impl CertificateParams { /// See [`from_ca_cert_der`](Self::from_ca_cert_der) for more details. #[cfg(all(feature = "pem", feature = "x509-parser"))] pub fn from_ca_cert_pem(pem_str: &str) -> Result { - let certificate = pem::parse(pem_str).or(Err(Error::CouldNotParseCertificate))?; + let certificate = pem::parse(pem_str).map_err(|_| Error::CouldNotParseCertificate)?; Self::from_ca_cert_der(&certificate.contents().into()) } @@ -200,7 +200,7 @@ impl CertificateParams { #[cfg(feature = "x509-parser")] pub fn from_ca_cert_der(ca_cert: &CertificateDer<'_>) -> Result { let (_remainder, x509) = x509_parser::parse_x509_certificate(ca_cert) - .or(Err(Error::CouldNotParseCertificate))?; + .map_err(|_| Error::CouldNotParseCertificate)?; Ok(CertificateParams { is_ca: IsCa::from_x509(&x509)?, @@ -805,7 +805,7 @@ impl ExtendedKeyUsagePurpose { fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result, Error> { let extended_key_usage = x509 .extended_key_usage() - .or(Err(Error::CouldNotParseCertificate))? + .map_err(|_| Error::CouldNotParseCertificate)? .map(|ext| ext.value); let mut extended_key_usages = Vec::new(); @@ -872,7 +872,7 @@ impl NameConstraints { ) -> Result, Error> { let constraints = x509 .name_constraints() - .or(Err(Error::CouldNotParseCertificate))? + .map_err(|_| Error::CouldNotParseCertificate)? .map(|ext| ext.value); let Some(constraints) = constraints else { @@ -1097,7 +1097,7 @@ impl IsCa { let basic_constraints = x509 .basic_constraints() - .or(Err(Error::CouldNotParseCertificate))? + .map_err(|_| Error::CouldNotParseCertificate)? .map(|ext| ext.value); Ok(match basic_constraints { diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index a6f0d816..c6d6cb15 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -80,7 +80,7 @@ impl CertificateSigningRequestParams { /// See [`from_der`](Self::from_der) for more details. #[cfg(all(feature = "pem", feature = "x509-parser"))] pub fn from_pem(pem_str: &str) -> Result { - let csr = pem::parse(pem_str).or(Err(Error::CouldNotParseCertificationRequest))?; + let csr = pem::parse(pem_str).map_err(|_| Error::CouldNotParseCertificationRequest)?; Self::from_der(&csr.contents().into()) } diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 34d25631..2bbc7c1d 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -182,7 +182,7 @@ impl SanType { fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result, Error> { let sans = x509 .subject_alternative_name() - .or(Err(Error::CouldNotParseCertificate))? + .map_err(|_| Error::CouldNotParseCertificate)? .map(|ext| &ext.value.general_names); let Some(sans) = sans else { @@ -473,7 +473,7 @@ impl KeyUsagePurpose { fn from_x509(x509: &x509_parser::certificate::X509Certificate<'_>) -> Result, Error> { let key_usage = x509 .key_usage() - .or(Err(Error::CouldNotParseCertificate))? + .map_err(|_| Error::CouldNotParseCertificate)? .map(|ext| ext.value); // This x509 parser stores flags in reversed bit BIT STRING order let flags = key_usage.map_or(0u16, |k| k.flags).reverse_bits(); diff --git a/rustls-cert-gen/src/cert.rs b/rustls-cert-gen/src/cert.rs index 53e7dc62..4cf24ac7 100644 --- a/rustls-cert-gen/src/cert.rs +++ b/rustls-cert-gen/src/cert.rs @@ -250,8 +250,8 @@ impl KeyPairAlgorithm { let rng = ring_like::rand::SystemRandom::new(); let alg = &rcgen::PKCS_ED25519; - let pkcs8_bytes = - Ed25519KeyPair::generate_pkcs8(&rng).or(Err(rcgen::Error::RingUnspecified))?; + let pkcs8_bytes = Ed25519KeyPair::generate_pkcs8(&rng) + .map_err(|_| rcgen::Error::RingUnspecified)?; rcgen::KeyPair::from_pkcs8_der_and_sign_algo(&pkcs8_bytes.as_ref().into(), alg) }, @@ -263,7 +263,7 @@ impl KeyPairAlgorithm { let alg = &rcgen::PKCS_ECDSA_P256_SHA256; let pkcs8_bytes = EcdsaKeyPair::generate_pkcs8(&ECDSA_P256_SHA256_ASN1_SIGNING, &rng) - .or(Err(rcgen::Error::RingUnspecified))?; + .map_err(|_| rcgen::Error::RingUnspecified)?; rcgen::KeyPair::from_pkcs8_der_and_sign_algo(&pkcs8_bytes.as_ref().into(), alg) }, KeyPairAlgorithm::EcdsaP384 => { @@ -274,7 +274,7 @@ impl KeyPairAlgorithm { let alg = &rcgen::PKCS_ECDSA_P384_SHA384; let pkcs8_bytes = EcdsaKeyPair::generate_pkcs8(&ECDSA_P384_SHA384_ASN1_SIGNING, &rng) - .or(Err(rcgen::Error::RingUnspecified))?; + .map_err(|_| rcgen::Error::RingUnspecified)?; rcgen::KeyPair::from_pkcs8_der_and_sign_algo(&pkcs8_bytes.as_ref().into(), alg) }, @@ -287,7 +287,7 @@ impl KeyPairAlgorithm { let alg = &rcgen::PKCS_ECDSA_P521_SHA512; let pkcs8_bytes = EcdsaKeyPair::generate_pkcs8(&ECDSA_P521_SHA512_ASN1_SIGNING, &rng) - .or(Err(rcgen::Error::RingUnspecified))?; + .map_err(|_| rcgen::Error::RingUnspecified)?; rcgen::KeyPair::from_pkcs8_der_and_sign_algo(&pkcs8_bytes.as_ref().into(), alg) },