diff --git a/CHANGELOG.md b/CHANGELOG.md index e0ae0e78..bbdd2c53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If - OAuth Profile for Open Public Clients ([draft-ietf-mailmaint-oauth-public](https://datatracker.ietf.org/doc/draft-ietf-mailmaint-oauth-public/)) - Client secret verification for confidential clients. - HTTP: Add `redirectRoot` option to `Http` object to allow redirecting requests to the root path to a different path (e.g. `/account`). +- ACME: `reuseKey` option to allow reusing private keys in renewals. ## Changed diff --git a/crates/common/src/network/acme/order.rs b/crates/common/src/network/acme/order.rs index cc45a653..9a624a3f 100644 --- a/crates/common/src/network/acme/order.rs +++ b/crates/common/src/network/acme/order.rs @@ -93,14 +93,21 @@ impl AcmeRequestBuilder { &self, server: &Server, domains: Vec, + reuse_key_pem: Option, dns_parameters: Option, ) -> AcmeResult { let mut params = CertificateParams::new(domains.clone()).map_err(|err| { AcmeError::Crypto(format!("Failed to create certificate params: {}", err)) })?; params.distinguished_name = DistinguishedName::new(); - let key_pair = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256) - .map_err(|err| AcmeError::Crypto(format!("Failed to generate key pair: {}", err)))?; + let key_pair = match reuse_key_pem { + Some(pem) => KeyPair::from_pem(&pem).map_err(|err| { + AcmeError::Crypto(format!("Failed to load private key for reuse: {}", err)) + })?, + None => KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).map_err(|err| { + AcmeError::Crypto(format!("Failed to generate key pair: {}", err)) + })?, + }; let response = self.new_order(domains.clone()).await?; let order_url = response.location; let mut order = response.body; diff --git a/crates/common/src/network/acme/renew.rs b/crates/common/src/network/acme/renew.rs index 02de5488..3cb3b75c 100644 --- a/crates/common/src/network/acme/renew.rs +++ b/crates/common/src/network/acme/renew.rs @@ -14,7 +14,7 @@ use crate::{ use registry::{ schema::{ enums::{AcmeChallengeType, AcmeRenewBefore, DnsRecordType}, - prelude::ObjectType, + prelude::{ObjectType, Property}, structs::{ AcmeProvider, Certificate, CertificateManagement, DnsManagement, Domain, PublicText, PublicTextValue, SecretText, SecretTextValue, SystemSettings, Task, TaskDnsManagement, @@ -24,7 +24,10 @@ use registry::{ types::{datetime::UTCDateTime, id::ObjectId, map::Map}, }; use store::{ - registry::write::{RegistryWrite, RegistryWriteResult}, + registry::{ + RegistryQuery, + write::{RegistryWrite, RegistryWriteResult}, + }, write::now, }; use types::id::Id; @@ -57,6 +60,7 @@ impl Server { }; let challenge_type = acme_provider.challenge_type; let renew_before = acme_provider.renew_before; + let reuse_key = acme_provider.reuse_key; let request = AcmeRequestBuilder::new(acme_provider).await?; let domains = request.build_domains( self, @@ -64,7 +68,10 @@ impl Server { &cert.subject_alternative_names.into_inner(), ); - if let Some(renew_at) = self.acme_certificate_renewal_due(&domains, renew_before, now()) { + if let Some(renew_at) = self + .acme_certificate_renewal_due(&domains, renew_before, now()) + .await? + { return Err(AcmeError::NotDue(format!( "Certificate for domain {} is still valid; renewal is not due until {}", domain.name, @@ -95,7 +102,25 @@ impl Server { .to_string(), )); } - let pem_cert = request.renew(self, domains, dns_parameters).await?; + let reuse_key_pem = if reuse_key { + match self.acme_certificate_by_domains(&domains).await? { + Some(certificate) => certificate + .private_key + .secret() + .await + .map(std::borrow::Cow::into_owned) + .map_err(|err| { + AcmeError::Crypto(format!("Failed to load certificate private key: {err}")) + })? + .into(), + None => None, + } + } else { + None + }; + let pem_cert = request + .renew(self, domains, reuse_key_pem, dns_parameters) + .await?; let parsed_cert = ParsedCert::parse(&pem_cert.certificate)?; let mut new_sans = parsed_cert.sans.clone(); new_sans.sort(); @@ -198,50 +223,63 @@ impl Server { } } - fn acme_certificate_renewal_due( + async fn acme_certificate_by_domains( + &self, + domains: &[String], + ) -> AcmeResult> { + let mut wanted = domains.iter().collect::>(); + wanted.sort(); + let Some(reference) = wanted.first() else { + return Ok(None); + }; + + let candidate_ids = self + .registry() + .query::>( + RegistryQuery::new(ObjectType::Certificate) + .text(Property::SubjectAlternativeNames, reference.as_str()), + ) + .await?; + + for id in candidate_ids { + let Some(certificate) = self.registry().object::(id).await? else { + continue; + }; + let mut sans = certificate + .subject_alternative_names + .iter() + .collect::>(); + sans.sort(); + if sans == wanted { + return Ok(Some(certificate)); + } + } + + Ok(None) + } + + async fn acme_certificate_renewal_due( &self, domains: &[String], renew_before: AcmeRenewBefore, now: u64, - ) -> Option { - let mut target = domains.iter().map(|d| d.as_str()).collect::>(); - target.sort(); - + ) -> AcmeResult> { let now = now as i64; - let certificates = self.inner.data.tls_certificates.load(); - for name in &target { - let Some(certified_key) = certificates.get(name.strip_prefix("*.").unwrap_or(name)) - else { - continue; - }; - let Ok(leaf) = certified_key.end_entity_cert() else { - continue; - }; - let Ok(parsed) = ParsedCert::parse_der(leaf.as_ref()) else { - continue; - }; + let Some(certificate) = self.acme_certificate_by_domains(domains).await? else { + return Ok(None); + }; - let mut sans = parsed.sans; - sans.sort(); - if sans != target { - continue; - } - - let not_valid_after = parsed.valid_not_after.timestamp(); - if not_valid_after <= now { - return None; - } - let not_valid_before = parsed.valid_not_before.timestamp(); - let renew_at = - Self::acme_renewal_due_at(not_valid_before, not_valid_after, renew_before); - return if now < renew_at { - Some(renew_at as u64) - } else { - None - }; + let not_valid_after = certificate.not_valid_after.timestamp(); + if not_valid_after <= now { + return Ok(None); } - - None + let not_valid_before = certificate.not_valid_before.timestamp(); + let renew_at = Self::acme_renewal_due_at(not_valid_before, not_valid_after, renew_before); + Ok(if now < renew_at { + Some(renew_at as u64) + } else { + None + }) } fn acme_renewal_due_at( diff --git a/crates/registry/src/schema/properties.rs b/crates/registry/src/schema/properties.rs index a12e6c67..b7254e72 100644 --- a/crates/registry/src/schema/properties.rs +++ b/crates/registry/src/schema/properties.rs @@ -988,6 +988,7 @@ pub enum Property { RetryCount = 640, RetryDue = 641, ReturnPath = 635, + ReuseKey = 912, ReverseIpVerify = 692, Rewrite = 565, RoleIds = 193, diff --git a/crates/registry/src/schema/properties_impl.rs b/crates/registry/src/schema/properties_impl.rs index afe9d578..a6f27ba6 100644 --- a/crates/registry/src/schema/properties_impl.rs +++ b/crates/registry/src/schema/properties_impl.rs @@ -1141,6 +1141,7 @@ impl EnumImpl for Property { b"retryCount" => Property::RetryCount, b"retryDue" => Property::RetryDue, b"returnPath" => Property::ReturnPath, + b"reuseKey" => Property::ReuseKey, b"reverseIpVerify" => Property::ReverseIpVerify, b"rewrite" => Property::Rewrite, b"roleIds" => Property::RoleIds, @@ -2058,6 +2059,7 @@ impl EnumImpl for Property { Property::RetryCount => "retryCount", Property::RetryDue => "retryDue", Property::ReturnPath => "returnPath", + Property::ReuseKey => "reuseKey", Property::ReverseIpVerify => "reverseIpVerify", Property::Rewrite => "rewrite", Property::RoleIds => "roleIds", @@ -2979,6 +2981,7 @@ impl EnumImpl for Property { 640 => Some(Property::RetryCount), 641 => Some(Property::RetryDue), 635 => Some(Property::ReturnPath), + 912 => Some(Property::ReuseKey), 692 => Some(Property::ReverseIpVerify), 565 => Some(Property::Rewrite), 193 => Some(Property::RoleIds), @@ -3161,7 +3164,7 @@ impl EnumImpl for Property { } } - const COUNT: usize = 912; + const COUNT: usize = 913; } impl serde::Serialize for Property { diff --git a/crates/registry/src/schema/structs.rs b/crates/registry/src/schema/structs.rs index c5fb832f..ac046613 100644 --- a/crates/registry/src/schema/structs.rs +++ b/crates/registry/src/schema/structs.rs @@ -59,6 +59,8 @@ pub struct AcmeProvider { pub member_tenant_id: Option, #[serde(rename = "preferredChain")] pub preferred_chain: Option, + #[serde(rename = "reuseKey")] + pub reuse_key: bool, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/crates/registry/src/schema/structs_impl.rs b/crates/registry/src/schema/structs_impl.rs index 4816267c..2e09f31a 100644 --- a/crates/registry/src/schema/structs_impl.rs +++ b/crates/registry/src/schema/structs_impl.rs @@ -289,7 +289,7 @@ impl RegistryJsonPropertyPatch for AccountSettings { impl ObjectImpl for AcmeProvider { const FLAGS: u64 = OBJ_FILTER_TENANT; - const VERSION: u8 = 1; + const VERSION: u8 = 2; const OBJECT: ObjectType = ObjectType::AcmeProvider; fn validate(&self, errors: &mut Vec) -> bool { @@ -351,6 +351,7 @@ impl Pickle for AcmeProvider { self.max_retries.pickle(out); self.member_tenant_id.pickle(out); self.preferred_chain.pickle(out); + self.reuse_key.pickle(out); } fn unpickle(stream: &mut crate::pickle::PickledStream<'_>) -> Option { @@ -366,6 +367,9 @@ impl Pickle for AcmeProvider { if stream.version() >= 1 { this.preferred_chain = Pickle::unpickle(stream)?; } + if stream.version() >= 2 { + this.reuse_key = Pickle::unpickle(stream)?; + } Some(this) } } @@ -382,13 +386,14 @@ impl Default for AcmeProvider { max_retries: 10i64, member_tenant_id: Default::default(), preferred_chain: Default::default(), + reuse_key: false, } } } impl IntoValue for AcmeProvider { fn into_value(self) -> JmapValue<'static> { - let mut map = jmap_tools::Map::with_capacity(11); + let mut map = jmap_tools::Map::with_capacity(12); map.insert_unchecked(Property::ChallengeType, self.challenge_type.into_value()); map.insert_unchecked(Property::Contact, self.contact.into_value()); map.insert_unchecked(Property::Directory, self.directory.into_value()); @@ -398,6 +403,7 @@ impl IntoValue for AcmeProvider { map.insert_unchecked(Property::MaxRetries, self.max_retries.into_value()); map.insert_unchecked(Property::MemberTenantId, self.member_tenant_id.into_value()); map.insert_unchecked(Property::PreferredChain, self.preferred_chain.into_value()); + map.insert_unchecked(Property::ReuseKey, self.reuse_key.into_value()); JmapValue::Object(map) } } @@ -435,6 +441,7 @@ impl RegistryJsonPropertyPatch for AcmeProvider { Some(Property::PreferredChain) => self .preferred_chain .patch(pointer.with_validators(&[StringValidator::Trim]), value), + Some(Property::ReuseKey) => self.reuse_key.patch(pointer, value), Some(Property::Type) => Ok(MaybeUnpatched::Unpatched { property: Property::Type, value, diff --git a/resources/schema/schema.json.gz b/resources/schema/schema.json.gz index 2cf95620..8b79fc30 100644 Binary files a/resources/schema/schema.json.gz and b/resources/schema/schema.json.gz differ diff --git a/resources/schema/schema.json.sha256 b/resources/schema/schema.json.sha256 index 2ec3207b..f7bd6655 100644 --- a/resources/schema/schema.json.sha256 +++ b/resources/schema/schema.json.sha256 @@ -1 +1 @@ -qtv0KUvdELIzxfAvonxvf3PW10RnWMB9iADw0FtHMF4 \ No newline at end of file +wtAcis1aWGGJNdxRmc-Mibt95tE_ulNQWxYULQ7a094 \ No newline at end of file diff --git a/tests/src/automation/acme.rs b/tests/src/automation/acme.rs index f812bbcc..80fa4ca0 100644 --- a/tests/src/automation/acme.rs +++ b/tests/src/automation/acme.rs @@ -21,7 +21,7 @@ use registry::{ TaskDomainManagement, }, }, - types::{datetime::UTCDateTime, map::Map}, + types::{datetime::UTCDateTime, id::ObjectId, map::Map}, }; use serde_json::json; use store::{registry::write::RegistryWrite, write::now}; @@ -280,10 +280,10 @@ pub async fn test(test: &TestServer) { let not_valid_before = certificate.not_valid_before.timestamp(); let length = not_valid_after - not_valid_before; assert_eq!( - not_valid_after - length / 2, + not_valid_before + length / 2, task.due_timestamp() as i64, "ACME renewal task has incorrect due timestamp, expected around {} but found {}", - not_valid_after - length / 2, + not_valid_before + length / 2, task.due_timestamp() as i64 ); account.registry_destroy_all(ObjectType::Certificate).await; @@ -537,6 +537,105 @@ pub async fn test(test: &TestServer) { account.registry_destroy_all(ObjectType::Task).await; account.registry_destroy_all(ObjectType::AcmeProvider).await; + // reuse_key: the keypair (and thus the SPKI published in DANE "3 1 1" records) must + // stay stable across renewals when enabled, and rotate when disabled. + for (reuse_key, expect_stable) in [(true, true), (false, false)] { + account.registry_destroy_all(ObjectType::Certificate).await; + account.registry_destroy_all(ObjectType::Task).await; + + let reuse_acme_id = account + .registry_create_object(AcmeProvider { + directory: "https://localhost:14000/dir".to_string(), + contact: Map::new(vec!["mailto:hello@reuse.org".to_string()]), + challenge_type: AcmeChallengeType::TlsAlpn01, + reuse_key, + ..Default::default() + }) + .await; + let reuse_domain_id = account + .registry_create_object(Domain { + name: "reuse.org".to_string(), + certificate_management: CertificateManagement::Automatic( + CertificateManagementProperties { + acme_provider_id: reuse_acme_id, + subject_alternative_names: Default::default(), + }, + ), + dkim_management: DkimManagement::Manual, + dns_management: DnsManagement::Automatic(DnsManagementProperties { + dns_server_id: in_memory_dns_id, + ..Default::default() + }), + ..Default::default() + }) + .await; + + // Initial issuance + test.wait_for_tasks_skip_not_due().await; + let (first_id, first_cert) = account + .registry_get_all::() + .await + .into_iter() + .next() + .expect("a certificate to be issued"); + let first_chain = first_cert.certificate.value().await.unwrap().into_owned(); + let first_key = leaf_public_key(&first_chain); + + // Backdate the stored certificate so a renewal is immediately due, then renew it. + // The reuse path must locate this certificate by its SANs and reuse its private key. + let reference = store::write::now() as i64; + let object_id = ObjectId::new(ObjectType::Certificate, first_id); + let old = test + .server + .registry() + .get(object_id) + .await + .unwrap() + .expect("stored certificate"); + let mut backdated = Certificate::from(old.clone()); + backdated.not_valid_before = UTCDateTime::from_timestamp(reference - 1_000_000); + backdated.not_valid_after = UTCDateTime::from_timestamp(reference - 10); + test.server + .registry() + .write(RegistryWrite::update(first_id, &backdated.into(), &old)) + .await + .unwrap(); + test.server + .acme_renew(reuse_domain_id) + .await + .ok() + .expect("certificate renewal to succeed"); + + let (_, renewed_cert) = account + .registry_get_all::() + .await + .into_iter() + .find(|(id, _)| *id != first_id) + .expect("a renewed certificate"); + let renewed_chain = renewed_cert.certificate.value().await.unwrap().into_owned(); + let second_key = leaf_public_key(&renewed_chain); + + if expect_stable { + assert_eq!( + first_key, second_key, + "reuse_key=true must preserve the certificate public key across renewals" + ); + } else { + assert_ne!( + first_key, second_key, + "reuse_key=false must rotate the certificate public key on renewal" + ); + } + + account + .registry_destroy(ObjectType::Domain, [reuse_domain_id]) + .await + .assert_destroyed(&[reuse_domain_id]); + account.registry_destroy_all(ObjectType::Certificate).await; + account.registry_destroy_all(ObjectType::Task).await; + account.registry_destroy_all(ObjectType::AcmeProvider).await; + } + // Cleanup account .registry_update_object( @@ -750,6 +849,15 @@ fn subject_common_name(pem: &str) -> Option { .map(str::to_string) } +fn leaf_public_key(chain: &str) -> Vec { + let block = Pem::iter_from_buffer(chain.as_bytes()) + .next() + .expect("certificate chain should contain a leaf") + .expect("valid PEM block"); + let cert = block.parse_x509().expect("valid leaf certificate"); + cert.public_key().raw.to_vec() +} + fn top_issuer_common_name(chain: &str) -> Option { let block = Pem::iter_from_buffer(chain.as_bytes()) .filter_map(Result::ok) diff --git a/tests/src/smtp/outbound/dane.rs b/tests/src/smtp/outbound/dane.rs index 2624ba25..943183cc 100644 --- a/tests/src/smtp/outbound/dane.rs +++ b/tests/src/smtp/outbound/dane.rs @@ -34,7 +34,6 @@ use rustls_pki_types::CertificateDer; use sha2::{Digest, Sha256}; use smtp::outbound::dane::{dnssec::TlsaLookup, verify::TlsaVerify}; use smtp::queue::{Error, ErrorDetails, Status}; -use store::write::now; use std::{ collections::BTreeSet, fs::{self, File}, @@ -44,6 +43,7 @@ use std::{ sync::Arc, time::{Duration, Instant}, }; +use store::write::now; #[tokio::test] #[serial_test::serial]