From 8dad8ebc31c81ac3567e448abdf37105be6ce46a Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 28 Jul 2026 02:11:15 -0700 Subject: [PATCH 1/2] fix(dstack-ingress): verify delegated records with dnsguide, not a second checker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- custom-domain/dstack-ingress/README.md | 24 ++-- custom-domain/dstack-ingress/scripts/dns01.sh | 125 ++++++++---------- .../scripts/dns_providers/base.py | 24 +++- .../scripts/dns_providers/cloudflare.py | 12 +- .../dstack-ingress/scripts/dnsguide.py | 57 +++++++- .../scripts/tests/test_dnsguide.py | 57 ++++++++ 6 files changed, 213 insertions(+), 86 deletions(-) diff --git a/custom-domain/dstack-ingress/README.md b/custom-domain/dstack-ingress/README.md index a0db066e..f209c6ce 100644 --- a/custom-domain/dstack-ingress/README.md +++ b/custom-domain/dstack-ingress/README.md @@ -173,9 +173,9 @@ environment: | `TXT_PREFIX` | `_dstack-app-address` | DNS TXT record prefix | | `ACME_STAGING` | `false` | Use Let's Encrypt staging, in either mode. `CERTBOT_STAGING` is the historical name and still works | | `CHALLENGE_TYPE` | `dns-01` | `dns-01` (certbot + DNS credentials) or `tls-alpn-01` (lego, no DNS credentials) | -| `DNS_SETUP_MODE` | `wait` | tls-alpn-01 only: `wait`, `print` or `webhook` — see below | -| `DNS_SETUP_TIMEOUT` | `1800` | tls-alpn-01 only: seconds to wait for the records to appear | -| `DNS_SETUP_INTERVAL` | `15` | tls-alpn-01 only: seconds between DNS checks | +| `DNS_SETUP_MODE` | `wait` | How to handle operator-managed records: `wait`, `print` or `webhook` — see below. Applies to tls-alpn-01 and to dns-01 challenge delegation | +| `DNS_SETUP_TIMEOUT` | `1800` | Seconds to wait for those records to appear | +| `DNS_SETUP_INTERVAL` | `15` | Seconds between DNS checks | | `DNS_WEBHOOK_URL` | | tls-alpn-01 + `DNS_SETUP_MODE=webhook`: endpoint to notify | | `DNS_WEBHOOK_TOKEN` | | Shared secret; the payload is HMAC-SHA256 signed with it | | `DOH_RESOLVERS` | Google + Cloudflare | Comma-separated DoH endpoints used to verify records | @@ -193,7 +193,7 @@ environment: | `ALPN` | | TLS ALPN protocols (e.g. `h2,http/1.1`). Only set if backends support h2c | | `ACME_CHALLENGE_ALIAS` | | Delegate the ACME DNS-01 challenge to this zone (see below) so the DNS token needs no access to the served domain's own zone | | `ACME_CHALLENGE_PROPAGATION_SECONDS` | `30` | Wait after writing the delegated challenge TXT before validation (only with `ACME_CHALLENGE_ALIAS`). Keep well under ~250s — certbot is killed after a 300s per-run timeout | -| `ALLOW_MISSING_CAA` | `false` | In delegation mode, continue even if the required `accounturi` CAA cannot be confirmed. Default fails closed (see below) | +| `ALLOW_MISSING_CAA` | `false` | In delegation mode, treat an unconfirmed `accounturi` CAA as a warning instead of a blocker. Default fails closed (see below) | For DNS provider credentials, see [DNS_PROVIDERS.md](DNS_PROVIDERS.md). @@ -221,11 +221,10 @@ svc.example.com CAA 0 issue "letsencrypt.org;validation > **Security note.** The `accounturi` CAA restricts issuance to this enclave's > ACME account and is the control that prevents forged certificates. In > delegation mode dstack-ingress cannot set it for you (no token for the served -> zone), so it prints the record and verifies its presence via DoH. **If the -> CAA is confirmed absent the container fails to start** (set `ALLOW_MISSING_CAA=true` -> to override; a transient DoH failure only warns, it does not block). Without -> this CAA, anyone who can satisfy the delegated challenge could obtain a -> certificate for the domain. +> zone), so it prints the record and verifies it. **A CAA that is confirmed +> absent stops issuance** — set `ALLOW_MISSING_CAA=true` to downgrade that to a +> warning. Without this CAA, anyone who can satisfy the delegated challenge +> could obtain a certificate for the domain. > > Use a **dedicated** delegation zone and scope the token to only that zone — a > zone shared with other tenants lets anyone with write access to it complete @@ -235,6 +234,13 @@ svc.example.com CAA 0 issue "letsencrypt.org;validation > delegation, otherwise `certbot renew` reuses the old plugin (which needs the > production-zone token this mode avoids). +The records above are checked the same way tls-alpn-01 checks its own: two DoH +resolvers, both CAA wire formats, and `DNS_SETUP_MODE` deciding what happens +while they are missing. `wait` (the default) blocks until they appear, so you can +start the container and create the records afterwards; `print` lists them and +continues without checking; `webhook` POSTs them to `DNS_WEBHOOK_URL` for an +operator service to create automatically. + ## Evidence & Attestation Evidence files are served at `https://your-domain.com/evidences/` by default (via payload inspection in HAProxy's TCP mode). They can also be accessed by the backend application through the shared `/evidences` volume. diff --git a/custom-domain/dstack-ingress/scripts/dns01.sh b/custom-domain/dstack-ingress/scripts/dns01.sh index ada2353e..abcfd4ee 100644 --- a/custom-domain/dstack-ingress/scripts/dns01.sh +++ b/custom-domain/dstack-ingress/scripts/dns01.sh @@ -76,16 +76,6 @@ EOF set_alias_record() { local domain="$1" - if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then - # certbot validates the challenge at the base name even for a wildcard, - # so the _acme-challenge CNAME must use the base domain (strip "*."). - local base="${domain#\*.}" - echo "[challenge-delegation] Not touching ${domain}'s own zone (token is scoped to the delegated zone)." - echo "[challenge-delegation] Set these in your production zone yourself (static, one-time):" - echo " ${domain} CNAME ${GATEWAY_DOMAIN}" - echo " _acme-challenge.${base} CNAME _acme-challenge.${base}.${ACME_CHALLENGE_ALIAS}" - return - fi echo "Setting alias record for $domain" dnsman.py set_alias \ --domain "$domain" \ @@ -105,12 +95,6 @@ set_txt_record() { local txt_domain txt_domain=$(txt_record_name "$domain") - if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then - echo "[challenge-delegation] Set this in your production zone yourself (static, one-time):" - echo " ${txt_domain} TXT \"$(txt_record_value)\"" - return - fi - dnsman.py set_txt \ --domain "$txt_domain" \ --content "$(txt_record_value)" @@ -121,70 +105,61 @@ set_txt_record() { fi } -# In delegation mode we have NO token for the served domain's zone, so we cannot -# set the accounturi CAA ourselves. That CAA is the forge-prevention (only this -# enclave's ACME account may issue), so this path is deliberately NOT gated by -# SET_CAA and fails closed if the record is confirmed absent -- otherwise anyone -# who can satisfy the delegated DNS-01 challenge could obtain a cert for the -# domain. Set ALLOW_MISSING_CAA=true to override (accept the risk). -verify_delegation_caa() { - local domain="$1" - local account_file account_uri caa_domain caa_tag caa_value resp status found - - if ! account_file=$(get_letsencrypt_account_file); then - echo "ERROR: cannot read the Let's Encrypt account file to determine the required accounturi CAA" >&2 - caa_fail_or_allow "$domain" - return - fi +# Challenge delegation: this container holds no token for the served domain's +# zone, so the operator creates those records. dnsguide.py is what prints and +# verifies operator-managed records everywhere else in this image, so use it +# here too rather than a second implementation -- it queries two resolvers +# instead of one, parses both CAA wire formats, and understands DNS_SETUP_MODE. +# +# $1 domain, $2 comma-separated record subset, $3.. extra dnsguide flags. +delegation_guide() { + local domain="$1" include="$2" + shift 2 + local caa_domain caa_tag caa_domain="${domain#\*.}" caa_tag=$(caa_tag_for "$domain") - account_uri=$(jq -j '.uri' "$account_file") - caa_value="letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" - echo "[challenge-delegation] Set this CAA in your production zone (static, one-time):" - echo " ${caa_domain} CAA 0 ${caa_tag} \"${caa_value}\"" - - # Verify via DNS-over-HTTPS (dig is not installed in the image; curl+jq are). - resp=$(curl -s --max-time 10 "https://dns.google/resolve?name=${caa_domain}&type=257" 2>/dev/null || true) - if [ -z "$resp" ]; then - echo "WARNING: could not reach dns.google to verify the CAA (network issue) — NOT confirmed; continuing" - return - fi - status=$(echo "$resp" | jq -r '.Status // empty' 2>/dev/null || true) - if [ "$status" != "0" ]; then - echo "WARNING: CAA DoH query for ${caa_domain} returned status ${status:-unknown} — NOT confirmed; continuing" - return - fi - # Literal (grep -F) match on the CAA data fields — the account URI contains - # '/' and '.', which must not be treated as regex. - found=$(echo "$resp" | jq -r '.Answer // [] | .[] | .data' 2>/dev/null | grep -F "accounturi=$account_uri" || true) - if [ -n "$found" ]; then - echo "[challenge-delegation] Verified: accounturi CAA is present for $caa_domain" - return - fi - echo "ERROR: the accounturi CAA is NOT present for $caa_domain." >&2 - echo "ERROR: without it, anyone who can satisfy the delegated DNS-01 challenge could obtain a" >&2 - echo "ERROR: publicly-trusted certificate for this domain (forged TLS termination)." >&2 - caa_fail_or_allow "$domain" + dnsguide.py \ + --domain "$domain" \ + --alias-target "$GATEWAY_DOMAIN" \ + --txt-name "$(txt_record_name "$domain")" \ + --txt-value "$(txt_record_value)" \ + --caa-name "$caa_domain" \ + --caa-tag "$caa_tag" \ + --challenge dns-01 \ + --challenge-alias "$ACME_CHALLENGE_ALIAS" \ + --mode "${DNS_SETUP_MODE:-wait}" \ + --include "$include" \ + "$@" } -caa_fail_or_allow() { - local domain="$1" - if [ "${ALLOW_MISSING_CAA:-false}" = "true" ]; then - echo "WARNING: ALLOW_MISSING_CAA=true — continuing without a verified accounturi CAA for $domain" - return 0 +# The accounturi CAA is what stops anyone else who can satisfy the delegated +# challenge from getting a certificate for this name, and we cannot write it, so +# a confirmed-absent record is fatal. ALLOW_MISSING_CAA downgrades it to a +# warning for operators who accept that risk. +delegation_verify_caa() { + local domain="$1" account_file account_uri advisory=() + if ! account_file=$(get_letsencrypt_account_file); then + echo "ERROR: cannot read the ACME account file to determine the required CAA" >&2 + return 1 fi - echo "ERROR: refusing to continue without the accounturi CAA for $domain." >&2 - echo "ERROR: set the CAA record shown above, or ALLOW_MISSING_CAA=true to override." >&2 - exit 1 + account_uri=$(jq -j '.uri' "$account_file") + [ "${ALLOW_MISSING_CAA:-false}" = "true" ] && advisory=(--caa-advisory) + + delegation_guide "$domain" caa \ + --caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" \ + --account-uri "$account_uri" \ + "${advisory[@]}" } set_caa_record() { local domain="$1" - # Delegation mode is handled separately and is NOT gated by SET_CAA. + # Delegation is handled separately and is deliberately NOT gated by + # SET_CAA: there we cannot write the record, so verifying it is the only + # protection left. if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then - verify_delegation_caa "$domain" + delegation_verify_caa "$domain" || exit 1 return fi @@ -223,8 +198,18 @@ set_caa_record() { process_domain() { local domain="$1" first status - set_alias_record "$domain" - set_txt_record "$domain" + if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then + # Delegation: the operator owns these. Verify them before spending an + # ACME attempt that would fail on records nobody has created yet. CAA + # is checked after the first issuance, once the account URI exists. + if ! delegation_guide "$domain" cname,txt,challenge-cname; then + echo "Error: required DNS records for $domain are not in place" >&2 + return 1 + fi + else + set_alias_record "$domain" + set_txt_record "$domain" + fi # The CAA record names our ACME account, so the account has to exist first: # try once, write CAA, try again. The first attempt is expected to fail when diff --git a/custom-domain/dstack-ingress/scripts/dns_providers/base.py b/custom-domain/dstack-ingress/scripts/dns_providers/base.py index e8e89570..ff5319a6 100644 --- a/custom-domain/dstack-ingress/scripts/dns_providers/base.py +++ b/custom-domain/dstack-ingress/scripts/dns_providers/base.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 import os +import sys from abc import ABC, abstractmethod from typing import Dict, List, Optional, Any @@ -247,6 +248,15 @@ def set_txt_record(self, name: str, content: str, ttl: int = 60) -> bool: ) return self.create_dns_record(new_record) + def zone_is_resolvable(self, name: str) -> bool: + """Whether the provider can find a zone that owns this name. + + Lets callers tell "nothing there" apart from "could not look". The base + answer is optimistic so providers that do not model zones keep their + current behaviour; override where the distinction is knowable. + """ + return True + def unset_txt_record(self, name: str) -> bool: """Delete all TXT records for a name. @@ -259,8 +269,20 @@ def unset_txt_record(self, name: str) -> bool: Returns: True if all matching records were removed (or none existed). """ + # get_dns_records returns [] both for "this name has no TXT records" and + # for "the zone could not be resolved", so an empty list on its own would + # report a failed cleanup as a success. Ask whether the zone resolves + # before believing the emptiness. + records = self.get_dns_records(name, RecordType.TXT) + if not records and not self.zone_is_resolvable(name): + print( + f"Error: cannot delete TXT records for {name}: zone lookup failed", + file=sys.stderr, + ) + return False + ok = True - for record in self.get_dns_records(name, RecordType.TXT): + for record in records: if not record.id: print(f"Warning: TXT record for {name} has no id; cannot delete") ok = False diff --git a/custom-domain/dstack-ingress/scripts/dns_providers/cloudflare.py b/custom-domain/dstack-ingress/scripts/dns_providers/cloudflare.py index ca1208be..578fba4b 100644 --- a/custom-domain/dstack-ingress/scripts/dns_providers/cloudflare.py +++ b/custom-domain/dstack-ingress/scripts/dns_providers/cloudflare.py @@ -158,10 +158,18 @@ def _ensure_zone_id(self, domain: str) -> Optional[str]: return self.zone_id zone_info = self._get_zone_info(domain) - if zone_info: - self.zone_id, self.zone_domain = zone_info + if not zone_info: + # Returning the cached id here would hand back a *different* zone's + # id, so a write meant for one zone could land in another. That is a + # correctness problem generally and a security one under challenge + # delegation, where "which zone are we writing to" is the point. + return None + self.zone_id, self.zone_domain = zone_info return self.zone_id + def zone_is_resolvable(self, name: str) -> bool: + return self._ensure_zone_id(name) is not None + def get_dns_records( self, name: str, record_type: Optional[RecordType] = None ) -> List[DNSRecord]: diff --git a/custom-domain/dstack-ingress/scripts/dnsguide.py b/custom-domain/dstack-ingress/scripts/dnsguide.py index 7971e864..afb38248 100644 --- a/custom-domain/dstack-ingress/scripts/dnsguide.py +++ b/custom-domain/dstack-ingress/scripts/dnsguide.py @@ -62,6 +62,11 @@ class Record: name: str value: str note: str = "" + # A CNAME the operator points at the gateway may legitimately be flattened + # to an address record, so check_alias accepts either. A delegation CNAME + # cannot: its target holds only a TXT, so there is no address to compare + # and the record has to be the CNAME itself. + exact: bool = False class ResolveError(RuntimeError): @@ -156,6 +161,17 @@ def check_txt(record: Record, resolvers: str) -> Tuple[bool, str]: ) +def check_cname_exact(record: Record, resolvers: str) -> Tuple[bool, str]: + """Verify a CNAME points at exactly this target, with no address fallback.""" + target = _fqdn(record.value) + cnames = [_fqdn(v) for v in query_union(record.name, RR_CNAME, resolvers)] + if target in cnames: + return True, "ok (CNAME)" + if cnames: + return False, f"CNAME points at {cnames!r}, expected {target!r}" + return False, "no CNAME record found" + + def check_alias(record: Record, resolvers: str) -> Tuple[bool, str]: """Verify the hostname ends up at the gateway. @@ -324,6 +340,7 @@ def verify_once( caa_method: str, account_uri: str, resolvers: str, + caa_advisory: bool = False, ) -> List[Tuple[str, bool, str]]: results = [] for rec in records: @@ -331,10 +348,16 @@ def verify_once( ok, why = check_txt(rec, resolvers) results.append((f"TXT {rec.name}", ok, why)) elif rec.type == "CNAME": - ok, why = check_alias(rec, resolvers) + if rec.exact: + ok, why = check_cname_exact(rec, resolvers) + else: + ok, why = check_alias(rec, resolvers) results.append((f"CNAME {rec.name}", ok, why)) ok, why = check_caa(domain, caa_tag, caa_method, account_uri, resolvers) - results.append((f"CAA {domain}", ok, why)) + if not ok and caa_advisory: + results.append((f"CAA {domain}", True, f"WARNING: {why} (continuing: CAA is advisory)")) + else: + results.append((f"CAA {domain}", ok, why)) return results @@ -359,6 +382,21 @@ def build_records(args: argparse.Namespace) -> List[Record]: note="tells the gateway which instance to route this hostname to", ) ) + if "challenge-cname" in include and args.challenge_alias: + # Challenge delegation: the CA follows this CNAME when it looks for + # _acme-challenge, so the TXT can live in a zone whose token we hold and + # the served zone never needs one. certbot validates at the base name + # even for a wildcard, so strip "*." here too. + base = args.domain[2:] if args.domain.startswith("*.") else args.domain + records.append( + Record( + type="CNAME", + name=f"_acme-challenge.{base}", + value=f"_acme-challenge.{base}.{args.challenge_alias}", + note="delegates the ACME challenge to a zone this container can write", + exact=True, + ) + ) if "caa" in include and args.caa_value: note = "optional; restricts issuance for this name to Let's Encrypt" if args.account_uri: @@ -385,6 +423,16 @@ def main() -> int: parser.add_argument("--caa-value", default="") parser.add_argument("--account-uri", default="") parser.add_argument("--challenge", default="tls-alpn-01") + parser.add_argument( + "--challenge-alias", + default=os.environ.get("ACME_CHALLENGE_ALIAS", ""), + help="delegation zone for the _acme-challenge CNAME (dns-01 delegation)", + ) + parser.add_argument( + "--caa-advisory", + action="store_true", + help="report a failing CAA check as a warning instead of blocking", + ) parser.add_argument( "--mode", default=os.environ.get("DNS_SETUP_MODE", "wait"), @@ -401,7 +449,8 @@ def main() -> int: parser.add_argument( "--include", default="cname,txt,caa", - help="comma-separated subset of records to handle (cname,txt,caa)", + help="comma-separated subset of records to handle " + "(cname,txt,caa,challenge-cname)", ) args = parser.parse_args() @@ -434,7 +483,7 @@ def main() -> int: try: results = verify_once( records, args.domain.lstrip("*."), args.caa_tag, args.challenge, - args.account_uri, args.resolver, + args.account_uri, args.resolver, args.caa_advisory, ) except ResolveError as exc: print(f"[dns-check #{attempt}] resolver problem: {exc}", flush=True) diff --git a/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py b/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py index 8a801b33..6a296ec6 100644 --- a/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py +++ b/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py @@ -119,6 +119,63 @@ def test_reason_is_not_repeated(self): self.assertEqual(reason.count("excludes tls-alpn-01"), 1, reason) +class TestDelegationRecord(unittest.TestCase): + """The _acme-challenge CNAME that makes challenge delegation work.""" + + def _args(self, domain, alias): + import argparse + return argparse.Namespace( + domain=domain, alias_target="gw.example.net", + txt_name="_dstack-app-address." + domain.lstrip("*."), + txt_value="appid:443", caa_name="", caa_tag="issue", caa_value="", + account_uri="", challenge="dns-01", challenge_alias=alias, + include="challenge-cname", + ) + + def test_points_at_the_delegation_zone(self): + rec = dnsguide.build_records(self._args("svc.example.com", "deleg.example.net"))[0] + self.assertEqual(rec.name, "_acme-challenge.svc.example.com") + self.assertEqual(rec.value, "_acme-challenge.svc.example.com.deleg.example.net") + self.assertTrue(rec.exact, "a delegation CNAME has no address to fall back to") + + def test_wildcard_validates_at_the_base_name(self): + rec = dnsguide.build_records(self._args("*.example.com", "deleg.example.net"))[0] + self.assertEqual(rec.name, "_acme-challenge.example.com") + + def test_absent_without_a_delegation_zone(self): + self.assertEqual(dnsguide.build_records(self._args("svc.example.com", "")), []) + + +class TestExactCnameCheck(unittest.TestCase): + """check_alias falls back to comparing addresses; a delegation target has none.""" + + REC = dnsguide.Record(type="CNAME", name="_acme-challenge.svc.example.com", + value="_acme-challenge.svc.example.com.deleg.example.net", + exact=True) + + def _with(self, cnames): + saved = dnsguide.query_union + dnsguide.query_union = lambda name, rr, r: cnames if rr == dnsguide.RR_CNAME else [] + self.addCleanup(lambda: setattr(dnsguide, "query_union", saved)) + + def test_accepts_the_exact_target(self): + self._with(["_acme-challenge.svc.example.com.deleg.example.net."]) + ok, _ = dnsguide.check_cname_exact(self.REC, "r") + self.assertTrue(ok) + + def test_rejects_a_different_target(self): + self._with(["somewhere.else.example.net."]) + ok, why = dnsguide.check_cname_exact(self.REC, "r") + self.assertFalse(ok) + self.assertIn("expected", why) + + def test_reports_absence_without_claiming_it_does_not_resolve(self): + self._with([]) + ok, why = dnsguide.check_cname_exact(self.REC, "r") + self.assertFalse(ok) + self.assertIn("no CNAME record found", why) + + class TestTxtNormalisation(unittest.TestCase): def test_strips_quotes(self): self.assertEqual(dnsguide._unquote_txt('"abc:443"'), "abc:443") From 4d06340eb5aa3694329abaef29c2f3466e8d933a Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 28 Jul 2026 02:42:10 -0700 Subject: [PATCH 2/2] fix(dstack-ingress): keep delegation's CAA gate fail-closed 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. --- custom-domain/dstack-ingress/TESTING.md | 46 +++++++++++++++++++ custom-domain/dstack-ingress/scripts/dns01.sh | 11 +++-- .../dstack-ingress/scripts/dnsguide.py | 33 +++++++++++-- .../scripts/tests/test_dnsguide.py | 42 +++++++++++++++++ 4 files changed, 126 insertions(+), 6 deletions(-) diff --git a/custom-domain/dstack-ingress/TESTING.md b/custom-domain/dstack-ingress/TESTING.md index 4ba1d820..e2edc04e 100644 --- a/custom-domain/dstack-ingress/TESTING.md +++ b/custom-domain/dstack-ingress/TESTING.md @@ -199,6 +199,51 @@ first issuance. payload, and a TDX quote whose `report_data` is `sha256(payload)`. Verify both: the HMAC proves the shared secret, the quote proves the enclave. +### Challenge delegation (dns-01) + +`ACME_CHALLENGE_ALIAS` answers the DNS-01 challenge in a zone the container's +token controls, so the token needs no access to the served domain's own zone. +Testing it looks like it needs a second registrar account. It does not. + +Any name under a zone you already control works as the delegation zone, because +the provider resolves a zone by longest-suffix match on the record name. With +`ACME_CHALLENGE_ALIAS=deleg.example.com` and a served domain of +`svc.example.com`, the challenge record is +`_acme-challenge.svc.example.com.deleg.example.com`, which lands in +`example.com` — the same zone, but through the delegation code path. + +Play the operator and create the three static records the container prints: + +``` +svc.example.com CNAME +_dstack-app-address.svc.example.com TXT ":443" +_acme-challenge.svc.example.com CNAME _acme-challenge.svc.example.com.deleg.example.com +``` + +Then start the container with a real provider token and `ACME_STAGING=true`, and +watch for: the three records verifying, `Executing (challenge-delegation):` with +`--manual` and both hooks, the certificate arriving, and the challenge TXT being +cleaned up afterwards (query `_acme-challenge..` at the +authoritative nameserver — it should be gone). + +The CAA step comes last, once the ACME account exists and its URI is known. It +blocks until you create the record it prints. Cloudflare's API wants CAA as +structured fields rather than one string, so most quick DNS scripts cannot write +it; the image's own `dnsman.py set_caa` can, and running it in a one-off +container is the easiest way to play the operator here: + +```bash +docker run --rm --env-file .env --entrypoint dnsman.py \ + set_caa --domain svc.example.com --caa-tag issue \ + --caa-value 'letsencrypt.org;validationmethods=dns-01;accounturi=' +``` + +**What this does not test.** The whole point of delegation is that a token scoped +to *only* the delegation zone is enough. Sharing one zone exercises every line of +the mechanism but leaves that property unverified — the token still has access to +the served zone, so a bug that quietly writes there would not show up. Confirming +the scoping needs a genuinely separate zone with a separate token. + ### Negative paths These fail fast and are cheap, so run them on every change: @@ -206,6 +251,7 @@ These fail fast and are cheap, so run them on every change: | Scenario | Expected | |---|---| | tls-alpn-01 + a wildcard domain | Refused before any ACME call, citing RFC 8737 | +| Delegation with no CAA record at all | Blocked — unlike normal issuance, where "no CAA" means unrestricted and passes | | A CAA record with `validationmethods=dns-01`, mode tls-alpn-01 | Blocked with the restriction quoted back | | TXT holding the wrong value | Reported as `want , saw `, not "missing" | | An instance ID the gateway does not know | The CA reports a connection error — the gateway will not route to it | diff --git a/custom-domain/dstack-ingress/scripts/dns01.sh b/custom-domain/dstack-ingress/scripts/dns01.sh index abcfd4ee..0e0790d5 100644 --- a/custom-domain/dstack-ingress/scripts/dns01.sh +++ b/custom-domain/dstack-ingress/scripts/dns01.sh @@ -138,18 +138,23 @@ delegation_guide() { # a confirmed-absent record is fatal. ALLOW_MISSING_CAA downgrades it to a # warning for operators who accept that risk. delegation_verify_caa() { - local domain="$1" account_file account_uri advisory=() + local domain="$1" account_file account_uri if ! account_file=$(get_letsencrypt_account_file); then echo "ERROR: cannot read the ACME account file to determine the required CAA" >&2 return 1 fi account_uri=$(jq -j '.uri' "$account_file") - [ "${ALLOW_MISSING_CAA:-false}" = "true" ] && advisory=(--caa-advisory) + # Absent is the state to block on here, not "absent means unrestricted": + # we cannot create this record, so nothing else protects the name. + local gate=(--caa-required) + if [ "${ALLOW_MISSING_CAA:-false}" = "true" ]; then + gate=(--caa-required --caa-advisory) + fi delegation_guide "$domain" caa \ --caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" \ --account-uri "$account_uri" \ - "${advisory[@]}" + "${gate[@]}" } set_caa_record() { diff --git a/custom-domain/dstack-ingress/scripts/dnsguide.py b/custom-domain/dstack-ingress/scripts/dnsguide.py index afb38248..eb5ab6ce 100644 --- a/custom-domain/dstack-ingress/scripts/dnsguide.py +++ b/custom-domain/dstack-ingress/scripts/dnsguide.py @@ -269,12 +269,24 @@ def _caa_permits(value: str, tag: str, want_method: str, account_uri: str) -> Tu def check_caa( - domain: str, want_tag: str, want_method: str, account_uri: str, resolvers: str + domain: str, + want_tag: str, + want_method: str, + account_uri: str, + resolvers: str, + require_present: bool = False, ) -> Tuple[bool, str]: """Check CAA for domain, walking up to the closest ancestor that has one. An absent CAA record set means every CA may issue (RFC 8659), so "no CAA - anywhere" is a pass, not a failure. + anywhere" is normally a pass: we are asking whether *we* may issue, and + nothing forbids it. + + `require_present` inverts that for challenge delegation, where the question + is different. There the record is not a formality but the only thing stopping + someone else who can satisfy the delegated challenge from getting a + certificate for this name, and we hold no token to create it ourselves. An + unrestricted name is exactly the state that must block. """ labels = _fqdn(domain).split(".") for i in range(len(labels) - 1): @@ -308,6 +320,11 @@ def check_caa( reasons.append(reason) return False, f"CAA at {candidate} does not permit issuance: {'; '.join(reasons)}" + if require_present: + return False, ( + "no CAA record set, so any account that can satisfy the delegated " + "challenge could obtain a certificate for this name" + ) return True, "ok (no CAA record set; issuance is unrestricted)" @@ -341,6 +358,7 @@ def verify_once( account_uri: str, resolvers: str, caa_advisory: bool = False, + caa_required: bool = False, ) -> List[Tuple[str, bool, str]]: results = [] for rec in records: @@ -353,7 +371,9 @@ def verify_once( else: ok, why = check_alias(rec, resolvers) results.append((f"CNAME {rec.name}", ok, why)) - ok, why = check_caa(domain, caa_tag, caa_method, account_uri, resolvers) + ok, why = check_caa( + domain, caa_tag, caa_method, account_uri, resolvers, caa_required + ) if not ok and caa_advisory: results.append((f"CAA {domain}", True, f"WARNING: {why} (continuing: CAA is advisory)")) else: @@ -428,6 +448,12 @@ def main() -> int: default=os.environ.get("ACME_CHALLENGE_ALIAS", ""), help="delegation zone for the _acme-challenge CNAME (dns-01 delegation)", ) + parser.add_argument( + "--caa-required", + action="store_true", + help="treat an absent CAA record set as a failure (challenge delegation: " + "the record is the only protection and we cannot create it)", + ) parser.add_argument( "--caa-advisory", action="store_true", @@ -484,6 +510,7 @@ def main() -> int: results = verify_once( records, args.domain.lstrip("*."), args.caa_tag, args.challenge, args.account_uri, args.resolver, args.caa_advisory, + args.caa_required, ) except ResolveError as exc: print(f"[dns-check #{attempt}] resolver problem: {exc}", flush=True) diff --git a/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py b/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py index 6a296ec6..8112e747 100644 --- a/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py +++ b/custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py @@ -176,6 +176,48 @@ def test_reports_absence_without_claiming_it_does_not_resolve(self): self.assertIn("no CNAME record found", why) +class TestCaaRequiredForDelegation(unittest.TestCase): + """Delegation inverts the usual CAA question. + + Normally we ask "does anything forbid us from issuing", and no CAA at all + means no. Under challenge delegation the record is the only thing stopping + someone else who can satisfy the delegated challenge, and we cannot create + it, so its absence has to block. + """ + + def _no_caa(self): + saved = dnsguide.query_union + dnsguide.query_union = lambda name, rr, r: [] + self.addCleanup(lambda: setattr(dnsguide, "query_union", saved)) + + def test_absent_caa_passes_by_default(self): + self._no_caa() + ok, why = dnsguide.check_caa("app.example.com", "issue", "dns-01", "", "r") + self.assertTrue(ok) + self.assertIn("unrestricted", why) + + def test_absent_caa_blocks_when_required(self): + self._no_caa() + ok, why = dnsguide.check_caa( + "app.example.com", "issue", "dns-01", "", "r", require_present=True + ) + self.assertFalse(ok) + self.assertIn("no CAA record set", why) + + def test_present_and_matching_passes_when_required(self): + saved = dnsguide.query_union + dnsguide.query_union = lambda name, rr, r: ( + ['0 issue "letsencrypt.org;validationmethods=dns-01;accounturi=https://acme/1"'] + if name == "app.example.com" else [] + ) + self.addCleanup(lambda: setattr(dnsguide, "query_union", saved)) + ok, _ = dnsguide.check_caa( + "app.example.com", "issue", "dns-01", "https://acme/1", "r", + require_present=True, + ) + self.assertTrue(ok) + + class TestTxtNormalisation(unittest.TestCase): def test_strips_quotes(self): self.assertEqual(dnsguide._unquote_txt('"abc:443"'), "abc:443")