From 91f9e77934b2f32b3ee1d83b1b38ba7246bf9122 Mon Sep 17 00:00:00 2001 From: Maurus Decimus <11444311+mdecimus@users.noreply.github.com> Date: Mon, 15 Jun 2026 15:19:14 +0200 Subject: [PATCH] Fix ACME: Add freshness check when renewing certificates --- CHANGELOG.md | 1 + README.md | 4 +- crates/common/src/network/acme/order.rs | 146 ++++++++++++------------ crates/common/src/network/acme/renew.rs | 85 +++++++++++--- 4 files changed, 144 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b947b41..dbff97cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If - FoundationDB: Fix read version cache expiration logic. - MTA: Re-scheduling or editing a queued message reports success but persists nothing for recipients in a non-`default` virtual queue. - CardDAV: Version requests included in `address-data` are ignored. +- ACME: Add freshness check when renewing certificates. ## [0.16.8] - 2026-06-06 diff --git a/README.md b/README.md index e347df81..e085ee30 100644 --- a/README.md +++ b/README.md @@ -168,10 +168,10 @@ If you find the project useful you can help by [becoming a sponsor](https://open ## License -This project is dual-licensed under the **GNU Affero General Public License v3.0** (AGPL-3.0; as published by the Free Software Foundation) and the **Stalwart Enterprise License v1 (SELv1)**: +This project is dual-licensed under the **GNU Affero General Public License v3.0** (AGPL-3.0; as published by the Free Software Foundation) and the **Stalwart Enterprise License v2 (SELv2)**: - The [GNU Affero General Public License v3.0](./LICENSES/AGPL-3.0-only.txt) is a free software license that ensures your freedom to use, modify, and distribute the software, with the condition that any modified versions of the software must also be distributed under the same license. -- The [Stalwart Enterprise License v1 (SELv1)](./LICENSES/LicenseRef-SEL.txt) is a proprietary license designed for commercial use. It offers additional features and greater flexibility for businesses that do not wish to comply with the AGPL-3.0 license requirements. +- The [Stalwart Enterprise License v2 (SELv2)](./LICENSES/LicenseRef-SEL.txt) is a proprietary license designed for commercial use. It offers additional features and greater flexibility for businesses that do not wish to comply with the AGPL-3.0 license requirements. Each file in this project contains a license notice at the top, indicating the applicable license(s). The license notice follows the [REUSE guidelines](https://reuse.software/) to ensure clarity and consistency. The full text of each license is available in the [LICENSES](./LICENSES/) directory. diff --git a/crates/common/src/network/acme/order.rs b/crates/common/src/network/acme/order.rs index 60395c4a..c9a75a19 100644 --- a/crates/common/src/network/acme/order.rs +++ b/crates/common/src/network/acme/order.rs @@ -27,14 +27,13 @@ use x509_parser::prelude::{GeneralName, ParsedExtension}; const HOSTNAMES: &[&str] = &["mta-sts", "ua-auto-config", "autoconfig", "autodiscover"]; impl AcmeRequestBuilder { - pub async fn renew( + pub fn build_domains( &self, server: &Server, domain: &str, hostnames: &[String], - dns_parameters: Option, - ) -> AcmeResult { - let domains = if hostnames.is_empty() { + ) -> Vec { + if hostnames.is_empty() { if matches!( self.challenge, ChallengeType::Dns01 | ChallengeType::DnsPersist01 @@ -87,8 +86,15 @@ impl AcmeRequestBuilder { } }) .collect() - }; + } + } + pub async fn renew( + &self, + server: &Server, + domains: Vec, + dns_parameters: Option, + ) -> AcmeResult { let mut params = CertificateParams::new(domains.clone()).map_err(|err| { AcmeError::Crypto(format!("Failed to create certificate params: {}", err)) })?; @@ -339,78 +345,66 @@ impl AcmeRequestBuilder { impl ParsedCert { pub fn parse(certificate: impl AsRef<[u8]>) -> AcmeResult { - pem::parse_many(certificate) - .map_err(|err| AcmeError::Crypto(format!("Failed to parse PEM: {}", err))) - .and_then(|pems| { - pems.into_iter() - .next() - .ok_or_else(|| AcmeError::Crypto("No certificates found in PEM".to_string())) - }) - .and_then(|der| { - parse_x509_certificate(der.contents()) - .map_err(|err| { - AcmeError::Crypto(format!("Failed to parse X.509 certificate: {}", err)) - }) - .and_then(|(_, cert)| { - // Add CNs and SANs to the list of names - let mut names: BTreeSet = BTreeSet::new(); - for name in cert.subject().iter_common_name() { - if let Ok(name) = name.as_str() { - names.insert(name.into()); - } - } - for ext in cert.extensions() { - if let ParsedExtension::SubjectAlternativeName(san) = - ext.parsed_extension() - { - for name in &san.general_names { - let name = match name { - GeneralName::DNSName(name) => (*name).into(), - GeneralName::IPAddress(ip) => match ip.len() { - 4 => Ipv4Addr::from(<[u8; 4]>::try_from(*ip).unwrap()) - .to_string(), - 16 => { - Ipv6Addr::from(<[u8; 16]>::try_from(*ip).unwrap()) - .to_string() - } - _ => continue, - }, - _ => { - continue; - } - }; - names.insert(name); - } - } - } + let der = pem::parse_many(certificate) + .map_err(|err| AcmeError::Crypto(format!("Failed to parse PEM: {}", err)))? + .into_iter() + .next() + .ok_or_else(|| AcmeError::Crypto("No certificates found in PEM".to_string()))?; + Self::parse_der(der.contents()) + } - Ok(ParsedCert { - sans: names.into_iter().collect(), - issuer: cert.tbs_certificate.issuer().to_string(), - valid_not_before: Utc - .timestamp_opt( - cert.tbs_certificate.validity().not_before.timestamp(), - 0, - ) - .single() - .ok_or_else(|| { - AcmeError::Crypto( - "Certificate not_before time is out of range".to_string(), - ) - })?, - valid_not_after: Utc - .timestamp_opt( - cert.tbs_certificate.validity().not_after.timestamp(), - 0, - ) - .single() - .ok_or_else(|| { - AcmeError::Crypto( - "Certificate not_after time is out of range".to_string(), - ) - })?, - }) - }) + pub fn parse_der(der: &[u8]) -> AcmeResult { + parse_x509_certificate(der) + .map_err(|err| AcmeError::Crypto(format!("Failed to parse X.509 certificate: {}", err))) + .and_then(|(_, cert)| { + // Add CNs and SANs to the list of names + let mut names: BTreeSet = BTreeSet::new(); + for name in cert.subject().iter_common_name() { + if let Ok(name) = name.as_str() { + names.insert(name.into()); + } + } + for ext in cert.extensions() { + if let ParsedExtension::SubjectAlternativeName(san) = ext.parsed_extension() { + for name in &san.general_names { + let name = match name { + GeneralName::DNSName(name) => (*name).into(), + GeneralName::IPAddress(ip) => match ip.len() { + 4 => Ipv4Addr::from(<[u8; 4]>::try_from(*ip).unwrap()) + .to_string(), + 16 => Ipv6Addr::from(<[u8; 16]>::try_from(*ip).unwrap()) + .to_string(), + _ => continue, + }, + _ => { + continue; + } + }; + names.insert(name); + } + } + } + + Ok(ParsedCert { + sans: names.into_iter().collect(), + issuer: cert.tbs_certificate.issuer().to_string(), + valid_not_before: Utc + .timestamp_opt(cert.tbs_certificate.validity().not_before.timestamp(), 0) + .single() + .ok_or_else(|| { + AcmeError::Crypto( + "Certificate not_before time is out of range".to_string(), + ) + })?, + valid_not_after: Utc + .timestamp_opt(cert.tbs_certificate.validity().not_after.timestamp(), 0) + .single() + .ok_or_else(|| { + AcmeError::Crypto( + "Certificate not_after time is out of range".to_string(), + ) + })?, + }) }) } } diff --git a/crates/common/src/network/acme/renew.rs b/crates/common/src/network/acme/renew.rs index 8ef885b9..1cbe41fd 100644 --- a/crates/common/src/network/acme/renew.rs +++ b/crates/common/src/network/acme/renew.rs @@ -55,10 +55,24 @@ impl Server { cert.acme_provider_id ))); }; + let challenge_type = acme_provider.challenge_type; + let renew_before = acme_provider.renew_before; + let request = AcmeRequestBuilder::new(acme_provider).await?; + let domains = request.build_domains( + self, + &domain.name, + &cert.subject_alternative_names.into_inner(), + ); + + if let Some(renew_at) = self.acme_certificate_renewal_due(&domains, renew_before, now()) { + return Ok(vec![Task::AcmeRenewal(TaskDomainManagement { + domain_id, + status: TaskStatus::at(renew_at as i64), + })]); + } + let dns_parameters = match &domain.dns_management { - DnsManagement::Automatic(props) - if acme_provider.challenge_type == AcmeChallengeType::Dns01 => - { + DnsManagement::Automatic(props) if challenge_type == AcmeChallengeType::Dns01 => { match self.build_dns_updater(props.dns_server_id).await? { Ok(updater) => Some(AcmeDnsParameters { updater, @@ -74,22 +88,13 @@ impl Server { } _ => None, }; - if acme_provider.challenge_type == AcmeChallengeType::Dns01 && dns_parameters.is_none() { + if challenge_type == AcmeChallengeType::Dns01 && dns_parameters.is_none() { return Err(AcmeError::Invalid( "ACME provider requires DNS challenge but a DNS provider was not configured" .to_string(), )); } - let renew_before = acme_provider.renew_before; - let pem_cert = AcmeRequestBuilder::new(acme_provider) - .await? - .renew( - self, - &domain.name, - &cert.subject_alternative_names.into_inner(), - dns_parameters, - ) - .await?; + let pem_cert = request.renew(self, domains, dns_parameters).await?; let parsed_cert = ParsedCert::parse(&pem_cert.certificate)?; let mut new_sans = parsed_cert.sans.clone(); new_sans.sort(); @@ -205,4 +210,56 @@ impl Server { err => Err(AcmeError::Registry(err)), } } + + 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(); + + 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 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 total = not_valid_after.saturating_sub(not_valid_before); + let (numerator, denominator) = match renew_before { + AcmeRenewBefore::R12 => (1, 2), + AcmeRenewBefore::R23 => (2, 3), + AcmeRenewBefore::R34 => (3, 4), + AcmeRenewBefore::R45 => (4, 5), + }; + let renew_at = not_valid_before + total * numerator / denominator; + return if now < renew_at { + Some(renew_at as u64) + } else { + None + }; + } + + None + } }