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

[Samuel Rufinatscha <s.rufinatscha@proxmox.com>]

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