Skip to content

Commit 8dad8eb

Browse files
committed
fix(dstack-ingress): verify delegated records with dnsguide, not a second 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.
1 parent ab6d7e1 commit 8dad8eb

6 files changed

Lines changed: 213 additions & 86 deletions

File tree

custom-domain/dstack-ingress/README.md

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,9 @@ environment:
173173
| `TXT_PREFIX` | `_dstack-app-address` | DNS TXT record prefix |
174174
| `ACME_STAGING` | `false` | Use Let's Encrypt staging, in either mode. `CERTBOT_STAGING` is the historical name and still works |
175175
| `CHALLENGE_TYPE` | `dns-01` | `dns-01` (certbot + DNS credentials) or `tls-alpn-01` (lego, no DNS credentials) |
176-
| `DNS_SETUP_MODE` | `wait` | tls-alpn-01 only: `wait`, `print` or `webhook` — see below |
177-
| `DNS_SETUP_TIMEOUT` | `1800` | tls-alpn-01 only: seconds to wait for the records to appear |
178-
| `DNS_SETUP_INTERVAL` | `15` | tls-alpn-01 only: seconds between DNS checks |
176+
| `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 |
177+
| `DNS_SETUP_TIMEOUT` | `1800` | Seconds to wait for those records to appear |
178+
| `DNS_SETUP_INTERVAL` | `15` | Seconds between DNS checks |
179179
| `DNS_WEBHOOK_URL` | | tls-alpn-01 + `DNS_SETUP_MODE=webhook`: endpoint to notify |
180180
| `DNS_WEBHOOK_TOKEN` | | Shared secret; the payload is HMAC-SHA256 signed with it |
181181
| `DOH_RESOLVERS` | Google + Cloudflare | Comma-separated DoH endpoints used to verify records |
@@ -193,7 +193,7 @@ environment:
193193
| `ALPN` | | TLS ALPN protocols (e.g. `h2,http/1.1`). Only set if backends support h2c |
194194
| `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 |
195195
| `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 |
196-
| `ALLOW_MISSING_CAA` | `false` | In delegation mode, continue even if the required `accounturi` CAA cannot be confirmed. Default fails closed (see below) |
196+
| `ALLOW_MISSING_CAA` | `false` | In delegation mode, treat an unconfirmed `accounturi` CAA as a warning instead of a blocker. Default fails closed (see below) |
197197

198198
For DNS provider credentials, see [DNS_PROVIDERS.md](DNS_PROVIDERS.md).
199199

@@ -221,11 +221,10 @@ svc.example.com CAA 0 issue "letsencrypt.org;validation
221221
> **Security note.** The `accounturi` CAA restricts issuance to this enclave's
222222
> ACME account and is the control that prevents forged certificates. In
223223
> delegation mode dstack-ingress cannot set it for you (no token for the served
224-
> zone), so it prints the record and verifies its presence via DoH. **If the
225-
> CAA is confirmed absent the container fails to start** (set `ALLOW_MISSING_CAA=true`
226-
> to override; a transient DoH failure only warns, it does not block). Without
227-
> this CAA, anyone who can satisfy the delegated challenge could obtain a
228-
> certificate for the domain.
224+
> zone), so it prints the record and verifies it. **A CAA that is confirmed
225+
> absent stops issuance** — set `ALLOW_MISSING_CAA=true` to downgrade that to a
226+
> warning. Without this CAA, anyone who can satisfy the delegated challenge
227+
> could obtain a certificate for the domain.
229228
>
230229
> Use a **dedicated** delegation zone and scope the token to only that zone — a
231230
> 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
235234
> delegation, otherwise `certbot renew` reuses the old plugin (which needs the
236235
> production-zone token this mode avoids).
237236

237+
The records above are checked the same way tls-alpn-01 checks its own: two DoH
238+
resolvers, both CAA wire formats, and `DNS_SETUP_MODE` deciding what happens
239+
while they are missing. `wait` (the default) blocks until they appear, so you can
240+
start the container and create the records afterwards; `print` lists them and
241+
continues without checking; `webhook` POSTs them to `DNS_WEBHOOK_URL` for an
242+
operator service to create automatically.
243+
238244
## Evidence & Attestation
239245

240246
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/scripts/dns01.sh

Lines changed: 55 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,6 @@ EOF
7676

7777
set_alias_record() {
7878
local domain="$1"
79-
if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then
80-
# certbot validates the challenge at the base name even for a wildcard,
81-
# so the _acme-challenge CNAME must use the base domain (strip "*.").
82-
local base="${domain#\*.}"
83-
echo "[challenge-delegation] Not touching ${domain}'s own zone (token is scoped to the delegated zone)."
84-
echo "[challenge-delegation] Set these in your production zone yourself (static, one-time):"
85-
echo " ${domain} CNAME ${GATEWAY_DOMAIN}"
86-
echo " _acme-challenge.${base} CNAME _acme-challenge.${base}.${ACME_CHALLENGE_ALIAS}"
87-
return
88-
fi
8979
echo "Setting alias record for $domain"
9080
dnsman.py set_alias \
9181
--domain "$domain" \
@@ -105,12 +95,6 @@ set_txt_record() {
10595
local txt_domain
10696
txt_domain=$(txt_record_name "$domain")
10797

108-
if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then
109-
echo "[challenge-delegation] Set this in your production zone yourself (static, one-time):"
110-
echo " ${txt_domain} TXT \"$(txt_record_value)\""
111-
return
112-
fi
113-
11498
dnsman.py set_txt \
11599
--domain "$txt_domain" \
116100
--content "$(txt_record_value)"
@@ -121,70 +105,61 @@ set_txt_record() {
121105
fi
122106
}
123107

124-
# In delegation mode we have NO token for the served domain's zone, so we cannot
125-
# set the accounturi CAA ourselves. That CAA is the forge-prevention (only this
126-
# enclave's ACME account may issue), so this path is deliberately NOT gated by
127-
# SET_CAA and fails closed if the record is confirmed absent -- otherwise anyone
128-
# who can satisfy the delegated DNS-01 challenge could obtain a cert for the
129-
# domain. Set ALLOW_MISSING_CAA=true to override (accept the risk).
130-
verify_delegation_caa() {
131-
local domain="$1"
132-
local account_file account_uri caa_domain caa_tag caa_value resp status found
133-
134-
if ! account_file=$(get_letsencrypt_account_file); then
135-
echo "ERROR: cannot read the Let's Encrypt account file to determine the required accounturi CAA" >&2
136-
caa_fail_or_allow "$domain"
137-
return
138-
fi
108+
# Challenge delegation: this container holds no token for the served domain's
109+
# zone, so the operator creates those records. dnsguide.py is what prints and
110+
# verifies operator-managed records everywhere else in this image, so use it
111+
# here too rather than a second implementation -- it queries two resolvers
112+
# instead of one, parses both CAA wire formats, and understands DNS_SETUP_MODE.
113+
#
114+
# $1 domain, $2 comma-separated record subset, $3.. extra dnsguide flags.
115+
delegation_guide() {
116+
local domain="$1" include="$2"
117+
shift 2
118+
local caa_domain caa_tag
139119
caa_domain="${domain#\*.}"
140120
caa_tag=$(caa_tag_for "$domain")
141-
account_uri=$(jq -j '.uri' "$account_file")
142-
caa_value="letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri"
143121

144-
echo "[challenge-delegation] Set this CAA in your production zone (static, one-time):"
145-
echo " ${caa_domain} CAA 0 ${caa_tag} \"${caa_value}\""
146-
147-
# Verify via DNS-over-HTTPS (dig is not installed in the image; curl+jq are).
148-
resp=$(curl -s --max-time 10 "https://dns.google/resolve?name=${caa_domain}&type=257" 2>/dev/null || true)
149-
if [ -z "$resp" ]; then
150-
echo "WARNING: could not reach dns.google to verify the CAA (network issue) — NOT confirmed; continuing"
151-
return
152-
fi
153-
status=$(echo "$resp" | jq -r '.Status // empty' 2>/dev/null || true)
154-
if [ "$status" != "0" ]; then
155-
echo "WARNING: CAA DoH query for ${caa_domain} returned status ${status:-unknown} — NOT confirmed; continuing"
156-
return
157-
fi
158-
# Literal (grep -F) match on the CAA data fields — the account URI contains
159-
# '/' and '.', which must not be treated as regex.
160-
found=$(echo "$resp" | jq -r '.Answer // [] | .[] | .data' 2>/dev/null | grep -F "accounturi=$account_uri" || true)
161-
if [ -n "$found" ]; then
162-
echo "[challenge-delegation] Verified: accounturi CAA is present for $caa_domain"
163-
return
164-
fi
165-
echo "ERROR: the accounturi CAA is NOT present for $caa_domain." >&2
166-
echo "ERROR: without it, anyone who can satisfy the delegated DNS-01 challenge could obtain a" >&2
167-
echo "ERROR: publicly-trusted certificate for this domain (forged TLS termination)." >&2
168-
caa_fail_or_allow "$domain"
122+
dnsguide.py \
123+
--domain "$domain" \
124+
--alias-target "$GATEWAY_DOMAIN" \
125+
--txt-name "$(txt_record_name "$domain")" \
126+
--txt-value "$(txt_record_value)" \
127+
--caa-name "$caa_domain" \
128+
--caa-tag "$caa_tag" \
129+
--challenge dns-01 \
130+
--challenge-alias "$ACME_CHALLENGE_ALIAS" \
131+
--mode "${DNS_SETUP_MODE:-wait}" \
132+
--include "$include" \
133+
"$@"
169134
}
170135

171-
caa_fail_or_allow() {
172-
local domain="$1"
173-
if [ "${ALLOW_MISSING_CAA:-false}" = "true" ]; then
174-
echo "WARNING: ALLOW_MISSING_CAA=true — continuing without a verified accounturi CAA for $domain"
175-
return 0
136+
# The accounturi CAA is what stops anyone else who can satisfy the delegated
137+
# challenge from getting a certificate for this name, and we cannot write it, so
138+
# a confirmed-absent record is fatal. ALLOW_MISSING_CAA downgrades it to a
139+
# warning for operators who accept that risk.
140+
delegation_verify_caa() {
141+
local domain="$1" account_file account_uri advisory=()
142+
if ! account_file=$(get_letsencrypt_account_file); then
143+
echo "ERROR: cannot read the ACME account file to determine the required CAA" >&2
144+
return 1
176145
fi
177-
echo "ERROR: refusing to continue without the accounturi CAA for $domain." >&2
178-
echo "ERROR: set the CAA record shown above, or ALLOW_MISSING_CAA=true to override." >&2
179-
exit 1
146+
account_uri=$(jq -j '.uri' "$account_file")
147+
[ "${ALLOW_MISSING_CAA:-false}" = "true" ] && advisory=(--caa-advisory)
148+
149+
delegation_guide "$domain" caa \
150+
--caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" \
151+
--account-uri "$account_uri" \
152+
"${advisory[@]}"
180153
}
181154

182155
set_caa_record() {
183156
local domain="$1"
184157

185-
# Delegation mode is handled separately and is NOT gated by SET_CAA.
158+
# Delegation is handled separately and is deliberately NOT gated by
159+
# SET_CAA: there we cannot write the record, so verifying it is the only
160+
# protection left.
186161
if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then
187-
verify_delegation_caa "$domain"
162+
delegation_verify_caa "$domain" || exit 1
188163
return
189164
fi
190165

@@ -223,8 +198,18 @@ set_caa_record() {
223198
process_domain() {
224199
local domain="$1" first status
225200

226-
set_alias_record "$domain"
227-
set_txt_record "$domain"
201+
if [ -n "${ACME_CHALLENGE_ALIAS:-}" ]; then
202+
# Delegation: the operator owns these. Verify them before spending an
203+
# ACME attempt that would fail on records nobody has created yet. CAA
204+
# is checked after the first issuance, once the account URI exists.
205+
if ! delegation_guide "$domain" cname,txt,challenge-cname; then
206+
echo "Error: required DNS records for $domain are not in place" >&2
207+
return 1
208+
fi
209+
else
210+
set_alias_record "$domain"
211+
set_txt_record "$domain"
212+
fi
228213

229214
# The CAA record names our ACME account, so the account has to exist first:
230215
# try once, write CAA, try again. The first attempt is expected to fail when

custom-domain/dstack-ingress/scripts/dns_providers/base.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#!/usr/bin/env python3
22

33
import os
4+
import sys
45

56
from abc import ABC, abstractmethod
67
from typing import Dict, List, Optional, Any
@@ -247,6 +248,15 @@ def set_txt_record(self, name: str, content: str, ttl: int = 60) -> bool:
247248
)
248249
return self.create_dns_record(new_record)
249250

251+
def zone_is_resolvable(self, name: str) -> bool:
252+
"""Whether the provider can find a zone that owns this name.
253+
254+
Lets callers tell "nothing there" apart from "could not look". The base
255+
answer is optimistic so providers that do not model zones keep their
256+
current behaviour; override where the distinction is knowable.
257+
"""
258+
return True
259+
250260
def unset_txt_record(self, name: str) -> bool:
251261
"""Delete all TXT records for a name.
252262
@@ -259,8 +269,20 @@ def unset_txt_record(self, name: str) -> bool:
259269
Returns:
260270
True if all matching records were removed (or none existed).
261271
"""
272+
# get_dns_records returns [] both for "this name has no TXT records" and
273+
# for "the zone could not be resolved", so an empty list on its own would
274+
# report a failed cleanup as a success. Ask whether the zone resolves
275+
# before believing the emptiness.
276+
records = self.get_dns_records(name, RecordType.TXT)
277+
if not records and not self.zone_is_resolvable(name):
278+
print(
279+
f"Error: cannot delete TXT records for {name}: zone lookup failed",
280+
file=sys.stderr,
281+
)
282+
return False
283+
262284
ok = True
263-
for record in self.get_dns_records(name, RecordType.TXT):
285+
for record in records:
264286
if not record.id:
265287
print(f"Warning: TXT record for {name} has no id; cannot delete")
266288
ok = False

custom-domain/dstack-ingress/scripts/dns_providers/cloudflare.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,18 @@ def _ensure_zone_id(self, domain: str) -> Optional[str]:
158158
return self.zone_id
159159

160160
zone_info = self._get_zone_info(domain)
161-
if zone_info:
162-
self.zone_id, self.zone_domain = zone_info
161+
if not zone_info:
162+
# Returning the cached id here would hand back a *different* zone's
163+
# id, so a write meant for one zone could land in another. That is a
164+
# correctness problem generally and a security one under challenge
165+
# delegation, where "which zone are we writing to" is the point.
166+
return None
167+
self.zone_id, self.zone_domain = zone_info
163168
return self.zone_id
164169

170+
def zone_is_resolvable(self, name: str) -> bool:
171+
return self._ensure_zone_id(name) is not None
172+
165173
def get_dns_records(
166174
self, name: str, record_type: Optional[RecordType] = None
167175
) -> List[DNSRecord]:

custom-domain/dstack-ingress/scripts/dnsguide.py

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ class Record:
6262
name: str
6363
value: str
6464
note: str = ""
65+
# A CNAME the operator points at the gateway may legitimately be flattened
66+
# to an address record, so check_alias accepts either. A delegation CNAME
67+
# cannot: its target holds only a TXT, so there is no address to compare
68+
# and the record has to be the CNAME itself.
69+
exact: bool = False
6570

6671

6772
class ResolveError(RuntimeError):
@@ -156,6 +161,17 @@ def check_txt(record: Record, resolvers: str) -> Tuple[bool, str]:
156161
)
157162

158163

164+
def check_cname_exact(record: Record, resolvers: str) -> Tuple[bool, str]:
165+
"""Verify a CNAME points at exactly this target, with no address fallback."""
166+
target = _fqdn(record.value)
167+
cnames = [_fqdn(v) for v in query_union(record.name, RR_CNAME, resolvers)]
168+
if target in cnames:
169+
return True, "ok (CNAME)"
170+
if cnames:
171+
return False, f"CNAME points at {cnames!r}, expected {target!r}"
172+
return False, "no CNAME record found"
173+
174+
159175
def check_alias(record: Record, resolvers: str) -> Tuple[bool, str]:
160176
"""Verify the hostname ends up at the gateway.
161177
@@ -324,17 +340,24 @@ def verify_once(
324340
caa_method: str,
325341
account_uri: str,
326342
resolvers: str,
343+
caa_advisory: bool = False,
327344
) -> List[Tuple[str, bool, str]]:
328345
results = []
329346
for rec in records:
330347
if rec.type == "TXT":
331348
ok, why = check_txt(rec, resolvers)
332349
results.append((f"TXT {rec.name}", ok, why))
333350
elif rec.type == "CNAME":
334-
ok, why = check_alias(rec, resolvers)
351+
if rec.exact:
352+
ok, why = check_cname_exact(rec, resolvers)
353+
else:
354+
ok, why = check_alias(rec, resolvers)
335355
results.append((f"CNAME {rec.name}", ok, why))
336356
ok, why = check_caa(domain, caa_tag, caa_method, account_uri, resolvers)
337-
results.append((f"CAA {domain}", ok, why))
357+
if not ok and caa_advisory:
358+
results.append((f"CAA {domain}", True, f"WARNING: {why} (continuing: CAA is advisory)"))
359+
else:
360+
results.append((f"CAA {domain}", ok, why))
338361
return results
339362

340363

@@ -359,6 +382,21 @@ def build_records(args: argparse.Namespace) -> List[Record]:
359382
note="tells the gateway which instance to route this hostname to",
360383
)
361384
)
385+
if "challenge-cname" in include and args.challenge_alias:
386+
# Challenge delegation: the CA follows this CNAME when it looks for
387+
# _acme-challenge, so the TXT can live in a zone whose token we hold and
388+
# the served zone never needs one. certbot validates at the base name
389+
# even for a wildcard, so strip "*." here too.
390+
base = args.domain[2:] if args.domain.startswith("*.") else args.domain
391+
records.append(
392+
Record(
393+
type="CNAME",
394+
name=f"_acme-challenge.{base}",
395+
value=f"_acme-challenge.{base}.{args.challenge_alias}",
396+
note="delegates the ACME challenge to a zone this container can write",
397+
exact=True,
398+
)
399+
)
362400
if "caa" in include and args.caa_value:
363401
note = "optional; restricts issuance for this name to Let's Encrypt"
364402
if args.account_uri:
@@ -385,6 +423,16 @@ def main() -> int:
385423
parser.add_argument("--caa-value", default="")
386424
parser.add_argument("--account-uri", default="")
387425
parser.add_argument("--challenge", default="tls-alpn-01")
426+
parser.add_argument(
427+
"--challenge-alias",
428+
default=os.environ.get("ACME_CHALLENGE_ALIAS", ""),
429+
help="delegation zone for the _acme-challenge CNAME (dns-01 delegation)",
430+
)
431+
parser.add_argument(
432+
"--caa-advisory",
433+
action="store_true",
434+
help="report a failing CAA check as a warning instead of blocking",
435+
)
388436
parser.add_argument(
389437
"--mode",
390438
default=os.environ.get("DNS_SETUP_MODE", "wait"),
@@ -401,7 +449,8 @@ def main() -> int:
401449
parser.add_argument(
402450
"--include",
403451
default="cname,txt,caa",
404-
help="comma-separated subset of records to handle (cname,txt,caa)",
452+
help="comma-separated subset of records to handle "
453+
"(cname,txt,caa,challenge-cname)",
405454
)
406455
args = parser.parse_args()
407456

@@ -434,7 +483,7 @@ def main() -> int:
434483
try:
435484
results = verify_once(
436485
records, args.domain.lstrip("*."), args.caa_tag, args.challenge,
437-
args.account_uri, args.resolver,
486+
args.account_uri, args.resolver, args.caa_advisory,
438487
)
439488
except ResolveError as exc:
440489
print(f"[dns-check #{attempt}] resolver problem: {exc}", flush=True)

0 commit comments

Comments
 (0)