From 87e644141db4e24c9d2999cb4455382991bfc9d4 Mon Sep 17 00:00:00 2001 From: Lukas Velikov Date: Wed, 7 Aug 2024 12:23:29 -0400 Subject: [PATCH 1/5] Add KeyUsagePurpose::to_u16, simplify KeyUsage encoding --- rcgen/src/certificate.rs | 40 +++++++++++----------------------------- rcgen/src/lib.rs | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index e00b8e8b..b74b2d5b 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -672,36 +672,18 @@ impl CertificateParams { // Write standard key usage if !self.key_usages.is_empty() { + // 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; + + // "When present, conforming CAs SHOULD mark this extension as critical." write_x509_extension(writer.next(), oid::KEY_USAGE, true, |writer| { - let mut bits: u16 = 0; - - for entry in self.key_usages.iter() { - // Map the index to a value - let index = match entry { - KeyUsagePurpose::DigitalSignature => 0, - KeyUsagePurpose::ContentCommitment => 1, - KeyUsagePurpose::KeyEncipherment => 2, - KeyUsagePurpose::DataEncipherment => 3, - KeyUsagePurpose::KeyAgreement => 4, - KeyUsagePurpose::KeyCertSign => 5, - KeyUsagePurpose::CrlSign => 6, - KeyUsagePurpose::EncipherOnly => 7, - KeyUsagePurpose::DecipherOnly => 8, - }; - - bits |= 1 << index; - } - - // Compute the 1-based most significant bit - let msb = 16 - bits.leading_zeros(); - let nb = if msb <= 8 { 1 } else { 2 }; - - let bits = bits.reverse_bits().to_be_bytes(); - - // Finally take only the bytes != 0 - let bits = &bits[..nb]; - - writer.write_bitvec_bytes(bits, msb as usize) + // u16 is large enough to encode the largest possible key usage (two-bytes) + let bit_string = + self.key_usages.iter().fold(0u16, |bit_string, key_usage| { + bit_string | key_usage.to_u16() + }); + writer.write_bitvec_bytes(&bit_string.to_be_bytes(), KEY_USAGE_BITS); }); } diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index 912e9d91..ea7f0e83 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -435,6 +435,25 @@ pub enum KeyUsagePurpose { DecipherOnly, } +impl KeyUsagePurpose { + /// 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 { + const FLAG: u16 = 0b1000_0000_0000_0000; + FLAG >> match self { + KeyUsagePurpose::DigitalSignature => 0, + KeyUsagePurpose::ContentCommitment => 1, + KeyUsagePurpose::KeyEncipherment => 2, + KeyUsagePurpose::DataEncipherment => 3, + KeyUsagePurpose::KeyAgreement => 4, + KeyUsagePurpose::KeyCertSign => 5, + KeyUsagePurpose::CrlSign => 6, + KeyUsagePurpose::EncipherOnly => 7, + KeyUsagePurpose::DecipherOnly => 8, + } + } +} + /// Method to generate key identifiers from public keys. /// /// Key identifiers should be derived from the public key data. [RFC 7093] defines From fa7289f41e953a801f452ee702b72098c25435f7 Mon Sep 17 00:00:00 2001 From: Lukas Velikov Date: Tue, 6 Aug 2024 21:37:51 -0400 Subject: [PATCH 2/5] Extract KeyUsage encoding into CertificateParams::write_key_usage --- rcgen/src/certificate.rs | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index b74b2d5b..d080abf0 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -455,6 +455,25 @@ impl CertificateParams { Ok(result) } + /// Write a certificate's KeyUsage as defined in RFC 5280. + 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; + if self.key_usages.is_empty() { + return; + } + + // "When present, conforming CAs SHOULD mark this extension as critical." + write_x509_extension(writer, oid::KEY_USAGE, true, |writer| { + // u16 is large enough to encode the largest possible key usage (two-bytes) + let bit_string = self.key_usages.iter().fold(0u16, |bit_string, key_usage| { + bit_string | key_usage.to_u16() + }); + writer.write_bitvec_bytes(&bit_string.to_be_bytes(), KEY_USAGE_BITS); + }); + } + 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| { @@ -671,21 +690,7 @@ impl CertificateParams { } // Write standard key usage - if !self.key_usages.is_empty() { - // 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; - - // "When present, conforming CAs SHOULD mark this extension as critical." - write_x509_extension(writer.next(), oid::KEY_USAGE, true, |writer| { - // u16 is large enough to encode the largest possible key usage (two-bytes) - let bit_string = - self.key_usages.iter().fold(0u16, |bit_string, key_usage| { - bit_string | key_usage.to_u16() - }); - writer.write_bitvec_bytes(&bit_string.to_be_bytes(), KEY_USAGE_BITS); - }); - } + self.write_key_usage(writer.next()); // Write extended key usage if !self.extended_key_usages.is_empty() { From ac04eaa7365a45183e9136888c8c088caa82b088 Mon Sep 17 00:00:00 2001 From: Lukas Velikov Date: Tue, 6 Aug 2024 21:40:03 -0400 Subject: [PATCH 3/5] Add KeyUsage support for CSR generation --- rcgen/src/certificate.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index d080abf0..6763a02d 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -563,7 +563,6 @@ impl CertificateParams { ); if serial_number.is_some() || *is_ca != IsCa::NoCa - || !key_usages.is_empty() || name_constraints.is_some() || !crl_distribution_points.is_empty() || *use_authority_key_identifier_extension @@ -581,12 +580,17 @@ impl CertificateParams { // Write extensions // According to the spec in RFC 2986, even if attributes are empty we need the empty attribute tag writer.next().write_tagged(Tag::context(0), |writer| { - if !subject_alt_names.is_empty() || !custom_extensions.is_empty() { + if !key_usages.is_empty() + || !subject_alt_names.is_empty() + || !custom_extensions.is_empty() + { writer.write_sequence(|writer| { let oid = ObjectIdentifier::from_slice(oid::PKCS_9_AT_EXTENSION_REQUEST); writer.next().write_oid(&oid); 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()); @@ -613,6 +617,7 @@ impl CertificateParams { der: CertificateSigningRequestDer::from(der), }) } + pub(crate) fn serialize_der_with_signer( &self, pub_key: &K, From b6ea5f1248929cc2fd51541e8a195d421b167507 Mon Sep 17 00:00:00 2001 From: Lukas Velikov Date: Wed, 7 Aug 2024 14:46:00 -0400 Subject: [PATCH 4/5] Add KeyUsagePurpose::from_u16 --- rcgen/src/certificate.rs | 35 +++-------------------------------- rcgen/src/lib.rs | 25 ++++++++++++++++++++++++- rcgen/tests/webpki.rs | 5 +++++ 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 6763a02d..1d634c37 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -324,38 +324,9 @@ impl CertificateParams { .key_usage() .or(Err(Error::CouldNotParseCertificate))? .map(|ext| ext.value); - - let mut key_usages = Vec::new(); - if let Some(key_usage) = key_usage { - if key_usage.digital_signature() { - key_usages.push(KeyUsagePurpose::DigitalSignature); - } - if key_usage.non_repudiation() { - key_usages.push(KeyUsagePurpose::ContentCommitment); - } - if key_usage.key_encipherment() { - key_usages.push(KeyUsagePurpose::KeyEncipherment); - } - if key_usage.data_encipherment() { - key_usages.push(KeyUsagePurpose::DataEncipherment); - } - if key_usage.key_agreement() { - key_usages.push(KeyUsagePurpose::KeyAgreement); - } - if key_usage.key_cert_sign() { - key_usages.push(KeyUsagePurpose::KeyCertSign); - } - if key_usage.crl_sign() { - key_usages.push(KeyUsagePurpose::CrlSign); - } - if key_usage.encipher_only() { - key_usages.push(KeyUsagePurpose::EncipherOnly); - } - if key_usage.decipher_only() { - key_usages.push(KeyUsagePurpose::DecipherOnly); - } - } - Ok(key_usages) + // 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( diff --git a/rcgen/src/lib.rs b/rcgen/src/lib.rs index ea7f0e83..0caecbeb 100644 --- a/rcgen/src/lib.rs +++ b/rcgen/src/lib.rs @@ -413,7 +413,7 @@ impl<'a> Iterator for DistinguishedNameIterator<'a> { } /// One of the purposes contained in the [key usage](https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.3) extension -#[derive(Debug, PartialEq, Eq, Hash, Clone)] +#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)] pub enum KeyUsagePurpose { /// digitalSignature DigitalSignature, @@ -452,6 +452,29 @@ impl KeyUsagePurpose { KeyUsagePurpose::DecipherOnly => 8, } } + + /// Parse a collection of key usages from a [`u16`] representing the value + /// of a KeyUsage BIT STRING as defined by RFC 5280. + #[cfg(feature = "x509-parser")] + fn from_u16(value: u16) -> Vec { + [ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::ContentCommitment, + KeyUsagePurpose::KeyEncipherment, + KeyUsagePurpose::DataEncipherment, + KeyUsagePurpose::KeyAgreement, + KeyUsagePurpose::KeyCertSign, + KeyUsagePurpose::CrlSign, + KeyUsagePurpose::EncipherOnly, + KeyUsagePurpose::DecipherOnly, + ] + .iter() + .filter_map(|key_usage| { + let present = key_usage.to_u16() & value != 0; + present.then_some(*key_usage) + }) + .collect() + } } /// Method to generate key identifiers from public keys. diff --git a/rcgen/tests/webpki.rs b/rcgen/tests/webpki.rs index 8f076952..01eec1ec 100644 --- a/rcgen/tests/webpki.rs +++ b/rcgen/tests/webpki.rs @@ -412,11 +412,16 @@ fn test_webpki_separate_ca_name_constraints() { fn test_webpki_imported_ca() { let (mut params, ca_key) = util::default_params(); params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.key_usages.push(KeyUsagePurpose::KeyCertSign); let ca_cert = params.self_signed(&ca_key).unwrap(); let ca_cert_der = ca_cert.der(); let imported_ca_cert_params = CertificateParams::from_ca_cert_der(ca_cert_der).unwrap(); + assert_eq!( + imported_ca_cert_params.key_usages, + vec![KeyUsagePurpose::KeyCertSign] + ); let imported_ca_cert = imported_ca_cert_params.self_signed(&ca_key).unwrap(); let mut params = CertificateParams::new(vec!["crabs.crabs".to_string()]).unwrap(); From 42f1e3efcbe878a9ec251d21c72b19ac917330c0 Mon Sep 17 00:00:00 2001 From: Lukas Velikov Date: Wed, 7 Aug 2024 14:50:04 -0400 Subject: [PATCH 5/5] Add KeyUsagePurpose support to CSR from DER; add CSR test --- rcgen/src/csr.rs | 7 +++++++ rcgen/tests/generic.rs | 30 +++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index e79e21eb..79fab8e2 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -94,7 +94,9 @@ impl CertificateSigningRequestParams { /// [`rustls_pemfile::csr()`]: https://docs.rs/rustls-pemfile/latest/rustls_pemfile/fn.csr.html #[cfg(feature = "x509-parser")] pub fn from_der(csr: &CertificateSigningRequestDer<'_>) -> Result { + use crate::KeyUsagePurpose; use x509_parser::prelude::FromDer; + let csr = x509_parser::certification_request::X509CertificationRequest::from_der(csr) .map_err(|_| Error::CouldNotParseCertificationRequest)? .1; @@ -117,6 +119,11 @@ impl CertificateSigningRequestParams { if let Some(extensions) = csr.requested_extensions() { for ext in extensions { match ext { + x509_parser::extensions::ParsedExtension::KeyUsage(key_usage) => { + // This x509 parser stores flags in reversed bit BIT STRING order + params.key_usages = + KeyUsagePurpose::from_u16(key_usage.flags.reverse_bits()); + }, x509_parser::extensions::ParsedExtension::SubjectAlternativeName(san) => { for name in &san.general_names { params diff --git a/rcgen/tests/generic.rs b/rcgen/tests/generic.rs index 0af46b17..d9eef933 100644 --- a/rcgen/tests/generic.rs +++ b/rcgen/tests/generic.rs @@ -360,7 +360,7 @@ mod test_parse_other_name_alt_name { #[cfg(feature = "x509-parser")] mod test_csr { - use rcgen::{CertificateParams, CertificateSigningRequestParams, KeyPair}; + use rcgen::{CertificateParams, CertificateSigningRequestParams, KeyPair, KeyUsagePurpose}; #[test] fn test_csr_roundtrip() { @@ -375,4 +375,32 @@ mod test_csr { // Ensure algorithms match. assert_eq!(key_pair.algorithm(), csrp.public_key.algorithm()); } + + #[test] + fn test_nontrivial_csr_roundtrip() { + let key_pair = KeyPair::generate().unwrap(); + + // We should be able to serialize a CSR, and then parse the CSR. + let mut params = CertificateParams::default(); + params.key_usages = vec![ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::ContentCommitment, + KeyUsagePurpose::KeyEncipherment, + KeyUsagePurpose::DataEncipherment, + KeyUsagePurpose::KeyAgreement, + KeyUsagePurpose::KeyCertSign, + KeyUsagePurpose::CrlSign, + // It doesn't make sense to have both encipher and decipher only + // So we'll take this opportunity to test omitting a key usage + // KeyUsagePurpose::EncipherOnly, + KeyUsagePurpose::DecipherOnly, + ]; + let csr = params.serialize_request(&key_pair).unwrap(); + let csrp = CertificateSigningRequestParams::from_der(csr.der()).unwrap(); + + // Ensure algorithms match. + assert_eq!(key_pair.algorithm(), csrp.public_key.algorithm()); + // Ensure key usages match. + assert_eq!(csrp.params.key_usages, params.key_usages); + } }