From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id BDCD51FF13B for ; Tue, 13 Jan 2026 14:45:42 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E95E318439; Tue, 13 Jan 2026 14:45:45 +0100 (CET) Date: Tue, 13 Jan 2026 14:45:06 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20260108112629.189670-1-s.rufinatscha@proxmox.com> <20260108112629.189670-10-s.rufinatscha@proxmox.com> In-Reply-To: <20260108112629.189670-10-s.rufinatscha@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1768310071.o1dolezxdy.astroid@yuna.none> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1768311864805 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup v5 5/5] acme: certificate ordering through proxmox-acme-api X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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 > --- > 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, > -} > - > -async fn order_certificate( > - worker: Arc, > - node_config: &NodeConfig, > -) -> Result, Error> { > - use proxmox_acme::authorization::Status; > - use proxmox_acme::order::Identifier; > - > - let domains = node_config.acme_domains().try_fold( > - Vec::::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 = 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::::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 > 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 = 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, > - > - /// Flag to disable the config. > - #[serde(skip_serializing_if = "Option::is_none", default)] > - pub disable: Option, > -} > - > -#[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) -> 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 { > 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