fix(certbot): read the dns-01 challenge from the authoritative nameservers

[kvinwang]

Stacked on #1129 — that PR gets issuance as far as publishing the TXT record; this one is about what happens next. Review after it merges, or read the single-file diff against its branch.

Problem

check_dns verifies the challenge record it just wrote. On a zone whose SOA minimum exceeds max_dns_wait, that check can never pass, however long it waits.

The record is created and queried moments apart. DNS has not propagated in that window, so the first lookup returns NXDOMAIN — and the recursive resolver caches that negative answer for the zone's SOA minimum. Every retry inside the wait window is then answered from that cache, describing a world where the record does not exist.

Observed on a live gateway issuing for 06rc0.kvin.wang:

DEBUG certbot::acme_client: Unsettled challenges: [
        acme_domain: "_acme-challenge.06rc0.kvin.wang",
        dns_value: "a-RtPuJHEnlOmefe8XWQDWlfLJPd8b6Rrfsw6Ygma8o" ]
DEBUG certbot::acme_client: challenge not found, waiting for 32s tries=16
WARN  cert[06rc0.kvin.wang]: auto-renewal failed: certificate request timed out

The record existed the whole time and was globally visible:

$ dig +short TXT _acme-challenge.06rc0.kvin.wang @1.1.1.1
"a-RtPuJHEnlOmefe8XWQDWlfLJPd8b6Rrfsw6Ygma8o"

The zone's negative TTL is 1800s against a 300s budget:

$ dig +short SOA kvin.wang
jocelyn.ns.cloudflare.com. dns.cloudflare.com. 2413134396 10000 2400 604800 1800

Rebuilding the TokioResolver each iteration — which the loop already did — does not help. That clears hickory's in-process cache; the negative answer lives in the recursive resolver upstream.

Fix

Read the challenge record from the zone's authoritative nameservers, so no recursive cache sits in the path.

Discovery walks up a label at a time until a name actually carries NS records, because neither _acme-challenge.<name> nor the name below it is usually a zone cut — for _acme-challenge.06rc0.kvin.wang the NS records live on kvin.wang. The walk stops before the public suffix; a TLD's nameservers cannot answer for the record, so falling back beats querying them.

The authoritative resolver runs with caching disabled. Its only job is to observe a record written seconds ago, so a cached answer of any age is the wrong answer.

If the nameservers cannot be reached, it warns and falls back to the system resolver rather than blocking issuance — the ACME server has its own DNS view either way, and the existing timeout already proceeds on expiry.

Two things this deliberately does not do:

• No new configuration. An earlier draft added a configurable bootstrap resolver defaulting to 1.1.1.1, threaded through the DNS credential, the proto and the admin API. That was unnecessary: the bootstrap lookup reads NS records and their addresses, which are stable records where ordinary caching is correct and wanted. Only the challenge record must dodge the cache. Dropping the knob removed 40 lines and left the gateway untouched.
• No change to the wait or retry policy. The bug is not that the budget is too short; it is that the check was asking something that could not answer.

Verification

cargo check --workspace --locked, cargo test -p certbot, cargo fmt --check, cargo clippy -p certbot -p dstack-gateway -- -D warnings all pass.

A unit test pins the label walk, including the boundary:

Input	Expected
06rc0.kvin.wang	kvin.wang
a.b.example.com	b.example.com
kvin.wang	None — stop at the registrable name
wang	None — never query a TLD

That test exists because the first implementation got this wrong: it looked up NS records on the challenge's own name, which returns NODATA, found nothing, and silently fell back to the recursive resolver. The change would have had no effect while appearing to work. dig NS 06rc0.kvin.wang returning only a SOA is what surfaced it.

Not yet covered: a live issuance through a deployed gateway. The image was still building when this was opened; I will add the result as a comment. One risk I want to state rather than discover in review — the resolver is configured udp_and_tcp, so if UDP/53 to external addresses is restricted inside a CVM it should fall back to TCP, but that path has not been exercised on real hardware yet.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[kvinwang]

Thanks — this caught a real hole, and #1 in particular is the kind I would rather not have found in production.

Pushed 97c36206fd addressing #1–#5.

#1 — hard bail! on unreachable nameservers (blocking)

You're right, and I confirmed the mechanism rather than taking it on faith:

// hickory-net-0.26.1/src/error.rs:160
pub fn is_no_records_found(&self) -> bool {
    matches!(self, Self::Dns(DnsError::NoRecordsFound { .. }))
}

Transport errors are NetError::Proto(..) / NoConnections, so they miss that arm and hit bail!. What makes this bad is exactly your framing: before the PR the blocked-egress case worked — NoRecordsFound from the local stub, retry, timeout, proceed-anyway, and the ACME server validates from its own view. After it, issuance dies on the first iteration and the timeout escape hatch is never reached. I turned a self-healing path into a deterministic failure, and the warn!-and-fall-back comment claimed a safety property the code did not implement. That comment covering only discovery failure while the query path bailed is precisely the gap.

Fixed with the sticky per-domain fallback you suggested: a transport error swaps that domain's resolver for the system one and re-queues the challenge, so the rest of the wait behaves exactly as it did before this PR.

#2 — one resolver for all challenges

Fixed. authoritative_resolver now takes a single domain, and resolvers are keyed by acme_domain in a BTreeMap, so a SAN list spanning zones gets one resolver per zone rather than the first challenge's nameservers applied to all of them.

#3 — discovery repeated every iteration

Fixed. Discovery is hoisted above 'outer. You're right that the original per-iteration rebuild existed to drop hickory's in-process cache and that cache_size = 0 makes it redundant. I left the per-NS lookup_ip calls sequential — it runs once now, over a handful of records.

#4 — parent_zone doc overstates the guarantee

Fixed, and you're right that both the comment and the test name claimed a public-suffix guarantee. The comment now says what the code does — stops at the last two labels, a heuristic rather than a PSL lookup — and names the multi-label-suffix case explicitly, including why the outcome is a wasted lookup and a fallback rather than a wrong answer. The test was renamed to the_walk_climbs_to_a_name_that_can_carry_ns_records.

#5 — displaced doc comment

Fixed. The dns-persist-01 comment is back above an_authorization_survives_a_challenge_type_without_a_token, and the NS test moved to its own mod ns_discovery_tests with use super::parent_zone; at module level.

Smaller notes

• Trailing dot / search list — not applied yet. Your reasoning about raw_name_first is right, so it is a free tightening rather than a fix; I would rather land it with the live result in hand than churn the diff again first.
• CNAME-delegated _acme-challenge — worth documenting; not added yet for the same reason. Real constraint, and correct that it does not apply to dstack's own DNS-01 provider path today.
• remove_txt_records clobbering the sibling authorization — agreed this is pre-existing and out of scope, and I had not noticed it. It looks real for a example.com + *.example.com cert: both authorizations publish to _acme-challenge.example.com with different values and the second remove deletes the first. Worth its own issue.

On the live result

Still building. Your closing point is the right one to hold me to — whether the CVM can reach external :53 decides whether #1 is theoretical, and it is now the specific thing I will report rather than just "issuance succeeded". If egress is blocked, the fallback path is what gets exercised, and that is the more interesting result of the two.

[kvinwang]

The egress data point

You asked for the one measurement that decides whether #1 is theoretical: a CVM on this host can reach external authoritative nameservers over UDP/53.

I deployed a throwaway CVM on the same guest image and VMM config as the gateway, running nothing but dig:

--- full status UDP ---
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 21233
;; SERVER: 172.64.32.174#53(jocelyn.ns.cloudflare.com) (UDP)

NOERROR with a SERVER: line naming Cloudflare's authoritative IP over UDP. A blocked path gives ;; connection timed out; no servers could be reached instead.

The CVM's /etc/resolv.conf points at Docker's stub (127.0.0.11) chaining to the host's 127.0.0.53, and that path resolves normally too — the A and NS lookups both answered. So discovery and the authoritative query both work here.

So on this host #1 is not the failing case — but that does not make it theoretical. It means this environment happens to have open egress. The scenario you described is a policy choice, not a law, and the change should not depend on which way that choice went on the machine I happened to test. The fallback stands on its own.

Two corrections to my own testing, since they nearly produced wrong answers

First probe was ambiguous and I almost read it backwards. It queried _acme-challenge.06rc0.kvin.wang against the authoritative server and got nothing, which looked like blocked egress. It was not: certbot had already cleaned up the challenge record after the successful issuance in #1129. dig +short prints nothing for both NODATA and timeout, so the probe could not distinguish "unreachable" from "no such record" — the two hypotheses I was trying to separate. Confirmed from the host that the record was genuinely gone before drawing any conclusion.

Second probe had a shell-quoting bug. The $q loop variable was eaten passing through the compose JSON, so those dig +short lines ran with an empty query and printed nothing. Those empties are my harness misfiring, not evidence. The reachability result above comes from the one query that used no variables.

I am flagging both because a reader skimming the log would see a lot of empty output and could reasonably conclude egress is blocked. It is not.

Still outstanding

A live issuance through a gateway carrying the review fixes. The image I had building was for b775f51fae, which predates 97c36206fd, so it would exercise the old bail! path rather than the fallback — verifying the wrong artifact. Rebuilding against the current head.

[kvinwang]

Live A/B

Ran both arms on the same host within the same hour, changing only the gateway image.

	Control	Treatment
Gateway code	#1129 only	#1129 + this PR at 97c36206fd
Domain	ctl.06rc0.kvin.wang	exp.06rc0.kvin.wang
Retries	16	—
Elapsed	300s, budget exhausted	17s
Outcome	certificate request timed out, has_cert=False	successfully issued/renewed, has_cert=True

Control:

10:01:28  challenge not found, waiting for 32s tries=12
10:02:00  challenge not found, waiting for 32s tries=13
10:02:32  challenge not found, waiting for 32s tries=14
10:03:04  challenge not found, waiting for 32s tries=15
10:03:36  challenge not found, waiting for 32s tries=16
10:03:47  cert[ctl.06rc0.kvin.wang]: auto-renewal failed: certificate request timed out

Treatment:

10:10:26  AddZtDomain exp.06rc0.kvin.wang
10:10:43  cert[exp.06rc0.kvin.wang]: successfully issued/renewed

Everything else was held constant: same host, same VMM and guest image, same KMS CVM, same Cloudflare zone and credential, same Let's Encrypt staging endpoint, and both names freshly created and never queried before the run.

Both arms are fresh runs. I had originally planned to use the earlier 09:04 failure as the control, but that was a different gateway image with different credential and certificate state — close enough to look like a comparison without being one. Re-running it against the current build was worth the extra five minutes.

On the fallback path

The egress probe reported earlier means the treatment arm took the authoritative path, not the fallback — UDP/53 to Cloudflare's nameservers is open from a CVM on this host. So this run demonstrates the fix working, and does not exercise the transport-error fallback that finding #1 was about.

That path remains covered by reasoning and by the code review rather than by a live run, and I would rather say so than let a green A/B imply more coverage than it has. Forcing it would mean blocking egress on the host, which I did not want to do on a shared machine.

[kvinwang]

Fixed both in cb2cc93cec. I worked through the consequences rather than taking the severity as given, and both turned out to matter a little more than "polish".

(a) No backoff and a misleading log after fallback

The log line is the worse half. Once a domain has fallen back, a subsequent transport error re-enters the same arm and announces "falling back to the system resolver" — while already on the system resolver. Anyone reading that log is being told something untrue about the current state.

The missing backoff compounds it: delay stays pinned at its initial 250ms because the path never reaches the !settled arm. Each iteration is still gated by hickory's query timeout, so it is not a spin — but over a 300s budget that is 50-odd iterations instead of ~16, each emitting that same wrong line. Fifty self-contradicting log lines is a materially worse debugging experience than sixteen correct ones.

Now the fallback announces itself once, tracked in a fell_back set, and a failure after that returns false so it flows into the normal backoff path with a debug! recording the error.

(b) Discovery outside the budget

I think this one is closer to a bug than a tradeoff, because of how the two timeouts interact.

max_dns_wait and renew_timeout are commonly the same value — they are both 300s in the config I have been testing with. With discovery outside the budget, check_dns can run for discovery + max_dns_wait, which exceeds renew_timeout. The outer timeout then kills the renewal mid-wait, so control never reaches the graceful "proceed anyway" exit.

That exit is what finding #1 was about: it is the reason a DNS problem degrades to "the ACME server validates from its own view" instead of failing issuance. So leaving discovery outside the budget reintroduces, by a different route, the failure mode #1 closed — and I introduced the discovery phase that makes it reachable.

start_time now begins before discovery, so max_dns_wait bounds the whole function and check_dns cannot overrun the outer timeout. If discovery alone exhausts the budget, the loop takes the graceful exit immediately, which is the right outcome.

Not applied

The trailing-dot FQDN tightening and the CNAME-delegation note — agreed both are minor and neither changes behaviour on dstack's own DNS-01 path. Happy to add them if you would rather they land here than as follow-ups.

cargo test -p certbot, fmt, and clippy -D warnings all pass.

One note on the push: the branch had been rebased remotely when #1129 merged and GitHub retargeted the base. I verified the remote tip was content-identical to my local pre-rebase tip (git diff empty) before replaying the new commit on top, so this was a fast-forward rather than a force-push over someone else's work.