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

[Manuel Federanko <m.federanko@proxmox.com>]

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