From a5e9a7e06a1e6cf7fa8ad35f19bab6e0c2b16acf Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 3 Aug 2026 07:12:46 +0000 Subject: [PATCH] fix(dns): finish the CNAME chain instead of handing clients a dead end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Moshpit name pointed at a hostname answered with a CNAME and nothing else: seo.rank AAAA -> rcode=0, 1 answer type=CNAME ttl=60 target=dev.profullstack.com That is a correct authoritative answer — and this resolver is not in the position that makes it correct. It sets RA=1 and is used directly by stub clients: browsers, curl, and every machine pointed at the DoH endpoint. A stub does not chase CNAMEs. It reads the answer section for an address, finds none, and reports failure: curl --doh-url https://dns.moshcode.sh/dns-query http://seo.rank/ curl: (6) Could not resolve host: seo.rank So the endpoint resolved google.com (forwarded upstream, which chases) while failing every name it exists to serve. moshpitResponse now resolves the CNAME target and appends the leaf address records. Best-effort: an upstream failure still returns the CNAME, because failing closed would turn one hiccup into "this name does not exist". Answer TTLs are clamped to the registry's, since the owner can repoint a name at any moment. Co-Authored-By: Claude Opus 5 --- lib/dns/server.ts | 55 +++++++++++++++++++++++++++++++++++++++ tests/dns-server.test.mjs | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/lib/dns/server.ts b/lib/dns/server.ts index dae3828..f32aa01 100644 --- a/lib/dns/server.ts +++ b/lib/dns/server.ts @@ -24,6 +24,7 @@ import { setMessageId, udpPayloadSize, type Message, + type Question, } from "./wire"; export type ServerStats = { @@ -164,6 +165,59 @@ export function createDnsServer(options: DnsServerOptions): DnsServer { return setMessageId(response, clientId); } + /** + * Finish a CNAME chain the client will not finish itself. + * + * A name pointed at a hostname answers with a CNAME, which is correct for an + * authoritative server: its answer gets completed by whichever recursive + * resolver asked. This resolver is not in that position. It sets RA=1 and is + * used *directly* by stub clients — browsers, curl, and every machine + * pointed at the DoH endpoint — and a stub does not chase CNAMEs. It reads + * the address out of the answer section, finds none, and reports failure. + * + * So a bare CNAME reads to all of them as "no such host": `curl` says + * "Could not resolve host: seo.rank" for a name that resolves perfectly. + * + * Best-effort on purpose. If the upstream lookup fails we still return the + * CNAME rather than nothing — a resolver that chases well is better than the + * one we had, and a resolver that fails closed on an upstream hiccup is + * worse. + */ + async function completeCnameChain(message: Message, question: Question): Promise { + if (question.type !== TYPE.A && question.type !== TYPE.AAAA) return; + // Already has what was asked for — a name pointed at a literal address. + if (message.answers.some((r) => r.type === question.type)) return; + + const cname = message.answers.find((r) => r.type === TYPE.CNAME && r.target); + if (!cname?.target) return; + + try { + const probe = encodeMessage({ + id: randomId(), + flags: { qr: false, opcode: 0, aa: false, tc: false, rd: true, ra: false, z: false, ad: false, cd: false, rcode: 0 }, + questions: [{ name: cname.target, type: question.type, class: CLASS.IN }], + }); + const resolved = decodeMessage(await forwarder.query(probe)); + for (const record of resolved.answers) { + // Leaves only. Relaying the upstream's own CNAMEs would rebuild the + // same dead end one link further along. + if (record.type !== question.type || !record.address) continue; + message.answers.push({ + name: cname.target, + type: question.type, + class: CLASS.IN, + // Never outlive the registry's own TTL: the owner can repoint this + // name at any moment, and an address cached past that is the one + // failure nobody can debug from outside. + ttl: Math.min(ttl, record.ttl ?? ttl), + address: record.address, + }); + } + } catch { + // Keep the CNAME. See above. + } + } + async function moshpitResponse(query: Message, name: string): Promise<{ buffer: Buffer; message: Message } | null> { const lookup = await registry.lookup(name); if (!lookup?.registered) return null; @@ -187,6 +241,7 @@ export function createDnsServer(options: DnsServerOptions): DnsServer { ttl, }); if (!message) return null; + await completeCnameChain(message, query.questions[0]); message.additionals = echoOpt(query); return { buffer: encodeMessage(message), message }; } diff --git a/tests/dns-server.test.mjs b/tests/dns-server.test.mjs index 57fc0f3..8ee2d63 100644 --- a/tests/dns-server.test.mjs +++ b/tests/dns-server.test.mjs @@ -55,6 +55,7 @@ function stubRegistry(names, options = {}) { resolved: entry?.resolved ?? name, registered: Boolean(entry), aliased: Boolean(entry?.resolved && entry.resolved !== name), + target: entry?.target ?? null, }), { status: 200, headers: { "content-type": "application/json" } }, ); @@ -286,3 +287,47 @@ test("the registry is asked once for a name, however many clients ask us", async await registry.lookup("scrambled.eggs"); assert.equal(calls, 1, "coalesced in flight, then cached"); }); + +// A name pointed at a hostname is the case `seo.rank` hit in production: the +// answer was a CNAME to dev.profullstack.com and nothing else, and every stub +// client read that as "no such host". +test("a name pointed at a hostname answers with the address, not just the CNAME", async () => { + const dns = harness({ + names: { "seo.rank": { target: "dev.profullstack.com" } }, + zone: { "dev.profullstack.com": CLEARNET_V4 }, + }); + const response = decodeMessage(await dns.handle(query("seo.rank"))); + + assert.equal(response.flags.rcode, RCODE.NOERROR); + // The CNAME still goes out — a `dig` should show where the name points. + assert.ok( + response.answers.some((r) => r.type === TYPE.CNAME && r.target === "dev.profullstack.com"), + "the CNAME is what makes the indirection visible", + ); + // ...but the address has to be there too. This resolver sets RA=1 and talks + // to stub clients directly; a stub reads the answer section for an address + // and gives up when there is none, so a bare CNAME is a failed lookup. + assert.deepEqual(addresses(response), [CLEARNET_V4], "a stub client needs the leaf address"); + await dns.close(); +}); + +test("an unreachable upstream still yields the CNAME rather than nothing", async () => { + // Chasing is best-effort. Failing closed here would turn one upstream + // hiccup into "this name does not exist". + const dns = harness({ names: { "seo.rank": { target: "dev.profullstack.com" } }, zone: {} }); + const response = decodeMessage(await dns.handle(query("seo.rank"))); + + assert.equal(response.flags.rcode, RCODE.NOERROR); + assert.ok(response.answers.some((r) => r.type === TYPE.CNAME), "the CNAME survives an upstream failure"); + await dns.close(); +}); + +test("a name pointed at a literal address does not get a spurious lookup", async () => { + // Nothing to chase: the address is already the answer. + const dns = harness({ names: { "pinned.rank": { target: "203.0.113.55" } } }); + const response = decodeMessage(await dns.handle(query("pinned.rank"))); + + assert.deepEqual(addresses(response), ["203.0.113.55"]); + assert.ok(!response.answers.some((r) => r.type === TYPE.CNAME), "no CNAME when the target is an address"); + await dns.close(); +});