ACME: post a challenge once, then poll the authorization
The renewal loop re-posted the challenge every time it polled and found the authorization still pending. RFC 8555 section 7.5.1 has the client post a challenge once to say it is ready and then poll; a server that has already moved the challenge to "processing" refuses a second post, and pebble answers 400 malformed, which failed the whole renewal. Verified against pebble: the 400s are gone and the client polls. The suite still can't finish on this machine, because pebble never reaches the test server to validate the challenge; that path is the environment, and the runbook now says so.
This commit is contained in:
@@ -234,7 +234,7 @@ impl AcmeRequestBuilder {
|
|||||||
let mut retry_after = response.retry_after;
|
let mut retry_after = response.retry_after;
|
||||||
let auth = response.body;
|
let auth = response.body;
|
||||||
|
|
||||||
let (domain, challenge_url) = match auth.status {
|
let domain = match auth.status {
|
||||||
AuthStatus::Pending => {
|
AuthStatus::Pending => {
|
||||||
let Identifier::Dns(domain) = auth.identifier;
|
let Identifier::Dns(domain) = auth.identifier;
|
||||||
|
|
||||||
@@ -318,7 +318,7 @@ impl AcmeRequestBuilder {
|
|||||||
}
|
}
|
||||||
|
|
||||||
self.challenge(&challenge.url).await?;
|
self.challenge(&challenge.url).await?;
|
||||||
(domain, challenge.url.clone())
|
domain
|
||||||
}
|
}
|
||||||
AuthStatus::Valid => return Ok(()),
|
AuthStatus::Valid => return Ok(()),
|
||||||
_ => {
|
_ => {
|
||||||
@@ -345,14 +345,20 @@ impl AcmeRequestBuilder {
|
|||||||
|
|
||||||
match response.body.status {
|
match response.body.status {
|
||||||
AuthStatus::Pending => {
|
AuthStatus::Pending => {
|
||||||
|
// inbuxa: keep polling, don't post the challenge again.
|
||||||
|
// RFC 8555 section 7.5.1 has the client post a challenge
|
||||||
|
// once to say it's ready and then poll the authorization,
|
||||||
|
// which stays pending while validation runs. Posting it
|
||||||
|
// again is refused once the server has moved the
|
||||||
|
// challenge to "processing" (pebble answers 400
|
||||||
|
// malformed, "Cannot update challenge with status
|
||||||
|
// processing"), and that refusal failed the renewal.
|
||||||
trc::event!(
|
trc::event!(
|
||||||
Acme(AcmeEvent::AuthPending),
|
Acme(AcmeEvent::AuthPending),
|
||||||
Hostname = domain.to_string(),
|
Hostname = domain.to_string(),
|
||||||
Url = self.directory.new_order.to_string(),
|
Url = self.directory.new_order.to_string(),
|
||||||
Total = i,
|
Total = i,
|
||||||
);
|
);
|
||||||
|
|
||||||
self.challenge(&challenge_url).await?
|
|
||||||
}
|
}
|
||||||
AuthStatus::Valid => {
|
AuthStatus::Valid => {
|
||||||
trc::event!(
|
trc::event!(
|
||||||
|
|||||||
@@ -63,6 +63,28 @@ STORE=PostgreSqlReplicated cargo test -p tests --features postgres,redis \
|
|||||||
replica_cluster_tests -- --ignored
|
replica_cluster_tests -- --ignored
|
||||||
```
|
```
|
||||||
|
|
||||||
|
`LOG=<level>` turns on the test server's own logging, which is the only way
|
||||||
|
to see why a task failed rather than that it failed: `LOG=error` is what
|
||||||
|
found the ACME fault below.
|
||||||
|
|
||||||
|
## The suites the regression runs, but not with its own settings
|
||||||
|
|
||||||
|
Three of the suites a plain `cargo test -p tests` runs can't pass on the
|
||||||
|
settings it uses. Run them by name, with these:
|
||||||
|
|
||||||
|
```
|
||||||
|
# Two nodes, so a shared store and a coordinator, never RocksDb
|
||||||
|
STORE=PostgreSql COORDINATOR=Redis cargo test -p tests --features postgres,redis \
|
||||||
|
-- --exact cluster::broadcast::cluster_tests
|
||||||
|
|
||||||
|
# The spam rules the expectations were recorded against
|
||||||
|
STORE=RocksDb SPAM_RULES_URL=file:///path/to/spam-filter-rules.json.gz \
|
||||||
|
cargo test -p tests -- --exact smtp::inbound::antispam::antispam
|
||||||
|
|
||||||
|
# The ACME pair, which keeps its containers between runs
|
||||||
|
STORE=RocksDb cargo test -p tests -- --exact automation::automation_tests
|
||||||
|
```
|
||||||
|
|
||||||
## What a plain regression leaves failing
|
## What a plain regression leaves failing
|
||||||
|
|
||||||
`STORE=RocksDb cargo test -p tests -- --test-threads=1` was 86 passed, 4
|
`STORE=RocksDb cargo test -p tests -- --test-threads=1` was 86 passed, 4
|
||||||
@@ -79,13 +101,19 @@ and three are the invocation or the environment rather than the code:
|
|||||||
can't work on RocksDb, where each node gets its own.
|
can't work on RocksDb, where each node gets its own.
|
||||||
- `smtp::outbound::lmtp::lmtp_delivery` counted three DSNs where it wanted
|
- `smtp::outbound::lmtp::lmtp_delivery` counted three DSNs where it wanted
|
||||||
four, and passes on its own: queue timing under a loaded sequential run.
|
four, and passes on its own: queue timing under a loaded sequential run.
|
||||||
- `automation::automation_tests` fails against pebble with
|
- `automation::automation_tests` failed against pebble with
|
||||||
`400 malformed: "Cannot update challenge with status processing, only
|
`400 malformed: "Cannot update challenge with status processing, only
|
||||||
status pending"`. `crates/common/src/network/acme/order.rs` re-POSTs the
|
status pending"`, because the client re-posted the challenge on every
|
||||||
challenge each time it polls an authorization that is still pending;
|
poll. That was a real renewal bug and is fixed; the 400s are gone and the
|
||||||
pebble accepts that only while the challenge itself is pending. Upstream
|
client polls as RFC 8555 section 7.5.1 says to.
|
||||||
code, untouched by the fork, and unfixed: worth treating as a real
|
|
||||||
renewal bug rather than a test artefact.
|
The suite still doesn't pass here: pebble never validates the TLS-ALPN
|
||||||
|
challenge, so the authorizations stay pending until the client gives up
|
||||||
|
and no certificate is issued. Validation needs pebble, in its container,
|
||||||
|
to reach the test server's `0.0.0.0:8899` across the docker bridge, and
|
||||||
|
`ufw` is active on this machine. That wasn't proved — standing up a
|
||||||
|
listener to test it needs a permission this session didn't have — so
|
||||||
|
before reading anything into an ACME failure, check that path first.
|
||||||
|
|
||||||
## When one fails
|
## When one fails
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user