Re: [pbs-devel] [PATCH proxmox-backup v5 5/5] acme: certificate ordering through proxmox-acme-api

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On January 8, 2026 12:26 pm, Samuel Rufinatscha wrote:
> PBS currently uses its own ACME client and API logic, while PDM uses the
> factored out proxmox-acme and proxmox-acme-api crates. This duplication
> risks differences in behaviour and requires ACME maintenance in two
> places. This patch is part of a series to move PBS over to the shared
> ACME stack.
> 
> Changes:
> - Replace the custom ACME order/authorization loop in node certificates
> with a call to proxmox_acme_api::order_certificate.
> - Build domain + config data as proxmox-acme-api types
> - Remove obsolete local ACME ordering and plugin glue code.
> 
> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
> ---
>  src/acme/mod.rs               |   2 -
>  src/acme/plugin.rs            | 335 ----------------------------------
>  src/api2/node/certificates.rs | 229 ++++-------------------
>  src/api2/types/acme.rs        |  73 --------
>  src/api2/types/mod.rs         |   3 -
>  src/config/acme/mod.rs        |   8 +-
>  src/config/acme/plugin.rs     |  92 +---------
>  src/config/node.rs            |  20 +-
>  src/lib.rs                    |   2 -
>  9 files changed, 38 insertions(+), 726 deletions(-)
>  delete mode 100644 src/acme/mod.rs
>  delete mode 100644 src/acme/plugin.rs
>  delete mode 100644 src/api2/types/acme.rs
> 

[..]

> diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
> index 47ff8de5..73401c41 100644
> --- a/src/api2/node/certificates.rs
> +++ b/src/api2/node/certificates.rs
> @@ -1,14 +1,11 @@
> -use std::sync::Arc;
> -use std::time::Duration;
> -
>  use anyhow::{bail, format_err, Error};
>  use openssl::pkey::PKey;
>  use openssl::x509::X509;
>  use serde::{Deserialize, Serialize};
> -use tracing::{info, warn};
> +use tracing::info;
>  
>  use pbs_api_types::{NODE_SCHEMA, PRIV_SYS_MODIFY};
> -use proxmox_acme::async_client::AcmeClient;
> +use proxmox_acme_api::AcmeDomain;
>  use proxmox_rest_server::WorkerTask;
>  use proxmox_router::list_subdirs_api_method;
>  use proxmox_router::SubdirMap;
> @@ -18,8 +15,6 @@ use proxmox_schema::api;
>  use pbs_buildcfg::configdir;
>  use pbs_tools::cert;
>  
> -use crate::api2::types::AcmeDomain;
> -use crate::config::node::NodeConfig;
>  use crate::server::send_certificate_renewal_mail;
>  
>  pub const ROUTER: Router = Router::new()
> @@ -268,193 +263,6 @@ pub async fn delete_custom_certificate() -> Result<(), Error> {
>      Ok(())
>  }
>  
> -struct OrderedCertificate {
> -    certificate: hyper::body::Bytes,
> -    private_key_pem: Vec<u8>,
> -}
> -
> -async fn order_certificate(
> -    worker: Arc<WorkerTask>,
> -    node_config: &NodeConfig,
> -) -> Result<Option<OrderedCertificate>, Error> {
> -    use proxmox_acme::authorization::Status;
> -    use proxmox_acme::order::Identifier;
> -
> -    let domains = node_config.acme_domains().try_fold(
> -        Vec::<AcmeDomain>::new(),
> -        |mut acc, domain| -> Result<_, Error> {
> -            let mut domain = domain?;
> -            domain.domain.make_ascii_lowercase();
> -            if let Some(alias) = &mut domain.alias {
> -                alias.make_ascii_lowercase();
> -            }
> -            acc.push(domain);
> -            Ok(acc)
> -        },
> -    )?;
> -
> -    let get_domain_config = |domain: &str| {
> -        domains
> -            .iter()
> -            .find(|d| d.domain == domain)
> -            .ok_or_else(|| format_err!("no config for domain '{}'", domain))
> -    };
> -
> -    if domains.is_empty() {
> -        info!("No domains configured to be ordered from an ACME server.");
> -        return Ok(None);
> -    }
> -
> -    let (plugins, _) = crate::config::acme::plugin::config()?;
> -
> -    let mut acme = node_config.acme_client().await?;
> -
> -    info!("Placing ACME order");
> -    let order = acme
> -        .new_order(domains.iter().map(|d| d.domain.to_ascii_lowercase()))
> -        .await?;
> -    info!("Order URL: {}", order.location);
> -
> -    let identifiers: Vec<String> = order
> -        .data
> -        .identifiers
> -        .iter()
> -        .map(|identifier| match identifier {
> -            Identifier::Dns(domain) => domain.clone(),
> -        })
> -        .collect();
> -
> -    for auth_url in &order.data.authorizations {
> -        info!("Getting authorization details from '{auth_url}'");
> -        let mut auth = acme.get_authorization(auth_url).await?;
> -
> -        let domain = match &mut auth.identifier {
> -            Identifier::Dns(domain) => domain.to_ascii_lowercase(),
> -        };
> -
> -        if auth.status == Status::Valid {
> -            info!("{domain} is already validated!");
> -            continue;
> -        }
> -
> -        info!("The validation for {domain} is pending");
> -        let domain_config: &AcmeDomain = get_domain_config(&domain)?;
> -        let plugin_id = domain_config.plugin.as_deref().unwrap_or("standalone");
> -        let mut plugin_cfg = crate::acme::get_acme_plugin(&plugins, plugin_id)?
> -            .ok_or_else(|| format_err!("plugin '{plugin_id}' for domain '{domain}' not found!"))?;
> -
> -        info!("Setting up validation plugin");
> -        let validation_url = plugin_cfg
> -            .setup(&mut acme, &auth, domain_config, Arc::clone(&worker))
> -            .await?;
> -
> -        let result = request_validation(&mut acme, auth_url, validation_url).await;
> -
> -        if let Err(err) = plugin_cfg
> -            .teardown(&mut acme, &auth, domain_config, Arc::clone(&worker))
> -            .await
> -        {
> -            warn!("Failed to teardown plugin '{plugin_id}' for domain '{domain}' - {err}");
> -        }
> -
> -        result?;
> -    }
> -
> -    info!("All domains validated");
> -    info!("Creating CSR");
> -
> -    let csr = proxmox_acme::util::Csr::generate(&identifiers, &Default::default())?;
> -    let mut finalize_error_cnt = 0u8;
> -    let order_url = &order.location;
> -    let mut order;
> -    loop {
> -        use proxmox_acme::order::Status;
> -
> -        order = acme.get_order(order_url).await?;
> -
> -        match order.status {
> -            Status::Pending => {
> -                info!("still pending, trying to finalize anyway");
> -                let finalize = order
> -                    .finalize
> -                    .as_deref()
> -                    .ok_or_else(|| format_err!("missing 'finalize' URL in order"))?;
> -                if let Err(err) = acme.finalize(finalize, &csr.data).await {
> -                    if finalize_error_cnt >= 5 {
> -                        return Err(err);
> -                    }
> -
> -                    finalize_error_cnt += 1;
> -                }
> -                tokio::time::sleep(Duration::from_secs(5)).await;
> -            }
> -            Status::Ready => {
> -                info!("order is ready, finalizing");
> -                let finalize = order
> -                    .finalize
> -                    .as_deref()
> -                    .ok_or_else(|| format_err!("missing 'finalize' URL in order"))?;
> -                acme.finalize(finalize, &csr.data).await?;
> -                tokio::time::sleep(Duration::from_secs(5)).await;
> -            }
> -            Status::Processing => {
> -                info!("still processing, trying again in 30 seconds");
> -                tokio::time::sleep(Duration::from_secs(30)).await;
> -            }
> -            Status::Valid => {
> -                info!("valid");
> -                break;
> -            }
> -            other => bail!("order status: {:?}", other),
> -        }
> -    }
> -
> -    info!("Downloading certificate");
> -    let certificate = acme
> -        .get_certificate(
> -            order
> -                .certificate
> -                .as_deref()
> -                .ok_or_else(|| format_err!("missing certificate url in finalized order"))?,
> -        )
> -        .await?;
> -
> -    Ok(Some(OrderedCertificate {
> -        certificate,
> -        private_key_pem: csr.private_key_pem,
> -    }))
> -}
> -
> -async fn request_validation(
> -    acme: &mut AcmeClient,
> -    auth_url: &str,
> -    validation_url: &str,
> -) -> Result<(), Error> {
> -    info!("Triggering validation");
> -    acme.request_challenge_validation(validation_url).await?;
> -
> -    info!("Sleeping for 5 seconds");
> -    tokio::time::sleep(Duration::from_secs(5)).await;
> -
> -    loop {
> -        use proxmox_acme::authorization::Status;
> -
> -        let auth = acme.get_authorization(auth_url).await?;
> -        match auth.status {
> -            Status::Pending => {
> -                info!("Status is still 'pending', trying again in 10 seconds");
> -                tokio::time::sleep(Duration::from_secs(10)).await;
> -            }
> -            Status::Valid => return Ok(()),
> -            other => bail!(
> -                "validating challenge '{}' failed - status: {:?}",
> -                validation_url,
> -                other
> -            ),
> -        }
> -    }
> -}
> -
>  #[api(
>      input: {
>          properties: {
> @@ -524,9 +332,30 @@ fn spawn_certificate_worker(
>  
>      let auth_id = rpcenv.get_auth_id().unwrap();
>  
> +    let acme_config = if let Some(cfg) = node_config.acme_config().transpose()? {
> +        cfg
> +    } else {
> +        proxmox_acme_api::parse_acme_config_string("account=default")?
> +    };

wouldn't it make sense to inline this into acme_config() ? the same
fallback is already there for acme_client()

> +
> +    let domains = node_config.acme_domains().try_fold(
> +        Vec::<AcmeDomain>::new(),
> +        |mut acc, domain| -> Result<_, Error> {
> +            let mut domain = domain?;
> +            domain.domain.make_ascii_lowercase();
> +            if let Some(alias) = &mut domain.alias {
> +                alias.make_ascii_lowercase();
> +            }
> +            acc.push(domain);
> +            Ok(acc)
> +        },
> +    )?;
> +
>      WorkerTask::spawn(name, None, auth_id, true, move |worker| async move {
>          let work = || async {
> -            if let Some(cert) = order_certificate(worker, &node_config).await? {
> +            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?;
>              }
> @@ -562,16 +391,20 @@ pub fn revoke_acme_cert(rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error
>  
>      let auth_id = rpcenv.get_auth_id().unwrap();
>  
> +    let acme_config = if let Some(cfg) = node_config.acme_config().transpose()? {
> +        cfg
> +    } else {
> +        proxmox_acme_api::parse_acme_config_string("account=default")?
> +    };

here as well

> +
>      WorkerTask::spawn(
>          "acme-revoke-cert",
>          None,
>          auth_id,
>          true,
>          move |_worker| async move {
> -            info!("Loading ACME account");
> -            let mut acme = node_config.acme_client().await?;
>              info!("Revoking old certificate");
> -            acme.revoke_certificate(cert_pem.as_bytes(), None).await?;
> +            proxmox_acme_api::revoke_certificate(&acme_config, &cert_pem.as_bytes()).await?;
>              info!("Deleting certificate and regenerating a self-signed one");
>              delete_custom_certificate().await?;
>              Ok(())

[..]

> diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
> index 8ce852ec..4b4a216e 100644
> --- a/src/config/acme/plugin.rs
> +++ b/src/config/acme/plugin.rs
> @@ -1,104 +1,16 @@
>  use std::sync::LazyLock;
>  
>  use anyhow::Error;
> -use serde::{Deserialize, Serialize};
>  use serde_json::Value;
>  
> -use pbs_api_types::PROXMOX_SAFE_ID_FORMAT;
> -use proxmox_schema::{api, ApiType, Schema, StringSchema, Updater};
> +use proxmox_acme_api::{DnsPlugin, StandalonePlugin, PLUGIN_ID_SCHEMA};
> +use proxmox_schema::{ApiType, Schema};
>  use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
>  
>  use pbs_config::{open_backup_lockfile, BackupLockGuard};
>  
> -pub const PLUGIN_ID_SCHEMA: Schema = StringSchema::new("ACME Challenge Plugin ID.")
> -    .format(&PROXMOX_SAFE_ID_FORMAT)
> -    .min_length(1)
> -    .max_length(32)
> -    .schema();
> -
>  pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
>  
> -#[api(
> -    properties: {
> -        id: { schema: PLUGIN_ID_SCHEMA },
> -    },
> -)]
> -#[derive(Deserialize, Serialize)]
> -/// Standalone ACME Plugin for the http-1 challenge.
> -pub struct StandalonePlugin {
> -    /// Plugin ID.
> -    id: String,
> -}
> -
> -impl Default for StandalonePlugin {
> -    fn default() -> Self {
> -        Self {
> -            id: "standalone".to_string(),
> -        }
> -    }
> -}
> -
> -#[api(
> -    properties: {
> -        id: { schema: PLUGIN_ID_SCHEMA },
> -        disable: {
> -            optional: true,
> -            default: false,
> -        },
> -        "validation-delay": {
> -            default: 30,
> -            optional: true,
> -            minimum: 0,
> -            maximum: 2 * 24 * 60 * 60,
> -        },
> -    },
> -)]
> -/// DNS ACME Challenge Plugin core data.
> -#[derive(Deserialize, Serialize, Updater)]
> -#[serde(rename_all = "kebab-case")]
> -pub struct DnsPluginCore {
> -    /// Plugin ID.
> -    #[updater(skip)]
> -    pub id: String,
> -
> -    /// DNS API Plugin Id.
> -    pub api: String,
> -
> -    /// Extra delay in seconds to wait before requesting validation.
> -    ///
> -    /// Allows to cope with long TTL of DNS records.
> -    #[serde(skip_serializing_if = "Option::is_none", default)]
> -    pub validation_delay: Option<u32>,
> -
> -    /// Flag to disable the config.
> -    #[serde(skip_serializing_if = "Option::is_none", default)]
> -    pub disable: Option<bool>,
> -}
> -
> -#[api(
> -    properties: {
> -        core: { type: DnsPluginCore },
> -    },
> -)]
> -/// DNS ACME Challenge Plugin.
> -#[derive(Deserialize, Serialize)]
> -#[serde(rename_all = "kebab-case")]
> -pub struct DnsPlugin {
> -    #[serde(flatten)]
> -    pub core: DnsPluginCore,
> -
> -    // We handle this property separately in the API calls.
> -    /// DNS plugin data (base64url encoded without padding).
> -    #[serde(with = "proxmox_serde::string_as_base64url_nopad")]
> -    pub data: String,
> -}
> -
> -impl DnsPlugin {
> -    pub fn decode_data(&self, output: &mut Vec<u8>) -> Result<(), Error> {
> -        Ok(proxmox_base64::url::decode_to_vec(&self.data, output)?)
> -    }
> -}
> -
>  fn init() -> SectionConfig {
>      let mut config = SectionConfig::new(&PLUGIN_ID_SCHEMA);
>  
> diff --git a/src/config/node.rs b/src/config/node.rs
> index e4b66a20..6865b815 100644
> --- a/src/config/node.rs
> +++ b/src/config/node.rs
> @@ -9,14 +9,14 @@ use pbs_api_types::{
>      OPENSSL_CIPHERS_TLS_1_3_SCHEMA,
>  };
>  use proxmox_acme::async_client::AcmeClient;
> -use proxmox_acme_api::AcmeAccountName;
> +use proxmox_acme_api::{AcmeAccountName, AcmeConfig, AcmeDomain, ACME_DOMAIN_PROPERTY_SCHEMA};
>  use proxmox_http::ProxyConfig;
>  use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
>  
>  use pbs_buildcfg::configdir;
>  use pbs_config::{open_backup_lockfile, BackupLockGuard};
>  
> -use crate::api2::types::{AcmeDomain, ACME_DOMAIN_PROPERTY_SCHEMA, HTTP_PROXY_SCHEMA};
> +use crate::api2::types::HTTP_PROXY_SCHEMA;
>  
>  const CONF_FILE: &str = configdir!("/node.cfg");
>  const LOCK_FILE: &str = configdir!("/.node.lck");
> @@ -43,20 +43,6 @@ pub fn save_config(config: &NodeConfig) -> Result<(), Error> {
>      pbs_config::replace_backup_config(CONF_FILE, &raw)
>  }
>  
> -#[api(
> -    properties: {
> -        account: { type: AcmeAccountName },
> -    }
> -)]
> -#[derive(Deserialize, Serialize)]
> -/// The ACME configuration.
> -///
> -/// Currently only contains the name of the account use.
> -pub struct AcmeConfig {
> -    /// Account to use to acquire ACME certificates.
> -    account: AcmeAccountName,
> -}
> -
>  /// All available languages in Proxmox. Taken from proxmox-i18n repository.
>  /// pt_BR, zh_CN, and zh_TW use the same case in the translation files.
>  // TODO: auto-generate from available translations
> @@ -242,7 +228,7 @@ impl NodeConfig {
>  
>      pub async fn acme_client(&self) -> Result<AcmeClient, Error> {
>          let account = if let Some(cfg) = self.acme_config().transpose()? {
> -            cfg.account
> +            AcmeAccountName::from_string(cfg.account)?
>          } else {
>              AcmeAccountName::from_string("default".to_string())? // should really not happen
>          };
> diff --git a/src/lib.rs b/src/lib.rs
> index 8633378c..828f5842 100644
> --- a/src/lib.rs
> +++ b/src/lib.rs
> @@ -27,8 +27,6 @@ pub(crate) mod auth;
>  
>  pub mod tape;
>  
> -pub mod acme;
> -
>  pub mod client_helpers;
>  
>  pub mod traffic_control_cache;
> -- 
> 2.47.3
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel