[PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime

[PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime

[Manuel Federanko]

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 <m.federanko@proxmox.com>
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<Str
 /// Renew the current ACME certificate if it expires within 30 days (or always if the `force`
 /// parameter is set).
 pub fn renew_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error> {
-    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<bool, Error> {
+/// When to start checking for new certs.
+pub fn cert_renew_lead_time() -> Result<i64, Error> {
     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<i64>) -> Result<bool, Error> {
+    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

Re: [PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime

[Fabian Grünbichler]

seems like v1 of this got applied (with some follow-ups), would you mind
checking if rebasing the diff between v1 and v2 still makes sense?

On April 23, 2026 3:46 pm, Manuel Federanko wrote:
> 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 <m.federanko@proxmox.com>
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372

Re: [PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime

[Thomas Lamprecht]

Am 24.04.26 um 10:35 schrieb Fabian Grünbichler:
> seems like v1 of this got applied (with some follow-ups), would you mind
> checking if rebasing the diff between v1 and v2 still makes sense?
Argh, b4 should have pulled in the newer revision in any case, I'll
recheck what was going on here.

Re: [PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime

[Thomas Lamprecht]

Am 24.04.26 um 12:36 schrieb Thomas Lamprecht:
> Am 24.04.26 um 10:35 schrieb Fabian Grünbichler:
>> seems like v1 of this got applied (with some follow-ups), would you mind
>> checking if rebasing the diff between v1 and v2 still makes sense?
> Argh, b4 should have pulled in the newer revision in any case, I'll
> recheck what was going on here.
> 
Probably just timing, but b4 still isn't able to find v2, might be
due to Manuel not sending out a superseded reply to the old submission
that points to the new one, which then made b4 not recognize the newer
revision.

Re: [PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime

[Fabian Grünbichler]

On April 24, 2026 12:50 pm, Thomas Lamprecht wrote:
> Am 24.04.26 um 12:36 schrieb Thomas Lamprecht:
>> Am 24.04.26 um 10:35 schrieb Fabian Grünbichler:
>>> seems like v1 of this got applied (with some follow-ups), would you mind
>>> checking if rebasing the diff between v1 and v2 still makes sense?
>> Argh, b4 should have pulled in the newer revision in any case, I'll
>> recheck what was going on here.
>> 
> Probably just timing, but b4 still isn't able to find v2, might be
> due to Manuel not sending out a superseded reply to the old submission
> that points to the new one, which then made b4 not recognize the newer
> revision.

b4 doesn't look at superseded, it searches lore for matching newer
versions of the series, and it seems this particular thread is not
handled correctly, since the query ends up looking for:

((s:"acme: partially fix #6372: scale certificate renewal checks by lifetime" AND f:"s.sterz@proxmox.com")) AND d:20260422..

because it thinks the series is authored by Shannon, instead of Manuel
because it picks one of the replies as base message..

if it wouldn't misattribute that, it would also find the v2 thread (and
presumably handle it correctly)

Re: [PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime

[Manuel Federanko]

On 2026-04-24 10:35 AM, Fabian Grünbichler wrote:
> seems like v1 of this got applied (with some follow-ups), would you mind
> checking if rebasing the diff between v1 and v2 still makes sense?

They could be rebased, but seeing as ARI support would change some stuff
anyways I'm not quiet sure if it is worth it. I'd rather incorporate
these improvements into the ARI patch-series I'm working on.

> On April 23, 2026 3:46 pm, Manuel Federanko wrote:
>> 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 <m.federanko@proxmox.com>
>> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372

applied: [PATCH proxmox-backup v2] acme: partially fix #6372: scale certificate renewal checks by lifetime

[Thomas Lamprecht]

On Thu, 23 Apr 2026 15:46:08 +0200, Manuel Federanko wrote:
> 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.
> 
> [...]

Just again for the record: v1 was already applied when v2 landed, to avoid
dropping v2, I pulled its improvements in as follow-ups on top of v1, so
this should be basically (semantically) applied now too.

Follow-ups on top of v1 I pushed:

- 3ee7129c6 acme: certificates: update stale 30-day doc comments
- 034652107 acme: certificates: warn on fallback to 30-day renewal lead time
- cec290d10 acme: certificates: parse certificate once per renewal check
- 3d819b132 acme: certificates: factor out SECONDS_PER_DAY constant

Ported from v2:

- 74bc33071 acme: certificates: use 1/2 lifetime lead for short-lived certs

I kept the 3-day floor inside cert_renew_lead_time rather than moving it to
the daily-update caller as you did - behavior is equivalent for every
lifetime, and it keeps the policy in one place with fewer cert reads per
cycle.

For the PDM port: the same improvements apply there too. One option would be
to add a renewal_lead_time(&self) -> i64 helper on CertificateInfo in
proxmox-acme-api::certificate_helpers, use it from PDM, and then switch PBS
over to it here as well - single source of truth for both products.