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 68C801FF13C for ; Thu, 25 Jun 2026 16:14:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 62D1813864; Thu, 25 Jun 2026 16:14:16 +0200 (CEST) From: Manuel Federanko To: pbs-devel@lists.proxmox.com, pdm-devel@lists.proxmox.com Subject: [PATCH proxmox-datacenter-manager 7/7] acme: fix #6372 use ARI for renewal if available. Date: Thu, 25 Jun 2026 16:13:37 +0200 Message-ID: <20260625141337.181684-8-m.federanko@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260625141337.181684-1-m.federanko@proxmox.com> References: <20260625141337.181684-1-m.federanko@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -1.703 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.249 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 RCVD_IN_SBL_CSS 3.335 Received via a relay in Spamhaus SBL-CSS 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 Message-ID-Hash: E62O3I45F6AEWVD3AO5B6ZMOG5PDEAHA X-Message-ID-Hash: E62O3I45F6AEWVD3AO5B6ZMOG5PDEAHA 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: Try to fetch ARI renewal information if it is available and renew based on that. If not fall back to 1/3 of the remaining lifetime for long-lived certificates or 1/2 of it for short-lived ones. This thus also incorporates some of the changes already present in backup server [0]. Since the ARI check needs to talk to the ACME directory the check is move to the worker. [0] https://lore.proxmox.com/pbs-devel/b8e5bd1b-bfbc-4b9e-befa-cd4b0157ed22@proxmox.com/ Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372 Signed-off-by: Manuel Federanko --- cli/admin/src/acme.rs | 7 - server/src/api/nodes/certificates.rs | 129 +++++++++++++++--- ...proxmox-datacenter-manager-daily-update.rs | 5 - 3 files changed, 113 insertions(+), 28 deletions(-) diff --git a/cli/admin/src/acme.rs b/cli/admin/src/acme.rs index e61bb1ef..be6e0018 100644 --- a/cli/admin/src/acme.rs +++ b/cli/admin/src/acme.rs @@ -405,13 +405,6 @@ 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) - && !dc_api::nodes::certificates::cert_expires_soon()? - { - println!("Certificate does not expire within the next 30 days, not renewing."); - return Ok(()); - } - let info = &dc_api::nodes::certificates::API_METHOD_RENEW_ACME_CERT; let result = match info.handler { ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?, diff --git a/server/src/api/nodes/certificates.rs b/server/src/api/nodes/certificates.rs index 0f391499..08838bea 100644 --- a/server/src/api/nodes/certificates.rs +++ b/server/src/api/nodes/certificates.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Error, bail, format_err}; +use anyhow::{Context, Error, format_err}; use openssl::pkey::PKey; use openssl::x509::X509; @@ -8,7 +8,7 @@ use proxmox_router::list_subdirs_api_method; use proxmox_router::{Permission, Router, RpcEnvironment}; use proxmox_schema::api; -use proxmox_acme_api::{AcmeDomain, CertificateInfo}; +use proxmox_acme_api::{AcmeConfig, AcmeDomain, CertificateInfo}; use proxmox_rest_server::WorkerTask; use proxmox_schema::api_types::NODE_SCHEMA; @@ -43,12 +43,14 @@ const ACME_SUBDIRS: SubdirMap = &[( .put(&API_METHOD_RENEW_ACME_CERT), )]; +const SECONDS_PER_DAY: i64 = 24 * 60 * 60; + fn get_certificate_pem() -> Result, Error> { let cert_pem = proxmox_sys::fs::file_get_contents(API_CERT_FN)?; Ok(cert_pem) } -fn get_certificate_info() -> Result { +pub fn get_certificate_info() -> Result { let cert_pem = get_certificate_pem()?; CertificateInfo::from_pem("proxy.pem", &cert_pem) } @@ -233,18 +235,93 @@ 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.") + spawn_certificate_worker("acme-renew-cert", force, rpcenv) +} + +/// Renewal lead time in seconds for the given certificate. +/// +/// Long-lived certs are renewed once 2/3 of their lifetime has elapsed; short-lived ones (under +/// ten days) already at 1/2, following Let's Encrypt's integration guide. A 3-day floor still +/// applies so the daily-update service has a couple of chances to retry transient failures. +fn cert_renew_lead_time(cert: &CertificateInfo) -> i64 { + if let (Some(notafter), Some(notbefore)) = (cert.notafter, cert.notbefore) { + let lifetime = notafter - notbefore; + let scale = if lifetime < 10 * SECONDS_PER_DAY { + 2 + } else { + 3 + }; + std::cmp::max(lifetime / scale, 3 * SECONDS_PER_DAY) + } else { + log::warn!( + "certificate notBefore/notAfter unavailable, falling back to 30-day renewal lead time" + ); + 30 * SECONDS_PER_DAY } +} - spawn_certificate_worker("acme-renew-cert", force, rpcenv) +/// ARI renewal time if available +/// +/// Query the ARI endpoint for a suggested renewal window, draw a uniform random time in this window +/// Return None if ARI does not apply. +async fn cert_renew_lead_time_ari( + acme_config: &AcmeConfig, + cert_info: &CertificateInfo, +) -> Result, Error> { + let now = proxmox_time::epoch_i64(); + if cert_info.is_expired_after_epoch(now)? { + return Ok(Some(0)); + } + let ari_id = match cert_info.ari_id.as_deref() { + Some(x) => x, + None => return Ok(None), + }; + + let window = match proxmox_acme_api::get_renewal_info(acme_config, ari_id).await? { + Some(x) => x, + None => return Ok(None), + }; + if let Some(reason) = window.data.explanation_url { + info!( + "Obtained renewal window, for information on this chosen window please visit {reason}" + ); + } + let window_start = proxmox_time::parse_rfc3339(&window.data.suggested_window.start)?; + let window_end = proxmox_time::parse_rfc3339(&window.data.suggested_window.end)?; + let rand = proxmox_sys::linux::random_data(8)? + .into_iter() + .enumerate() + .fold(0, |acc, (index, x)| acc + ((x as u64) << (index * 8))) as f64 + / (u64::MAX as f64); + let renew = window_start + (((window_end - window_start) as f64) * rand) as i64; + // need max since the randomness could result in negative values + Ok(Some(std::cmp::max(0, renew - now))) } -/// Check whether the current certificate expires within the next 30 days. -pub fn cert_expires_soon() -> Result { - let cert = get_certificate_info()?; - 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)) +/// Should the certificate be renewed now. +/// +/// Is true if the ceriticates expires within its lead time. +/// Returns if the certificate should be renewed and the lead time in days. +pub async fn check_renewal_needed( + acme_config: &AcmeConfig, + cert_info: &CertificateInfo, +) -> Result { + let lead_ari = cert_renew_lead_time_ari(acme_config, cert_info).await?; + if let Some(lead_ari) = lead_ari { + // rfc9773 section 4.2 tells us to renew if the chosen renewal time would be before the next check + let expires_soon = lead_ari < SECONDS_PER_DAY; + let ts = proxmox_time::TimeSpan::from(std::time::Duration::new(lead_ari as u64, 0)); + info!("Certificate is scheduled for renewal in {ts:.0} by ARI"); + Ok(expires_soon) + } else { + let lead = cert_renew_lead_time(cert_info); + let expires_soon = cert_info + .is_expired_after_epoch(proxmox_time::epoch_i64() + lead) + .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err))?; + let ts = proxmox_time::TimeSpan::from(std::time::Duration::new(lead as u64, 0)); + info!("Certificate renewal lead time is {ts:.0}"); + Ok(expires_soon) + } } fn spawn_certificate_worker( @@ -281,11 +358,31 @@ fn spawn_certificate_worker( WorkerTask::spawn(name, None, auth_id, true, move |worker| async move { let work = || async { - if let Some(cert) = - proxmox_acme_api::order_certificate(worker, &acme_config, &domains).await? - { - crate::auth::certs::set_api_certificate(&cert.certificate, &cert.private_key_pem)?; - crate::reload_api_certificate().await?; + let cert_info = get_certificate_info()?; + let expires_soon = if !force { + let expires_soon = check_renewal_needed(&acme_config, &cert_info).await?; + if !expires_soon { + info!("Certificate does not expire soon and 'force' was not set, not renewing"); + } + expires_soon + } else { + false + }; + if force || expires_soon { + if let Some(cert) = proxmox_acme_api::order_certificate( + worker, + &acme_config, + &domains, + cert_info.ari_id.as_deref(), + ) + .await? + { + crate::auth::certs::set_api_certificate( + &cert.certificate, + &cert.private_key_pem, + )?; + crate::reload_api_certificate().await?; + } } Ok(()) diff --git a/server/src/bin/proxmox-datacenter-manager-daily-update.rs b/server/src/bin/proxmox-datacenter-manager-daily-update.rs index 314b3399..e70033a4 100644 --- a/server/src/bin/proxmox-datacenter-manager-daily-update.rs +++ b/server/src/bin/proxmox-datacenter-manager-daily-update.rs @@ -72,11 +72,6 @@ async fn check_acme_certificates(rpcenv: &mut dyn RpcEnvironment) -> Result<(), return Ok(()); } - if !api::nodes::certificates::cert_expires_soon()? { - log::info!("Certificate does not expire within the next 30 days, not renewing."); - return Ok(()); - } - let info = &api::nodes::certificates::API_METHOD_RENEW_ACME_CERT; let result = match info.handler { ApiHandler::Sync(handler) => (handler)(json!({}), info, rpcenv)?, -- 2.47.3