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 F083F1FF13C for ; Thu, 25 Jun 2026 16:13:58 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E0640132BB; Thu, 25 Jun 2026 16:13:46 +0200 (CEST) From: Manuel Federanko To: pbs-devel@lists.proxmox.com, pdm-devel@lists.proxmox.com Subject: [PATCH proxmox-backup 6/7] acme: fix #6372 implement ARI renewal information fetching. Date: Thu, 25 Jun 2026 16:13:36 +0200 Message-ID: <20260625141337.181684-7-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.738 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: UC7PYZ6YTSM64YZD6PA6VN4PZGKV62KF X-Message-ID-Hash: UC7PYZ6YTSM64YZD6PA6VN4PZGKV62KF 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 and renew based on that, if it is not available fall back to normal lifetime based renewal logic. The ARI check needs to talk to the server, move all checks into the worker process and unconditionally call the worker process from all entry points. Add a method that returns the ARI ID from the CertInfo struct, which is easier than trying to pass along other information or switching to proxmox-acme-api's CertificateInfo struct. Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372 Signed-off-by: Manuel Federanko --- src/api2/node/certificates.rs | 106 +++++++++++++++++++------ src/bin/proxmox-daily-update.rs | 6 -- src/bin/proxmox_backup_manager/acme.rs | 8 -- 3 files changed, 81 insertions(+), 39 deletions(-) diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs index 3df05b020..a6a4b67cc 100644 --- a/src/api2/node/certificates.rs +++ b/src/api2/node/certificates.rs @@ -1,11 +1,11 @@ -use anyhow::{Error, bail, format_err}; +use anyhow::{Error, format_err}; use openssl::pkey::PKey; use openssl::x509::X509; use serde::{Deserialize, Serialize}; use tracing::info; use pbs_api_types::{NODE_SCHEMA, PRIV_SYS_MODIFY}; -use proxmox_acme_api::AcmeDomain; +use proxmox_acme_api::{AcmeConfig, AcmeDomain}; use proxmox_rest_server::WorkerTask; use proxmox_router::SubdirMap; use proxmox_router::list_subdirs_api_method; @@ -307,13 +307,6 @@ pub fn new_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result Result { - let (expires_soon, lead_days) = check_renewal_needed()?; - if !expires_soon && !force { - bail!( - "Certificate does not expire within the next {lead_days} days and 'force' is not set." - ) - } - spawn_certificate_worker("acme-renew-cert", force, rpcenv) } @@ -341,17 +334,67 @@ fn cert_renew_lead_time(cert: &cert::CertInfo) -> i64 { } } +/// 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: &cert::CertInfo, +) -> 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() { + 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 its renewal lead time. /// /// Returns `(expires_soon, lead_time_in_days)`; the lead time is returned so callers can produce /// consistent user-facing messages without re-reading and re-parsing the certificate. -pub fn check_renewal_needed() -> Result<(bool, i64), Error> { - let cert = pem_to_cert_info(get_certificate_pem()?.as_bytes())?; - let lead = cert_renew_lead_time(&cert); - let expires_soon = cert - .is_expired_after_epoch(proxmox_time::epoch_i64() + lead) - .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err))?; - Ok((expires_soon, lead / SECONDS_PER_DAY)) +async fn check_renewal_needed( + acme_config: &AcmeConfig, + cert_info: &cert::CertInfo, +) -> 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( @@ -359,10 +402,6 @@ fn spawn_certificate_worker( force: bool, rpcenv: &mut dyn RpcEnvironment, ) -> Result { - // We only have 1 certificate path in PBS which makes figuring out whether or not it is a - // custom one too hard... We keep the parameter because the widget-toolkit may be using it... - let _ = force; - let (node_config, _digest) = pbs_config::node::config()?; let auth_id = rpcenv.get_auth_id().unwrap(); @@ -384,11 +423,28 @@ 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::config::set_proxy_certificate(&cert.certificate, &cert.private_key_pem)?; - crate::server::reload_proxy_certificate().await?; + let cert_info = pem_to_cert_info(get_certificate_pem()?.as_bytes())?; + 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::config::set_proxy_certificate(&cert.certificate, &cert.private_key_pem)?; + crate::server::reload_proxy_certificate().await?; + } } Ok(()) diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs index 42ce62d16..eeadf9d13 100644 --- a/src/bin/proxmox-daily-update.rs +++ b/src/bin/proxmox-daily-update.rs @@ -74,12 +74,6 @@ async fn check_acme_certificates(rpcenv: &mut dyn RpcEnvironment) -> Result<(), return Ok(()); } - let (expires_soon, lead_days) = api2::node::certificates::check_renewal_needed()?; - if !expires_soon { - log::info!("Certificate does not expire within the next {lead_days} 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)?, diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs index ed9e5868c..ea14cfe2b 100644 --- a/src/bin/proxmox_backup_manager/acme.rs +++ b/src/bin/proxmox_backup_manager/acme.rs @@ -413,14 +413,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) { - let (expires_soon, lead_days) = api2::node::certificates::check_renewal_needed()?; - if !expires_soon { - println!("Certificate does not expire within the next {lead_days} days, not renewing."); - return Ok(()); - } - } - let info = &api2::node::certificates::API_METHOD_RENEW_ACME_CERT; let result = match info.handler { ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?, -- 2.47.3