From ed5446a47741af85bff8cccba20455a95aecd096 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 3 Apr 2024 17:42:35 -0400 Subject: [PATCH 1/3] cert: use key_identifier_method of issuer for AKI Previously when issuing a certificate with an authority key identifier (AKI) extension that's signed by an issuer certificate we had a small bug where we used the to-be-issued certificate's param's `key_identifier_method` to derive the key identifier of the issuing certificate to use for the issued certificate's AKI. Instead we should be using the issuer certificate's param's `key_identifier_method`, taking care to mind the pre-specified variant. We missed this with our unit testing of the pre-specified key identifier method because we only issued a self-signed test certificate, never issuing a certificate signed by the CA that has the customization. This commit fixes the bug and extends test coverage to prevent further regression. --- rcgen/src/certificate.rs | 55 +++++++++++++++++++++++++++------------- rcgen/src/csr.rs | 8 +++--- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 1c70120d..77985b47 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -154,11 +154,7 @@ impl CertificateParams { issuer_key: &KeyPair, ) -> Result { let subject_public_key_info = key_pair.public_key_der(); - let der = self.serialize_der_with_signer( - key_pair, - issuer_key, - &issuer.params.distinguished_name, - )?; + let der = self.serialize_der_with_signer(key_pair, issuer_key, &issuer.params)?; Ok(Certificate { params: self, subject_public_key_info, @@ -172,7 +168,7 @@ impl CertificateParams { /// [`Certificate::pem`]. pub fn self_signed(self, key_pair: &KeyPair) -> Result { let subject_public_key_info = key_pair.public_key_der(); - let der = self.serialize_der_with_signer(key_pair, key_pair, &self.distinguished_name)?; + let der = self.serialize_der_with_signer(key_pair, key_pair, &self)?; Ok(Certificate { params: self, subject_public_key_info, @@ -567,7 +563,7 @@ impl CertificateParams { &self, pub_key: &K, issuer: &KeyPair, - issuer_name: &DistinguishedName, + issuer_params: &CertificateParams, ) -> Result, Error> { let der = issuer.sign_der(|writer| { let pub_key_spki = @@ -596,7 +592,7 @@ impl CertificateParams { // Write signature algorithm issuer.alg.write_alg_ident(writer.next()); // Write issuer name - write_distinguished_name(writer.next(), issuer_name); + write_distinguished_name(writer.next(), &issuer_params.distinguished_name); // Write validity writer.next().write_sequence(|writer| { // Not before @@ -626,7 +622,13 @@ impl CertificateParams { if self.use_authority_key_identifier_extension { write_x509_authority_key_identifier( writer.next(), - self.key_identifier_method.derive(issuer.public_key_der()), + match &issuer_params.key_identifier_method { + KeyIdMethod::PreSpecified(aki) => aki.clone(), + #[cfg(feature = "crypto")] + _ => issuer_params + .key_identifier_method + .derive(issuer.public_key_der()), + }, ); } // Write subject_alt_names @@ -1397,24 +1399,24 @@ PITGdT9dgN88nHPCle0B1+OY+OZ5 -----END PRIVATE KEY-----"#; let params = CertificateParams::from_ca_cert_pem(ca_cert).unwrap(); - let expected_ski = vec![ + let ca_ski = vec![ 0x97, 0xD4, 0x76, 0xA1, 0x9B, 0x1A, 0x71, 0x35, 0x2A, 0xC7, 0xF4, 0xA1, 0x84, 0x12, 0x56, 0x06, 0xBA, 0x5D, 0x61, 0x84, ]; assert_eq!( - KeyIdMethod::PreSpecified(expected_ski.clone()), + KeyIdMethod::PreSpecified(ca_ski.clone()), params.key_identifier_method ); - let kp = KeyPair::from_pem(ca_key).unwrap(); - let ca_cert = params.self_signed(&kp).unwrap(); - assert_eq!(&expected_ski, &ca_cert.key_identifier()); + let ca_kp = KeyPair::from_pem(ca_key).unwrap(); + let ca_cert = params.self_signed(&ca_kp).unwrap(); + assert_eq!(&ca_ski, &ca_cert.key_identifier()); - let (_remainder, x509) = x509_parser::parse_x509_certificate(ca_cert.der()).unwrap(); + let (_, x509_ca) = x509_parser::parse_x509_certificate(ca_cert.der()).unwrap(); assert_eq!( - &expected_ski, - &x509 + &ca_ski, + &x509_ca .iter_extensions() .find_map(|ext| match ext.parsed_extension() { x509_parser::extensions::ParsedExtension::SubjectKeyIdentifier(key_id) => { @@ -1424,6 +1426,25 @@ PITGdT9dgN88nHPCle0B1+OY+OZ5 }) .unwrap() ); + + let ee_key = KeyPair::generate().unwrap(); + let mut ee_params = CertificateParams::default(); + ee_params.use_authority_key_identifier_extension = true; + let ee_cert = ee_params.signed_by(&ee_key, &ca_cert, &ee_key).unwrap(); + + let (_, x509_ee) = x509_parser::parse_x509_certificate(ee_cert.der()).unwrap(); + assert_eq!( + &ca_ski, + &x509_ee + .iter_extensions() + .find_map(|ext| match ext.parsed_extension() { + x509_parser::extensions::ParsedExtension::AuthorityKeyIdentifier(aki) => { + aki.key_identifier.as_ref().map(|ki| ki.0.to_vec()) + }, + _ => None, + }) + .unwrap() + ); } } } diff --git a/rcgen/src/csr.rs b/rcgen/src/csr.rs index aa2c2770..d4f0023c 100644 --- a/rcgen/src/csr.rs +++ b/rcgen/src/csr.rs @@ -149,11 +149,9 @@ impl CertificateSigningRequestParams { issuer: &Certificate, issuer_key: &KeyPair, ) -> Result { - let der = self.params.serialize_der_with_signer( - &self.public_key, - issuer_key, - &issuer.params.distinguished_name, - )?; + let der = + self.params + .serialize_der_with_signer(&self.public_key, issuer_key, &issuer.params)?; let subject_public_key_info = yasna::construct_der(|writer| { self.public_key.serialize_public_key_der(writer); }); From ba652b66524601dad927905ca66917c723f24bcd Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 3 Apr 2024 18:19:27 -0400 Subject: [PATCH 2/3] docs: update CHANGELOG for 0.13.1 Also fixes the release date for 0.13.0. --- rcgen/CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/rcgen/CHANGELOG.md b/rcgen/CHANGELOG.md index c3ad0bd3..ed7bace3 100644 --- a/rcgen/CHANGELOG.md +++ b/rcgen/CHANGELOG.md @@ -1,7 +1,15 @@ # Changes -## Release 0.13.0 - March XX, 2024 +## Release 0.13.1 - April 4th, 2024 + +Fixed: + +- Fixed incorrect usage of the subject certificate's parameter's key identifier + method when computing the key identifier of the issuer for the subject's + authority key identifier (AKI) extension. + +## Release 0.13.0 - March 28th, 2024 Breaking changes: From 66359a48b5e5927a5b0ba5a13aa7adef0548feb0 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 3 Apr 2024 18:16:41 -0400 Subject: [PATCH 3/3] rcgen: 0.13.0 -> 0.13.1 --- Cargo.lock | 2 +- rcgen/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e9cfffa8..3530f13a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -866,7 +866,7 @@ dependencies = [ [[package]] name = "rcgen" -version = "0.13.0" +version = "0.13.1" dependencies = [ "aws-lc-rs", "botan", diff --git a/rcgen/Cargo.toml b/rcgen/Cargo.toml index 5f79cc74..4550d34e 100644 --- a/rcgen/Cargo.toml +++ b/rcgen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rcgen" -version = "0.13.0" +version = "0.13.1" documentation = "https://docs.rs/rcgen" description.workspace = true repository.workspace = true