fix(acme): stop punishing certificate issuance prepared before a DNS cutover - #119
Merged
Conversation
Before a DNS cutover the domain still points at the old server, so an HTTP-01 order cannot succeed. Every handshake carrying that SNI spent one anyway: the triggering domain was exempt from the batch guard, so a smoke test or a scanner could burn Let's Encrypt's failed-authorization budget (five per hostname per hour). The rate-limit hold that followed is what delayed the certificate at the actual cutover — which is why preparing in advance had become something to avoid. Probe the trigger before assembling its order. A failing probe refuses the handshake and records a cheap preflight hold without spending anything; a passing probe lifts any existing hold and proceeds. Keeping the probe as the gate preserves what the old unconditional pass protected — a just-repointed domain gets its certificate on the next handshake instead of waiting out a stale ladder entry — while removing a burn no quarantine check could have stopped, since it happens before any hold exists. Also stop HTTP-probing domains a DNS-01 provider answers for. probeDomains skipped only wildcards, so a host in a DNS-01 zone was probed, failed while its DNS still pointed elsewhere, and was dropped from an order that would have succeeded — punishing exactly the case DNS-01 makes safe. Holds now record which kind they are, so a later change can tell a backoff it may lift from one the CA imposed. The persisted form is numeric and a state file without the key decodes as an ACME hold, the conservative default; the status API renders it as a string. Refs #118
…gain The quarantine ladder runs to 24 hours, but the usual reason a domain cannot be issued is temporary and operator-driven: its DNS has not been repointed at this proxy yet. Waiting out a full step after the repoint is what made preparing a cutover in advance feel like a punishment — the failed attempts beforehand delayed the certificate afterwards. Sweep the held domains on an interval and lift the hold from any that now answer the pre-flight probe, so a cutover costs about one probe interval instead of a ladder step. Released dynamic domains are queued for issuance immediately; a deploy-registered host needs only the release, since its handshake path re-probes the trigger itself and would otherwise race a background order for the same name. Rate-limit holds are never lifted: that end time is the CA's, not ours, and ordering inside the window both fails and pushes it further out. Wildcards and DNS-01 zones are not probed at all — neither depends on where the name points. A failing sweep records nothing, so a domain cannot be driven up the ladder merely for continuing to be in the state it is already held for. New --acme-release-probe-interval (default 1m, negative disables). Refs #118
`domains list` only ever reported services with a domain source, so the hosts an operator names in `deploy --host` — the common case, and the one they stare at during a DNS cutover — were invisible while their issuance was held. Report them alongside the dynamic sets, with the certificate state and the hold, and count them in `domains stats`. Holds now render why they are held, not just when they lift, because the kinds behave differently: a preflight or ACME hold lifts on its own once the domain routes here, a rate-limit hold waits out the CA's window. `domains retry [domain] --all` is the escape hatch. Until now the only way to clear a hold was a proxy restart, which did not work either — quarantine is persisted. It is deliberately blunter than the automatic release: it clears rate-limit holds, and it wipes the failure history rather than only the hold, because asking for a retry is a claim that the cause is fixed. The issuance rate limit still applies. Refs #118
The operational question this answers is whether you can configure a host before its DNS is repointed. You can, and the README never said so — which is how teams arrive at the opposite rule of thumb: that setting it up in advance leaves it failing and delays the real cutover. Refs #118
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deploying a host before its DNS is repointed made the certificate arrive later, not sooner — so the working rule of thumb became "don't set it up in advance, do it at the exact moment of the cutover", which is the manual step automatic TLS exists to remove. Four defects produced that, all fixed here.
The trigger domain was never reachability-checked before an order.
GetCertificatecalledprovisionCertificateunconditionally for any registered host, andfilterBatchMatesdeliberately exempted the triggering domain. Any HTTPS request carrying that SNI before the cutover — a smoke test, a monitor, a scanner — spent a real ACME order that could not succeed. Five in an hour trips Let's Encrypt's failed-authorization limit, and the hold that follows is what delayed the real cutover. Now the trigger is probed first: a failing probe refuses the handshake and records a cheap preflight hold having spent nothing.Nothing lifted a hold when DNS landed. The ladder runs to 24h and persists to
dynamic-domains.state, so a restart did not help either. A new release prober re-probes held domains every minute (--acme-release-probe-interval, negative to disable) and lifts the hold as soon as the domain routes back. A cutover now costs about a probe interval instead of a backoff step.DNS-01 domains were HTTP-probed anyway.
probeDomainsskipped only wildcards, so a non-wildcard host in a DNS-01 zone was probed, failed while its DNS still pointed elsewhere, and was dropped from an order that would have succeeded — inverting the main advantage of DNS-01.No visibility, no escape hatch.
domains listreported only services with a domain source, so deploy-registered hosts were invisible while held. They are now listed with their certificate state and hold, holds render why they are held, anddomains retry [domain] --allclears one on demand.Closes #118
Design notes
The probe, not the quarantine, is the gate on the handshake path. That keeps the property the old unconditional pass was protecting — a just-repointed domain gets its certificate on the very next handshake rather than waiting out a stale ladder entry — while removing a burn that a quarantine check could never have stopped, because it happens on the first attempt, before any hold exists.
Holds now carry a kind, so the release prober can tell its own backoff from one the CA imposed. Rate-limit holds are never lifted early: that end time is Let's Encrypt's, the domain routing here again says nothing about it, and ordering inside the window both fails and pushes it further out. The persisted form is numeric so a state file predating the field decodes as an ACME hold, the conservative default; the status API renders it as a string.
A failing sweep records nothing. The domain is already held, and counting each sweep as a fresh failure would drive it to the top of the ladder within minutes purely for continuing to be in the state it was already held for.
Test plan
go test ./...— greengo test -race ./...— green (this adds a background goroutine)go test ./internal/server/ -run 'Quarantine|BatchGuard|DomainIssuer|CertRenewer|Release|Retry|RegisteredDomains' -count=2— green, per the global-registry isolation problem from Global Prometheus registry breaks test isolation: -count=2 panics, cache-metrics assertions flake #104gofmt -lclean,go vet ./...clean,make buildokdomains --help,domains retry --help,run --helpshowing the new flagNew tests worth reading as the spec:
TestBatchGuard_UnreachableTriggerIsRefusedWithoutBurningAnOrder,TestBatchGuard_ReachableTriggerProvisionsDespiteAStaleHold,TestBatchGuard_DNSSolvableTriggerIsNotProbed,TestReleaseProber_NeverLiftsARateLimitHold,TestReleaseProber_SkipsDomainsTheProbeCannotSpeakFor,TestReleaseProber_FailedProbeDoesNotEscalateTheLadder.TestBatchGuard_TriggerDomainIsNeverDroppedencoded the old contract and was rewritten, not deleted — the trigger is still never dropped from its own batch, but the handshake is now gated by a live probe upstream of it.Deviations & judgment calls
Deviations from the issue
The trigger gate went into
provisionCertificate, notGetCertificate.GetCertificateis its only caller, so the two are equivalent in production — butprovisionCertificateis directly callable from tests, whereas reachingGetCertificate's provisioning branch needs atls.ClientHelloInfocarrying a real context, whichcrypto/tlsgives no exported way to construct. One level down also covers the directory-mismatch synchronous reprovision path (--tls-stagingflips), which enters through the same function.Lifting a hold uses a new
Release, not the existingClear.Clearwipes the failure count as well as the hold, which is right after a successful order but wrong here: a domain flapping between routing here and not would reset to the bottom of the ladder on every brief green probe and could loop against the CA indefinitely.ReleasezeroesUntil, keepsFailures, and reports whether it actually held so callers only log real transitions. The manualdomains retrydeliberately still usesClear— asking for a retry is a claim that the cause is fixed.Judgment calls
QuarantineStatus.Kindis a string on the wire, not the raw enum. AddingKindtoquarantineEntrybroke theQuarantineStatus(entry)struct conversion, which only compiles while the two types are field-identical. Rather than mirror an unexported enum into an exported API type and emit"kind":2to operators, the enum got aString()andQuarantineStatusis built explicitly.The release prober requests issuance only for dynamic domains. A deploy-registered host is issued on its handshake, and that path re-probes the trigger itself, so lifting the hold is all it needs — routing the two through different mechanisms would risk a background order racing a synchronous one for the same name. The practical cost is that a registered host's certificate arrives on the first visit after the cutover rather than strictly before it.
statslabels changed from a bareCertified:toDynamic certified:/Registered certified:, since there are now two counts and one unqualified label would have been ambiguous.Discoveries
The issuance guard is installed unconditionally whenever
--acme-emailis set, so the "behaviour unchanged when no guard is installed" fallback is a test-only concern in practice, not a production one.The renewal path already carried a cruder form of the DNS-01 guard: it probes only dynamic domains, on the reasoning that a registered host may be DNS-01-only. That rule is both too broad (never probes a registered HTTP-01 host, even when probing would help) and too narrow (always probes a dynamic domain, even one in a DNS-01 zone). The new predicate composes with it rather than replacing it. I did not widen the registered-host side — the renewal path is not what this issue is about, and changing it would alter renewal behaviour for hosts currently never probed. Worth a follow-up.
Not done
v1.0.0.7tag without it, since the gem'sMINIMUM_VERSIONgate makes that tag hard to walk back.Follow-ups
dash proxy domains retry,release_probe_intervalindeploy.yml, hold reporting indash doctor). Blocked on this shipping and an image being published.Summary by cubic
Gates handshake-driven certificate issuance on a live preflight probe and adds an automatic release prober. Before, pre‑cutover handshakes spent failing ACME orders and hit rate limits; now we probe first, refuse unreachable triggers without spending an order, and lift holds as soon as DNS routes here.
New Features
--acme-release-probe-interval(default 1m; negative disables).preflight,acme,rate_limited); rate-limited holds are never lifted early.domains listshows deploy-registered hosts with certificate state and hold reason/time.domains retry [domain] --allclears holds and immediately re-requests issuance.domains statssplits “Certified” into “Dynamic certified” and “Registered certified”.Bug Fixes
preflightand the handshake is refused instead of burning an order.Closes #118.
Written for commit ec4981d. Summary will update on new commits.