* [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests
@ 2025-12-02 15:56 Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 1/4] acme: include proxmox-acme-api dependency Samuel Rufinatscha
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 UTC (permalink / raw)
To: pbs-devel
Hi,
this series fixes account registration for ACME providers that return
HTTP 204 No Content to the newNonce request. Currently, both the PBS
ACME client and the shared ACME client in proxmox-acme only accept
HTTP 200 OK for this request. The issue was observed in PBS against a
custom ACME deployment and reported as bug #6939 [1].
## Problem
During ACME account registration, PBS first fetches an anti-replay
nonce by sending a HEAD request to the CA’s newNonce URL.
RFC 8555 §7.2 [2] states that:
* the server MUST include a Replay-Nonce header with a fresh nonce,
* the server SHOULD use status 200 OK for the HEAD request,
* the server MUST also handle GET on the same resource and may return
204 No Content with an empty body.
The reporter observed the following error message:
*ACME server responded with unexpected status code: 204*
and mentioned that the issue did not appear with PVE 9 [1]. Looking at
PVE’s Perl ACME client [3], it uses a GET request instead of HEAD and
accepts any 2xx success code when retrieving the nonce. This difference
in behavior does not affect functionality but is worth noting for
consistency across implementations.
## Approach
To support ACME providers which return 204 No Content, the Rust ACME
clients in proxmox-backup and proxmox need to treat both 200 OK and 204
No Content as valid responses for the nonce request, as long as a
Replay-Nonce header is present.
This series changes the expected field of the internal Request type
from a single u16 to a list of allowed status codes
(e.g. &'static [u16]), so one request can explicitly accept multiple
success codes.
To avoid fixing the issue twice (once in PBS’ own ACME client and once
in the shared Rust client), this series first refactors PBS to use the
shared AcmeClient from proxmox-acme / proxmox-acme-api, similar to PDM,
and then applies the bug fix in that shared implementation so that all
consumers benefit from the more tolerant behavior.
## Testing
*Testing the refactor*
To test the refactor, I
(1) installed latest stable PBS on a VM
(2) created .deb package from latest PBS (master), containing the
refactor
(3) installed created .deb package
(4) installed Pebble from Let's Encrypt from Let's Encrypt [5] on the
same VM
(5) created an ACME account and ordered the new certificate for the
host domain.
Steps to reproduce:
(1) install latest stable PBS on a VM, created .deb package from latest
PBS (master) containing the refactor, install created .deb package
(2) install Pebble from Let's Encrypt from Let's Encrypt [5] on the
same VM:
cd
apt update
apt install -y golang git
git clone https://github.com/letsencrypt/pebble
cd pebble
go build ./cmd/pebble
then, downloaded and trusted the Pebble cert:
wget https://raw.githubusercontent.com/letsencrypt/pebble/main/test/certs/pebble.minica.pem
cp pebble.minica.pem /usr/local/share/ca-certificates/pebble.minica.crt
update-ca-certificates
We want Pebble to perform HTTP-01 validation against port 80, because
PBS’s standalone plugin will bind port 80. Set httpPort to 80.
nano ./test/config/pebble-config.json
Started the Pebble server in the background:
./pebble -config ./test/config/pebble-config.json &
Created a Pebble ACME account:
proxmox-backup-manager acme account register default admin@example.com \
--directory 'https://127.0.0.1:14000/dir'
To verify persistence of the account I checked
ls /etc/proxmox-backup/acme/accounts
Verified if update-account works
proxmox-backup-manager acme account update default --contact "a@example.com,b@example.com"
proxmox-backup-manager acme account info default
In the PBS GUI, you can create a new domain. You can use your host
domain name (see /etc/hosts). Select the created account and order the
certificate.
After a page reload, you might need to accept the new certificate in the browser.
In the PBS dashboard, you should then see the new Pebble certificate.
*Note: on reboot, the created Pebble ACME account will be gone and you
will need to create a new one. Pebble does not persist account info.
In that case remove your previously created account in
/etc/proxmox-backup/acme/accounts.
*Testing the newNonce fix*
To prove the ACME newNonce fix, I put nginx in front of Pebble, to
intercept the newNonce request in order to return 204 No Content
instead of 200 OK, all other requests are unchanged and forwarded to
Pebble. Requires trusting the nginx CAs via
/usr/local/share/ca-certificates + update-ca-certificates on the VM.
Then I ran following command against nginx:
proxmox-backup-manager acme account register proxytest root@backup.local --directory 'https://nginx-address/dir
The account could be created successfully. When adjusting the nginx
configuration to return any other non-expected success status code,
PBS expectely rejects.
## Patch summary
[PATCH proxmox-backup v4 1/4] acme: include proxmox-acme-api dependency
[PATCH proxmox-backup v4 2/4] acme: drop local AcmeClient
[PATCH proxmox-backup v4 3/4] acme: change API impls to use proxmox-acme-api handlers
[PATCH proxmox-backup v4 4/4] acme: certificate ordering through proxmox-acme-api
[PATCH proxmox v4 1/4] acme: reduce visibility of Request type
[PATCH proxmox v4 2/4] acme: introduce http_status module
[PATCH proxmox v4 3/4] acme-api: add helper to load client for an account
[PATCH proxmox v4 4/4] fix #6939: support servers returning 204 for newNonce
Thanks for considering this patch series, I look forward to your
feedback.
Best,
Samuel Rufinatscha
## Changes from v1:
[PATCH proxmox v2 1/1] fix #6939: support providers returning 204 for nonce
requests
* Introduced `http_success` module to contain the http success codes
* Replaced `Vec<u16>` with `&[u16]` for expected codes to avoid
allocations.
* Clarified the PVEs Perl ACME client behaviour in the commit message.
[PATCH proxmox-backup v2 1/1] acme: accept HTTP 204 from newNonce endpoint
* Integrated the `http_success` module, replacing `Vec<u16>` with `&[u16]`
* Clarified the PVEs Perl ACME client behaviour in the commit message.
## Changes from v2:
[PATCH proxmox v3 1/1] fix #6939: support providers returning 204 for nonce
requests
* Rename `http_success` module to `http_status`
[PATCH proxmox-backup v3 1/1] acme: accept HTTP 204 from newNonce endpoint
* Replace `http_success` usage
## Changes from v3:
Removed: [PATCH proxmox-backup v3 1/1].
Added:
[PATCH proxmox-backup v4 1/4] acme: include proxmox-acme-api dependency
* New: add proxmox-acme-api as a dependency and initialize it in
PBS so PBS can use the shared ACME API instead.
[PATCH proxmox-backup v4 2/4] acme: drop local AcmeClient
* New: remove the PBS-local AcmeClient implementation and switch PBS
over to the shared proxmox-acme async client.
[PATCH proxmox-backup v4 3/4] acme: change API impls to use proxmox-acme-api
handlers
* New: rework PBS’ ACME API endpoints to delegate to
proxmox-acme-api handlers instead of duplicating logic locally.
[PATCH proxmox-backup v4 4/4] acme: certificate ordering through
proxmox-acme-api
* New: move PBS’ ACME certificate ordering logic over to
proxmox-acme-api, keeping only certificate installation/reload in
PBS.
[PATCH proxmox v4 1/4] acme: reduce visibility of Request type
* New: hide the low-level Request type and its fields behind
constructors / reduced visibility so changes to “expected” no longer
affect the public API as they did in v3.
[PATCH proxmox v4 2/4] acme: introduce http_status module
* New: split out the HTTP status constants into an internal
http_status module as a separate preparatory cleanup before the bug
fix, instead of doing this inline like in v3.
[PATCH proxmox v4 3/4] acme-api: add helper to load client for an account
* New: add a load_client_with_account helper in proxmox-acme-api so
PBS (and others) can construct an AcmeClient for a configured account
without duplicating boilerplate.
Changed:
[PATCH proxmox v3 1/1] -> [PATCH proxmox v4 4/4]
fix #6939: acme: support server returning 204 for nonce requests
* Rebased on top of the refactor: keep the same behavioural fix as in v3
(accept 204 for newNonce with Replay-Nonce present), but implement it
on top of the http_status module that is part of the refactor.
proxmox-backup:
Samuel Rufinatscha (4):
acme: include proxmox-acme-api dependency
acme: drop local AcmeClient
acme: change API impls to use proxmox-acme-api handlers
acme: certificate ordering through proxmox-acme-api
Cargo.toml | 3 +
src/acme/client.rs | 691 -------------------------
src/acme/mod.rs | 5 -
src/acme/plugin.rs | 336 ------------
src/api2/config/acme.rs | 407 ++-------------
src/api2/node/certificates.rs | 240 ++-------
src/api2/types/acme.rs | 98 ----
src/api2/types/mod.rs | 3 -
src/bin/proxmox-backup-api.rs | 2 +
src/bin/proxmox-backup-manager.rs | 2 +
src/bin/proxmox-backup-proxy.rs | 1 +
src/bin/proxmox_backup_manager/acme.rs | 21 +-
src/config/acme/mod.rs | 51 +-
src/config/acme/plugin.rs | 99 +---
src/config/node.rs | 29 +-
src/lib.rs | 2 -
16 files changed, 103 insertions(+), 1887 deletions(-)
delete mode 100644 src/acme/client.rs
delete mode 100644 src/acme/mod.rs
delete mode 100644 src/acme/plugin.rs
delete mode 100644 src/api2/types/acme.rs
proxmox:
Samuel Rufinatscha (4):
acme: reduce visibility of Request type
acme: introduce http_status module
acme-api: add helper to load client for an account
fix #6939: acme: support servers returning 204 for nonce requests
proxmox-acme-api/src/account_api_impl.rs | 5 +++++
proxmox-acme-api/src/lib.rs | 3 ++-
proxmox-acme/src/account.rs | 27 +++++++++++++-----------
proxmox-acme/src/async_client.rs | 8 +++----
proxmox-acme/src/authorization.rs | 2 +-
proxmox-acme/src/client.rs | 8 +++----
proxmox-acme/src/lib.rs | 6 ++----
proxmox-acme/src/order.rs | 2 +-
proxmox-acme/src/request.rs | 25 +++++++++++++++-------
9 files changed, 51 insertions(+), 35 deletions(-)
Summary over all repositories:
25 files changed, 154 insertions(+), 1922 deletions(-)
--
Generated by git-murpp 0.8.1
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/4] acme: include proxmox-acme-api dependency
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
@ 2025-12-02 15:56 ` Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 2/4] acme: drop local AcmeClient Samuel Rufinatscha
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 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] 10+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/4] acme: drop local AcmeClient
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 1/4] acme: include proxmox-acme-api dependency Samuel Rufinatscha
@ 2025-12-02 15:56 ` Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 3/4] acme: change API impls to use proxmox-acme-api handlers Samuel Rufinatscha
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 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] 10+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 3/4] acme: change API impls to use proxmox-acme-api handlers
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 1/4] acme: include proxmox-acme-api dependency Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 2/4] acme: drop local AcmeClient Samuel Rufinatscha
@ 2025-12-02 15:56 ` Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 4/4] acme: certificate ordering through proxmox-acme-api Samuel Rufinatscha
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 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] 10+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 4/4] acme: certificate ordering through proxmox-acme-api
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (2 preceding siblings ...)
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 3/4] acme: change API impls to use proxmox-acme-api handlers Samuel Rufinatscha
@ 2025-12-02 15:56 ` Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 1/4] acme: reduce visibility of Request type Samuel Rufinatscha
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 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] 10+ messages in thread
* [pbs-devel] [PATCH proxmox 1/4] acme: reduce visibility of Request type
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (3 preceding siblings ...)
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 4/4] acme: certificate ordering through proxmox-acme-api Samuel Rufinatscha
@ 2025-12-02 15:56 ` Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 2/4] acme: introduce http_status module Samuel Rufinatscha
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 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] 10+ messages in thread
* [pbs-devel] [PATCH proxmox 2/4] acme: introduce http_status module
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (4 preceding siblings ...)
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 1/4] acme: reduce visibility of Request type Samuel Rufinatscha
@ 2025-12-02 15:56 ` Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 3/4] acme-api: add helper to load client for an account Samuel Rufinatscha
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 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] 10+ messages in thread
* [pbs-devel] [PATCH proxmox 3/4] acme-api: add helper to load client for an account
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (5 preceding siblings ...)
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 2/4] acme: introduce http_status module Samuel Rufinatscha
@ 2025-12-02 15:56 ` Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 4/4] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
2025-12-02 16:02 ` [pbs-devel] [PATCH proxmox{-backup, } 0/8] " Samuel Rufinatscha
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 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] 10+ messages in thread
* [pbs-devel] [PATCH proxmox 4/4] fix #6939: acme: support servers returning 204 for nonce requests
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (6 preceding siblings ...)
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 3/4] acme-api: add helper to load client for an account Samuel Rufinatscha
@ 2025-12-02 15:56 ` Samuel Rufinatscha
2025-12-02 16:02 ` [pbs-devel] [PATCH proxmox{-backup, } 0/8] " Samuel Rufinatscha
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 15:56 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] 10+ messages in thread
* Re: [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
` (7 preceding siblings ...)
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 4/4] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
@ 2025-12-02 16:02 ` Samuel Rufinatscha
8 siblings, 0 replies; 10+ messages in thread
From: Samuel Rufinatscha @ 2025-12-02 16:02 UTC (permalink / raw)
To: pbs-devel
Ignore this please, forgot to add the version in the subject.
Will send a new one.
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-12-02 16:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-02 15:56 [pbs-devel] [PATCH proxmox{-backup, } 0/8] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 1/4] acme: include proxmox-acme-api dependency Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 2/4] acme: drop local AcmeClient Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 3/4] acme: change API impls to use proxmox-acme-api handlers Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox-backup 4/4] acme: certificate ordering through proxmox-acme-api Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 1/4] acme: reduce visibility of Request type Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 2/4] acme: introduce http_status module Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 3/4] acme-api: add helper to load client for an account Samuel Rufinatscha
2025-12-02 15:56 ` [pbs-devel] [PATCH proxmox 4/4] fix #6939: acme: support servers returning 204 for nonce requests Samuel Rufinatscha
2025-12-02 16:02 ` [pbs-devel] [PATCH proxmox{-backup, } 0/8] " Samuel Rufinatscha
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.