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

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

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