Skip to content

Commit 50443e2

Browse files
committed
test: resolve the challenge through the authoritative nameservers
certbot reads challenge records straight from the zone's authoritative servers, because a resolver that cached NXDOMAIN moments before the record was written answers from that cache for the whole wait. It gets there by asking NS for the zone and then A for each answer -- and the mock served neither, so `authoritative_resolver` bailed on the first question of every issuance and fell back to the system resolver. The fallback is deliberate and issuance still worked, which is exactly why nothing noticed the path was dead -- including this branch's own change to it, generalizing the stripped label from `_acme-challenge` to any challenge prefix, which had only unit coverage. The mock now answers NS at a zone apex and A for the name it gives. Only at the apex: a name below it must stay NODATA or the walk up looking for the zone cut stops in the wrong place, and walking up is what it does against a real zone. Both are gated on MOCK_CF_NS_ADDR, so a mock that is not told where it can be reached advertises nothing. The new assertion reads the mock's own query log, so it observes that both questions were asked and answered rather than inferring it.
1 parent 05f76c1 commit 50443e2

3 files changed

Lines changed: 72 additions & 6 deletions

File tree

dstack/gateway/test-run/e2e/docker-compose.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ services:
5252
- DEBUG=true
5353
# The zones certbot writes into and Pebble reads back out of.
5454
- MOCK_CF_ZONES=test0.local,test1.local,test2.local,persist0.local,persist1.local,persist2.local,selfcheck0.local
55+
# Advertise this container as the authoritative nameserver for those
56+
# zones, so certbot's self-check can reach it the way it reaches a real
57+
# one -- NS for the zone, A for the answer -- instead of failing that
58+
# lookup and quietly falling back to its system resolver.
59+
- MOCK_CF_NS_ADDR=172.30.0.10
5560
healthcheck:
5661
test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://localhost:8080/health')"]
5762
interval: 5s

dstack/gateway/test-run/e2e/test.sh

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,22 @@ test_self_check_resolves_a_published_record() {
383383
[ "$(answered_queries_for "_validation-persist.${PERSIST_DOMAIN}")" -gt 0 ]
384384
}
385385

386+
# The self-check does not ask its system resolver for the challenge record: a
387+
# resolver that cached NXDOMAIN moments before the record was written answers
388+
# from that cache for the whole wait, so the check reads the authoritative
389+
# servers instead. Getting there is two questions -- NS for the zone, then A for
390+
# the name that answers -- and if either goes unanswered certbot logs a warning
391+
# and silently falls back to the very resolver the path exists to avoid. The
392+
# fallback is deliberate and issuance still works, which is exactly why nothing
393+
# else here would notice that the path never ran.
394+
test_self_check_reads_the_authoritative_nameservers() {
395+
# `_validation-persist.<zone>` is not a zone cut, so certbot strips the
396+
# leading label and asks at the apex. That strip is what makes one code path
397+
# serve both challenges, and this is where it is exercised for real.
398+
[ "$(answered_queries_for "${PERSIST_DOMAIN}")" -gt 0 ] || return 1
399+
[ "$(answered_queries_for "ns.${PERSIST_DOMAIN}")" -gt 0 ]
400+
}
401+
386402
# The unhappy path, and the reason the check is advisory at all. A name with no
387403
# record has to be polled and given up on -- the record may still be
388404
# propagating -- rather than asked once and abandoned.
@@ -792,6 +808,8 @@ main() {
792808
"$(test_self_check_resolves_a_published_record; echo $?)"
793809
run_test "Self-check polls and gives up when the record is absent" \
794810
"$(test_self_check_gives_up_on_a_missing_record; echo $?)"
811+
run_test "Self-check reads the authoritative nameservers" \
812+
"$(test_self_check_reads_the_authoritative_nameservers; echo $?)"
795813

796814
# Summary
797815
log_section "Test Summary"

tools/mock-cf-dns/server.py

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
# under test resolved a name rather than inferring it from what happened next.
4646
QUERIES: list[dict[str, Any]] = []
4747
MAX_QUERIES = 2000
48+
QTYPE_ANY = 255
4849
NEXT_ID = 1
4950
# Names whose issuer CAA set went from non-empty to empty at some point.
5051
#
@@ -294,6 +295,46 @@ def txt_rdata(text: str) -> bytes:
294295
return b"".join(bytes([len(c)]) + c for c in chunks)
295296

296297

298+
def wire_name(name: str) -> bytes:
299+
"""Encode a domain name uncompressed, for use in RDATA."""
300+
out = b""
301+
for label in name.strip(".").split("."):
302+
raw = label.encode("ascii", "ignore")[:63]
303+
out += bytes([len(raw)]) + raw
304+
return out + b"\x00"
305+
306+
307+
def ns_address() -> str | None:
308+
"""Address to advertise as this mock's nameserver, if it is to advertise one.
309+
310+
A client that reads a challenge record straight from the authoritative
311+
servers -- which is the only way to dodge a recursive resolver's negative
312+
cache -- gets there by asking NS for the zone and then A for each answer.
313+
Unset, this mock answers neither and that client falls back to its system
314+
resolver, so the authoritative path never runs. Set to the address this
315+
container is reachable at, and the path is exercised end to end.
316+
"""
317+
return os.environ.get("MOCK_CF_NS_ADDR") or None
318+
319+
320+
def _synthesized(name: str, qtype: int) -> list[tuple[int, bytes]]:
321+
"""NS at a zone apex and A for the nameserver it names.
322+
323+
Only at the apex: a name below it must answer NODATA, or a client walking
324+
up from `_acme-challenge.<name>` looking for the zone cut would stop at the
325+
wrong place -- and walking up is what it does against a real zone.
326+
"""
327+
addr = ns_address()
328+
if not addr:
329+
return []
330+
apexes = {zone["name"] for zone in _zones()}
331+
if qtype in (2, QTYPE_ANY) and name in apexes:
332+
return [(2, wire_name(f"ns.{name}"))]
333+
if qtype in (1, QTYPE_ANY) and name.startswith("ns.") and name[3:] in apexes:
334+
return [(1, socket.inet_aton(addr))]
335+
return []
336+
337+
297338
def _upstream() -> str:
298339
"""Where to send questions this mock is not authoritative for.
299340
@@ -371,21 +412,23 @@ def dns_response(packet: bytes) -> bytes:
371412
return txid + struct.pack("!HHHHH", 0x8182, 1, 0, 0, 0) + packet[12:qend + 4]
372413

373414
with STATE_LOCK:
374-
answers = [
415+
stored = [
375416
r
376417
for r in RECORDS
377418
if r["type"] == "TXT" and r["name"].strip(".").lower() == name
378419
]
379-
if qtype not in (16, 255):
380-
answers = []
420+
if qtype not in (16, QTYPE_ANY):
421+
stored = []
422+
answers = [(16, txt_rdata(r["content"])) for r in stored]
423+
answers += _synthesized(name, qtype)
424+
381425
_record_query(name, qtype, len(answers), False)
382426
header = txid + struct.pack("!HHHHH", 0x8180, 1, len(answers), 0, 0)
383427
body = question
384-
for record in answers:
385-
rdata = txt_rdata(record["content"])
428+
for rtype, rdata in answers:
386429
body += (
387430
b"\xc0\x0c"
388-
+ struct.pack("!HHIH", 16, 1, int(record.get("ttl", 60)), len(rdata))
431+
+ struct.pack("!HHIH", rtype, 1, 60, len(rdata))
389432
+ rdata
390433
)
391434
return header + body

0 commit comments

Comments
 (0)