Merge pull request #107 from Dstack-TEE/fix/ingress-104-followups

fix(dstack-ingress): verify delegated records with dnsguide, not a second checker

Author: kvinwang

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.

custom-domain/dstack-ingress/TESTING.md

@@ -199,13 +199,59 @@ 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  <GATEWAY_DOMAIN>
+_dstack-app-address.svc.example.com  TXT  "<app-id>: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.<domain>.<alias>` 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 <image> \
+  set_caa --domain svc.example.com --caa-tag issue \
+  --caa-value 'letsencrypt.org;validationmethods=dns-01;accounturi=<uri>'
+```
+
+**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:
 
 | 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 <x>, saw <y>`, not "missing" |
 | An instance ID the gateway does not know | The CA reports a connection error — the gateway will not route to it |

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,66 @@ 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"
+# 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
+    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")
+    # 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
-        echo "WARNING: ALLOW_MISSING_CAA=true — continuing without a verified accounturi CAA for $domain"
-        return 0
+        gate=(--caa-required --caa-advisory)
     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
+
+    delegation_guide "$domain" caa \
+        --caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" \
+        --account-uri "$account_uri" \
+        "${gate[@]}"
 }
 
 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 +203,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

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

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]: