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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 29.10.25 um 17:45 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.
> 
> [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>
> ---

Thanks for the v2, looks OK in general, one naming issue – that irked
my a tiny bit to much to just ignore it – inline.

>  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/request.rs b/proxmox-acme/src/request.rs
> index 78a90913..0532528e 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 success status codes used in ACME responses.
> +pub mod http_success {

It's not wrong, but reads a bit odd to me; is it really relevant to differ
between success and (in the future, e.g. if this gets copied+adapted somewhere
else) get a http_client_error or http_server_error module?

I'd probably rather just use one of http_status or http_status_code, as the
codes themselves are uniquely named already anyway and with "status" included
on the name it would be IMO slightly better describe what this refers to without
having to read the doc comment.

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



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