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

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

[Manuel Federanko]

Start renewing a certificate once 2/3 of its total lifetime have passed,
instead of the hardcoded 30 days. This stays consistent with many
certificates, which are valid for 90 days.

The update service runs daily, impose a 3 day minimum remaining lifetime
to still be able to handle transient failures for certificate renewals.

Signed-off-by: Manuel Federanko <m.federanko@proxmox.com>
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372
---
 src/api2/node/certificates.rs          | 21 +++++++++++++++++++--
 src/bin/proxmox-daily-update.rs        |  3 ++-
 src/bin/proxmox_backup_manager/acme.rs |  3 ++-
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
index a69f6511..6e7b3326 100644
--- a/src/api2/node/certificates.rs
+++ b/src/api2/node/certificates.rs
@@ -306,16 +306,33 @@ pub fn new_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result<Str
 /// 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.")
+        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)
 }
 
+/// 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())?;
+    if let (Some(notafter), Some(notbefore)) =
+        (cert.not_after_unix().ok(), cert.not_before_unix().ok())
+    {
+        // gets usually checked every day by the daily-update service,
+        // start checking at least 3 days before expiry
+        let lifetime = notafter - notbefore;
+        let lead = std::cmp::max(lifetime / 3, 3 * 24 * 60 * 60);
+        Ok(lead)
+    } else {
+        Ok(30 * 24 * 60 * 60)
+    }
+}
+
 /// Check whether the current certificate expires within the next 30 days.
 pub fn cert_expires_soon() -> Result<bool, 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)
+    cert.is_expired_after_epoch(proxmox_time::epoch_i64() + cert_renew_lead_time()?)
         .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err))
 }
 
diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs
index c4d68e30..e5e96eb9 100644
--- a/src/bin/proxmox-daily-update.rs
+++ b/src/bin/proxmox-daily-update.rs
@@ -75,7 +75,8 @@ async fn check_acme_certificates(rpcenv: &mut dyn RpcEnvironment) -> Result<(),
     }
 
     if !api2::node::certificates::cert_expires_soon()? {
-        log::info!("Certificate does not expire within the next 30 days, not renewing.");
+        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(());
     }
 
diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs
index 57431225..d1a2323f 100644
--- a/src/bin/proxmox_backup_manager/acme.rs
+++ b/src/bin/proxmox_backup_manager/acme.rs
@@ -415,7 +415,8 @@ pub fn plugin_cli() -> CommandLineInterface {
 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()?
     {
-        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] acme: partially fix #6372: scale certificate renewal checks by lifetime

[Christian Ebner]

On 4/21/26 4:45 PM, Manuel Federanko wrote:
> Start renewing a certificate once 2/3 of its total lifetime have passed,
> instead of the hardcoded 30 days. This stays consistent with many
> certificates, which are valid for 90 days.
> 
> The update service runs daily, impose a 3 day minimum remaining lifetime
> to still be able to handle transient failures for certificate renewals.
> 
> Signed-off-by: Manuel Federanko <m.federanko@proxmox.com>
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372
> ---

@Shannon, can you have a look at this to avoid potential conflicts with 
your TLS certificate rotation series [0]?

[0] 
https://lore.proxmox.com/pbs-devel/20260407135714.490747-1-s.sterz@proxmox.com/T/#t

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

[Shannon Sterz]

On Tue Apr 21, 2026 at 4:46 PM CEST, Manuel Federanko wrote:
> Start renewing a certificate once 2/3 of its total lifetime have passed,
> instead of the hardcoded 30 days. This stays consistent with many
> certificates, which are valid for 90 days.
>
> The update service runs daily, impose a 3 day minimum remaining lifetime
> to still be able to handle transient failures for certificate renewals.
>
> Signed-off-by: Manuel Federanko <m.federanko@proxmox.com>
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372
> ---
>  src/api2/node/certificates.rs          | 21 +++++++++++++++++++--
>  src/bin/proxmox-daily-update.rs        |  3 ++-
>  src/bin/proxmox_backup_manager/acme.rs |  3 ++-
>  3 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
> index a69f6511..6e7b3326 100644
> --- a/src/api2/node/certificates.rs
> +++ b/src/api2/node/certificates.rs
> @@ -306,16 +306,33 @@ pub fn new_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result<Str
>  /// 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.")
> +        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)
>  }
>
> +/// 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())?;
> +    if let (Some(notafter), Some(notbefore)) =
> +        (cert.not_after_unix().ok(), cert.not_before_unix().ok())

no need to convert between `Option` and `Result` here. simply do:

    if let (Ok(notafter), Ok(notbefore)) = (cert.not_after_unix(), cert.not_before_unix()) {

here :)

> +    {
> +        // gets usually checked every day by the daily-update service,
> +        // start checking at least 3 days before expiry
> +        let lifetime = notafter - notbefore;
> +        let lead = std::cmp::max(lifetime / 3, 3 * 24 * 60 * 60);

i talked to Fabian a bit and we came to the following consensus
regarding the 3 day cut-off here. this function should probably just
encode the lead time itself, not the 3 day cut off as that is an
artifact of how we refresh acme certificates in the daily-update task.
essentially the plan is to have this function return the lead time as
follows:

1. for short-lived certificates (< 10 days) the lead time should be half
   of the lifetime of the certificate [1].
2. for other certificates the lead time should be 1/3 of the
   certificates lifetime [2].

this is based on recommendations by let's encrypt [1,2]. for
ultra-short-lived certificates, however, the daily update service will
only have very limited chances to successfully renew the certificate,
since it will only run once a day. hence, the 3 day cut-off should be
moved to the daily update service. the service should check the validity
of the certificate with the 3 day cut-off in mind. it should then call
the acme renewal endpoint with the `force` parameter set to `true` to
by-pass the validity check based on the lead time outlined above. does
that make sense?

[1]: https://letsencrypt.org/docs/integration-guide/
[2]: https://letsencrypt.org/2025/12/02/from-90-to-45#action-required

> +        Ok(lead)
> +    } else {
> +        Ok(30 * 24 * 60 * 60)
> +    }

tiny nit: imo it might be a bit cleaner to do `return Ok(lead)` in the
if branch above, drop the else and return `Ok(30 * 24 * 60 * 60)` by
default.

> +}
> +
>  /// Check whether the current certificate expires within the next 30 days.
>  pub fn cert_expires_soon() -> Result<bool, 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)
> +    cert.is_expired_after_epoch(proxmox_time::epoch_i64() + cert_renew_lead_time()?)
>          .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err))
>  }
>
> diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs
> index c4d68e30..e5e96eb9 100644
> --- a/src/bin/proxmox-daily-update.rs
> +++ b/src/bin/proxmox-daily-update.rs
> @@ -75,7 +75,8 @@ async fn check_acme_certificates(rpcenv: &mut dyn RpcEnvironment) -> Result<(),
>      }
>
>      if !api2::node::certificates::cert_expires_soon()? {
> -        log::info!("Certificate does not expire within the next 30 days, not renewing.");
> +        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(());
>      }
>
> diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs
> index 57431225..d1a2323f 100644
> --- a/src/bin/proxmox_backup_manager/acme.rs
> +++ b/src/bin/proxmox_backup_manager/acme.rs
> @@ -415,7 +415,8 @@ pub fn plugin_cli() -> CommandLineInterface {
>  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()?
>      {
> -        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(());
>      }
>

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

[Shannon Sterz]

On Wed Apr 22, 2026 at 8:49 AM CEST, Christian Ebner wrote:
> On 4/21/26 4:45 PM, Manuel Federanko wrote:
>> Start renewing a certificate once 2/3 of its total lifetime have passed,
>> instead of the hardcoded 30 days. This stays consistent with many
>> certificates, which are valid for 90 days.
>>
>> The update service runs daily, impose a 3 day minimum remaining lifetime
>> to still be able to handle transient failures for certificate renewals.
>>
>> Signed-off-by: Manuel Federanko <m.federanko@proxmox.com>
>> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372
>> ---
>
> @Shannon, can you have a look at this to avoid potential conflicts with
> your TLS certificate rotation series [0]?
>
> [0]
> https://lore.proxmox.com/pbs-devel/20260407135714.490747-1-s.sterz@proxmox.com/T/#t

thanks for the heads-up, but from what i can tell not really. this
series addresses acme certificates, whereas mine is only relevant for
the self-signed certificates that pbs & pdm fall back on by default. imo
it would make sense to have these changes expand to include pdm too,
tho.

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

[Manuel Federanko]

On 2026-04-22 11:08 AM, Shannon Sterz wrote:
> On Tue Apr 21, 2026 at 4:46 PM CEST, Manuel Federanko wrote:
>> Start renewing a certificate once 2/3 of its total lifetime have passed,
>> instead of the hardcoded 30 days. This stays consistent with many
>> certificates, which are valid for 90 days.
>>
>> The update service runs daily, impose a 3 day minimum remaining lifetime
>> to still be able to handle transient failures for certificate renewals.
>>
>> Signed-off-by: Manuel Federanko <m.federanko@proxmox.com>
>> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372
>> ---
>>  src/api2/node/certificates.rs          | 21 +++++++++++++++++++--
>>  src/bin/proxmox-daily-update.rs        |  3 ++-
>>  src/bin/proxmox_backup_manager/acme.rs |  3 ++-
>>  3 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
>> index a69f6511..6e7b3326 100644
>> --- a/src/api2/node/certificates.rs
>> +++ b/src/api2/node/certificates.rs
>> @@ -306,16 +306,33 @@ pub fn new_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result<Str
>>  /// 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.")
>> +        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)
>>  }
>>
>> +/// 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())?;
>> +    if let (Some(notafter), Some(notbefore)) =
>> +        (cert.not_after_unix().ok(), cert.not_before_unix().ok())
> 
> no need to convert between `Option` and `Result` here. simply do:
> 
>     if let (Ok(notafter), Ok(notbefore)) = (cert.not_after_unix(), cert.not_before_unix()) {
> 
> here :)

right, that makes sense. Will change in v2

>> +    {
>> +        // gets usually checked every day by the daily-update service,
>> +        // start checking at least 3 days before expiry
>> +        let lifetime = notafter - notbefore;
>> +        let lead = std::cmp::max(lifetime / 3, 3 * 24 * 60 * 60);
> 
> i talked to Fabian a bit and we came to the following consensus
> regarding the 3 day cut-off here. this function should probably just
> encode the lead time itself, not the 3 day cut off as that is an
> artifact of how we refresh acme certificates in the daily-update task.
> essentially the plan is to have this function return the lead time as
> follows:
> 
> 1. for short-lived certificates (< 10 days) the lead time should be half
>    of the lifetime of the certificate [1].
> 2. for other certificates the lead time should be 1/3 of the
>    certificates lifetime [2].
> 
> this is based on recommendations by let's encrypt [1,2]. for
> ultra-short-lived certificates, however, the daily update service will
> only have very limited chances to successfully renew the certificate,
> since it will only run once a day. hence, the 3 day cut-off should be
> moved to the daily update service. the service should check the validity
> of the certificate with the 3 day cut-off in mind. it should then call
> the acme renewal endpoint with the `force` parameter set to `true` to
> by-pass the validity check based on the lead time outlined above. does
> that make sense?
> 
> [1]: https://letsencrypt.org/docs/integration-guide/
> [2]: https://letsencrypt.org/2025/12/02/from-90-to-45#action-required
> 
>> +        Ok(lead)
>> +    } else {
>> +        Ok(30 * 24 * 60 * 60)
>> +    }
> 
> tiny nit: imo it might be a bit cleaner to do `return Ok(lead)` in the
> if branch above, drop the else and return `Ok(30 * 24 * 60 * 60)` by
> default.

ack, will change in v2

>> +}
>> +
>>  /// Check whether the current certificate expires within the next 30 days.
>>  pub fn cert_expires_soon() -> Result<bool, 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)
>> +    cert.is_expired_after_epoch(proxmox_time::epoch_i64() + cert_renew_lead_time()?)
>>          .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err))
>>  }
>>
>> diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs
>> index c4d68e30..e5e96eb9 100644
>> --- a/src/bin/proxmox-daily-update.rs
>> +++ b/src/bin/proxmox-daily-update.rs
>> @@ -75,7 +75,8 @@ async fn check_acme_certificates(rpcenv: &mut dyn RpcEnvironment) -> Result<(),
>>      }
>>
>>      if !api2::node::certificates::cert_expires_soon()? {
>> -        log::info!("Certificate does not expire within the next 30 days, not renewing.");
>> +        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(());
>>      }
>>
>> diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs
>> index 57431225..d1a2323f 100644
>> --- a/src/bin/proxmox_backup_manager/acme.rs
>> +++ b/src/bin/proxmox_backup_manager/acme.rs
>> @@ -415,7 +415,8 @@ pub fn plugin_cli() -> CommandLineInterface {
>>  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()?
>>      {
>> -        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(());
>>      }
>>
> 
> 

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

[Manuel Federanko]

On 2026-04-22 11:08 AM, Shannon Sterz wrote:
> On Wed Apr 22, 2026 at 8:49 AM CEST, Christian Ebner wrote:
>> On 4/21/26 4:45 PM, Manuel Federanko wrote:
>>> Start renewing a certificate once 2/3 of its total lifetime have passed,
>>> instead of the hardcoded 30 days. This stays consistent with many
>>> certificates, which are valid for 90 days.
>>>
>>> The update service runs daily, impose a 3 day minimum remaining lifetime
>>> to still be able to handle transient failures for certificate renewals.
>>>
>>> Signed-off-by: Manuel Federanko <m.federanko@proxmox.com>
>>> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6372
>>> ---
>>
>> @Shannon, can you have a look at this to avoid potential conflicts with
>> your TLS certificate rotation series [0]?
>>
>> [0]
>> https://lore.proxmox.com/pbs-devel/20260407135714.490747-1-s.sterz@proxmox.com/T/#t
> 
> thanks for the heads-up, but from what i can tell not really. this
> series addresses acme certificates, whereas mine is only relevant for
> the self-signed certificates that pbs & pdm fall back on by default. imo
> it would make sense to have these changes expand to include pdm too,
> tho.
Thanks for the input. I will also include this in PDM.
I'm currently working on ARI, so this might change anyways, though I think it is
still valuable until ARI is implemented, also, not all CAs support ARI.

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

[Manuel Federanko]

Superseded-by: https://lore.proxmox.com/pbs-devel/20260423134607.105229-2-m.federanko@proxmox.com/

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

[Thomas Lamprecht]

On Tue, 21 Apr 2026 16:46:45 +0200, Manuel Federanko wrote:
> Start renewing a certificate once 2/3 of its total lifetime have passed,
> instead of the hardcoded 30 days. This stays consistent with many
> certificates, which are valid for 90 days.
> 
> The update service runs daily, impose a 3 day minimum remaining lifetime
> to still be able to handle transient failures for certificate renewals.
> 
> [...]

Applied with a small set of mostly cosmetic follow-ups on top, thanks!

[1/1] acme: partially fix #6372: scale certificate renewal checks by lifetime
      commit: 29bd6b89ca3b3d33e89bd0277daf46b39b210273