From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 244571FF13F for ; Thu, 23 Apr 2026 15:47:01 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id ECA4B19903; Thu, 23 Apr 2026 15:47:00 +0200 (CEST) From: Manuel Federanko To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime Date: Thu, 23 Apr 2026 15:46:08 +0200 Message-ID: <20260423134607.105229-2-m.federanko@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.223 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy HEADER_FROM_DIFFERENT_DOMAINS 0.25 From and EnvelopeFrom 2nd level mail domains are different KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [acme.rs,proxmox.com,proxmox-daily-update.rs,letsencrypt.org,certificates.rs] Message-ID-Hash: FGGT22ZMHZUQ25SCAKI4DN4BVGKCPZIS X-Message-ID-Hash: FGGT22ZMHZUQ25SCAKI4DN4BVGKCPZIS X-MailFrom: mfederanko@dev.localdomain X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Start renewing a certificate once 2/3 or 1/2 (for short-lived certificates) of its total lifetime have passed, instead of the hardcoded 30 days. This stays consistent with many certificates, which are valid for 90 days and is recommended by letsencrypt [1]. The update service runs daily, impose a 3 day minimum remaining lifetime to still be able to handle transient failures for certificate renewals. [1] https://letsencrypt.org/docs/integration-guide/#when-to-renew Signed-off-by: Manuel Federanko Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372 --- I have a nearly identical patch ready for pdm, if there is no v3 for this patch I will send that too. changed since v2: * move the 3day cutoff to the daily-update service * use half of total lifetime as lead time for short-lived certificates * improved commit message src/api2/node/certificates.rs | 30 ++++++++++++++++++++------ src/bin/proxmox-daily-update.rs | 10 ++++++--- src/bin/proxmox_backup_manager/acme.rs | 6 ++++-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs index a69f6511..7b57d731 100644 --- a/src/api2/node/certificates.rs +++ b/src/api2/node/certificates.rs @@ -305,18 +305,36 @@ pub fn new_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result Result { - if !cert_expires_soon()? && !force { - bail!("Certificate does not expire within the next 30 days and 'force' is not set.") + if !force && !cert_expires_soon(None)? { + let lead = cert_renew_lead_time()? / (24 * 60 * 60); + bail!("Certificate does not expire within the next {lead} days and 'force' is not set.") } spawn_certificate_worker("acme-renew-cert", force, rpcenv) } -/// Check whether the current certificate expires within the next 30 days. -pub fn cert_expires_soon() -> Result { +/// When to start checking for new certs. +pub fn cert_renew_lead_time() -> Result { let cert = pem_to_cert_info(get_certificate_pem()?.as_bytes())?; - cert.is_expired_after_epoch(proxmox_time::epoch_i64() + 30 * 24 * 60 * 60) - .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err)) + if let (Ok(notafter), Ok(notbefore)) = (cert.not_after_unix(), cert.not_before_unix()) { + let lifetime = notafter - notbefore; + // certificates lived for 10 days or shorter should be renewed once half their lifetime is + // over + let scale = if lifetime < (10 * 24 * 60 * 60) { 2 } else { 3 }; + let lead = lifetime / scale; + return Ok(lead); + } + Ok(30 * 24 * 60 * 60) +} + +/// Check whether the current certificate expires within the next seconds, or +/// cert_renew_lead_time() if not given +pub fn cert_expires_soon(seconds: Option) -> Result { + let cert = pem_to_cert_info(get_certificate_pem()?.as_bytes())?; + cert.is_expired_after_epoch( + proxmox_time::epoch_i64() + seconds.unwrap_or(cert_renew_lead_time()?), + ) + .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err)) } fn spawn_certificate_worker( diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs index c4d68e30..dc1af8bf 100644 --- a/src/bin/proxmox-daily-update.rs +++ b/src/bin/proxmox-daily-update.rs @@ -74,14 +74,18 @@ async fn check_acme_certificates(rpcenv: &mut dyn RpcEnvironment) -> Result<(), return Ok(()); } - if !api2::node::certificates::cert_expires_soon()? { - log::info!("Certificate does not expire within the next 30 days, not renewing."); + // force renewal if we expire within 3 days, this gives some chance + // to succeed, since we currently only run daily + let force = api2::node::certificates::cert_expires_soon(Some(3 * 24 * 60 * 60))?; + if !force && !api2::node::certificates::cert_expires_soon(None)? { + let lead = api2::node::certificates::cert_renew_lead_time()? / (24 * 60 * 60); + log::info!("Certificate does not expire within the next {lead} days, not renewing."); return Ok(()); } let info = &api2::node::certificates::API_METHOD_RENEW_ACME_CERT; let result = match info.handler { - ApiHandler::Sync(handler) => (handler)(json!({}), info, rpcenv)?, + ApiHandler::Sync(handler) => (handler)(json!({"force": force}), info, rpcenv)?, _ => unreachable!(), }; wait_for_local_worker(result.as_str().unwrap()).await?; diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs index 57431225..8ba9be05 100644 --- a/src/bin/proxmox_backup_manager/acme.rs +++ b/src/bin/proxmox_backup_manager/acme.rs @@ -413,9 +413,11 @@ pub fn plugin_cli() -> CommandLineInterface { )] /// Order a new ACME certificate. async fn order_acme_cert(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> { - if !param["force"].as_bool().unwrap_or(false) && !api2::node::certificates::cert_expires_soon()? + if !param["force"].as_bool().unwrap_or(false) + && !api2::node::certificates::cert_expires_soon(None)? { - println!("Certificate does not expire within the next 30 days, not renewing."); + let lead = api2::node::certificates::cert_renew_lead_time()? / (24 * 60 * 60); + println!("Certificate does not expire within the next {lead} days, not renewing."); return Ok(()); } -- 2.47.3