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

[pbs-devel] [PATCH proxmox{, -backup} 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*

This is also stricter than the PVE Perl ACME client, which tolerates any
2xx success codes [3]. The author mentions, the issue did not appear
with PVE9 [1].

## 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

[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:
[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)

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/request.rs      |  4 ++--
 4 files changed, 11 insertions(+), 11 deletions(-)


proxmox-backup:

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

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


Summary over all repositories:
  5 files changed, 14 insertions(+), 14 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 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 functionally harmless. 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.

This aligns behavior with PVE’s more tolerant Perl ACME client and
avoids regressions.

[1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
[2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939

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 73d786b8..60719865 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: vec![crate::request::CREATED],
         };
 
         Ok(NewOrder::new(request))
@@ -107,7 +107,7 @@ impl Account {
             method: "POST",
             content_type: crate::request::JSON_CONTENT_TYPE,
             body,
-            expected: 200,
+            expected: vec![200],
         })
     }
 
@@ -132,7 +132,7 @@ impl Account {
             method: "POST",
             content_type: crate::request::JSON_CONTENT_TYPE,
             body,
-            expected: 200,
+            expected: vec![200],
         })
     }
 
@@ -157,7 +157,7 @@ impl Account {
             method: "POST",
             content_type: crate::request::JSON_CONTENT_TYPE,
             body,
-            expected: 200,
+            expected: vec![200],
         })
     }
 
@@ -405,7 +405,7 @@ impl AccountCreator {
             method: "POST",
             content_type: crate::request::JSON_CONTENT_TYPE,
             body,
-            expected: crate::request::CREATED,
+            expected: vec![crate::request::CREATED],
         })
     }
 
diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
index 60e1f359..0901aa8d 100644
--- a/proxmox-acme/src/async_client.rs
+++ b/proxmox-acme/src/async_client.rs
@@ -421,7 +421,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
@@ -501,7 +501,7 @@ impl AcmeClient {
                 method: "GET",
                 content_type: "",
                 body: String::new(),
-                expected: 200,
+                expected: vec![200],
             },
             nonce,
         )
@@ -553,7 +553,7 @@ impl AcmeClient {
                 method: "HEAD",
                 content_type: "",
                 body: String::new(),
-                expected: 200,
+                expected: vec![200, 204],
             },
             nonce,
         )
diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
index d8a62081..ea8a8655 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 78a90913..38e825d6 100644
--- a/proxmox-acme/src/request.rs
+++ b/proxmox-acme/src/request.rs
@@ -17,8 +17,8 @@ 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: Vec<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

[pbs-devel] [PATCH proxmox-backup 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 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.

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.

This restores interoperability with ACME servers that send 204 for
newNonce, and aligns PBS' behavior with the updated proxmox-acme library
as well as PVE's more tolerant ACME client.

Fixes: #6939
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 src/acme/client.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/acme/client.rs b/src/acme/client.rs
index 1c12a4b9..0dabf676 100644
--- a/src/acme/client.rs
+++ b/src/acme/client.rs
@@ -530,7 +530,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
@@ -609,7 +609,7 @@ impl AcmeClient {
                 method: "GET",
                 content_type: "",
                 body: String::new(),
-                expected: 200,
+                expected: vec![200],
             },
             nonce,
         )
@@ -657,7 +657,7 @@ impl AcmeClient {
                 method: "HEAD",
                 content_type: "",
                 body: String::new(),
-                expected: 200,
+                expected: vec![200, 204],
             },
             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 1/1] fix #6939: acme: support servers returning 204 for nonce requests

[Christian Ebner]

Hi, thanks for the patches!

comments inline

On 10/28/25 8:34 PM, Samuel Rufinatscha wrote:
> 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 functionally harmless. 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.
> 
> This aligns behavior with PVE’s more tolerant Perl ACME client and
> avoids regressions.
> 
> [1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
> [2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939
> 
> 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 73d786b8..60719865 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: vec![crate::request::CREATED],

while this is defined as dedicated constant...

>           };
>   
>           Ok(NewOrder::new(request))
> @@ -107,7 +107,7 @@ impl Account {
>               method: "POST",
>               content_type: crate::request::JSON_CONTENT_TYPE,
>               body,
> -            expected: 200,
> +            expected: vec![200],

... these and the others below are not. Same for the 204 status code you 
are about to add.

So in preparation for adding the new status code, these should probably 
be defined as, either:
- as dedicated status code constants as well, or
- all moved over to directly use 
https://docs.rs/http/1.3.1/http/status/struct.StatusCode.html

I feel like the latter is not done here intentionally to avoid the 
dependency on hyper or http (re-exported by hyper) for the api types only.

@wolfgang, comments on that?

>           })
>       }
>   
> @@ -132,7 +132,7 @@ impl Account {
>               method: "POST",
>               content_type: crate::request::JSON_CONTENT_TYPE,
>               body,
> -            expected: 200,
> +            expected: vec![200],
>           })
>       }
>   
> @@ -157,7 +157,7 @@ impl Account {
>               method: "POST",
>               content_type: crate::request::JSON_CONTENT_TYPE,
>               body,
> -            expected: 200,
> +            expected: vec![200],
>           })
>       }
>   
> @@ -405,7 +405,7 @@ impl AccountCreator {
>               method: "POST",
>               content_type: crate::request::JSON_CONTENT_TYPE,
>               body,
> -            expected: crate::request::CREATED,
> +            expected: vec![crate::request::CREATED],
>           })
>       }
>   
> diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
> index 60e1f359..0901aa8d 100644
> --- a/proxmox-acme/src/async_client.rs
> +++ b/proxmox-acme/src/async_client.rs
> @@ -421,7 +421,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
> @@ -501,7 +501,7 @@ impl AcmeClient {
>                   method: "GET",
>                   content_type: "",
>                   body: String::new(),
> -                expected: 200,
> +                expected: vec![200],
>               },
>               nonce,
>           )
> @@ -553,7 +553,7 @@ impl AcmeClient {
>                   method: "HEAD",
>                   content_type: "",
>                   body: String::new(),
> -                expected: 200,
> +                expected: vec![200, 204],
>               },
>               nonce,
>           )
> diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
> index d8a62081..ea8a8655 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 78a90913..38e825d6 100644
> --- a/proxmox-acme/src/request.rs
> +++ b/proxmox-acme/src/request.rs
> @@ -17,8 +17,8 @@ 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: Vec<u16>,
>   }
>   
>   /// An ACME error response contains a specially formatted type string, and can optionally



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

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

[Thomas Lamprecht]

Thanks for the patch and actually also checking if PVE is affected, and why
it isn't.

Am 28.10.25 um 20:34 schrieb Samuel Rufinatscha:
> This is also stricter than the PVE Perl ACME client, which tolerates any
> 2xx success codes [3]. The author mentions, the issue did not appear
> with PVE9 [1].
> 
> ## 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?

There probably isn't answer that strictly right in all cases, but in
general it's good to have similar behavior across implementations,
especially given that we do not know of any report where the PVE behavior
of accepting all 2xx response codes caused any problems, from that 3.
would be best, or does the RFC forbid the server to accept other status
codes?

That said, in practice only 201 (Created) might make sense for ACME in
addition to the referenced 200 (OK) and 204 (No Content), and following
the RFC is fine, so 1. is IMO also a good solution here.
Please note the difference to PVE in the commit message, there you write
that behavior is now aligned with PVE, but it's rather "closer aligned"
to, not fully.


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

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

[Thomas Lamprecht]

Am 29.10.25 um 08:23 schrieb Christian Ebner:
> Hi, thanks for the patches!
> 
> comments inline
> 
> On 10/28/25 8:34 PM, Samuel Rufinatscha wrote:
>> 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 functionally harmless. 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.
>>
>> This aligns behavior with PVE’s more tolerant Perl ACME client and
>> avoids regressions.
>>
>> [1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
>> [2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939
>>
>> 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 73d786b8..60719865 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: vec![crate::request::CREATED],
> 
> while this is defined as dedicated constant...
> 
>>           };
>>             Ok(NewOrder::new(request))
>> @@ -107,7 +107,7 @@ impl Account {
>>               method: "POST",
>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>               body,
>> -            expected: 200,
>> +            expected: vec![200],
> 
> ... these and the others below are not. Same for the 204 status code you are about to add.
> 
> So in preparation for adding the new status code, these should probably be defined as, either:
> - as dedicated status code constants as well, or
> - all moved over to directly use https://docs.rs/http/1.3.1/http/status/struct.StatusCode.html
> 
> I feel like the latter is not done here intentionally to avoid the dependency on hyper or http (re-exported by hyper) for the api types only.

While you are right that constants are generally nicer, IMO HTTP codes are
very stable and universal to be fine to be used directly as numbers in the few
limited instances here.

If we already (even just transitively) would get them from a dependency we still
should switch to that, but I'd not introduce a new dependency just for that; IMO
to high of a cost.


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

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

[Christian Ebner]

On 10/29/25 8:53 AM, Thomas Lamprecht wrote:
> Am 29.10.25 um 08:23 schrieb Christian Ebner:
>> Hi, thanks for the patches!
>>
>> comments inline
>>
>> On 10/28/25 8:34 PM, Samuel Rufinatscha wrote:
>>> 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 functionally harmless. 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.
>>>
>>> This aligns behavior with PVE’s more tolerant Perl ACME client and
>>> avoids regressions.
>>>
>>> [1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
>>> [2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939
>>>
>>> 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 73d786b8..60719865 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: vec![crate::request::CREATED],
>>
>> while this is defined as dedicated constant...
>>
>>>            };
>>>              Ok(NewOrder::new(request))
>>> @@ -107,7 +107,7 @@ impl Account {
>>>                method: "POST",
>>>                content_type: crate::request::JSON_CONTENT_TYPE,
>>>                body,
>>> -            expected: 200,
>>> +            expected: vec![200],
>>
>> ... these and the others below are not. Same for the 204 status code you are about to add.
>>
>> So in preparation for adding the new status code, these should probably be defined as, either:
>> - as dedicated status code constants as well, or
>> - all moved over to directly use https://docs.rs/http/1.3.1/http/status/struct.StatusCode.html
>>
>> I feel like the latter is not done here intentionally to avoid the dependency on hyper or http (re-exported by hyper) for the api types only.
> 
> While you are right that constants are generally nicer, IMO HTTP codes are
> very stable and universal to be fine to be used directly as numbers in the few
> limited instances here.
> 
> If we already (even just transitively) would get them from a dependency we still
> should switch to that, but I'd not introduce a new dependency just for that; IMO
> to high of a cost.

Agreed, given that the `create::request::CREATED` constant should be 
inlined and dropped then for consistency.


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

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

[Wolfgang Bumiller]

On Wed, Oct 29, 2025 at 08:53:34AM +0100, Thomas Lamprecht wrote:
> Am 29.10.25 um 08:23 schrieb Christian Ebner:
> > Hi, thanks for the patches!
> > 
> > comments inline
> > 
> > On 10/28/25 8:34 PM, Samuel Rufinatscha wrote:
> >> 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 functionally harmless. 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.
> >>
> >> This aligns behavior with PVE’s more tolerant Perl ACME client and
> >> avoids regressions.
> >>
> >> [1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
> >> [2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939
> >>
> >> 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 73d786b8..60719865 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: vec![crate::request::CREATED],
> > 
> > while this is defined as dedicated constant...
> > 
> >>           };
> >>             Ok(NewOrder::new(request))
> >> @@ -107,7 +107,7 @@ impl Account {
> >>               method: "POST",
> >>               content_type: crate::request::JSON_CONTENT_TYPE,
> >>               body,
> >> -            expected: 200,
> >> +            expected: vec![200],
> > 
> > ... these and the others below are not. Same for the 204 status code you are about to add.
> > 
> > So in preparation for adding the new status code, these should probably be defined as, either:
> > - as dedicated status code constants as well, or
> > - all moved over to directly use https://docs.rs/http/1.3.1/http/status/struct.StatusCode.html
> > 
> > I feel like the latter is not done here intentionally to avoid the dependency on hyper or http (re-exported by hyper) for the api types only.
> 
> While you are right that constants are generally nicer, IMO HTTP codes are
> very stable and universal to be fine to be used directly as numbers in the few
> limited instances here.

Mostly this, but we can also just add internal constants as well. 200
just seemed common enough...
> 
> If we already (even just transitively) would get them from a dependency we still
> should switch to that, but I'd not introduce a new dependency just for that; IMO
> to high of a cost.


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

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

[Wolfgang Bumiller]

On Tue, Oct 28, 2025 at 04:22:00PM +0100, Samuel Rufinatscha wrote:
> 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 functionally harmless. 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.
> 
> This aligns behavior with PVE’s more tolerant Perl ACME client and
> avoids regressions.
> 
> [1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
> [2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939
> 
> 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 73d786b8..60719865 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: vec![crate::request::CREATED],
>          };
>  
>          Ok(NewOrder::new(request))
> @@ -107,7 +107,7 @@ impl Account {
>              method: "POST",
>              content_type: crate::request::JSON_CONTENT_TYPE,
>              body,
> -            expected: 200,
> +            expected: vec![200],
>          })
>      }
>  
> @@ -132,7 +132,7 @@ impl Account {
>              method: "POST",
>              content_type: crate::request::JSON_CONTENT_TYPE,
>              body,
> -            expected: 200,
> +            expected: vec![200],
>          })
>      }
>  
> @@ -157,7 +157,7 @@ impl Account {
>              method: "POST",
>              content_type: crate::request::JSON_CONTENT_TYPE,
>              body,
> -            expected: 200,
> +            expected: vec![200],
>          })
>      }
>  
> @@ -405,7 +405,7 @@ impl AccountCreator {
>              method: "POST",
>              content_type: crate::request::JSON_CONTENT_TYPE,
>              body,
> -            expected: crate::request::CREATED,
> +            expected: vec![crate::request::CREATED],
>          })
>      }
>  
> diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
> index 60e1f359..0901aa8d 100644
> --- a/proxmox-acme/src/async_client.rs
> +++ b/proxmox-acme/src/async_client.rs
> @@ -421,7 +421,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
> @@ -501,7 +501,7 @@ impl AcmeClient {
>                  method: "GET",
>                  content_type: "",
>                  body: String::new(),
> -                expected: 200,
> +                expected: vec![200],
>              },
>              nonce,
>          )
> @@ -553,7 +553,7 @@ impl AcmeClient {
>                  method: "HEAD",
>                  content_type: "",
>                  body: String::new(),
> -                expected: 200,
> +                expected: vec![200, 204],
>              },
>              nonce,
>          )
> diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
> index d8a62081..ea8a8655 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 78a90913..38e825d6 100644
> --- a/proxmox-acme/src/request.rs
> +++ b/proxmox-acme/src/request.rs
> @@ -17,8 +17,8 @@ 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: Vec<u16>,

We always have a static set, so I'd rather use `&'static [u16]` here.
There's no need to allocate usually-single-element vectors everywhere.

>  }
>  
>  /// 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

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

[Thomas Lamprecht]

Am 29.10.25 um 11:35 schrieb Wolfgang Bumiller:
>> While you are right that constants are generally nicer, IMO HTTP codes are
>> very stable and universal to be fine to be used directly as numbers in the few
>> limited instances here.
> Mostly this, but we can also just add internal constants as well.

Definitively, but then let's prefix them with HTTP_ or HTTP_STATUS_ to actually
be telling about what they are. As, e.g., just reading "CREATED" is (in this
specific case) IMO rather worse than just "200". 


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

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

[Samuel Rufinatscha]

On 10/29/25 11:35 AM, Wolfgang Bumiller wrote:
> On Wed, Oct 29, 2025 at 08:53:34AM +0100, Thomas Lamprecht wrote:
>> Am 29.10.25 um 08:23 schrieb Christian Ebner:
>>> Hi, thanks for the patches!
>>>
>>> comments inline
>>>
>>> On 10/28/25 8:34 PM, Samuel Rufinatscha wrote:
>>>> 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 functionally harmless. 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.
>>>>
>>>> This aligns behavior with PVE’s more tolerant Perl ACME client and
>>>> avoids regressions.
>>>>
>>>> [1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
>>>> [2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939
>>>>
>>>> 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 73d786b8..60719865 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: vec![crate::request::CREATED],
>>>
>>> while this is defined as dedicated constant...
>>>
>>>>            };
>>>>              Ok(NewOrder::new(request))
>>>> @@ -107,7 +107,7 @@ impl Account {
>>>>                method: "POST",
>>>>                content_type: crate::request::JSON_CONTENT_TYPE,
>>>>                body,
>>>> -            expected: 200,
>>>> +            expected: vec![200],
>>>
>>> ... these and the others below are not. Same for the 204 status code you are about to add.
>>>
>>> So in preparation for adding the new status code, these should probably be defined as, either:
>>> - as dedicated status code constants as well, or
>>> - all moved over to directly use https://docs.rs/http/1.3.1/http/status/struct.StatusCode.html
>>>
>>> I feel like the latter is not done here intentionally to avoid the dependency on hyper or http (re-exported by hyper) for the api types only.
>>
>> While you are right that constants are generally nicer, IMO HTTP codes are
>> very stable and universal to be fine to be used directly as numbers in the few
>> limited instances here.
> 
> Mostly this, but we can also just add internal constants as well. 200
> just seemed common enough...
>>
>> If we already (even just transitively) would get them from a dependency we still
>> should switch to that, but I'd not introduce a new dependency just for that; IMO
>> to high of a cost.

First thanks for the review Christian, Thomas, Wolfgang - agree!

I checked the option of using `StatusCode`, but as you mentioned, that 
would require adding the `http` or `hyper` dependency, which we 
currently don’t include in the core types. I would therefore drop the 
constant as suggested and introduced a small, dedicated internal module 
to hold the ACME HTTP success constants. This would keep them together 
and would make imports handy.


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

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

[Samuel Rufinatscha]

On 10/29/25 11:38 AM, Wolfgang Bumiller wrote:
> On Tue, Oct 28, 2025 at 04:22:00PM +0100, Samuel Rufinatscha wrote:
>> 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 functionally harmless. 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.
>>
>> This aligns behavior with PVE’s more tolerant Perl ACME client and
>> avoids regressions.
>>
>> [1] https://datatracker.ietf.org/doc/html/rfc8555/#section-7.2
>> [2] https://bugzilla.proxmox.com/show_bug.cgi?id=6939
>>
>> 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 73d786b8..60719865 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: vec![crate::request::CREATED],
>>           };
>>   
>>           Ok(NewOrder::new(request))
>> @@ -107,7 +107,7 @@ impl Account {
>>               method: "POST",
>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>               body,
>> -            expected: 200,
>> +            expected: vec![200],
>>           })
>>       }
>>   
>> @@ -132,7 +132,7 @@ impl Account {
>>               method: "POST",
>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>               body,
>> -            expected: 200,
>> +            expected: vec![200],
>>           })
>>       }
>>   
>> @@ -157,7 +157,7 @@ impl Account {
>>               method: "POST",
>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>               body,
>> -            expected: 200,
>> +            expected: vec![200],
>>           })
>>       }
>>   
>> @@ -405,7 +405,7 @@ impl AccountCreator {
>>               method: "POST",
>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>               body,
>> -            expected: crate::request::CREATED,
>> +            expected: vec![crate::request::CREATED],
>>           })
>>       }
>>   
>> diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
>> index 60e1f359..0901aa8d 100644
>> --- a/proxmox-acme/src/async_client.rs
>> +++ b/proxmox-acme/src/async_client.rs
>> @@ -421,7 +421,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
>> @@ -501,7 +501,7 @@ impl AcmeClient {
>>                   method: "GET",
>>                   content_type: "",
>>                   body: String::new(),
>> -                expected: 200,
>> +                expected: vec![200],
>>               },
>>               nonce,
>>           )
>> @@ -553,7 +553,7 @@ impl AcmeClient {
>>                   method: "HEAD",
>>                   content_type: "",
>>                   body: String::new(),
>> -                expected: 200,
>> +                expected: vec![200, 204],
>>               },
>>               nonce,
>>           )
>> diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
>> index d8a62081..ea8a8655 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 78a90913..38e825d6 100644
>> --- a/proxmox-acme/src/request.rs
>> +++ b/proxmox-acme/src/request.rs
>> @@ -17,8 +17,8 @@ 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: Vec<u16>,
> 
> We always have a static set, so I'd rather use `&'static [u16]` here.
> There's no need to allocate usually-single-element vectors everywhere.

Agree, will replace the `Vec` with `&'static [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