[pbs-devel] [PATCH proxmox{, -backup} v3 0/2] fix #6939: acme: support servers returning 204 for nonce requests

[pbs-devel] [PATCH proxmox{, -backup} v3 0/2] fix #6939: acme: support servers returning 204 for nonce requests

[Samuel Rufinatscha]

Hi,

this series proposes a change to ACME account registration in Proxmox
Backup Server (PBS), so that it also works with ACME servers that return
HTTP 204 No Content to the HEAD request for newNonce.

This behaviour was observed against a specific ACME deployment and
reported as bug #6939 [1]. Currently, PBS cannot register an ACME
account for this CA.

## 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]
says:

* 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 with status 204 No
  Content and an empty body [2].

Currently, our Rust ACME clients only accept 200 OK. PBS inherits that
strictness and aborts with:

*ACME server responded with unexpected status code: 204*

The author mentions, the issue did not appear with PVE9 [1].
After looking into PVE’s Perl ACME client [3] it appears it uses a GET
request instead of a HEAD request and accepts any 2xx success code
when retrieving the nonce [5]. This difference in behavior does not
affect functionality but is worth noting for consistency across
implementations.

## Ideas to solve the problem

To support ACME providers which return 204 No Content, the underlying
ACME clients need to tolerate both 200 OK and 204 No Content as valid
responses for the nonce HEAD request, as long as the Replay-Nonce is
provided.

I considered following solutions:

1. Change the `expected` field of the `AcmeRequest` type from `u16` to
   `Vec<u16>`, to support multiple success codes

2. Keep `expected: u16` and add a second field e.g. `expected_other:
   Vec<u16>` for "also allowed" codes.

3. Support any 2xx success codes, and remove the `expected` check

I thought (1) might be reasonable, because:

* It stays explicit and makes it clear which statuses are considered
  success.
* We don’t create two parallel concepts ("expected" vs
  "expected_other") which introduces additional complexity
* Can be extend later if we meet yet another harmless but not 200
  variant.
* We don’t allow arbitrary 2xx.

What do you think? Do you maybe have any other solution in mind that
would fit better?

## Testing

To prove the proposed fix, I reproduced the scenario:

Pebble (release 2.8.0) from Let's Encrypt [5] running on a Debian 9 VM
as the ACME server. 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. Trust the
Pebble and ngix CAs via `/usr/local/share/ca-certificates` +
`update-ca-certificates` on the PBS VM.

Then I ran following command against nginx:

```
proxmox-backup-manager acme account register proxytest root@backup.local
--directory 'https://nginx-address/dir

Attempting to fetch Terms of
Service from "https://acme-vm/dir"
Terms of Service:
data:text/plain,Do%20what%20thou%20wilt
Do you agree to the above terms?
[y|N]: y
Do you want to use external account binding? [y|N]: N
Attempting
to register account with "https://acme-vm/dir"...
Registration
successful, account URL: https://acme-vm/my-account/160e58b66bdd72da
```

When adjusting the nginx configuration to return any other non-expected
success status code, e.g. 205, PBS expectely rejects with `API
misbehaved: ACME server responded with unexpected status code: 205`.

## Maintainer notes:

The patch series involves the following components:

proxmox-acme: Apply PATCH 1 to change `expected` from `u16` to
`Vec<u16>`. This results in a breaking change, as it changes the public
API of the `AcmeRequest` type that is used by other components.

proxmox-acme-api: Needs to depend on the new proxmox-acme; patch bump

proxmox-backup: Apply PATCH 2 to use the new API changes; no breaking
change as of only internal changes; patch bump

proxmox-perl-rs / proxmox-datacenter-manager: Will need to use the
dependency version bumps to follow the new proxmox-acme.

## Patch summary

[PATCH 1/2] fix #6939: support providers returning 204 for nonce
requests

* Make the expected-status logic accept multiple allowed codes.
* Treat both 200 OK and 204 No Content as valid for HEAD /newNonce,
  provided Replay-Nonce is present.
* Keep rejecting other codes.

[PATCH 2/2] acme: accept HTTP 204 from newNonce endpoint

* Use the updated proxmox-acme behavior in PBS.
* PBS can now register an ACME account against servers that return 204
  for the nonce HEAD request.
* Still rejects unexpected codes.

Thanks for considering this patch series, I look forward to your
feedback.

Best,
Samuel Rufinatscha

## Changes from v1:

[PATCH 1/2] 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 2/2] 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 1/2] fix #6939: support providers returning 204 for nonce
requests
    * Rename `http_success` module to `http_status`

[PATCH 2/2] acme: accept HTTP 204 from newNonce endpoint
    * Replace `http_success` usage

[1] Bugzilla report #6939:
[https://bugzilla.proxmox.com/show_bug.cgi?id=6939](https://bugzilla.proxmox.com/show_bug.cgi?id=6939)
[2] RFC 8555 (ACME):
[https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2](https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2)
[3] PVE’s Perl ACME client (allow 2xx codes for nonce requests):
[https://git.proxmox.com/?p=proxmox-acme.git;a=blob;f=src/PVE/ACME.pm;h=f1e9bb7d316e3cea1e376c610b0479119217aecc;hb=HEAD#l597](https://git.proxmox.com/?p=proxmox-acme.git;a=blob;f=src/PVE/ACME.pm;h=f1e9bb7d316e3cea1e376c610b0479119217aecc;hb=HEAD#l597)
[4] Pebble ACME server:
[https://github.com/letsencrypt/pebble](https://github.com/letsencrypt/pebble)
[5] Pebble ACME server (perform GET request:
[https://git.proxmox.com/?p=proxmox-acme.git;a=blob;f=src/PVE/ACME.pm;h=f1e9bb7d316e3cea1e376c610b0479119217aecc;hb=HEAD#l219](https://git.proxmox.com/?p=proxmox-acme.git;a=blob;f=src/PVE/ACME.pm;h=f1e9bb7d316e3cea1e376c610b0479119217aecc;hb=HEAD#l219)

proxmox:

Samuel Rufinatscha (1):
  fix #6939: acme: support servers returning 204 for nonce requests

 proxmox-acme/src/account.rs      | 10 +++++-----
 proxmox-acme/src/async_client.rs |  6 +++---
 proxmox-acme/src/client.rs       |  2 +-
 proxmox-acme/src/lib.rs          |  4 ++++
 proxmox-acme/src/request.rs      | 15 ++++++++++++---
 5 files changed, 25 insertions(+), 12 deletions(-)


proxmox-backup:

Samuel Rufinatscha (1):
  fix #6939: acme: accept HTTP 204 from newNonce endpoint

 src/acme/client.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


Summary over all repositories:
  6 files changed, 29 insertions(+), 16 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

[pbs-devel] [PATCH proxmox v3 1/1] fix #6939: acme: support servers returning 204 for nonce requests

[Samuel Rufinatscha]

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/lib.rs          |  4 ++++
 proxmox-acme/src/request.rs      | 15 ++++++++++++---
 5 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/proxmox-acme/src/account.rs b/proxmox-acme/src/account.rs
index 0bbf0027..1ad485a2 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],
         })
     }
 
@@ -405,7 +405,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 dc755fb9..66ec6024 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: 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, crate::http_status::NO_CONTENT],
             },
             nonce,
         )
diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
index 931f7245..881ee83d 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/lib.rs b/proxmox-acme/src/lib.rs
index df722629..cf1bc68d 100644
--- a/proxmox-acme/src/lib.rs
+++ b/proxmox-acme/src/lib.rs
@@ -70,6 +70,10 @@ pub use order::Order;
 #[doc(inline)]
 pub use request::Request;
 
+#[cfg(feature = "impl")]
+#[doc(inline)]
+pub use request::http_status;
+
 // we don't inline these:
 #[cfg(feature = "impl")]
 pub use order::NewOrder;
diff --git a/proxmox-acme/src/request.rs b/proxmox-acme/src/request.rs
index 78a90913..24e669c5 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 struct Request {
@@ -17,8 +16,18 @@ pub struct Request {
     /// The body to pass along with request, or an empty string.
     pub body: String,
 
-    /// The expected status code a compliant ACME provider will return on success.
-    pub expected: u16,
+    /// The set of HTTP status codes that indicate a successful response from an ACME provider.
+    pub expected: &'static [u16],
+}
+
+/// Common HTTP status codes used in ACME responses.
+pub mod http_status {
+    /// 200 OK
+    pub const OK: u16 = 200;
+    /// 201 Created
+    pub const CREATED: u16 = 201;
+    /// 204 No Content
+    pub const NO_CONTENT: u16 = 204;
 }
 
 /// 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

[pbs-devel] [PATCH proxmox-backup v3 1/1] fix #6939: acme: accept HTTP 204 from newNonce endpoint

[Samuel Rufinatscha]

When registering an ACME account, PBS fetches a fresh nonce by issuing a
HEAD request to the server's newNonce URL. Until now we assumed this
request would return HTTP 200 OK.

In practice, some ACME servers [1] respond with HTTP 204 No Content for
this HEAD request while still providing a valid Replay-Nonce header.
This causes PBS to abort registration with "ACME server responded with
unexpected status code: 204", even though the server would otherwise
issue certificates correctly. This behavior is technically
off-spec [2], however not forbidden.

Adjust the ACME client code in PBS to accept both 200 OK and 204 No
Content as successful results for the newNonce step. We continue to
reject other status codes so we don't silently accept arbitrary 2xx
responses.

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://bugzilla.proxmox.com/show_bug.cgi?id=6939
[2] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
[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>
---
 src/acme/client.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/acme/client.rs b/src/acme/client.rs
index 9fb6ad55..1962529a 100644
--- a/src/acme/client.rs
+++ b/src/acme/client.rs
@@ -15,7 +15,7 @@ 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::{http_status, Request as AcmeRequest};
 use proxmox_acme::{Account, Authorization, Challenge, Directory, Error, ErrorResponse};
 use proxmox_http::client::Client;
 use proxmox_sys::fs::{replace_file, CreateOptions};
@@ -529,7 +529,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
@@ -606,7 +606,7 @@ impl AcmeClient {
                 method: "GET",
                 content_type: "",
                 body: String::new(),
-                expected: 200,
+                expected: &[http_status::OK],
             },
             nonce,
         )
@@ -654,7 +654,7 @@ impl AcmeClient {
                 method: "HEAD",
                 content_type: "",
                 body: String::new(),
-                expected: 200,
+                expected: &[http_status::OK, http_status::NO_CONTENT],
             },
             nonce,
         )
-- 
2.47.3



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

Re: [pbs-devel] [PATCH proxmox v3 1/1] fix #6939: acme: support servers returning 204 for nonce requests

[Thomas Lamprecht]

Am 03.11.25 um 11:13 schrieb Samuel Rufinatscha:
> 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.

The resulting code now looks mostly OK, but when doing a final review
before pushing this out I noticed two things that I should have actually
caught earlier:

1. Introducing + using the new http_status module should be a separate
   patch upfront, it has nothing to do with the actual bug fix but is
   rather an independent cleanup.

2. The Request struct and it's expected member are pub and is still
   used directly in PBS - where it was originally factored out from
   but not yet replaced. So the type change from `u16` to `&'static [u16]`
   causes a breaking ABI change. That change itself can be manageable,
   albeit if it can be easily avoided it's always nicer to do so; using
   a constructor (potentially with builder pattern) and changing the
   visibility of the members to pub(crate) or even making them private
   to the module, can often be a good option to reduce friction for
   any future change.
   But here it would be nicer to clear the tech debt and switch PBS fully
   over to the factored out impl, like e.g. PDM uses already. This should
   then also allow us to reduce the visibility of the struct members and
   the http_status module, which as of now I'd also rather see as
   proxmox-acme specific and thus not exposing it to the public would be
   better.

Do you want to give switching PBS over to the factored-out impl a try?
It adds a bit to the scope, but we have to clean this up (or accumulate
tech debt interest) sooner or later anyway, and if we do already a breaking
change I'd prefer sooner.
For the acme side of your changes nothing should change – besides maybe
already reducing visibility of structs and/or their members in a separate
patch.

In summary, a good order/split for the resulting patches could look
something like:

1. change PBS over to proxmox-acme, at least the client part.
2. reduce the visibility of types that now are only used in proxmox-acme
   internally
3. introduce and use http_status mod in proxmox-acme
4. fix #6939

What do you think? If moving PBS to proxmox-acme turns out to be tricky,
or even more work than expected, we can also go this route here as stop
gap; but IMO it would be favorable to spend a bit more time in actually
cleaning this up and reduce code duplication than doing so.
Again, I should have caught that early, but FWIW, almost all of the work
you done so far will still be relevant, so no real harm done FWICT.


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