public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [PATCH proxmox 1/1] fix #6939: acme: support servers returning 204 for nonce requests
Date: Wed, 29 Oct 2025 11:38:14 +0100	[thread overview]
Message-ID: <zawvujvxw3lwpu3q2fvz3xdshofmzs2fepcnktufbhlidcukut@pqxx4bbhtnhn> (raw)
In-Reply-To: <20251028152201.216603-2-s.rufinatscha@proxmox.com>

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

  parent reply	other threads:[~2025-10-29 10:37 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-28 15:21 [pbs-devel] [PATCH proxmox{, -backup} 0/2] " Samuel Rufinatscha
2025-10-28 15:22 ` [pbs-devel] [PATCH proxmox 1/1] " Samuel Rufinatscha
2025-10-29  7:23   ` Christian Ebner
2025-10-29  7:53     ` Thomas Lamprecht
2025-10-29  8:07       ` Christian Ebner
2025-10-29 10:36       ` Wolfgang Bumiller
2025-10-29 11:27         ` Thomas Lamprecht
2025-10-29 15:50         ` Samuel Rufinatscha
2025-10-29 10:38   ` Wolfgang Bumiller [this message]
2025-10-29 15:56     ` Samuel Rufinatscha
2025-10-28 15:22 ` [pbs-devel] [PATCH proxmox-backup 1/1] fix #6939: acme: accept HTTP 204 from newNonce endpoint Samuel Rufinatscha
2025-10-29  7:51 ` [pbs-devel] [PATCH proxmox{, -backup} 0/2] fix #6939: acme: support servers returning 204 for nonce requests Thomas Lamprecht
2025-10-29 16:02   ` Samuel Rufinatscha
2025-10-29 16:49 ` [pbs-devel] superseded: " Samuel Rufinatscha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=zawvujvxw3lwpu3q2fvz3xdshofmzs2fepcnktufbhlidcukut@pqxx4bbhtnhn \
    --to=w.bumiller@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=s.rufinatscha@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal