[PATCH proxmox 0/3] fix #7166: acme: stop and time out stalled certificate orders

[PATCH proxmox 0/3] fix #7166: acme: stop and time out stalled certificate orders

[Samuel Rufinatscha]

Certificate orders can get stuck waiting for the CA and clicking on
stop does not end the task [0]. With HTTP-01, the task also keeps
port 80 busy, so another order fails with "Address already in use".

This series makes validation and finalization abortable and gives each
domain's validation and order finalization five minutes to finish. The
validation timeout starts after plugin setup to preserve configured DNS
propagation waits. The plugin is cleaned up on cancellation or timeout.

Testing:

Reproduced the issue on latest PDM with Pebble v2.10.1 by delaying
HTTP-01 validation and pausing the CA during finalization. Tasks ignored
Stop and kept running. With the patches applied, tasks stopped promptly
on request or timed out after five minutes, releasing port 80.
Certificate ordering and renewal still worked.

Maintainer notes:

* both PBS and PDM require the updated dependency

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=7166

proxmox:

Samuel Rufinatscha (3):
  acme-api: extract certificate order finalization helper
  fix #7166: acme-api: allow stopping certificate orders
  fix #7166: acme-api: time out validation and finalization

 proxmox-acme-api/src/certificate_helpers.rs | 39 ++++++++++++++++-----
 1 file changed, 31 insertions(+), 8 deletions(-)


Summary over all repositories:
  1 files changed, 31 insertions(+), 8 deletions(-)

-- 
Generated by git-murpp 0.8.1

[PATCH proxmox 1/3] acme-api: extract certificate order finalization helper

[Samuel Rufinatscha]

Move order finalization and certificate download into a private helper.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 proxmox-acme-api/src/certificate_helpers.rs | 23 ++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/proxmox-acme-api/src/certificate_helpers.rs b/proxmox-acme-api/src/certificate_helpers.rs
index 3921b18e..1db41564 100644
--- a/proxmox-acme-api/src/certificate_helpers.rs
+++ b/proxmox-acme-api/src/certificate_helpers.rs
@@ -119,8 +119,20 @@ pub async fn order_certificate(
     info!("Creating CSR");
 
     let csr = proxmox_acme::util::Csr::generate(&identifiers, &Default::default())?;
+    let certificate = finalize_order(&mut acme, &order.location, &csr.data).await?;
+
+    Ok(Some(OrderedCertificate {
+        certificate,
+        private_key_pem: csr.private_key_pem,
+    }))
+}
+
+async fn finalize_order(
+    acme: &mut AcmeClient,
+    order_url: &str,
+    csr: &[u8],
+) -> Result<Vec<u8>, Error> {
     let mut finalize_error_cnt = 0u8;
-    let order_url = &order.location;
     let mut order;
     loop {
         use proxmox_acme::order::Status;
@@ -134,7 +146,7 @@ pub async fn order_certificate(
                     .finalize
                     .as_deref()
                     .ok_or_else(|| format_err!("missing 'finalize' URL in order"))?;
-                if let Err(err) = acme.finalize(finalize, &csr.data).await {
+                if let Err(err) = acme.finalize(finalize, csr).await {
                     if finalize_error_cnt >= 5 {
                         return Err(err);
                     }
@@ -149,7 +161,7 @@ pub async fn order_certificate(
                     .finalize
                     .as_deref()
                     .ok_or_else(|| format_err!("missing 'finalize' URL in order"))?;
-                acme.finalize(finalize, &csr.data).await?;
+                acme.finalize(finalize, csr).await?;
                 tokio::time::sleep(Duration::from_secs(5)).await;
             }
             Status::Processing => {
@@ -174,10 +186,7 @@ pub async fn order_certificate(
         )
         .await?;
 
-    Ok(Some(OrderedCertificate {
-        certificate: certificate.to_vec(),
-        private_key_pem: csr.private_key_pem,
-    }))
+    Ok(certificate.to_vec())
 }
 
 async fn request_validation(
-- 
2.47.3

[PATCH proxmox 2/3] fix #7166: acme-api: allow stopping certificate orders

[Samuel Rufinatscha]

Handle abort requests during validation and finalization and clean up
the challenge plugin when validation is stopped.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 proxmox-acme-api/src/certificate_helpers.rs | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/proxmox-acme-api/src/certificate_helpers.rs b/proxmox-acme-api/src/certificate_helpers.rs
index 1db41564..771111fb 100644
--- a/proxmox-acme-api/src/certificate_helpers.rs
+++ b/proxmox-acme-api/src/certificate_helpers.rs
@@ -100,7 +100,11 @@ pub async fn order_certificate(
             .setup(&mut acme, &auth, domain_config, Arc::clone(&worker))
             .await?;
 
-        let result = request_validation(&mut acme, auth_url, validation_url).await;
+        let result = tokio::select! {
+            biased;
+            _ = worker.abort_future() => Err(format_err!("abort requested - aborting task")),
+            result = request_validation(&mut acme, auth_url, validation_url) => result,
+        };
 
         if let Err(err) = plugin_cfg
             .teardown(&mut acme, &auth, domain_config, Arc::clone(&worker))
@@ -119,7 +123,11 @@ pub async fn order_certificate(
     info!("Creating CSR");
 
     let csr = proxmox_acme::util::Csr::generate(&identifiers, &Default::default())?;
-    let certificate = finalize_order(&mut acme, &order.location, &csr.data).await?;
+    let certificate = tokio::select! {
+        biased;
+        _ = worker.abort_future() => bail!("abort requested - aborting task"),
+        result = finalize_order(&mut acme, &order.location, &csr.data) => result?,
+    };
 
     Ok(Some(OrderedCertificate {
         certificate,
-- 
2.47.3

[PATCH proxmox 3/3] fix #7166: acme-api: time out validation and finalization

[Samuel Rufinatscha]

Limit each domain's validation and order finalization to five minutes.
Start the validation timeout after plugin setup so DNS propagation waits
are unaffected, and clean up the plugin on timeout.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 proxmox-acme-api/src/certificate_helpers.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/proxmox-acme-api/src/certificate_helpers.rs b/proxmox-acme-api/src/certificate_helpers.rs
index 771111fb..323f4b4a 100644
--- a/proxmox-acme-api/src/certificate_helpers.rs
+++ b/proxmox-acme-api/src/certificate_helpers.rs
@@ -16,6 +16,8 @@ use proxmox_rest_server::WorkerTask;
 use crate::CertificateInfo;
 use crate::types::{AcmeConfig, AcmeDomain};
 
+const ACME_POLL_TIMEOUT: Duration = Duration::from_secs(5 * 60);
+
 pub async fn revoke_certificate(acme_config: &AcmeConfig, certificate: &[u8]) -> Result<(), Error> {
     let mut acme = super::account_config::load_account_config(&acme_config.account)
         .await?
@@ -103,6 +105,9 @@ pub async fn order_certificate(
         let result = tokio::select! {
             biased;
             _ = worker.abort_future() => Err(format_err!("abort requested - aborting task")),
+            _ = tokio::time::sleep(ACME_POLL_TIMEOUT) => {
+                Err(format_err!("ACME validation for '{domain}' timed out"))
+            }
             result = request_validation(&mut acme, auth_url, validation_url) => result,
         };
 
@@ -126,6 +131,7 @@ pub async fn order_certificate(
     let certificate = tokio::select! {
         biased;
         _ = worker.abort_future() => bail!("abort requested - aborting task"),
+        _ = tokio::time::sleep(ACME_POLL_TIMEOUT) => bail!("ACME order finalization timed out"),
         result = finalize_order(&mut acme, &order.location, &csr.data) => result?,
     };
 
-- 
2.47.3