Skip to content

Commit 3677b99

Browse files
ralyodioclaude
andauthored
fix(dns): finish the CNAME chain instead of handing clients a dead end (#99)
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 <noreply@anthropic.com>
1 parent de1da21 commit 3677b99

2 files changed

Lines changed: 100 additions & 0 deletions

File tree

lib/dns/server.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
setMessageId,
2525
udpPayloadSize,
2626
type Message,
27+
type Question,
2728
} from "./wire";
2829

2930
export type ServerStats = {
@@ -164,6 +165,59 @@ export function createDnsServer(options: DnsServerOptions): DnsServer {
164165
return setMessageId(response, clientId);
165166
}
166167

168+
/**
169+
* Finish a CNAME chain the client will not finish itself.
170+
*
171+
* A name pointed at a hostname answers with a CNAME, which is correct for an
172+
* authoritative server: its answer gets completed by whichever recursive
173+
* resolver asked. This resolver is not in that position. It sets RA=1 and is
174+
* used *directly* by stub clients — browsers, curl, and every machine
175+
* pointed at the DoH endpoint — and a stub does not chase CNAMEs. It reads
176+
* the address out of the answer section, finds none, and reports failure.
177+
*
178+
* So a bare CNAME reads to all of them as "no such host": `curl` says
179+
* "Could not resolve host: seo.rank" for a name that resolves perfectly.
180+
*
181+
* Best-effort on purpose. If the upstream lookup fails we still return the
182+
* CNAME rather than nothing — a resolver that chases well is better than the
183+
* one we had, and a resolver that fails closed on an upstream hiccup is
184+
* worse.
185+
*/
186+
async function completeCnameChain(message: Message, question: Question): Promise<void> {
187+
if (question.type !== TYPE.A && question.type !== TYPE.AAAA) return;
188+
// Already has what was asked for — a name pointed at a literal address.
189+
if (message.answers.some((r) => r.type === question.type)) return;
190+
191+
const cname = message.answers.find((r) => r.type === TYPE.CNAME && r.target);
192+
if (!cname?.target) return;
193+
194+
try {
195+
const probe = encodeMessage({
196+
id: randomId(),
197+
flags: { qr: false, opcode: 0, aa: false, tc: false, rd: true, ra: false, z: false, ad: false, cd: false, rcode: 0 },
198+
questions: [{ name: cname.target, type: question.type, class: CLASS.IN }],
199+
});
200+
const resolved = decodeMessage(await forwarder.query(probe));
201+
for (const record of resolved.answers) {
202+
// Leaves only. Relaying the upstream's own CNAMEs would rebuild the
203+
// same dead end one link further along.
204+
if (record.type !== question.type || !record.address) continue;
205+
message.answers.push({
206+
name: cname.target,
207+
type: question.type,
208+
class: CLASS.IN,
209+
// Never outlive the registry's own TTL: the owner can repoint this
210+
// name at any moment, and an address cached past that is the one
211+
// failure nobody can debug from outside.
212+
ttl: Math.min(ttl, record.ttl ?? ttl),
213+
address: record.address,
214+
});
215+
}
216+
} catch {
217+
// Keep the CNAME. See above.
218+
}
219+
}
220+
167221
async function moshpitResponse(query: Message, name: string): Promise<{ buffer: Buffer; message: Message } | null> {
168222
const lookup = await registry.lookup(name);
169223
if (!lookup?.registered) return null;
@@ -187,6 +241,7 @@ export function createDnsServer(options: DnsServerOptions): DnsServer {
187241
ttl,
188242
});
189243
if (!message) return null;
244+
await completeCnameChain(message, query.questions[0]);
190245
message.additionals = echoOpt(query);
191246
return { buffer: encodeMessage(message), message };
192247
}

tests/dns-server.test.mjs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ function stubRegistry(names, options = {}) {
5555
resolved: entry?.resolved ?? name,
5656
registered: Boolean(entry),
5757
aliased: Boolean(entry?.resolved && entry.resolved !== name),
58+
target: entry?.target ?? null,
5859
}),
5960
{ status: 200, headers: { "content-type": "application/json" } },
6061
);
@@ -286,3 +287,47 @@ test("the registry is asked once for a name, however many clients ask us", async
286287
await registry.lookup("scrambled.eggs");
287288
assert.equal(calls, 1, "coalesced in flight, then cached");
288289
});
290+
291+
// A name pointed at a hostname is the case `seo.rank` hit in production: the
292+
// answer was a CNAME to dev.profullstack.com and nothing else, and every stub
293+
// client read that as "no such host".
294+
test("a name pointed at a hostname answers with the address, not just the CNAME", async () => {
295+
const dns = harness({
296+
names: { "seo.rank": { target: "dev.profullstack.com" } },
297+
zone: { "dev.profullstack.com": CLEARNET_V4 },
298+
});
299+
const response = decodeMessage(await dns.handle(query("seo.rank")));
300+
301+
assert.equal(response.flags.rcode, RCODE.NOERROR);
302+
// The CNAME still goes out — a `dig` should show where the name points.
303+
assert.ok(
304+
response.answers.some((r) => r.type === TYPE.CNAME && r.target === "dev.profullstack.com"),
305+
"the CNAME is what makes the indirection visible",
306+
);
307+
// ...but the address has to be there too. This resolver sets RA=1 and talks
308+
// to stub clients directly; a stub reads the answer section for an address
309+
// and gives up when there is none, so a bare CNAME is a failed lookup.
310+
assert.deepEqual(addresses(response), [CLEARNET_V4], "a stub client needs the leaf address");
311+
await dns.close();
312+
});
313+
314+
test("an unreachable upstream still yields the CNAME rather than nothing", async () => {
315+
// Chasing is best-effort. Failing closed here would turn one upstream
316+
// hiccup into "this name does not exist".
317+
const dns = harness({ names: { "seo.rank": { target: "dev.profullstack.com" } }, zone: {} });
318+
const response = decodeMessage(await dns.handle(query("seo.rank")));
319+
320+
assert.equal(response.flags.rcode, RCODE.NOERROR);
321+
assert.ok(response.answers.some((r) => r.type === TYPE.CNAME), "the CNAME survives an upstream failure");
322+
await dns.close();
323+
});
324+
325+
test("a name pointed at a literal address does not get a spurious lookup", async () => {
326+
// Nothing to chase: the address is already the answer.
327+
const dns = harness({ names: { "pinned.rank": { target: "203.0.113.55" } } });
328+
const response = decodeMessage(await dns.handle(query("pinned.rank")));
329+
330+
assert.deepEqual(addresses(response), ["203.0.113.55"]);
331+
assert.ok(!response.answers.some((r) => r.type === TYPE.CNAME), "no CNAME when the target is an address");
332+
await dns.close();
333+
});

0 commit comments

Comments
 (0)