Skip to content

Commit 4d06340

Browse files
committed
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.
1 parent 8dad8eb commit 4d06340

4 files changed

Lines changed: 126 additions & 6 deletions

File tree

custom-domain/dstack-ingress/TESTING.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,13 +199,59 @@ first issuance.
199199
payload, and a TDX quote whose `report_data` is `sha256(payload)`. Verify both:
200200
the HMAC proves the shared secret, the quote proves the enclave.
201201

202+
### Challenge delegation (dns-01)
203+
204+
`ACME_CHALLENGE_ALIAS` answers the DNS-01 challenge in a zone the container's
205+
token controls, so the token needs no access to the served domain's own zone.
206+
Testing it looks like it needs a second registrar account. It does not.
207+
208+
Any name under a zone you already control works as the delegation zone, because
209+
the provider resolves a zone by longest-suffix match on the record name. With
210+
`ACME_CHALLENGE_ALIAS=deleg.example.com` and a served domain of
211+
`svc.example.com`, the challenge record is
212+
`_acme-challenge.svc.example.com.deleg.example.com`, which lands in
213+
`example.com` — the same zone, but through the delegation code path.
214+
215+
Play the operator and create the three static records the container prints:
216+
217+
```
218+
svc.example.com CNAME <GATEWAY_DOMAIN>
219+
_dstack-app-address.svc.example.com TXT "<app-id>:443"
220+
_acme-challenge.svc.example.com CNAME _acme-challenge.svc.example.com.deleg.example.com
221+
```
222+
223+
Then start the container with a real provider token and `ACME_STAGING=true`, and
224+
watch for: the three records verifying, `Executing (challenge-delegation):` with
225+
`--manual` and both hooks, the certificate arriving, and the challenge TXT being
226+
cleaned up afterwards (query `_acme-challenge.<domain>.<alias>` at the
227+
authoritative nameserver — it should be gone).
228+
229+
The CAA step comes last, once the ACME account exists and its URI is known. It
230+
blocks until you create the record it prints. Cloudflare's API wants CAA as
231+
structured fields rather than one string, so most quick DNS scripts cannot write
232+
it; the image's own `dnsman.py set_caa` can, and running it in a one-off
233+
container is the easiest way to play the operator here:
234+
235+
```bash
236+
docker run --rm --env-file .env --entrypoint dnsman.py <image> \
237+
set_caa --domain svc.example.com --caa-tag issue \
238+
--caa-value 'letsencrypt.org;validationmethods=dns-01;accounturi=<uri>'
239+
```
240+
241+
**What this does not test.** The whole point of delegation is that a token scoped
242+
to *only* the delegation zone is enough. Sharing one zone exercises every line of
243+
the mechanism but leaves that property unverified — the token still has access to
244+
the served zone, so a bug that quietly writes there would not show up. Confirming
245+
the scoping needs a genuinely separate zone with a separate token.
246+
202247
### Negative paths
203248

204249
These fail fast and are cheap, so run them on every change:
205250

206251
| Scenario | Expected |
207252
|---|---|
208253
| tls-alpn-01 + a wildcard domain | Refused before any ACME call, citing RFC 8737 |
254+
| Delegation with no CAA record at all | Blocked — unlike normal issuance, where "no CAA" means unrestricted and passes |
209255
| A CAA record with `validationmethods=dns-01`, mode tls-alpn-01 | Blocked with the restriction quoted back |
210256
| TXT holding the wrong value | Reported as `want <x>, saw <y>`, not "missing" |
211257
| 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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,23 @@ delegation_guide() {
138138
# a confirmed-absent record is fatal. ALLOW_MISSING_CAA downgrades it to a
139139
# warning for operators who accept that risk.
140140
delegation_verify_caa() {
141-
local domain="$1" account_file account_uri advisory=()
141+
local domain="$1" account_file account_uri
142142
if ! account_file=$(get_letsencrypt_account_file); then
143143
echo "ERROR: cannot read the ACME account file to determine the required CAA" >&2
144144
return 1
145145
fi
146146
account_uri=$(jq -j '.uri' "$account_file")
147-
[ "${ALLOW_MISSING_CAA:-false}" = "true" ] && advisory=(--caa-advisory)
147+
# Absent is the state to block on here, not "absent means unrestricted":
148+
# we cannot create this record, so nothing else protects the name.
149+
local gate=(--caa-required)
150+
if [ "${ALLOW_MISSING_CAA:-false}" = "true" ]; then
151+
gate=(--caa-required --caa-advisory)
152+
fi
148153

149154
delegation_guide "$domain" caa \
150155
--caa-value "letsencrypt.org;validationmethods=dns-01;accounturi=$account_uri" \
151156
--account-uri "$account_uri" \
152-
"${advisory[@]}"
157+
"${gate[@]}"
153158
}
154159

155160
set_caa_record() {

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

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,12 +269,24 @@ def _caa_permits(value: str, tag: str, want_method: str, account_uri: str) -> Tu
269269

270270

271271
def check_caa(
272-
domain: str, want_tag: str, want_method: str, account_uri: str, resolvers: str
272+
domain: str,
273+
want_tag: str,
274+
want_method: str,
275+
account_uri: str,
276+
resolvers: str,
277+
require_present: bool = False,
273278
) -> Tuple[bool, str]:
274279
"""Check CAA for domain, walking up to the closest ancestor that has one.
275280
276281
An absent CAA record set means every CA may issue (RFC 8659), so "no CAA
277-
anywhere" is a pass, not a failure.
282+
anywhere" is normally a pass: we are asking whether *we* may issue, and
283+
nothing forbids it.
284+
285+
`require_present` inverts that for challenge delegation, where the question
286+
is different. There the record is not a formality but the only thing stopping
287+
someone else who can satisfy the delegated challenge from getting a
288+
certificate for this name, and we hold no token to create it ourselves. An
289+
unrestricted name is exactly the state that must block.
278290
"""
279291
labels = _fqdn(domain).split(".")
280292
for i in range(len(labels) - 1):
@@ -308,6 +320,11 @@ def check_caa(
308320
reasons.append(reason)
309321
return False, f"CAA at {candidate} does not permit issuance: {'; '.join(reasons)}"
310322

323+
if require_present:
324+
return False, (
325+
"no CAA record set, so any account that can satisfy the delegated "
326+
"challenge could obtain a certificate for this name"
327+
)
311328
return True, "ok (no CAA record set; issuance is unrestricted)"
312329

313330

@@ -341,6 +358,7 @@ def verify_once(
341358
account_uri: str,
342359
resolvers: str,
343360
caa_advisory: bool = False,
361+
caa_required: bool = False,
344362
) -> List[Tuple[str, bool, str]]:
345363
results = []
346364
for rec in records:
@@ -353,7 +371,9 @@ def verify_once(
353371
else:
354372
ok, why = check_alias(rec, resolvers)
355373
results.append((f"CNAME {rec.name}", ok, why))
356-
ok, why = check_caa(domain, caa_tag, caa_method, account_uri, resolvers)
374+
ok, why = check_caa(
375+
domain, caa_tag, caa_method, account_uri, resolvers, caa_required
376+
)
357377
if not ok and caa_advisory:
358378
results.append((f"CAA {domain}", True, f"WARNING: {why} (continuing: CAA is advisory)"))
359379
else:
@@ -428,6 +448,12 @@ def main() -> int:
428448
default=os.environ.get("ACME_CHALLENGE_ALIAS", ""),
429449
help="delegation zone for the _acme-challenge CNAME (dns-01 delegation)",
430450
)
451+
parser.add_argument(
452+
"--caa-required",
453+
action="store_true",
454+
help="treat an absent CAA record set as a failure (challenge delegation: "
455+
"the record is the only protection and we cannot create it)",
456+
)
431457
parser.add_argument(
432458
"--caa-advisory",
433459
action="store_true",
@@ -484,6 +510,7 @@ def main() -> int:
484510
results = verify_once(
485511
records, args.domain.lstrip("*."), args.caa_tag, args.challenge,
486512
args.account_uri, args.resolver, args.caa_advisory,
513+
args.caa_required,
487514
)
488515
except ResolveError as exc:
489516
print(f"[dns-check #{attempt}] resolver problem: {exc}", flush=True)

custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,48 @@ def test_reports_absence_without_claiming_it_does_not_resolve(self):
176176
self.assertIn("no CNAME record found", why)
177177

178178

179+
class TestCaaRequiredForDelegation(unittest.TestCase):
180+
"""Delegation inverts the usual CAA question.
181+
182+
Normally we ask "does anything forbid us from issuing", and no CAA at all
183+
means no. Under challenge delegation the record is the only thing stopping
184+
someone else who can satisfy the delegated challenge, and we cannot create
185+
it, so its absence has to block.
186+
"""
187+
188+
def _no_caa(self):
189+
saved = dnsguide.query_union
190+
dnsguide.query_union = lambda name, rr, r: []
191+
self.addCleanup(lambda: setattr(dnsguide, "query_union", saved))
192+
193+
def test_absent_caa_passes_by_default(self):
194+
self._no_caa()
195+
ok, why = dnsguide.check_caa("app.example.com", "issue", "dns-01", "", "r")
196+
self.assertTrue(ok)
197+
self.assertIn("unrestricted", why)
198+
199+
def test_absent_caa_blocks_when_required(self):
200+
self._no_caa()
201+
ok, why = dnsguide.check_caa(
202+
"app.example.com", "issue", "dns-01", "", "r", require_present=True
203+
)
204+
self.assertFalse(ok)
205+
self.assertIn("no CAA record set", why)
206+
207+
def test_present_and_matching_passes_when_required(self):
208+
saved = dnsguide.query_union
209+
dnsguide.query_union = lambda name, rr, r: (
210+
['0 issue "letsencrypt.org;validationmethods=dns-01;accounturi=https://acme/1"']
211+
if name == "app.example.com" else []
212+
)
213+
self.addCleanup(lambda: setattr(dnsguide, "query_union", saved))
214+
ok, _ = dnsguide.check_caa(
215+
"app.example.com", "issue", "dns-01", "https://acme/1", "r",
216+
require_present=True,
217+
)
218+
self.assertTrue(ok)
219+
220+
179221
class TestTxtNormalisation(unittest.TestCase):
180222
def test_strips_quotes(self):
181223
self.assertEqual(dnsguide._unquote_txt('"abc:443"'), "abc:443")

0 commit comments

Comments
 (0)