* [pbs-devel] [PATCH proxmox-backup v4 1/4] acme: include proxmox-acme-api dependency
2025-12-03 10:22 [pbs-devel] [PATCH proxmox{-backup, } v4 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
@ 2025-12-03 10:22 ` Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox-backup v4 2/4] acme: drop local AcmeClient Samuel Rufinatscha
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Samuel Rufinatscha @ 2025-12-03 10:22 UTC (permalink / raw)
To: pbs-devel
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:
- Add proxmox-acme-api with the "impl" feature as a dependency.
- Initialize proxmox_acme_api in proxmox-backup- api, manager and proxy.
* Inits PBS config dir /acme as proxmox ACME directory
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
Cargo.toml | 3 +++
src/bin/proxmox-backup-api.rs | 2 ++
src/bin/proxmox-backup-manager.rs | 2 ++
src/bin/proxmox-backup-proxy.rs | 1 +
4 files changed, 8 insertions(+)
diff --git a/Cargo.toml b/Cargo.toml
index ff143932..bdaf7d85 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -101,6 +101,7 @@ pbs-api-types = "1.0.8"
# other proxmox crates
pathpatterns = "1"
proxmox-acme = "1"
+proxmox-acme-api = { version = "1", features = [ "impl" ] }
pxar = "1"
# PBS workspace
@@ -251,6 +252,7 @@ pbs-api-types.workspace = true
# in their respective repo
proxmox-acme.workspace = true
+proxmox-acme-api.workspace = true
pxar.workspace = true
# proxmox-backup workspace/internal crates
@@ -269,6 +271,7 @@ proxmox-rrd-api-types.workspace = true
[patch.crates-io]
#pbs-api-types = { path = "../proxmox/pbs-api-types" }
#proxmox-acme = { path = "../proxmox/proxmox-acme" }
+#proxmox-acme-api = { path = "../proxmox/proxmox-acme-api" }
#proxmox-api-macro = { path = "../proxmox/proxmox-api-macro" }
#proxmox-apt = { path = "../proxmox/proxmox-apt" }
#proxmox-apt-api-types = { path = "../proxmox/proxmox-apt-api-types" }
diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index 417e9e97..48f10092 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -8,6 +8,7 @@ use hyper_util::server::graceful::GracefulShutdown;
use tokio::net::TcpListener;
use tracing::level_filters::LevelFilter;
+use pbs_buildcfg::configdir;
use proxmox_http::Body;
use proxmox_lang::try_block;
use proxmox_rest_server::{ApiConfig, RestServer};
@@ -78,6 +79,7 @@ async fn run() -> Result<(), Error> {
let mut command_sock = proxmox_daemon::command_socket::CommandSocket::new(backup_user.gid);
proxmox_product_config::init(backup_user.clone(), pbs_config::priv_user()?);
+ proxmox_acme_api::init(configdir!("/acme"), true)?;
let dir_opts = CreateOptions::new()
.owner(backup_user.uid)
diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index d9f41353..0facb76c 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -18,6 +18,7 @@ use pbs_api_types::{
VERIFICATION_OUTDATED_AFTER_SCHEMA, VERIFY_JOB_READ_THREADS_SCHEMA,
VERIFY_JOB_VERIFY_THREADS_SCHEMA,
};
+use pbs_buildcfg::configdir;
use pbs_client::{display_task_log, view_task_result};
use pbs_config::sync;
use pbs_tools::json::required_string_param;
@@ -669,6 +670,7 @@ async fn run() -> Result<(), Error> {
.init()?;
proxmox_backup::server::notifications::init()?;
proxmox_product_config::init(pbs_config::backup_user()?, pbs_config::priv_user()?);
+ proxmox_acme_api::init(configdir!("/acme"), false)?;
let cmd_def = CliCommandMap::new()
.insert("acl", acl_commands())
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 92a8cb3c..0bab18ec 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -190,6 +190,7 @@ async fn run() -> Result<(), Error> {
proxmox_backup::server::notifications::init()?;
metric_collection::init()?;
proxmox_product_config::init(pbs_config::backup_user()?, pbs_config::priv_user()?);
+ proxmox_acme_api::init(configdir!("/acme"), false)?;
let mut indexpath = PathBuf::from(pbs_buildcfg::JS_DIR);
indexpath.push("index.hbs");
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH proxmox-backup v4 2/4] acme: drop local AcmeClient
2025-12-03 10:22 [pbs-devel] [PATCH proxmox{-backup, } v4 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox-backup v4 1/4] acme: include proxmox-acme-api dependency Samuel Rufinatscha
@ 2025-12-03 10:22 ` Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox-backup v4 3/4] acme: change API impls to use proxmox-acme-api handlers Samuel Rufinatscha
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Samuel Rufinatscha @ 2025-12-03 10:22 UTC (permalink / raw)
To: pbs-devel
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:
- Remove the local src/acme/client.rs and switch to
proxmox_acme::async_client::AcmeClient where needed.
- Use proxmox_acme_api::load_client_with_account to the custom
AcmeClient::load() function
- Replace the local do_register() logic with
proxmox_acme_api::register_account, to further ensure accounts are persisted
- Replace the local AcmeAccountName type, required for
proxmox_acme_api::register_account
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
src/acme/client.rs | 691 -------------------------
src/acme/mod.rs | 3 -
src/acme/plugin.rs | 2 +-
src/api2/config/acme.rs | 50 +-
src/api2/node/certificates.rs | 2 +-
src/api2/types/acme.rs | 8 -
src/bin/proxmox_backup_manager/acme.rs | 17 +-
src/config/acme/mod.rs | 8 +-
src/config/node.rs | 9 +-
9 files changed, 36 insertions(+), 754 deletions(-)
delete mode 100644 src/acme/client.rs
diff --git a/src/acme/client.rs b/src/acme/client.rs
deleted file mode 100644
index 9fb6ad55..00000000
--- a/src/acme/client.rs
+++ /dev/null
@@ -1,691 +0,0 @@
-//! HTTP Client for the ACME protocol.
-
-use std::fs::OpenOptions;
-use std::io;
-use std::os::unix::fs::OpenOptionsExt;
-
-use anyhow::{bail, format_err};
-use bytes::Bytes;
-use http_body_util::BodyExt;
-use hyper::Request;
-use nix::sys::stat::Mode;
-use proxmox_http::Body;
-use serde::{Deserialize, Serialize};
-
-use proxmox_acme::account::AccountCreator;
-use proxmox_acme::order::{Order, OrderData};
-use proxmox_acme::types::AccountData as AcmeAccountData;
-use proxmox_acme::Request as AcmeRequest;
-use proxmox_acme::{Account, Authorization, Challenge, Directory, Error, ErrorResponse};
-use proxmox_http::client::Client;
-use proxmox_sys::fs::{replace_file, CreateOptions};
-
-use crate::api2::types::AcmeAccountName;
-use crate::config::acme::account_path;
-use crate::tools::pbs_simple_http;
-
-/// Our on-disk format inherited from PVE's proxmox-acme code.
-#[derive(Deserialize, Serialize)]
-#[serde(rename_all = "camelCase")]
-pub struct AccountData {
- /// The account's location URL.
- location: String,
-
- /// The account data.
- account: AcmeAccountData,
-
- /// The private key as PEM formatted string.
- key: String,
-
- /// ToS URL the user agreed to.
- #[serde(skip_serializing_if = "Option::is_none")]
- tos: Option<String>,
-
- #[serde(skip_serializing_if = "is_false", default)]
- debug: bool,
-
- /// The directory's URL.
- directory_url: String,
-}
-
-#[inline]
-fn is_false(b: &bool) -> bool {
- !*b
-}
-
-pub struct AcmeClient {
- directory_url: String,
- debug: bool,
- account_path: Option<String>,
- tos: Option<String>,
- account: Option<Account>,
- directory: Option<Directory>,
- nonce: Option<String>,
- http_client: Client,
-}
-
-impl AcmeClient {
- /// Create a new ACME client for a given ACME directory URL.
- pub fn new(directory_url: String) -> Self {
- Self {
- directory_url,
- debug: false,
- account_path: None,
- tos: None,
- account: None,
- directory: None,
- nonce: None,
- http_client: pbs_simple_http(None),
- }
- }
-
- /// Load an existing ACME account by name.
- pub async fn load(account_name: &AcmeAccountName) -> Result<Self, anyhow::Error> {
- let account_path = account_path(account_name.as_ref());
- let data = match tokio::fs::read(&account_path).await {
- Ok(data) => data,
- Err(err) if err.kind() == io::ErrorKind::NotFound => {
- bail!("acme account '{}' does not exist", account_name)
- }
- Err(err) => bail!(
- "failed to load acme account from '{}' - {}",
- account_path,
- err
- ),
- };
- let data: AccountData = serde_json::from_slice(&data).map_err(|err| {
- format_err!(
- "failed to parse acme account from '{}' - {}",
- account_path,
- err
- )
- })?;
-
- let account = Account::from_parts(data.location, data.key, data.account);
-
- let mut me = Self::new(data.directory_url);
- me.debug = data.debug;
- me.account_path = Some(account_path);
- me.tos = data.tos;
- me.account = Some(account);
-
- Ok(me)
- }
-
- pub async fn new_account<'a>(
- &'a mut self,
- account_name: &AcmeAccountName,
- tos_agreed: bool,
- contact: Vec<String>,
- rsa_bits: Option<u32>,
- eab_creds: Option<(String, String)>,
- ) -> Result<&'a Account, anyhow::Error> {
- self.tos = if tos_agreed {
- self.terms_of_service_url().await?.map(str::to_owned)
- } else {
- None
- };
-
- let mut account = Account::creator()
- .set_contacts(contact)
- .agree_to_tos(tos_agreed);
-
- if let Some((eab_kid, eab_hmac_key)) = eab_creds {
- account = account.set_eab_credentials(eab_kid, eab_hmac_key)?;
- }
-
- let account = if let Some(bits) = rsa_bits {
- account.generate_rsa_key(bits)?
- } else {
- account.generate_ec_key()?
- };
-
- let _ = self.register_account(account).await?;
-
- crate::config::acme::make_acme_account_dir()?;
- let account_path = account_path(account_name.as_ref());
- let file = OpenOptions::new()
- .write(true)
- .create_new(true)
- .mode(0o600)
- .open(&account_path)
- .map_err(|err| format_err!("failed to open {:?} for writing: {}", account_path, err))?;
- self.write_to(file).map_err(|err| {
- format_err!(
- "failed to write acme account to {:?}: {}",
- account_path,
- err
- )
- })?;
- self.account_path = Some(account_path);
-
- // unwrap: Setting `self.account` is literally this function's job, we just can't keep
- // the borrow from from `self.register_account()` active due to clashes.
- Ok(self.account.as_ref().unwrap())
- }
-
- fn save(&self) -> Result<(), anyhow::Error> {
- let mut data = Vec::<u8>::new();
- self.write_to(&mut data)?;
- let account_path = self.account_path.as_ref().ok_or_else(|| {
- format_err!("no account path set, cannot save updated account information")
- })?;
- crate::config::acme::make_acme_account_dir()?;
- replace_file(
- account_path,
- &data,
- CreateOptions::new()
- .perm(Mode::from_bits_truncate(0o600))
- .owner(nix::unistd::ROOT)
- .group(nix::unistd::Gid::from_raw(0)),
- true,
- )
- }
-
- /// Shortcut to `account().ok_or_else(...).key_authorization()`.
- pub fn key_authorization(&self, token: &str) -> Result<String, anyhow::Error> {
- Ok(Self::need_account(&self.account)?.key_authorization(token)?)
- }
-
- /// Shortcut to `account().ok_or_else(...).dns_01_txt_value()`.
- /// the key authorization value.
- pub fn dns_01_txt_value(&self, token: &str) -> Result<String, anyhow::Error> {
- Ok(Self::need_account(&self.account)?.dns_01_txt_value(token)?)
- }
-
- async fn register_account(
- &mut self,
- account: AccountCreator,
- ) -> Result<&Account, anyhow::Error> {
- let mut retry = retry();
- let mut response = loop {
- retry.tick()?;
-
- let (directory, nonce) = Self::get_dir_nonce(
- &mut self.http_client,
- &self.directory_url,
- &mut self.directory,
- &mut self.nonce,
- )
- .await?;
- let request = account.request(directory, nonce)?;
- match self.run_request(request).await {
- Ok(response) => break response,
- Err(err) if err.is_bad_nonce() => continue,
- Err(err) => return Err(err.into()),
- }
- };
-
- let account = account.response(response.location_required()?, &response.body)?;
-
- self.account = Some(account);
- Ok(self.account.as_ref().unwrap())
- }
-
- pub async fn update_account<T: Serialize>(
- &mut self,
- data: &T,
- ) -> Result<&Account, anyhow::Error> {
- let account = Self::need_account(&self.account)?;
-
- let mut retry = retry();
- let response = loop {
- retry.tick()?;
-
- let (_directory, nonce) = Self::get_dir_nonce(
- &mut self.http_client,
- &self.directory_url,
- &mut self.directory,
- &mut self.nonce,
- )
- .await?;
-
- let request = account.post_request(&account.location, nonce, data)?;
- match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
- Ok(response) => break response,
- Err(err) if err.is_bad_nonce() => continue,
- Err(err) => return Err(err.into()),
- }
- };
-
- // unwrap: we've been keeping an immutable reference to it from the top of the method
- let _ = account;
- self.account.as_mut().unwrap().data = response.json()?;
- self.save()?;
- Ok(self.account.as_ref().unwrap())
- }
-
- pub async fn new_order<I>(&mut self, domains: I) -> Result<Order, anyhow::Error>
- where
- I: IntoIterator<Item = String>,
- {
- let account = Self::need_account(&self.account)?;
-
- let order = domains
- .into_iter()
- .fold(OrderData::new(), |order, domain| order.domain(domain));
-
- let mut retry = retry();
- loop {
- retry.tick()?;
-
- let (directory, nonce) = Self::get_dir_nonce(
- &mut self.http_client,
- &self.directory_url,
- &mut self.directory,
- &mut self.nonce,
- )
- .await?;
-
- let mut new_order = account.new_order(&order, directory, nonce)?;
- let mut response = match Self::execute(
- &mut self.http_client,
- new_order.request.take().unwrap(),
- &mut self.nonce,
- )
- .await
- {
- Ok(response) => response,
- Err(err) if err.is_bad_nonce() => continue,
- Err(err) => return Err(err.into()),
- };
-
- return Ok(
- new_order.response(response.location_required()?, response.bytes().as_ref())?
- );
- }
- }
-
- /// Low level "POST-as-GET" request.
- async fn post_as_get(&mut self, url: &str) -> Result<AcmeResponse, anyhow::Error> {
- let account = Self::need_account(&self.account)?;
-
- let mut retry = retry();
- loop {
- retry.tick()?;
-
- let (_directory, nonce) = Self::get_dir_nonce(
- &mut self.http_client,
- &self.directory_url,
- &mut self.directory,
- &mut self.nonce,
- )
- .await?;
-
- let request = account.get_request(url, nonce)?;
- match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
- Ok(response) => return Ok(response),
- Err(err) if err.is_bad_nonce() => continue,
- Err(err) => return Err(err.into()),
- }
- }
- }
-
- /// Low level POST request.
- async fn post<T: Serialize>(
- &mut self,
- url: &str,
- data: &T,
- ) -> Result<AcmeResponse, anyhow::Error> {
- let account = Self::need_account(&self.account)?;
-
- let mut retry = retry();
- loop {
- retry.tick()?;
-
- let (_directory, nonce) = Self::get_dir_nonce(
- &mut self.http_client,
- &self.directory_url,
- &mut self.directory,
- &mut self.nonce,
- )
- .await?;
-
- let request = account.post_request(url, nonce, data)?;
- match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
- Ok(response) => return Ok(response),
- Err(err) if err.is_bad_nonce() => continue,
- Err(err) => return Err(err.into()),
- }
- }
- }
-
- /// Request challenge validation. Afterwards, the challenge should be polled.
- pub async fn request_challenge_validation(
- &mut self,
- url: &str,
- ) -> Result<Challenge, anyhow::Error> {
- Ok(self
- .post(url, &serde_json::Value::Object(Default::default()))
- .await?
- .json()?)
- }
-
- /// Assuming the provided URL is an 'Authorization' URL, get and deserialize it.
- pub async fn get_authorization(&mut self, url: &str) -> Result<Authorization, anyhow::Error> {
- Ok(self.post_as_get(url).await?.json()?)
- }
-
- /// Assuming the provided URL is an 'Order' URL, get and deserialize it.
- pub async fn get_order(&mut self, url: &str) -> Result<OrderData, anyhow::Error> {
- Ok(self.post_as_get(url).await?.json()?)
- }
-
- /// Finalize an Order via its `finalize` URL property and the DER encoded CSR.
- pub async fn finalize(&mut self, url: &str, csr: &[u8]) -> Result<(), anyhow::Error> {
- let csr = proxmox_base64::url::encode_no_pad(csr);
- let data = serde_json::json!({ "csr": csr });
- self.post(url, &data).await?;
- Ok(())
- }
-
- /// Download a certificate via its 'certificate' URL property.
- ///
- /// The certificate will be a PEM certificate chain.
- pub async fn get_certificate(&mut self, url: &str) -> Result<Bytes, anyhow::Error> {
- Ok(self.post_as_get(url).await?.body)
- }
-
- /// Revoke an existing certificate (PEM or DER formatted).
- pub async fn revoke_certificate(
- &mut self,
- certificate: &[u8],
- reason: Option<u32>,
- ) -> Result<(), anyhow::Error> {
- // TODO: This can also work without an account.
- let account = Self::need_account(&self.account)?;
-
- let revocation = account.revoke_certificate(certificate, reason)?;
-
- let mut retry = retry();
- loop {
- retry.tick()?;
-
- let (directory, nonce) = Self::get_dir_nonce(
- &mut self.http_client,
- &self.directory_url,
- &mut self.directory,
- &mut self.nonce,
- )
- .await?;
-
- let request = revocation.request(directory, nonce)?;
- match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
- Ok(_response) => return Ok(()),
- Err(err) if err.is_bad_nonce() => continue,
- Err(err) => return Err(err.into()),
- }
- }
- }
-
- fn need_account(account: &Option<Account>) -> Result<&Account, anyhow::Error> {
- account
- .as_ref()
- .ok_or_else(|| format_err!("cannot use client without an account"))
- }
-
- pub(crate) fn account(&self) -> Result<&Account, anyhow::Error> {
- Self::need_account(&self.account)
- }
-
- pub fn tos(&self) -> Option<&str> {
- self.tos.as_deref()
- }
-
- pub fn directory_url(&self) -> &str {
- &self.directory_url
- }
-
- fn to_account_data(&self) -> Result<AccountData, anyhow::Error> {
- let account = self.account()?;
-
- Ok(AccountData {
- location: account.location.clone(),
- key: account.private_key.clone(),
- account: AcmeAccountData {
- only_return_existing: false, // don't actually write this out in case it's set
- ..account.data.clone()
- },
- tos: self.tos.clone(),
- debug: self.debug,
- directory_url: self.directory_url.clone(),
- })
- }
-
- fn write_to<T: io::Write>(&self, out: T) -> Result<(), anyhow::Error> {
- let data = self.to_account_data()?;
-
- Ok(serde_json::to_writer_pretty(out, &data)?)
- }
-}
-
-struct AcmeResponse {
- body: Bytes,
- location: Option<String>,
- got_nonce: bool,
-}
-
-impl AcmeResponse {
- /// Convenience helper to assert that a location header was part of the response.
- fn location_required(&mut self) -> Result<String, anyhow::Error> {
- self.location
- .take()
- .ok_or_else(|| format_err!("missing Location header"))
- }
-
- /// Convenience shortcut to perform json deserialization of the returned body.
- fn json<T: for<'a> Deserialize<'a>>(&self) -> Result<T, Error> {
- Ok(serde_json::from_slice(&self.body)?)
- }
-
- /// Convenience shortcut to get the body as bytes.
- fn bytes(&self) -> &[u8] {
- &self.body
- }
-}
-
-impl AcmeClient {
- /// Non-self-borrowing run_request version for borrow workarounds.
- async fn execute(
- http_client: &mut Client,
- request: AcmeRequest,
- nonce: &mut Option<String>,
- ) -> Result<AcmeResponse, Error> {
- let req_builder = Request::builder().method(request.method).uri(&request.url);
-
- let http_request = if !request.content_type.is_empty() {
- req_builder
- .header("Content-Type", request.content_type)
- .header("Content-Length", request.body.len())
- .body(request.body.into())
- } else {
- req_builder.body(Body::empty())
- }
- .map_err(|err| Error::Custom(format!("failed to create http request: {err}")))?;
-
- let response = http_client
- .request(http_request)
- .await
- .map_err(|err| Error::Custom(err.to_string()))?;
- let (parts, body) = response.into_parts();
-
- let status = parts.status.as_u16();
- let body = body
- .collect()
- .await
- .map_err(|err| Error::Custom(format!("failed to retrieve response body: {err}")))?
- .to_bytes();
-
- let got_nonce = if let Some(new_nonce) = parts.headers.get(proxmox_acme::REPLAY_NONCE) {
- let new_nonce = new_nonce.to_str().map_err(|err| {
- Error::Client(format!(
- "received invalid replay-nonce header from ACME server: {err}"
- ))
- })?;
- *nonce = Some(new_nonce.to_owned());
- true
- } else {
- false
- };
-
- if parts.status.is_success() {
- if status != request.expected {
- return Err(Error::InvalidApi(format!(
- "ACME server responded with unexpected status code: {:?}",
- parts.status
- )));
- }
-
- let location = parts
- .headers
- .get("Location")
- .map(|header| {
- header.to_str().map(str::to_owned).map_err(|err| {
- Error::Client(format!(
- "received invalid location header from ACME server: {err}"
- ))
- })
- })
- .transpose()?;
-
- return Ok(AcmeResponse {
- body,
- location,
- got_nonce,
- });
- }
-
- let error: ErrorResponse = serde_json::from_slice(&body).map_err(|err| {
- Error::Client(format!(
- "error status with improper error ACME response: {err}"
- ))
- })?;
-
- if error.ty == proxmox_acme::error::BAD_NONCE {
- if !got_nonce {
- return Err(Error::InvalidApi(
- "badNonce without a new Replay-Nonce header".to_string(),
- ));
- }
- return Err(Error::BadNonce);
- }
-
- Err(Error::Api(error))
- }
-
- /// Low-level API to run an n API request. This automatically updates the current nonce!
- async fn run_request(&mut self, request: AcmeRequest) -> Result<AcmeResponse, Error> {
- Self::execute(&mut self.http_client, request, &mut self.nonce).await
- }
-
- pub async fn directory(&mut self) -> Result<&Directory, Error> {
- Ok(Self::get_directory(
- &mut self.http_client,
- &self.directory_url,
- &mut self.directory,
- &mut self.nonce,
- )
- .await?
- .0)
- }
-
- async fn get_directory<'a, 'b>(
- http_client: &mut Client,
- directory_url: &str,
- directory: &'a mut Option<Directory>,
- nonce: &'b mut Option<String>,
- ) -> Result<(&'a Directory, Option<&'b str>), Error> {
- if let Some(d) = directory {
- return Ok((d, nonce.as_deref()));
- }
-
- let response = Self::execute(
- http_client,
- AcmeRequest {
- url: directory_url.to_string(),
- method: "GET",
- content_type: "",
- body: String::new(),
- expected: 200,
- },
- nonce,
- )
- .await?;
-
- *directory = Some(Directory::from_parts(
- directory_url.to_string(),
- response.json()?,
- ));
-
- Ok((directory.as_mut().unwrap(), nonce.as_deref()))
- }
-
- /// Like `get_directory`, but if the directory provides no nonce, also performs a `HEAD`
- /// request on the new nonce URL.
- async fn get_dir_nonce<'a, 'b>(
- http_client: &mut Client,
- directory_url: &str,
- directory: &'a mut Option<Directory>,
- nonce: &'b mut Option<String>,
- ) -> Result<(&'a Directory, &'b str), Error> {
- // this let construct is a lifetime workaround:
- let _ = Self::get_directory(http_client, directory_url, directory, nonce).await?;
- let dir = directory.as_ref().unwrap(); // the above fails if it couldn't fill this option
- if nonce.is_none() {
- // this is also a lifetime issue...
- let _ = Self::get_nonce(http_client, nonce, dir.new_nonce_url()).await?;
- };
- Ok((dir, nonce.as_deref().unwrap()))
- }
-
- pub async fn terms_of_service_url(&mut self) -> Result<Option<&str>, Error> {
- Ok(self.directory().await?.terms_of_service_url())
- }
-
- async fn get_nonce<'a>(
- http_client: &mut Client,
- nonce: &'a mut Option<String>,
- new_nonce_url: &str,
- ) -> Result<&'a str, Error> {
- let response = Self::execute(
- http_client,
- AcmeRequest {
- url: new_nonce_url.to_owned(),
- method: "HEAD",
- content_type: "",
- body: String::new(),
- expected: 200,
- },
- nonce,
- )
- .await?;
-
- if !response.got_nonce {
- return Err(Error::InvalidApi(
- "no new nonce received from new nonce URL".to_string(),
- ));
- }
-
- nonce
- .as_deref()
- .ok_or_else(|| Error::Client("failed to update nonce".to_string()))
- }
-}
-
-/// bad nonce retry count helper
-struct Retry(usize);
-
-const fn retry() -> Retry {
- Retry(0)
-}
-
-impl Retry {
- fn tick(&mut self) -> Result<(), Error> {
- if self.0 >= 3 {
- Err(Error::Client("kept getting a badNonce error!".to_string()))
- } else {
- self.0 += 1;
- Ok(())
- }
- }
-}
diff --git a/src/acme/mod.rs b/src/acme/mod.rs
index bf61811c..cc561f9a 100644
--- a/src/acme/mod.rs
+++ b/src/acme/mod.rs
@@ -1,5 +1,2 @@
-mod client;
-pub use client::AcmeClient;
-
pub(crate) mod plugin;
pub(crate) use plugin::get_acme_plugin;
diff --git a/src/acme/plugin.rs b/src/acme/plugin.rs
index f756e9b5..5bc09e1f 100644
--- a/src/acme/plugin.rs
+++ b/src/acme/plugin.rs
@@ -20,8 +20,8 @@ use tokio::process::Command;
use proxmox_acme::{Authorization, Challenge};
-use crate::acme::AcmeClient;
use crate::api2::types::AcmeDomain;
+use proxmox_acme::async_client::AcmeClient;
use proxmox_rest_server::WorkerTask;
use crate::config::acme::plugin::{DnsPlugin, PluginData};
diff --git a/src/api2/config/acme.rs b/src/api2/config/acme.rs
index 35c3fb77..02f88e2e 100644
--- a/src/api2/config/acme.rs
+++ b/src/api2/config/acme.rs
@@ -16,15 +16,15 @@ use proxmox_router::{
use proxmox_schema::{api, param_bail};
use proxmox_acme::types::AccountData as AcmeAccountData;
-use proxmox_acme::Account;
use pbs_api_types::{Authid, PRIV_SYS_MODIFY};
-use crate::acme::AcmeClient;
-use crate::api2::types::{AcmeAccountName, AcmeChallengeSchema, KnownAcmeDirectory};
+use crate::api2::types::{AcmeChallengeSchema, KnownAcmeDirectory};
use crate::config::acme::plugin::{
self, DnsPlugin, DnsPluginCore, DnsPluginCoreUpdater, PLUGIN_ID_SCHEMA,
};
+use proxmox_acme::async_client::AcmeClient;
+use proxmox_acme_api::AcmeAccountName;
use proxmox_rest_server::WorkerTask;
pub(crate) const ROUTER: Router = Router::new()
@@ -143,15 +143,15 @@ pub struct AccountInfo {
)]
/// Return existing ACME account information.
pub async fn get_account(name: AcmeAccountName) -> Result<AccountInfo, Error> {
- let client = AcmeClient::load(&name).await?;
- let account = client.account()?;
+ let account_info = proxmox_acme_api::get_account(name).await?;
+
Ok(AccountInfo {
- location: account.location.clone(),
- tos: client.tos().map(str::to_owned),
- directory: client.directory_url().to_owned(),
+ location: account_info.location,
+ tos: account_info.tos,
+ directory: account_info.directory,
account: AcmeAccountData {
only_return_existing: false, // don't actually write this out in case it's set
- ..account.data.clone()
+ ..account_info.account
},
})
}
@@ -240,41 +240,24 @@ fn register_account(
auth_id.to_string(),
true,
move |_worker| async move {
- let mut client = AcmeClient::new(directory);
-
info!("Registering ACME account '{}'...", &name);
- let account = do_register_account(
- &mut client,
+ let location = proxmox_acme_api::register_account(
&name,
- tos_url.is_some(),
contact,
- None,
+ tos_url,
+ Some(directory),
eab_kid.zip(eab_hmac_key),
)
.await?;
- info!("Registration successful, account URL: {}", account.location);
+ info!("Registration successful, account URL: {}", location);
Ok(())
},
)
}
-pub async fn do_register_account<'a>(
- client: &'a mut AcmeClient,
- name: &AcmeAccountName,
- agree_to_tos: bool,
- contact: String,
- rsa_bits: Option<u32>,
- eab_creds: Option<(String, String)>,
-) -> Result<&'a Account, Error> {
- let contact = account_contact_from_string(&contact);
- client
- .new_account(name, agree_to_tos, contact, rsa_bits, eab_creds)
- .await
-}
-
#[api(
input: {
properties: {
@@ -312,7 +295,10 @@ pub fn update_account(
None => json!({}),
};
- AcmeClient::load(&name).await?.update_account(&data).await?;
+ proxmox_acme_api::load_client_with_account(&name)
+ .await?
+ .update_account(&data)
+ .await?;
Ok(())
},
@@ -350,7 +336,7 @@ pub fn deactivate_account(
auth_id.to_string(),
true,
move |_worker| async move {
- match AcmeClient::load(&name)
+ match proxmox_acme_api::load_client_with_account(&name)
.await?
.update_account(&json!({"status": "deactivated"}))
.await
diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
index 61ef910e..31196715 100644
--- a/src/api2/node/certificates.rs
+++ b/src/api2/node/certificates.rs
@@ -17,10 +17,10 @@ use pbs_buildcfg::configdir;
use pbs_tools::cert;
use tracing::warn;
-use crate::acme::AcmeClient;
use crate::api2::types::AcmeDomain;
use crate::config::node::NodeConfig;
use crate::server::send_certificate_renewal_mail;
+use proxmox_acme::async_client::AcmeClient;
use proxmox_rest_server::WorkerTask;
pub const ROUTER: Router = Router::new()
diff --git a/src/api2/types/acme.rs b/src/api2/types/acme.rs
index 210ebdbc..7c9063c0 100644
--- a/src/api2/types/acme.rs
+++ b/src/api2/types/acme.rs
@@ -60,14 +60,6 @@ pub struct KnownAcmeDirectory {
pub url: &'static str,
}
-proxmox_schema::api_string_type! {
- #[api(format: &PROXMOX_SAFE_ID_FORMAT)]
- /// ACME account name.
- #[derive(Clone, Eq, PartialEq, Hash, Deserialize, Serialize)]
- #[serde(transparent)]
- pub struct AcmeAccountName(String);
-}
-
#[api(
properties: {
schema: {
diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs
index 0f0eafea..bb987b26 100644
--- a/src/bin/proxmox_backup_manager/acme.rs
+++ b/src/bin/proxmox_backup_manager/acme.rs
@@ -7,9 +7,9 @@ use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
use proxmox_schema::api;
use proxmox_sys::fs::file_get_contents;
-use proxmox_backup::acme::AcmeClient;
+use proxmox_acme::async_client::AcmeClient;
+use proxmox_acme_api::AcmeAccountName;
use proxmox_backup::api2;
-use proxmox_backup::api2::types::AcmeAccountName;
use proxmox_backup::config::acme::plugin::DnsPluginCore;
use proxmox_backup::config::acme::KNOWN_ACME_DIRECTORIES;
@@ -188,17 +188,20 @@ async fn register_account(
println!("Attempting to register account with {directory_url:?}...");
- let account = api2::config::acme::do_register_account(
- &mut client,
+ let tos_agreed = tos_agreed
+ .then(|| directory.terms_of_service_url().map(str::to_owned))
+ .flatten();
+
+ let location = proxmox_acme_api::register_account(
&name,
- tos_agreed,
contact,
- None,
+ tos_agreed,
+ Some(directory_url),
eab_creds,
)
.await?;
- println!("Registration successful, account URL: {}", account.location);
+ println!("Registration successful, account URL: {}", location);
Ok(())
}
diff --git a/src/config/acme/mod.rs b/src/config/acme/mod.rs
index 274a23fd..d31b2bc9 100644
--- a/src/config/acme/mod.rs
+++ b/src/config/acme/mod.rs
@@ -10,7 +10,8 @@ use proxmox_sys::fs::{file_read_string, CreateOptions};
use pbs_api_types::PROXMOX_SAFE_ID_REGEX;
-use crate::api2::types::{AcmeAccountName, AcmeChallengeSchema, KnownAcmeDirectory};
+use crate::api2::types::{AcmeChallengeSchema, KnownAcmeDirectory};
+use proxmox_acme_api::AcmeAccountName;
pub(crate) const ACME_DIR: &str = pbs_buildcfg::configdir!("/acme");
pub(crate) const ACME_ACCOUNT_DIR: &str = pbs_buildcfg::configdir!("/acme/accounts");
@@ -35,11 +36,6 @@ pub(crate) fn make_acme_dir() -> Result<(), Error> {
create_acme_subdir(ACME_DIR)
}
-pub(crate) fn make_acme_account_dir() -> Result<(), Error> {
- make_acme_dir()?;
- create_acme_subdir(ACME_ACCOUNT_DIR)
-}
-
pub const KNOWN_ACME_DIRECTORIES: &[KnownAcmeDirectory] = &[
KnownAcmeDirectory {
name: "Let's Encrypt V2",
diff --git a/src/config/node.rs b/src/config/node.rs
index d2d6e383..d2a17a49 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -16,10 +16,9 @@ use pbs_api_types::{
use pbs_buildcfg::configdir;
use pbs_config::{open_backup_lockfile, BackupLockGuard};
-use crate::acme::AcmeClient;
-use crate::api2::types::{
- AcmeAccountName, AcmeDomain, ACME_DOMAIN_PROPERTY_SCHEMA, HTTP_PROXY_SCHEMA,
-};
+use crate::api2::types::{AcmeDomain, ACME_DOMAIN_PROPERTY_SCHEMA, HTTP_PROXY_SCHEMA};
+use proxmox_acme::async_client::AcmeClient;
+use proxmox_acme_api::AcmeAccountName;
const CONF_FILE: &str = configdir!("/node.cfg");
const LOCK_FILE: &str = configdir!("/.node.lck");
@@ -249,7 +248,7 @@ impl NodeConfig {
} else {
AcmeAccountName::from_string("default".to_string())? // should really not happen
};
- AcmeClient::load(&account).await
+ proxmox_acme_api::load_client_with_account(&account).await
}
pub fn acme_domains(&'_ self) -> AcmeDomainIter<'_> {
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH proxmox-backup v4 3/4] acme: change API impls to use proxmox-acme-api handlers
2025-12-03 10:22 [pbs-devel] [PATCH proxmox{-backup, } v4 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox-backup v4 1/4] acme: include proxmox-acme-api dependency Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox-backup v4 2/4] acme: drop local AcmeClient Samuel Rufinatscha
@ 2025-12-03 10:22 ` Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox-backup v4 4/4] acme: certificate ordering through proxmox-acme-api Samuel Rufinatscha
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Samuel Rufinatscha @ 2025-12-03 10:22 UTC (permalink / raw)
To: pbs-devel
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 api2/config/acme.rs API logic with proxmox-acme-api handlers.
- Drop local caching and helper types that duplicate proxmox-acme-api.
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
src/api2/config/acme.rs | 385 ++-----------------------
src/api2/types/acme.rs | 16 -
src/bin/proxmox_backup_manager/acme.rs | 6 +-
src/config/acme/mod.rs | 44 +--
4 files changed, 35 insertions(+), 416 deletions(-)
diff --git a/src/api2/config/acme.rs b/src/api2/config/acme.rs
index 02f88e2e..a112c8ee 100644
--- a/src/api2/config/acme.rs
+++ b/src/api2/config/acme.rs
@@ -1,31 +1,17 @@
-use std::fs;
-use std::ops::ControlFlow;
-use std::path::Path;
-use std::sync::{Arc, LazyLock, Mutex};
-use std::time::SystemTime;
-
-use anyhow::{bail, format_err, Error};
-use hex::FromHex;
-use serde::{Deserialize, Serialize};
-use serde_json::{json, Value};
-use tracing::{info, warn};
-
-use proxmox_router::{
- http_bail, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap,
-};
-use proxmox_schema::{api, param_bail};
-
-use proxmox_acme::types::AccountData as AcmeAccountData;
-
+use anyhow::Error;
use pbs_api_types::{Authid, PRIV_SYS_MODIFY};
-
-use crate::api2::types::{AcmeChallengeSchema, KnownAcmeDirectory};
-use crate::config::acme::plugin::{
- self, DnsPlugin, DnsPluginCore, DnsPluginCoreUpdater, PLUGIN_ID_SCHEMA,
+use proxmox_acme_api::{
+ AccountEntry, AccountInfo, AcmeAccountName, AcmeChallengeSchema, ChallengeSchemaWrapper,
+ DeletablePluginProperty, DnsPluginCore, DnsPluginCoreUpdater, KnownAcmeDirectory, PluginConfig,
+ DEFAULT_ACME_DIRECTORY_ENTRY, PLUGIN_ID_SCHEMA,
};
-use proxmox_acme::async_client::AcmeClient;
-use proxmox_acme_api::AcmeAccountName;
+use proxmox_config_digest::ConfigDigest;
use proxmox_rest_server::WorkerTask;
+use proxmox_router::{
+ http_bail, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap,
+};
+use proxmox_schema::api;
+use tracing::info;
pub(crate) const ROUTER: Router = Router::new()
.get(&list_subdirs_api_method!(SUBDIRS))
@@ -67,19 +53,6 @@ const PLUGIN_ITEM_ROUTER: Router = Router::new()
.put(&API_METHOD_UPDATE_PLUGIN)
.delete(&API_METHOD_DELETE_PLUGIN);
-#[api(
- properties: {
- name: { type: AcmeAccountName },
- },
-)]
-/// An ACME Account entry.
-///
-/// Currently only contains a 'name' property.
-#[derive(Serialize)]
-pub struct AccountEntry {
- name: AcmeAccountName,
-}
-
#[api(
access: {
permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
@@ -93,40 +66,7 @@ pub struct AccountEntry {
)]
/// List ACME accounts.
pub fn list_accounts() -> Result<Vec<AccountEntry>, Error> {
- let mut entries = Vec::new();
- crate::config::acme::foreach_acme_account(|name| {
- entries.push(AccountEntry { name });
- ControlFlow::Continue(())
- })?;
- Ok(entries)
-}
-
-#[api(
- properties: {
- account: { type: Object, properties: {}, additional_properties: true },
- tos: {
- type: String,
- optional: true,
- },
- },
-)]
-/// ACME Account information.
-///
-/// This is what we return via the API.
-#[derive(Serialize)]
-pub struct AccountInfo {
- /// Raw account data.
- account: AcmeAccountData,
-
- /// The ACME directory URL the account was created at.
- directory: String,
-
- /// The account's own URL within the ACME directory.
- location: String,
-
- /// The ToS URL, if the user agreed to one.
- #[serde(skip_serializing_if = "Option::is_none")]
- tos: Option<String>,
+ proxmox_acme_api::list_accounts()
}
#[api(
@@ -143,23 +83,7 @@ pub struct AccountInfo {
)]
/// Return existing ACME account information.
pub async fn get_account(name: AcmeAccountName) -> Result<AccountInfo, Error> {
- let account_info = proxmox_acme_api::get_account(name).await?;
-
- Ok(AccountInfo {
- location: account_info.location,
- tos: account_info.tos,
- directory: account_info.directory,
- account: AcmeAccountData {
- only_return_existing: false, // don't actually write this out in case it's set
- ..account_info.account
- },
- })
-}
-
-fn account_contact_from_string(s: &str) -> Vec<String> {
- s.split(&[' ', ';', ',', '\0'][..])
- .map(|s| format!("mailto:{s}"))
- .collect()
+ proxmox_acme_api::get_account(name).await
}
#[api(
@@ -224,15 +148,11 @@ fn register_account(
);
}
- if Path::new(&crate::config::acme::account_path(&name)).exists() {
+ if std::path::Path::new(&proxmox_acme_api::account_config_filename(&name)).exists() {
http_bail!(BAD_REQUEST, "account {} already exists", name);
}
- let directory = directory.unwrap_or_else(|| {
- crate::config::acme::DEFAULT_ACME_DIRECTORY_ENTRY
- .url
- .to_owned()
- });
+ let directory = directory.unwrap_or_else(|| DEFAULT_ACME_DIRECTORY_ENTRY.url.to_string());
WorkerTask::spawn(
"acme-register",
@@ -288,17 +208,7 @@ pub fn update_account(
auth_id.to_string(),
true,
move |_worker| async move {
- let data = match contact {
- Some(data) => json!({
- "contact": account_contact_from_string(&data),
- }),
- None => json!({}),
- };
-
- proxmox_acme_api::load_client_with_account(&name)
- .await?
- .update_account(&data)
- .await?;
+ proxmox_acme_api::update_account(&name, contact).await?;
Ok(())
},
@@ -336,18 +246,8 @@ pub fn deactivate_account(
auth_id.to_string(),
true,
move |_worker| async move {
- match proxmox_acme_api::load_client_with_account(&name)
- .await?
- .update_account(&json!({"status": "deactivated"}))
- .await
- {
- Ok(_account) => (),
- Err(err) if !force => return Err(err),
- Err(err) => {
- warn!("error deactivating account {name}, proceeding anyway - {err}");
- }
- }
- crate::config::acme::mark_account_deactivated(&name)?;
+ proxmox_acme_api::deactivate_account(&name, force).await?;
+
Ok(())
},
)
@@ -374,15 +274,7 @@ pub fn deactivate_account(
)]
/// Get the Terms of Service URL for an ACME directory.
async fn get_tos(directory: Option<String>) -> Result<Option<String>, Error> {
- let directory = directory.unwrap_or_else(|| {
- crate::config::acme::DEFAULT_ACME_DIRECTORY_ENTRY
- .url
- .to_owned()
- });
- Ok(AcmeClient::new(directory)
- .terms_of_service_url()
- .await?
- .map(str::to_owned))
+ proxmox_acme_api::get_tos(directory).await
}
#[api(
@@ -397,52 +289,7 @@ async fn get_tos(directory: Option<String>) -> Result<Option<String>, Error> {
)]
/// Get named known ACME directory endpoints.
fn get_directories() -> Result<&'static [KnownAcmeDirectory], Error> {
- Ok(crate::config::acme::KNOWN_ACME_DIRECTORIES)
-}
-
-/// Wrapper for efficient Arc use when returning the ACME challenge-plugin schema for serializing
-struct ChallengeSchemaWrapper {
- inner: Arc<Vec<AcmeChallengeSchema>>,
-}
-
-impl Serialize for ChallengeSchemaWrapper {
- fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
- where
- S: serde::Serializer,
- {
- self.inner.serialize(serializer)
- }
-}
-
-struct CachedSchema {
- schema: Arc<Vec<AcmeChallengeSchema>>,
- cached_mtime: SystemTime,
-}
-
-fn get_cached_challenge_schemas() -> Result<ChallengeSchemaWrapper, Error> {
- static CACHE: LazyLock<Mutex<Option<CachedSchema>>> = LazyLock::new(|| Mutex::new(None));
-
- // the actual loading code
- let mut last = CACHE.lock().unwrap();
-
- let actual_mtime = fs::metadata(crate::config::acme::ACME_DNS_SCHEMA_FN)?.modified()?;
-
- let schema = match &*last {
- Some(CachedSchema {
- schema,
- cached_mtime,
- }) if *cached_mtime >= actual_mtime => schema.clone(),
- _ => {
- let new_schema = Arc::new(crate::config::acme::load_dns_challenge_schema()?);
- *last = Some(CachedSchema {
- schema: Arc::clone(&new_schema),
- cached_mtime: actual_mtime,
- });
- new_schema
- }
- };
-
- Ok(ChallengeSchemaWrapper { inner: schema })
+ Ok(proxmox_acme_api::KNOWN_ACME_DIRECTORIES)
}
#[api(
@@ -457,69 +304,7 @@ fn get_cached_challenge_schemas() -> Result<ChallengeSchemaWrapper, Error> {
)]
/// Get named known ACME directory endpoints.
fn get_challenge_schema() -> Result<ChallengeSchemaWrapper, Error> {
- get_cached_challenge_schemas()
-}
-
-#[api]
-#[derive(Default, Deserialize, Serialize)]
-#[serde(rename_all = "kebab-case")]
-/// The API's format is inherited from PVE/PMG:
-pub struct PluginConfig {
- /// Plugin ID.
- plugin: String,
-
- /// Plugin type.
- #[serde(rename = "type")]
- ty: String,
-
- /// DNS Api name.
- #[serde(skip_serializing_if = "Option::is_none", default)]
- api: Option<String>,
-
- /// Plugin configuration data.
- #[serde(skip_serializing_if = "Option::is_none", default)]
- data: Option<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)]
- validation_delay: Option<u32>,
-
- /// Flag to disable the config.
- #[serde(skip_serializing_if = "Option::is_none", default)]
- disable: Option<bool>,
-}
-
-// See PMG/PVE's $modify_cfg_for_api sub
-fn modify_cfg_for_api(id: &str, ty: &str, data: &Value) -> PluginConfig {
- let mut entry = data.clone();
-
- let obj = entry.as_object_mut().unwrap();
- obj.remove("id");
- obj.insert("plugin".to_string(), Value::String(id.to_owned()));
- obj.insert("type".to_string(), Value::String(ty.to_owned()));
-
- // FIXME: This needs to go once the `Updater` is fixed.
- // None of these should be able to fail unless the user changed the files by hand, in which
- // case we leave the unmodified string in the Value for now. This will be handled with an error
- // later.
- if let Some(Value::String(ref mut data)) = obj.get_mut("data") {
- if let Ok(new) = proxmox_base64::url::decode_no_pad(&data) {
- if let Ok(utf8) = String::from_utf8(new) {
- *data = utf8;
- }
- }
- }
-
- // PVE/PMG do this explicitly for ACME plugins...
- // obj.insert("digest".to_string(), Value::String(digest.clone()));
-
- serde_json::from_value(entry).unwrap_or_else(|_| PluginConfig {
- plugin: "*Error*".to_string(),
- ty: "*Error*".to_string(),
- ..Default::default()
- })
+ proxmox_acme_api::get_cached_challenge_schemas()
}
#[api(
@@ -535,12 +320,7 @@ fn modify_cfg_for_api(id: &str, ty: &str, data: &Value) -> PluginConfig {
)]
/// List ACME challenge plugins.
pub fn list_plugins(rpcenv: &mut dyn RpcEnvironment) -> Result<Vec<PluginConfig>, Error> {
- let (plugins, digest) = plugin::config()?;
- rpcenv["digest"] = hex::encode(digest).into();
- Ok(plugins
- .iter()
- .map(|(id, (ty, data))| modify_cfg_for_api(id, ty, data))
- .collect())
+ proxmox_acme_api::list_plugins(rpcenv)
}
#[api(
@@ -557,13 +337,7 @@ pub fn list_plugins(rpcenv: &mut dyn RpcEnvironment) -> Result<Vec<PluginConfig>
)]
/// List ACME challenge plugins.
pub fn get_plugin(id: String, rpcenv: &mut dyn RpcEnvironment) -> Result<PluginConfig, Error> {
- let (plugins, digest) = plugin::config()?;
- rpcenv["digest"] = hex::encode(digest).into();
-
- match plugins.get(&id) {
- Some((ty, data)) => Ok(modify_cfg_for_api(&id, ty, data)),
- None => http_bail!(NOT_FOUND, "no such plugin"),
- }
+ proxmox_acme_api::get_plugin(id, rpcenv)
}
// Currently we only have "the" standalone plugin and DNS plugins so we can just flatten a
@@ -595,30 +369,7 @@ pub fn get_plugin(id: String, rpcenv: &mut dyn RpcEnvironment) -> Result<PluginC
)]
/// Add ACME plugin configuration.
pub fn add_plugin(r#type: String, core: DnsPluginCore, data: String) -> Result<(), Error> {
- // Currently we only support DNS plugins and the standalone plugin is "fixed":
- if r#type != "dns" {
- param_bail!("type", "invalid ACME plugin type: {:?}", r#type);
- }
-
- let data = String::from_utf8(proxmox_base64::decode(data)?)
- .map_err(|_| format_err!("data must be valid UTF-8"))?;
-
- let id = core.id.clone();
-
- let _lock = plugin::lock()?;
-
- let (mut plugins, _digest) = plugin::config()?;
- if plugins.contains_key(&id) {
- param_bail!("id", "ACME plugin ID {:?} already exists", id);
- }
-
- let plugin = serde_json::to_value(DnsPlugin { core, data })?;
-
- plugins.insert(id, r#type, plugin);
-
- plugin::save_config(&plugins)?;
-
- Ok(())
+ proxmox_acme_api::add_plugin(r#type, core, data)
}
#[api(
@@ -634,26 +385,7 @@ pub fn add_plugin(r#type: String, core: DnsPluginCore, data: String) -> Result<(
)]
/// Delete an ACME plugin configuration.
pub fn delete_plugin(id: String) -> Result<(), Error> {
- let _lock = plugin::lock()?;
-
- let (mut plugins, _digest) = plugin::config()?;
- if plugins.remove(&id).is_none() {
- http_bail!(NOT_FOUND, "no such plugin");
- }
- plugin::save_config(&plugins)?;
-
- Ok(())
-}
-
-#[api()]
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-/// Deletable property name
-pub enum DeletableProperty {
- /// Delete the disable property
- Disable,
- /// Delete the validation-delay property
- ValidationDelay,
+ proxmox_acme_api::delete_plugin(id)
}
#[api(
@@ -675,12 +407,12 @@ pub enum DeletableProperty {
type: Array,
optional: true,
items: {
- type: DeletableProperty,
+ type: DeletablePluginProperty,
}
},
digest: {
- description: "Digest to protect against concurrent updates",
optional: true,
+ type: ConfigDigest,
},
},
},
@@ -694,65 +426,8 @@ pub fn update_plugin(
id: String,
update: DnsPluginCoreUpdater,
data: Option<String>,
- delete: Option<Vec<DeletableProperty>>,
- digest: Option<String>,
+ delete: Option<Vec<DeletablePluginProperty>>,
+ digest: Option<ConfigDigest>,
) -> Result<(), Error> {
- let data = data
- .as_deref()
- .map(proxmox_base64::decode)
- .transpose()?
- .map(String::from_utf8)
- .transpose()
- .map_err(|_| format_err!("data must be valid UTF-8"))?;
-
- let _lock = plugin::lock()?;
-
- let (mut plugins, expected_digest) = plugin::config()?;
-
- if let Some(digest) = digest {
- let digest = <[u8; 32]>::from_hex(digest)?;
- crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
- }
-
- match plugins.get_mut(&id) {
- Some((ty, ref mut entry)) => {
- if ty != "dns" {
- bail!("cannot update plugin of type {:?}", ty);
- }
-
- let mut plugin = DnsPlugin::deserialize(&*entry)?;
-
- if let Some(delete) = delete {
- for delete_prop in delete {
- match delete_prop {
- DeletableProperty::ValidationDelay => {
- plugin.core.validation_delay = None;
- }
- DeletableProperty::Disable => {
- plugin.core.disable = None;
- }
- }
- }
- }
- if let Some(data) = data {
- plugin.data = data;
- }
- if let Some(api) = update.api {
- plugin.core.api = api;
- }
- if update.validation_delay.is_some() {
- plugin.core.validation_delay = update.validation_delay;
- }
- if update.disable.is_some() {
- plugin.core.disable = update.disable;
- }
-
- *entry = serde_json::to_value(plugin)?;
- }
- None => http_bail!(NOT_FOUND, "no such plugin"),
- }
-
- plugin::save_config(&plugins)?;
-
- Ok(())
+ proxmox_acme_api::update_plugin(id, update, data, delete, digest)
}
diff --git a/src/api2/types/acme.rs b/src/api2/types/acme.rs
index 7c9063c0..2905b41b 100644
--- a/src/api2/types/acme.rs
+++ b/src/api2/types/acme.rs
@@ -44,22 +44,6 @@ pub const ACME_DOMAIN_PROPERTY_SCHEMA: Schema =
.format(&ApiStringFormat::PropertyString(&AcmeDomain::API_SCHEMA))
.schema();
-#[api(
- properties: {
- name: { type: String },
- url: { type: String },
- },
-)]
-/// An ACME directory endpoint with a name and URL.
-#[derive(Serialize)]
-pub struct KnownAcmeDirectory {
- /// The ACME directory's name.
- pub name: &'static str,
-
- /// The ACME directory's endpoint URL.
- pub url: &'static str,
-}
-
#[api(
properties: {
schema: {
diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs
index bb987b26..e7bd67af 100644
--- a/src/bin/proxmox_backup_manager/acme.rs
+++ b/src/bin/proxmox_backup_manager/acme.rs
@@ -8,10 +8,8 @@ use proxmox_schema::api;
use proxmox_sys::fs::file_get_contents;
use proxmox_acme::async_client::AcmeClient;
-use proxmox_acme_api::AcmeAccountName;
+use proxmox_acme_api::{AcmeAccountName, DnsPluginCore, KNOWN_ACME_DIRECTORIES};
use proxmox_backup::api2;
-use proxmox_backup::config::acme::plugin::DnsPluginCore;
-use proxmox_backup::config::acme::KNOWN_ACME_DIRECTORIES;
pub fn acme_mgmt_cli() -> CommandLineInterface {
let cmd_def = CliCommandMap::new()
@@ -122,7 +120,7 @@ async fn register_account(
match input.trim().parse::<usize>() {
Ok(n) if n < KNOWN_ACME_DIRECTORIES.len() => {
- break (KNOWN_ACME_DIRECTORIES[n].url.to_owned(), false);
+ break (KNOWN_ACME_DIRECTORIES[n].url.to_string(), false);
}
Ok(n) if n == KNOWN_ACME_DIRECTORIES.len() => {
input.clear();
diff --git a/src/config/acme/mod.rs b/src/config/acme/mod.rs
index d31b2bc9..35cda50b 100644
--- a/src/config/acme/mod.rs
+++ b/src/config/acme/mod.rs
@@ -1,8 +1,7 @@
use std::collections::HashMap;
use std::ops::ControlFlow;
-use std::path::Path;
-use anyhow::{bail, format_err, Error};
+use anyhow::Error;
use serde_json::Value;
use proxmox_sys::error::SysError;
@@ -10,8 +9,8 @@ use proxmox_sys::fs::{file_read_string, CreateOptions};
use pbs_api_types::PROXMOX_SAFE_ID_REGEX;
-use crate::api2::types::{AcmeChallengeSchema, KnownAcmeDirectory};
-use proxmox_acme_api::AcmeAccountName;
+use crate::api2::types::AcmeChallengeSchema;
+use proxmox_acme_api::{AcmeAccountName, KnownAcmeDirectory, KNOWN_ACME_DIRECTORIES};
pub(crate) const ACME_DIR: &str = pbs_buildcfg::configdir!("/acme");
pub(crate) const ACME_ACCOUNT_DIR: &str = pbs_buildcfg::configdir!("/acme/accounts");
@@ -36,23 +35,8 @@ pub(crate) fn make_acme_dir() -> Result<(), Error> {
create_acme_subdir(ACME_DIR)
}
-pub const KNOWN_ACME_DIRECTORIES: &[KnownAcmeDirectory] = &[
- KnownAcmeDirectory {
- name: "Let's Encrypt V2",
- url: "https://acme-v02.api.letsencrypt.org/directory",
- },
- KnownAcmeDirectory {
- name: "Let's Encrypt V2 Staging",
- url: "https://acme-staging-v02.api.letsencrypt.org/directory",
- },
-];
-
pub const DEFAULT_ACME_DIRECTORY_ENTRY: &KnownAcmeDirectory = &KNOWN_ACME_DIRECTORIES[0];
-pub fn account_path(name: &str) -> String {
- format!("{ACME_ACCOUNT_DIR}/{name}")
-}
-
pub fn foreach_acme_account<F>(mut func: F) -> Result<(), Error>
where
F: FnMut(AcmeAccountName) -> ControlFlow<Result<(), Error>>,
@@ -83,28 +67,6 @@ where
}
}
-pub fn mark_account_deactivated(name: &str) -> Result<(), Error> {
- let from = account_path(name);
- for i in 0..100 {
- let to = account_path(&format!("_deactivated_{name}_{i}"));
- if !Path::new(&to).exists() {
- return std::fs::rename(&from, &to).map_err(|err| {
- format_err!(
- "failed to move account path {:?} to {:?} - {}",
- from,
- to,
- err
- )
- });
- }
- }
- bail!(
- "No free slot to rename deactivated account {:?}, please cleanup {:?}",
- from,
- ACME_ACCOUNT_DIR
- );
-}
-
pub fn load_dns_challenge_schema() -> Result<Vec<AcmeChallengeSchema>, Error> {
let raw = file_read_string(ACME_DNS_SCHEMA_FN)?;
let schemas: serde_json::Map<String, Value> = serde_json::from_str(&raw)?;
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH proxmox-backup v4 4/4] acme: certificate ordering through proxmox-acme-api
2025-12-03 10:22 [pbs-devel] [PATCH proxmox{-backup, } v4 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (2 preceding siblings ...)
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox-backup v4 3/4] acme: change API impls to use proxmox-acme-api handlers Samuel Rufinatscha
@ 2025-12-03 10:22 ` Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox v4 1/4] acme-api: add helper to load client for an account Samuel Rufinatscha
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Samuel Rufinatscha @ 2025-12-03 10:22 UTC (permalink / raw)
To: pbs-devel
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 | 336 ----------------------------------
src/api2/node/certificates.rs | 240 ++++--------------------
src/api2/types/acme.rs | 74 --------
src/api2/types/mod.rs | 3 -
src/config/acme/mod.rs | 7 +-
src/config/acme/plugin.rs | 99 +---------
src/config/node.rs | 22 +--
src/lib.rs | 2 -
9 files changed, 46 insertions(+), 739 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/acme/mod.rs b/src/acme/mod.rs
deleted file mode 100644
index cc561f9a..00000000
--- a/src/acme/mod.rs
+++ /dev/null
@@ -1,2 +0,0 @@
-pub(crate) mod plugin;
-pub(crate) use plugin::get_acme_plugin;
diff --git a/src/acme/plugin.rs b/src/acme/plugin.rs
deleted file mode 100644
index 5bc09e1f..00000000
--- a/src/acme/plugin.rs
+++ /dev/null
@@ -1,336 +0,0 @@
-use std::future::Future;
-use std::net::{IpAddr, SocketAddr};
-use std::pin::Pin;
-use std::process::Stdio;
-use std::sync::Arc;
-use std::time::Duration;
-
-use anyhow::{bail, format_err, Error};
-use bytes::Bytes;
-use futures::TryFutureExt;
-use http_body_util::Full;
-use hyper::body::Incoming;
-use hyper::server::conn::http1;
-use hyper::service::service_fn;
-use hyper::{Request, Response};
-use hyper_util::rt::TokioIo;
-use tokio::io::{AsyncBufReadExt, AsyncRead, AsyncWriteExt, BufReader};
-use tokio::net::TcpListener;
-use tokio::process::Command;
-
-use proxmox_acme::{Authorization, Challenge};
-
-use crate::api2::types::AcmeDomain;
-use proxmox_acme::async_client::AcmeClient;
-use proxmox_rest_server::WorkerTask;
-
-use crate::config::acme::plugin::{DnsPlugin, PluginData};
-
-const PROXMOX_ACME_SH_PATH: &str = "/usr/share/proxmox-acme/proxmox-acme";
-
-pub(crate) fn get_acme_plugin(
- plugin_data: &PluginData,
- name: &str,
-) -> Result<Option<Box<dyn AcmePlugin + Send + Sync + 'static>>, Error> {
- let (ty, data) = match plugin_data.get(name) {
- Some(plugin) => plugin,
- None => return Ok(None),
- };
-
- Ok(Some(match ty.as_str() {
- "dns" => {
- let plugin: DnsPlugin = serde::Deserialize::deserialize(data)?;
- Box::new(plugin)
- }
- "standalone" => {
- // this one has no config
- Box::<StandaloneServer>::default()
- }
- other => bail!("missing implementation for plugin type '{}'", other),
- }))
-}
-
-pub(crate) trait AcmePlugin {
- /// Setup everything required to trigger the validation and return the corresponding validation
- /// URL.
- fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
- &'a mut self,
- client: &'b mut AcmeClient,
- authorization: &'c Authorization,
- domain: &'d AcmeDomain,
- task: Arc<WorkerTask>,
- ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>>;
-
- fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
- &'a mut self,
- client: &'b mut AcmeClient,
- authorization: &'c Authorization,
- domain: &'d AcmeDomain,
- task: Arc<WorkerTask>,
- ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>>;
-}
-
-fn extract_challenge<'a>(
- authorization: &'a Authorization,
- ty: &str,
-) -> Result<&'a Challenge, Error> {
- authorization
- .challenges
- .iter()
- .find(|ch| ch.ty == ty)
- .ok_or_else(|| format_err!("no supported challenge type ({}) found", ty))
-}
-
-async fn pipe_to_tasklog<T: AsyncRead + Unpin>(
- pipe: T,
- task: Arc<WorkerTask>,
-) -> Result<(), std::io::Error> {
- let mut pipe = BufReader::new(pipe);
- let mut line = String::new();
- loop {
- line.clear();
- match pipe.read_line(&mut line).await {
- Ok(0) => return Ok(()),
- Ok(_) => task.log_message(line.as_str()),
- Err(err) => return Err(err),
- }
- }
-}
-
-impl DnsPlugin {
- async fn action<'a>(
- &self,
- client: &mut AcmeClient,
- authorization: &'a Authorization,
- domain: &AcmeDomain,
- task: Arc<WorkerTask>,
- action: &str,
- ) -> Result<&'a str, Error> {
- let challenge = extract_challenge(authorization, "dns-01")?;
- let mut stdin_data = client
- .dns_01_txt_value(
- challenge
- .token()
- .ok_or_else(|| format_err!("missing token in challenge"))?,
- )?
- .into_bytes();
- stdin_data.push(b'\n');
- stdin_data.extend(self.data.as_bytes());
- if stdin_data.last() != Some(&b'\n') {
- stdin_data.push(b'\n');
- }
-
- let mut command = Command::new("/usr/bin/setpriv");
-
- #[rustfmt::skip]
- command.args([
- "--reuid", "nobody",
- "--regid", "nogroup",
- "--clear-groups",
- "--reset-env",
- "--",
- "/bin/bash",
- PROXMOX_ACME_SH_PATH,
- action,
- &self.core.api,
- domain.alias.as_deref().unwrap_or(&domain.domain),
- ]);
-
- // We could use 1 socketpair, but tokio wraps them all in `File` internally causing `close`
- // to be called separately on all of them without exception, so we need 3 pipes :-(
-
- let mut child = command
- .stdin(Stdio::piped())
- .stdout(Stdio::piped())
- .stderr(Stdio::piped())
- .spawn()?;
-
- let mut stdin = child.stdin.take().expect("Stdio::piped()");
- let stdout = child.stdout.take().expect("Stdio::piped() failed?");
- let stdout = pipe_to_tasklog(stdout, Arc::clone(&task));
- let stderr = child.stderr.take().expect("Stdio::piped() failed?");
- let stderr = pipe_to_tasklog(stderr, Arc::clone(&task));
- let stdin = async move {
- stdin.write_all(&stdin_data).await?;
- stdin.flush().await?;
- Ok::<_, std::io::Error>(())
- };
- match futures::try_join!(stdin, stdout, stderr) {
- Ok(((), (), ())) => (),
- Err(err) => {
- if let Err(err) = child.kill().await {
- task.log_message(format!(
- "failed to kill '{PROXMOX_ACME_SH_PATH} {action}' command: {err}"
- ));
- }
- bail!("'{}' failed: {}", PROXMOX_ACME_SH_PATH, err);
- }
- }
-
- let status = child.wait().await?;
- if !status.success() {
- bail!(
- "'{} {}' exited with error ({})",
- PROXMOX_ACME_SH_PATH,
- action,
- status.code().unwrap_or(-1)
- );
- }
-
- Ok(&challenge.url)
- }
-}
-
-impl AcmePlugin for DnsPlugin {
- fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
- &'a mut self,
- client: &'b mut AcmeClient,
- authorization: &'c Authorization,
- domain: &'d AcmeDomain,
- task: Arc<WorkerTask>,
- ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>> {
- Box::pin(async move {
- let result = self
- .action(client, authorization, domain, task.clone(), "setup")
- .await;
-
- let validation_delay = self.core.validation_delay.unwrap_or(30) as u64;
- if validation_delay > 0 {
- task.log_message(format!(
- "Sleeping {validation_delay} seconds to wait for TXT record propagation"
- ));
- tokio::time::sleep(Duration::from_secs(validation_delay)).await;
- }
- result
- })
- }
-
- fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
- &'a mut self,
- client: &'b mut AcmeClient,
- authorization: &'c Authorization,
- domain: &'d AcmeDomain,
- task: Arc<WorkerTask>,
- ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>> {
- Box::pin(async move {
- self.action(client, authorization, domain, task, "teardown")
- .await
- .map(drop)
- })
- }
-}
-
-#[derive(Default)]
-struct StandaloneServer {
- abort_handle: Option<futures::future::AbortHandle>,
-}
-
-// In case the "order_certificates" future gets dropped between setup & teardown, let's also cancel
-// the HTTP listener on Drop:
-impl Drop for StandaloneServer {
- fn drop(&mut self) {
- self.stop();
- }
-}
-
-impl StandaloneServer {
- fn stop(&mut self) {
- if let Some(abort) = self.abort_handle.take() {
- abort.abort();
- }
- }
-}
-
-async fn standalone_respond(
- req: Request<Incoming>,
- path: Arc<String>,
- key_auth: Arc<String>,
-) -> Result<Response<Full<Bytes>>, hyper::Error> {
- if req.method() == hyper::Method::GET && req.uri().path() == path.as_str() {
- Ok(Response::builder()
- .status(hyper::http::StatusCode::OK)
- .body(key_auth.as_bytes().to_vec().into())
- .unwrap())
- } else {
- Ok(Response::builder()
- .status(hyper::http::StatusCode::NOT_FOUND)
- .body("Not found.".into())
- .unwrap())
- }
-}
-
-impl AcmePlugin for StandaloneServer {
- fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
- &'a mut self,
- client: &'b mut AcmeClient,
- authorization: &'c Authorization,
- _domain: &'d AcmeDomain,
- _task: Arc<WorkerTask>,
- ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>> {
- Box::pin(async move {
- self.stop();
-
- let challenge = extract_challenge(authorization, "http-01")?;
- let token = challenge
- .token()
- .ok_or_else(|| format_err!("missing token in challenge"))?;
- let key_auth = Arc::new(client.key_authorization(token)?);
- let path = Arc::new(format!("/.well-known/acme-challenge/{token}"));
-
- // `[::]:80` first, then `*:80`
- let dual = SocketAddr::new(IpAddr::from([0u16; 8]), 80);
- let ipv4 = SocketAddr::new(IpAddr::from([0u8; 4]), 80);
- let incoming = TcpListener::bind(dual)
- .or_else(|_| TcpListener::bind(ipv4))
- .await?;
-
- let server = async move {
- loop {
- let key_auth = Arc::clone(&key_auth);
- let path = Arc::clone(&path);
- match incoming.accept().await {
- Ok((tcp, _)) => {
- let io = TokioIo::new(tcp);
- let service = service_fn(move |request| {
- standalone_respond(
- request,
- Arc::clone(&path),
- Arc::clone(&key_auth),
- )
- });
-
- tokio::task::spawn(async move {
- if let Err(err) =
- http1::Builder::new().serve_connection(io, service).await
- {
- println!("Error serving connection: {err:?}");
- }
- });
- }
- Err(err) => println!("Error accepting connection: {err:?}"),
- }
- }
- };
- let (future, abort) = futures::future::abortable(server);
- self.abort_handle = Some(abort);
- tokio::spawn(future);
-
- Ok(challenge.url.as_str())
- })
- }
-
- fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
- &'a mut self,
- _client: &'b mut AcmeClient,
- _authorization: &'c Authorization,
- _domain: &'d AcmeDomain,
- _task: Arc<WorkerTask>,
- ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>> {
- Box::pin(async move {
- if let Some(abort) = self.abort_handle.take() {
- abort.abort();
- }
- Ok(())
- })
- }
-}
diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
index 31196715..2a645b4a 100644
--- a/src/api2/node/certificates.rs
+++ b/src/api2/node/certificates.rs
@@ -1,27 +1,19 @@
-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;
-use proxmox_router::list_subdirs_api_method;
-use proxmox_router::SubdirMap;
-use proxmox_router::{Permission, Router, RpcEnvironment};
-use proxmox_schema::api;
-
+use crate::server::send_certificate_renewal_mail;
use pbs_api_types::{NODE_SCHEMA, PRIV_SYS_MODIFY};
use pbs_buildcfg::configdir;
use pbs_tools::cert;
-use tracing::warn;
-
-use crate::api2::types::AcmeDomain;
-use crate::config::node::NodeConfig;
-use crate::server::send_certificate_renewal_mail;
-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;
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
pub const ROUTER: Router = Router::new()
.get(&list_subdirs_api_method!(SUBDIRS))
@@ -269,193 +261,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: {
@@ -525,9 +330,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")?
+ };
+
+ 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?;
}
@@ -563,16 +389,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")?
+ };
+
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/api2/types/acme.rs b/src/api2/types/acme.rs
deleted file mode 100644
index 2905b41b..00000000
--- a/src/api2/types/acme.rs
+++ /dev/null
@@ -1,74 +0,0 @@
-use serde::{Deserialize, Serialize};
-use serde_json::Value;
-
-use proxmox_schema::{api, ApiStringFormat, ApiType, Schema, StringSchema};
-
-use pbs_api_types::{DNS_ALIAS_FORMAT, DNS_NAME_FORMAT, PROXMOX_SAFE_ID_FORMAT};
-
-#[api(
- properties: {
- "domain": { format: &DNS_NAME_FORMAT },
- "alias": {
- optional: true,
- format: &DNS_ALIAS_FORMAT,
- },
- "plugin": {
- optional: true,
- format: &PROXMOX_SAFE_ID_FORMAT,
- },
- },
- default_key: "domain",
-)]
-#[derive(Deserialize, Serialize)]
-/// A domain entry for an ACME certificate.
-pub struct AcmeDomain {
- /// The domain to certify for.
- pub domain: String,
-
- /// The domain to use for challenges instead of the default acme challenge domain.
- ///
- /// This is useful if you use CNAME entries to redirect `_acme-challenge.*` domains to a
- /// different DNS server.
- #[serde(skip_serializing_if = "Option::is_none")]
- pub alias: Option<String>,
-
- /// The plugin to use to validate this domain.
- ///
- /// Empty means standalone HTTP validation is used.
- #[serde(skip_serializing_if = "Option::is_none")]
- pub plugin: Option<String>,
-}
-
-pub const ACME_DOMAIN_PROPERTY_SCHEMA: Schema =
- StringSchema::new("ACME domain configuration string")
- .format(&ApiStringFormat::PropertyString(&AcmeDomain::API_SCHEMA))
- .schema();
-
-#[api(
- properties: {
- schema: {
- type: Object,
- additional_properties: true,
- properties: {},
- },
- type: {
- type: String,
- },
- },
-)]
-#[derive(Serialize)]
-/// Schema for an ACME challenge plugin.
-pub struct AcmeChallengeSchema {
- /// Plugin ID.
- pub id: String,
-
- /// Human readable name, falls back to id.
- pub name: String,
-
- /// Plugin Type.
- #[serde(rename = "type")]
- pub ty: &'static str,
-
- /// The plugin's parameter schema.
- pub schema: Value,
-}
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index afc34b30..34193685 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -4,9 +4,6 @@ use anyhow::bail;
use proxmox_schema::*;
-mod acme;
-pub use acme::*;
-
// File names: may not contain slashes, may not start with "."
pub const FILENAME_FORMAT: ApiStringFormat = ApiStringFormat::VerifyFn(|name| {
if name.starts_with('.') {
diff --git a/src/config/acme/mod.rs b/src/config/acme/mod.rs
index 35cda50b..afd7abf8 100644
--- a/src/config/acme/mod.rs
+++ b/src/config/acme/mod.rs
@@ -9,8 +9,7 @@ use proxmox_sys::fs::{file_read_string, CreateOptions};
use pbs_api_types::PROXMOX_SAFE_ID_REGEX;
-use crate::api2::types::AcmeChallengeSchema;
-use proxmox_acme_api::{AcmeAccountName, KnownAcmeDirectory, KNOWN_ACME_DIRECTORIES};
+use proxmox_acme_api::{AcmeAccountName, AcmeChallengeSchema};
pub(crate) const ACME_DIR: &str = pbs_buildcfg::configdir!("/acme");
pub(crate) const ACME_ACCOUNT_DIR: &str = pbs_buildcfg::configdir!("/acme/accounts");
@@ -35,8 +34,6 @@ pub(crate) fn make_acme_dir() -> Result<(), Error> {
create_acme_subdir(ACME_DIR)
}
-pub const DEFAULT_ACME_DIRECTORY_ENTRY: &KnownAcmeDirectory = &KNOWN_ACME_DIRECTORIES[0];
-
pub fn foreach_acme_account<F>(mut func: F) -> Result<(), Error>
where
F: FnMut(AcmeAccountName) -> ControlFlow<Result<(), Error>>,
@@ -80,7 +77,7 @@ pub fn load_dns_challenge_schema() -> Result<Vec<AcmeChallengeSchema>, Error> {
.and_then(Value::as_str)
.unwrap_or(id)
.to_owned(),
- ty: "dns",
+ ty: "dns".into(),
schema: schema.to_owned(),
})
.collect())
diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
index 18e71199..2e979ffe 100644
--- a/src/config/acme/plugin.rs
+++ b/src/config/acme/plugin.rs
@@ -1,104 +1,15 @@
use std::sync::LazyLock;
use anyhow::Error;
-use serde::{Deserialize, Serialize};
-use serde_json::Value;
-
-use proxmox_schema::{api, ApiType, Schema, StringSchema, Updater};
-use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
-
-use pbs_api_types::PROXMOX_SAFE_ID_FORMAT;
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();
+use proxmox_acme_api::PLUGIN_ID_SCHEMA;
+use proxmox_acme_api::{DnsPlugin, StandalonePlugin};
+use proxmox_schema::{ApiType, Schema};
+use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
+use serde_json::Value;
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 d2a17a49..b9257adf 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -6,17 +6,17 @@ use serde::{Deserialize, Serialize};
use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
-use proxmox_http::ProxyConfig;
-
use pbs_api_types::{
EMAIL_SCHEMA, MULTI_LINE_COMMENT_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA,
OPENSSL_CIPHERS_TLS_1_3_SCHEMA,
};
+use proxmox_acme_api::{AcmeConfig, AcmeDomain, ACME_DOMAIN_PROPERTY_SCHEMA};
+use proxmox_http::ProxyConfig;
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;
use proxmox_acme::async_client::AcmeClient;
use proxmox_acme_api::AcmeAccountName;
@@ -45,20 +45,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
@@ -244,7 +230,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
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH proxmox v4 1/4] acme-api: add helper to load client for an account
2025-12-03 10:22 [pbs-devel] [PATCH proxmox{-backup, } v4 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (3 preceding siblings ...)
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox-backup v4 4/4] acme: certificate ordering through proxmox-acme-api Samuel Rufinatscha
@ 2025-12-03 10:22 ` Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox v4 2/4] acme: reduce visibility of Request type Samuel Rufinatscha
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Samuel Rufinatscha @ 2025-12-03 10:22 UTC (permalink / raw)
To: pbs-devel
The PBS ACME refactoring needs a simple way to obtain an AcmeClient for
a given configured account without duplicating config wiring. This patch
adds a load_client_with_account helper in proxmox-acme-api that loads
the account and constructs a matching client, similarly as PBS previous
own AcmeClient::load() function.
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
proxmox-acme-api/src/account_api_impl.rs | 5 +++++
proxmox-acme-api/src/lib.rs | 3 ++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/proxmox-acme-api/src/account_api_impl.rs b/proxmox-acme-api/src/account_api_impl.rs
index ef195908..ca8c8655 100644
--- a/proxmox-acme-api/src/account_api_impl.rs
+++ b/proxmox-acme-api/src/account_api_impl.rs
@@ -116,3 +116,8 @@ pub async fn update_account(name: &AcmeAccountName, contact: Option<String>) ->
Ok(())
}
+
+pub async fn load_client_with_account(account_name: &AcmeAccountName) -> Result<AcmeClient, Error> {
+ let account_data = super::account_config::load_account_config(&account_name).await?;
+ Ok(account_data.client())
+}
diff --git a/proxmox-acme-api/src/lib.rs b/proxmox-acme-api/src/lib.rs
index 623e9e23..96f88ae2 100644
--- a/proxmox-acme-api/src/lib.rs
+++ b/proxmox-acme-api/src/lib.rs
@@ -31,7 +31,8 @@ mod plugin_config;
mod account_api_impl;
#[cfg(feature = "impl")]
pub use account_api_impl::{
- deactivate_account, get_account, get_tos, list_accounts, register_account, update_account,
+ deactivate_account, get_account, get_tos, list_accounts, load_client_with_account,
+ register_account, update_account,
};
#[cfg(feature = "impl")]
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH proxmox v4 2/4] acme: reduce visibility of Request type
2025-12-03 10:22 [pbs-devel] [PATCH proxmox{-backup, } v4 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (4 preceding siblings ...)
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox v4 1/4] acme-api: add helper to load client for an account Samuel Rufinatscha
@ 2025-12-03 10:22 ` Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox v4 3/4] acme: introduce http_status module Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox v4 4/4] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
7 siblings, 0 replies; 9+ messages in thread
From: Samuel Rufinatscha @ 2025-12-03 10:22 UTC (permalink / raw)
To: pbs-devel
Currently, the low-level ACME Request type is publicly exposed, even
though users are expected to go through AcmeClient and
proxmox-acme-api handlers. This patch reduces visibility so that
the Request type and related fields/methods are crate-internal only.
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
proxmox-acme/src/account.rs | 17 ++++++++++-------
proxmox-acme/src/async_client.rs | 2 +-
proxmox-acme/src/authorization.rs | 2 +-
proxmox-acme/src/client.rs | 6 +++---
proxmox-acme/src/lib.rs | 4 ----
proxmox-acme/src/order.rs | 2 +-
proxmox-acme/src/request.rs | 12 ++++++------
7 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/proxmox-acme/src/account.rs b/proxmox-acme/src/account.rs
index 0bbf0027..081ca986 100644
--- a/proxmox-acme/src/account.rs
+++ b/proxmox-acme/src/account.rs
@@ -92,7 +92,7 @@ impl Account {
}
/// Prepare a "POST-as-GET" request to fetch data. Low level helper.
- pub fn get_request(&self, url: &str, nonce: &str) -> Result<Request, Error> {
+ pub(crate) fn get_request(&self, url: &str, nonce: &str) -> Result<Request, Error> {
let key = PKey::private_key_from_pem(self.private_key.as_bytes())?;
let body = serde_json::to_string(&Jws::new_full(
&key,
@@ -112,7 +112,7 @@ impl Account {
}
/// Prepare a JSON POST request. Low level helper.
- pub fn post_request<T: Serialize>(
+ pub(crate) fn post_request<T: Serialize>(
&self,
url: &str,
nonce: &str,
@@ -179,7 +179,7 @@ impl Account {
/// Prepare a request to update account data.
///
/// This is a rather low level interface. You should know what you're doing.
- pub fn update_account_request<T: Serialize>(
+ pub(crate) fn update_account_request<T: Serialize>(
&self,
nonce: &str,
data: &T,
@@ -188,7 +188,10 @@ impl Account {
}
/// Prepare a request to deactivate this account.
- pub fn deactivate_account_request<T: Serialize>(&self, nonce: &str) -> Result<Request, Error> {
+ pub(crate) fn deactivate_account_request<T: Serialize>(
+ &self,
+ nonce: &str,
+ ) -> Result<Request, Error> {
self.post_request_raw_payload(
&self.location,
nonce,
@@ -220,7 +223,7 @@ impl Account {
///
/// This returns a raw `Request` since validation takes some time and the `Authorization`
/// object has to be re-queried and its `status` inspected.
- pub fn validate_challenge(
+ pub(crate) fn validate_challenge(
&self,
authorization: &Authorization,
challenge_index: usize,
@@ -274,7 +277,7 @@ pub struct CertificateRevocation<'a> {
impl CertificateRevocation<'_> {
/// Create the revocation request using the specified nonce for the given directory.
- pub fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
+ pub(crate) fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
let revoke_cert = directory.data.revoke_cert.as_ref().ok_or_else(|| {
Error::Custom("no 'revokeCert' URL specified by provider".to_string())
})?;
@@ -364,7 +367,7 @@ impl AccountCreator {
/// the resulting request.
/// Changing the private key between using the request and passing the response to
/// [`response`](AccountCreator::response()) will render the account unusable!
- pub fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
+ pub(crate) fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
let key = self.key.as_deref().ok_or(Error::MissingKey)?;
let url = directory.new_account_url().ok_or_else(|| {
Error::Custom("no 'newAccount' URL specified by provider".to_string())
diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
index dc755fb9..2ff3ba22 100644
--- a/proxmox-acme/src/async_client.rs
+++ b/proxmox-acme/src/async_client.rs
@@ -10,7 +10,7 @@ use proxmox_http::{client::Client, Body};
use crate::account::AccountCreator;
use crate::order::{Order, OrderData};
-use crate::Request as AcmeRequest;
+use crate::request::Request as AcmeRequest;
use crate::{Account, Authorization, Challenge, Directory, Error, ErrorResponse};
/// A non-blocking Acme client using tokio/hyper.
diff --git a/proxmox-acme/src/authorization.rs b/proxmox-acme/src/authorization.rs
index 28bc1b4b..765714fc 100644
--- a/proxmox-acme/src/authorization.rs
+++ b/proxmox-acme/src/authorization.rs
@@ -145,7 +145,7 @@ pub struct GetAuthorization {
/// this is guaranteed to be `Some`.
///
/// The response should be passed to the the [`response`](GetAuthorization::response()) method.
- pub request: Option<Request>,
+ pub(crate) request: Option<Request>,
}
impl GetAuthorization {
diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
index 931f7245..5c812567 100644
--- a/proxmox-acme/src/client.rs
+++ b/proxmox-acme/src/client.rs
@@ -7,8 +7,8 @@ use serde::{Deserialize, Serialize};
use crate::b64u;
use crate::error;
use crate::order::OrderData;
-use crate::request::ErrorResponse;
-use crate::{Account, Authorization, Challenge, Directory, Error, Order, Request};
+use crate::request::{ErrorResponse, Request};
+use crate::{Account, Authorization, Challenge, Directory, Error, Order};
macro_rules! format_err {
($($fmt:tt)*) => { Error::Client(format!($($fmt)*)) };
@@ -564,7 +564,7 @@ impl Client {
}
/// Low-level API to run an n API request. This automatically updates the current nonce!
- pub fn run_request(&mut self, request: Request) -> Result<HttpResponse, Error> {
+ pub(crate) fn run_request(&mut self, request: Request) -> Result<HttpResponse, Error> {
self.inner.run_request(request)
}
diff --git a/proxmox-acme/src/lib.rs b/proxmox-acme/src/lib.rs
index df722629..6722030c 100644
--- a/proxmox-acme/src/lib.rs
+++ b/proxmox-acme/src/lib.rs
@@ -66,10 +66,6 @@ pub use error::Error;
#[doc(inline)]
pub use order::Order;
-#[cfg(feature = "impl")]
-#[doc(inline)]
-pub use request::Request;
-
// we don't inline these:
#[cfg(feature = "impl")]
pub use order::NewOrder;
diff --git a/proxmox-acme/src/order.rs b/proxmox-acme/src/order.rs
index b6551004..432a81a4 100644
--- a/proxmox-acme/src/order.rs
+++ b/proxmox-acme/src/order.rs
@@ -153,7 +153,7 @@ pub struct NewOrder {
//order: OrderData,
/// The request to execute to place the order. When creating a [`NewOrder`] via
/// [`Account::new_order`](crate::Account::new_order) this is guaranteed to be `Some`.
- pub request: Option<Request>,
+ pub(crate) request: Option<Request>,
}
impl NewOrder {
diff --git a/proxmox-acme/src/request.rs b/proxmox-acme/src/request.rs
index 78a90913..dadfc5af 100644
--- a/proxmox-acme/src/request.rs
+++ b/proxmox-acme/src/request.rs
@@ -4,21 +4,21 @@ pub(crate) const JSON_CONTENT_TYPE: &str = "application/jose+json";
pub(crate) const CREATED: u16 = 201;
/// A request which should be performed on the ACME provider.
-pub struct Request {
+pub(crate) struct Request {
/// The complete URL to send the request to.
- pub url: String,
+ pub(crate) url: String,
/// The HTTP method name to use.
- pub method: &'static str,
+ pub(crate) method: &'static str,
/// The `Content-Type` header to pass along.
- pub content_type: &'static str,
+ pub(crate) content_type: &'static str,
/// The body to pass along with request, or an empty string.
- pub body: String,
+ pub(crate) body: String,
/// The expected status code a compliant ACME provider will return on success.
- pub expected: u16,
+ pub(crate) expected: u16,
}
/// An ACME error response contains a specially formatted type string, and can optionally
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH proxmox v4 3/4] acme: introduce http_status module
2025-12-03 10:22 [pbs-devel] [PATCH proxmox{-backup, } v4 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (5 preceding siblings ...)
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox v4 2/4] acme: reduce visibility of Request type Samuel Rufinatscha
@ 2025-12-03 10:22 ` Samuel Rufinatscha
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox v4 4/4] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
7 siblings, 0 replies; 9+ messages in thread
From: Samuel Rufinatscha @ 2025-12-03 10:22 UTC (permalink / raw)
To: pbs-devel
Introduce an internal http_status module with the common ACME HTTP
response codes, and replace use of crate::request::CREATED as well as
direct numeric status code usages.
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
proxmox-acme/src/account.rs | 10 +++++-----
proxmox-acme/src/async_client.rs | 4 ++--
proxmox-acme/src/lib.rs | 2 ++
proxmox-acme/src/request.rs | 11 ++++++++++-
4 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/proxmox-acme/src/account.rs b/proxmox-acme/src/account.rs
index 081ca986..350c78d4 100644
--- a/proxmox-acme/src/account.rs
+++ b/proxmox-acme/src/account.rs
@@ -85,7 +85,7 @@ impl Account {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: crate::request::CREATED,
+ expected: crate::http_status::CREATED,
};
Ok(NewOrder::new(request))
@@ -107,7 +107,7 @@ impl Account {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: 200,
+ expected: crate::http_status::OK,
})
}
@@ -132,7 +132,7 @@ impl Account {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: 200,
+ expected: crate::http_status::OK,
})
}
@@ -157,7 +157,7 @@ impl Account {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: 200,
+ expected: crate::http_status::OK,
})
}
@@ -408,7 +408,7 @@ impl AccountCreator {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: crate::request::CREATED,
+ expected: crate::http_status::CREATED,
})
}
diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
index 2ff3ba22..043648bb 100644
--- a/proxmox-acme/src/async_client.rs
+++ b/proxmox-acme/src/async_client.rs
@@ -498,7 +498,7 @@ impl AcmeClient {
method: "GET",
content_type: "",
body: String::new(),
- expected: 200,
+ expected: crate::http_status::OK,
},
nonce,
)
@@ -550,7 +550,7 @@ impl AcmeClient {
method: "HEAD",
content_type: "",
body: String::new(),
- expected: 200,
+ expected: crate::http_status::OK,
},
nonce,
)
diff --git a/proxmox-acme/src/lib.rs b/proxmox-acme/src/lib.rs
index 6722030c..6051a025 100644
--- a/proxmox-acme/src/lib.rs
+++ b/proxmox-acme/src/lib.rs
@@ -70,6 +70,8 @@ pub use order::Order;
#[cfg(feature = "impl")]
pub use order::NewOrder;
#[cfg(feature = "impl")]
+pub(crate) use request::http_status;
+#[cfg(feature = "impl")]
pub use request::ErrorResponse;
/// Header name for nonces.
diff --git a/proxmox-acme/src/request.rs b/proxmox-acme/src/request.rs
index dadfc5af..341ce53e 100644
--- a/proxmox-acme/src/request.rs
+++ b/proxmox-acme/src/request.rs
@@ -1,7 +1,6 @@
use serde::Deserialize;
pub(crate) const JSON_CONTENT_TYPE: &str = "application/jose+json";
-pub(crate) const CREATED: u16 = 201;
/// A request which should be performed on the ACME provider.
pub(crate) struct Request {
@@ -21,6 +20,16 @@ pub(crate) struct Request {
pub(crate) expected: u16,
}
+/// Common HTTP status codes used in ACME responses.
+pub(crate) mod http_status {
+ /// 200 OK
+ pub(crate) const OK: u16 = 200;
+ /// 201 Created
+ pub(crate) const CREATED: u16 = 201;
+ /// 204 No Content
+ pub(crate) const NO_CONTENT: u16 = 204;
+}
+
/// An ACME error response contains a specially formatted type string, and can optionally
/// contain textual details and a set of sub problems.
#[derive(Clone, Debug, Deserialize)]
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH proxmox v4 4/4] fix #6939: acme: support servers returning 204 for nonce requests
2025-12-03 10:22 [pbs-devel] [PATCH proxmox{-backup, } v4 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (6 preceding siblings ...)
2025-12-03 10:22 ` [pbs-devel] [PATCH proxmox v4 3/4] acme: introduce http_status module Samuel Rufinatscha
@ 2025-12-03 10:22 ` Samuel Rufinatscha
7 siblings, 0 replies; 9+ messages in thread
From: Samuel Rufinatscha @ 2025-12-03 10:22 UTC (permalink / raw)
To: pbs-devel
Some ACME servers (notably custom or legacy implementations) respond
to HEAD /newNonce with a 204 No Content instead of the
RFC 8555-recommended 200 OK [1]. While this behavior is technically
off-spec, it is not illegal. This issue was reported on our bug
tracker [2].
The previous implementation treated any non-200 response as an error,
causing account registration to fail against such servers. Relax the
status-code check to accept both 200 and 204 responses (and potentially
support other 2xx codes) to improve interoperability.
Note: In comparison, PVE’s Perl ACME client performs a GET request [3]
instead of a HEAD request and accepts any 2xx success code when
retrieving the nonce [4]. This difference in behavior does not affect
functionality but is worth noting for consistency across
implementations.
[1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
[2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939
[3] https://git.proxmox.com/?p=proxmox-acme.git;a=blob;f=src/PVE/ACME.pm;h=f1e9bb7d316e3cea1e376c610b0479119217aecc;hb=HEAD#l219
[4] https://git.proxmox.com/?p=proxmox-acme.git;a=blob;f=src/PVE/ACME.pm;h=f1e9bb7d316e3cea1e376c610b0479119217aecc;hb=HEAD#l597
Fixes: #6939
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
proxmox-acme/src/account.rs | 10 +++++-----
proxmox-acme/src/async_client.rs | 6 +++---
proxmox-acme/src/client.rs | 2 +-
proxmox-acme/src/request.rs | 4 ++--
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/proxmox-acme/src/account.rs b/proxmox-acme/src/account.rs
index 350c78d4..820b209d 100644
--- a/proxmox-acme/src/account.rs
+++ b/proxmox-acme/src/account.rs
@@ -85,7 +85,7 @@ impl Account {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: crate::http_status::CREATED,
+ expected: &[crate::http_status::CREATED],
};
Ok(NewOrder::new(request))
@@ -107,7 +107,7 @@ impl Account {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: crate::http_status::OK,
+ expected: &[crate::http_status::OK],
})
}
@@ -132,7 +132,7 @@ impl Account {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: crate::http_status::OK,
+ expected: &[crate::http_status::OK],
})
}
@@ -157,7 +157,7 @@ impl Account {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: crate::http_status::OK,
+ expected: &[crate::http_status::OK],
})
}
@@ -408,7 +408,7 @@ impl AccountCreator {
method: "POST",
content_type: crate::request::JSON_CONTENT_TYPE,
body,
- expected: crate::http_status::CREATED,
+ expected: &[crate::http_status::CREATED],
})
}
diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
index 043648bb..07da842c 100644
--- a/proxmox-acme/src/async_client.rs
+++ b/proxmox-acme/src/async_client.rs
@@ -420,7 +420,7 @@ impl AcmeClient {
};
if parts.status.is_success() {
- if status != request.expected {
+ if !request.expected.contains(&status) {
return Err(Error::InvalidApi(format!(
"ACME server responded with unexpected status code: {:?}",
parts.status
@@ -498,7 +498,7 @@ impl AcmeClient {
method: "GET",
content_type: "",
body: String::new(),
- expected: crate::http_status::OK,
+ expected: &[crate::http_status::OK],
},
nonce,
)
@@ -550,7 +550,7 @@ impl AcmeClient {
method: "HEAD",
content_type: "",
body: String::new(),
- expected: crate::http_status::OK,
+ expected: &[crate::http_status::OK, crate::http_status::NO_CONTENT],
},
nonce,
)
diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
index 5c812567..af250fb8 100644
--- a/proxmox-acme/src/client.rs
+++ b/proxmox-acme/src/client.rs
@@ -203,7 +203,7 @@ impl Inner {
let got_nonce = self.update_nonce(&mut response)?;
if response.is_success() {
- if response.status != request.expected {
+ if !request.expected.contains(&response.status) {
return Err(Error::InvalidApi(format!(
"API server responded with unexpected status code: {:?}",
response.status
diff --git a/proxmox-acme/src/request.rs b/proxmox-acme/src/request.rs
index 341ce53e..d782a7de 100644
--- a/proxmox-acme/src/request.rs
+++ b/proxmox-acme/src/request.rs
@@ -16,8 +16,8 @@ pub(crate) struct Request {
/// The body to pass along with request, or an empty string.
pub(crate) body: String,
- /// The expected status code a compliant ACME provider will return on success.
- pub(crate) expected: u16,
+ /// The set of HTTP status codes that indicate a successful response from an ACME provider.
+ pub(crate) expected: &'static [u16],
}
/// Common HTTP status codes used in ACME responses.
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread