fix(dstack-ingress): verify delegated records with dnsguide, not a second checker - #107
Conversation
…cond checker Challenge delegation (#104) grew its own CAA check: one DoH resolver, `grep -F` over the rdata, refuse to start if the record is not found. Three problems, all of which dnsguide.py already handles because tls-alpn-01 hit them first. Public resolvers cache *negative* answers for the zone's SOA minimum, up to an hour, so a resolver asked before the operator created the record keeps reporting it absent long after it exists. Combined with failing closed, that is a container refusing to start over a record that is there. The failure asymmetry ran the wrong way, too: an unreachable resolver warned and continued -- no CAA protection at all -- while a stale cache was fatal. The cheap failure was treated as serious and the serious one as cosmetic. And `grep -F` only matches presentation-form rdata, which is what dns.google returns. Adding a second resolver for the first problem would have quietly broken it, because Cloudflare answers CAA in RFC 3597 generic hex; a literal match fails, and failing closed turns that into an outage. dnsguide.py queries two resolvers with union quorum, parses both wire formats, and is unit-tested against strings real resolvers returned. Use it. Delegation also stops printing its own record list, since printing operator-managed records is what that tool is for. Two additions were needed: - `challenge-cname`, the `_acme-challenge` CNAME that makes delegation work. It gets an exact-match check rather than the gateway CNAME's: check_alias falls back to comparing addresses so a flattened apex ALIAS is not rejected, but a delegation target holds only a TXT, so there is no address and its absence would be reported as "hostname does not resolve yet". - `--caa-advisory`, so ALLOW_MISSING_CAA still downgrades a failing CAA check to a warning. Delegation gains DNS_SETUP_MODE from the same change. `wait` is the useful one: the operator can start the container and create the records while it waits, instead of the container failing on the first look. The routing records are now verified *before* the first issuance rather than after, so a first deployment no longer spends an ACME attempt on records nobody has created yet. Two smaller fixes in the same area: - `unset_txt_record` returned success when the zone lookup failed, because `get_dns_records` returns `[]` for both "no records" and "could not look". Observed as `Error: Could not find zone` immediately followed by `Successfully unset TXT record`, exit 0. - `_ensure_zone_id` returned the *previously cached* zone id when a lookup failed, so a write meant for one zone could land in another. Not currently reachable — `dnsman.py` runs fresh per call — but delegation is precisely the feature that makes "which zone are we writing to" a security property. Refs #106.
Routing the delegation checks through dnsguide silently changed what the CAA check means. dnsguide asks "does anything forbid us from issuing", and under RFC 8659 an absent CAA record set forbids nothing, so it passes. Delegation asks the opposite question: that record is the only thing stopping anyone else who can satisfy the delegated challenge from getting a certificate for the name, and we hold no token to create it, so its absence is exactly the state that has to block. The end-to-end run caught it -- with no CAA at all the check reported "ok (no CAA record set; issuance is unrestricted)" and let issuance proceed, where #104 would have refused to start. The unit tests did not, because they only ever asserted on records that exist. `--caa-required` inverts the rule for this one caller. ALLOW_MISSING_CAA still downgrades it to a warning, so the escape hatch behaves as documented. It is now better than the original in one respect: under DNS_SETUP_MODE wait the container waits for the operator to create the record rather than failing on the first look, and still fails after DNS_SETUP_TIMEOUT. TESTING.md documents how to exercise delegation without a second registrar account -- any name under a zone you already control works, because the provider resolves zones by longest-suffix match -- along with what that substitution does not cover.
Test report — delegated issuance, end to endThe path #104 shipped without executing has now been run: hook writes the TXT Scope, stated up front: this covers issuance. The serving path — gateway Substituting for a delegation zoneNo second registrar account was needed. The provider resolves a zone by What that does not test: the point of delegation is that a token scoped to The runReal Cloudflare credentials, Let's Encrypt staging, the three static records The challenge TXT was gone from the authoritative nameserver afterwards, so the Only the It caught a regression this PR had introducedWith no CAA record on the name at all, the check reported and let issuance through. #104 would have refused to start. Routing the delegation checks through dnsguide had silently changed the Fixed in 4d06340 with After the fix, with the CAA still missing: and once the record was created, without restarting the container: That is better than the original behaviour: under Suites27 unit tests (up from 18: six for the delegation record and exact CNAME check, Still not covered
|
Second test report — the serving path, on real hardwareThe earlier report covered issuance only. This run adds what it left out: a real Setup mirrors #105's tls-alpn-01 testing — TDX host running Issuance, in a CVMThe three operator records were created by hand, with the real app id this The CAA gate then blocked, as intended: and released once the record existed, without a restart. TrafficThat is the whoami backend inside the CVM, reached over the public internet. The gateway resolved the app-address TXT and load-balanced on it: and the CVM's own HAProxy logged the arrival from the gateway's WireGuard An incidental confirmation of something #105 documented: the first few Attestation, over the same pathFetched through the gateway from So the chain closes for a delegation-issued certificate too: Intel root → quote → Snags worth recordingNothing to do with this PR, but they cost time and would cost the next person the
Still not covered
|
Follow-ups to #104, from reviewing it after it merged. Tracked in #106; this PR
covers the P1 and both P3 items. No revert needed — the mechanism in #104 is
sound, these are the sharp edges around it.
The delegated-record check was a second, weaker implementation
verify_delegation_caa()did its own DoH lookup: one resolver,grep -Foverthe rdata, refuse to start if not found. Each of those has a failure mode that
dnsguide.pyalready handles, because tls-alpn-01 hit them first.Negative caching. Public resolvers cache "no such record" for the zone's SOA
minimum — up to an hour. A resolver asked before the operator created the record
keeps reporting it absent long after it exists. Fail closed on that and you have
a container refusing to start over a record that is there. This was observed
during #105's testing on exactly this kind of check.
The asymmetry ran backwards.
Wire format.
grep -F "accounturi=…"matches presentation-form rdata, whichis what
dns.googlereturns. Adding a second resolver to fix the first problemwould have quietly broken it: Cloudflare answers CAA in RFC 3597 generic hex
(
\# 47 00 05 69 73 73 75 65 …), the literal match fails, and — failing closed —that is an outage.
dnsguide.pyqueries two resolvers with union quorum, parses both encodings, andis unit-tested against strings real resolvers actually returned. Delegation now
calls it, and stops printing its own record list, since printing
operator-managed records is what that tool exists for.
What that needed
challenge-cname— the_acme-challengeCNAME. It gets an exact-matchcheck rather than the gateway CNAME's:
check_aliasfalls back to comparingaddresses so a flattened apex ALIAS is not rejected, but a delegation target
holds only a TXT, so there is no address and its absence was reported as
"hostname does not resolve yet".
--caa-advisory— soALLOW_MISSING_CAA=truestill downgrades a failing CAAcheck to a warning.
What it gains
DNS_SETUP_MODEnow applies to delegation.wait(the default) blocks until therecords appear, so the operator can start the container and create them
afterwards rather than having it fail on the first look;
webhookPOSTs them toan automation endpoint with an HMAC and a TDX quote.
The routing records are also verified before the first issuance instead of
after, so a first deployment no longer spends an ACME attempt — and one of Let's
Encrypt's five failed validations per hostname per hour — on records nobody has
created yet.
unset_txt_recordreported success when the zone lookup failedget_dns_records()returns[]both for "this name has no TXT records" and for"the zone could not be resolved", so the cleanup iterated nothing and returned
True:
Exit code 0. The hook discards cleanup failures on purpose, so the impact is a
stale TXT in the delegation zone — but the log stated the opposite of what
happened. Providers can now answer
zone_is_resolvable(); the base class staysoptimistic so providers that do not model zones are unaffected.
_ensure_zone_idfell back to a stale cached zoneA failed lookup returned whichever zone was cached last, so a write meant for one
zone could land in another. Not currently reachable —
dnsman.pyruns as a freshprocess per call — but delegation is precisely the feature that makes "which zone
are we writing to" a security property.
Verification
Container run in delegation mode,
DNS_SETUP_MODE=wait:All four records with correct values, two resolvers, and the delegation CNAME
reporting its own absence accurately rather than borrowing the address-based
message.
Six new unit tests cover the delegation record (target, wildcard base name,
absent without a delegation zone) and the exact CNAME check (accepts the target,
rejects a different one, reports absence). 24 tests pass, plus the sanitizer
suite.
Still not covered, and unchanged by this PR: a real delegated issuance —
hook writes the TXT, Let's Encrypt follows the CNAME, validation passes. That
needs a delegation zone.
TESTING.mddescribes the setup.Not in this PR
set_txt_recorddeletes every TXT at a name before writing. That is correct forthe routing records and wrong for challenges, which is invisible today because
process_domainpasses one-dper certbot invocation. It stops being invisiblewhen one certificate covers several identifiers validating at the same name —
example.complus*.example.com, or #86's SAN support. Left for whoever lands#86, noted in #106.