From f8f019e1040af9ecdb52bb120a21dc40703107e2 Mon Sep 17 00:00:00 2001 From: Alex Cinovoj <135995822+Alexi5000@users.noreply.github.com> Date: Fri, 13 Nov 2026 20:30:22 -0500 Subject: [PATCH] fix(pcap): preserve DNS RCODE, EDNS extended RCODE, and OPCODE status in address attribution Fixes #10211 Supersedes #10218 Co-Authored-By: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- src/pcap/PcapDnsWire.test.ts | 110 +++++++++++++++++++++++++++++++++++ src/pcap/PcapDnsWire.ts | 70 +++++++++++++++++----- 2 files changed, 164 insertions(+), 16 deletions(-) diff --git a/src/pcap/PcapDnsWire.test.ts b/src/pcap/PcapDnsWire.test.ts index d1d43e1637..ebdef8d290 100644 --- a/src/pcap/PcapDnsWire.test.ts +++ b/src/pcap/PcapDnsWire.test.ts @@ -37,6 +37,116 @@ it("preserves real TShark display collisions with distinct wire label identities }); }); +it("does not attribute addresses on NXDOMAIN (nonzero RCODE)", () => { + const query = label("612e62", "6578616d706c65"); + const answer = rr(query, 1, address); + const bytes = Buffer.concat([ + word(1), + word(0x8183), // QR=1, RCODE=3 (NXDOMAIN) + word(1), + word(1), + word(0), + word(0), + query, + word(1), + word(1), + answer, + ]); + const facts = parse(bytes); + expect(facts).toMatchObject({ + assigned: [], + unattributed: ["192.0.2.9"], + incomplete: true, + }); + expect(facts.incomplete_reasons).toContainEqual( + expect.stringContaining("RCODE"), + ); +}); + +it("does not attribute addresses on non-QUERY opcode", () => { + const query = label("612e62", "6578616d706c65"); + const answer = rr(query, 1, address); + const bytes = Buffer.concat([ + word(1), + word(0x9180), // QR=1, OPCODE=2 (STATUS) + word(1), + word(1), + word(0), + word(0), + query, + word(1), + word(1), + answer, + ]); + const facts = parse(bytes); + expect(facts).toMatchObject({ + assigned: [], + unattributed: ["192.0.2.9"], + incomplete: true, + }); +}); + +it("does not attribute addresses when EDNS OPT extended RCODE is nonzero (BADVERS)", () => { + const query = label("612e62", "6578616d706c65"); + const answer = rr(query, 1, address); + const optTtl = Buffer.alloc(4); + optTtl.writeUInt32BE(1 << 24); // extended RCODE = 1 -> full RCODE = 16 (BADVERS) + const opt = Buffer.concat([ + Buffer.from([0]), + word(41), + word(4096), + optTtl, + word(0), + ]); + const bytes = Buffer.concat([ + word(1), + word(0x8180), // QR=1, RD=1, header RCODE=0 + word(1), + word(1), + word(0), + word(1), + query, + word(1), + word(1), + answer, + opt, + ]); + const facts = parse(bytes); + expect(facts).toMatchObject({ + assigned: [], + unattributed: ["192.0.2.9"], + incomplete: true, + incomplete_reasons: [expect.stringContaining("RCODE is nonzero (16)")], + }); +}); + +it("preserves both nonzero RCODE rejection and TC truncation reasons", () => { + const query = label("612e62", "6578616d706c65"); + const answer = rr(query, 1, address); + const bytes = Buffer.concat([ + word(1), + word(0x8383), // QR=1, TC=1, RD=1, RCODE=3 (NXDOMAIN) + word(1), + word(1), + word(0), + word(0), + query, + word(1), + word(1), + answer, + ]); + const facts = parse(bytes); + expect(facts).toMatchObject({ + assigned: [], + unattributed: ["192.0.2.9"], + incomplete: true, + }); + expect(facts.incomplete_reasons).toEqual([ + expect.stringContaining("RCODE is nonzero (3)"), + "DNS message declares truncation (TC); captured records are incomplete", + ]); +}); + const label = (...labels: readonly string[]): Buffer => Buffer.concat([ ...labels.map((hex) => { diff --git a/src/pcap/PcapDnsWire.ts b/src/pcap/PcapDnsWire.ts index d6fa5719a8..519f539ea1 100644 --- a/src/pcap/PcapDnsWire.ts +++ b/src/pcap/PcapDnsWire.ts @@ -200,12 +200,16 @@ const decodeRecords = ( start: number, classValue: (value: number) => number, observed: readonly ObservedAddress[], -): Result => { +): Result< + { records: readonly RecordFact[]; extendedRcode: number }, + DnsWireFailure +> => { const u16 = (at: number): number => message.readUInt16BE(at); const counts = [u16(6), u16(8), u16(10)]; let at = start; const addressesByIdentity = addressInventory(observed); const records: RecordFact[] = []; + let extendedRcode = 0; for (let section = 0; section < counts.length; section += 1) { for (let index = 0; index < counts[section]!; index += 1) { const ownerResult = decodeName(message, at); @@ -220,6 +224,11 @@ const decodeRecords = ( const end = at + size; if (end > message.length) return invalidWire("DNS record data is truncated"); + // EDNS: an OPT pseudo-RR in the additional section carries the + // upper eight bits of the extended RCODE in its TTL field. + if (section === 2 && type === 41 && owner.end + 8 <= message.length) { + extendedRcode = message.readUInt32BE(owner.end + 4) >>> 24; + } let target: string | null = null; let address: string | null = null; if (type === 5 && dnsClass === 1) { @@ -262,28 +271,52 @@ const decodeRecords = ( } if (at !== message.length) return invalidWire("DNS message contains unexamined trailing bytes"); - return ok(records); + return ok({ records, extendedRcode }); }; const eligibleQuestion = ( decodedQuestions: readonly WireQuestion[], flags: number, -): WireQuestion | null => { + rcode: number = flags & 0x000f, +): { question: WireQuestion | null; reason: string | null } => { const question = decodedQuestions[0]; - if ( - (flags & 0x8000) === 0 || - decodedQuestions.length !== 1 || - question === undefined || - ![1, 255].includes(question.dnsClass) || - ![1, 28, 255].includes(question.type) - ) - return null; - return question; + // QR bit must be set (response, not query). + if ((flags & 0x8000) === 0) + return { question: null, reason: "DNS response flag (QR) is not set" }; + // RCODE must be zero (no error response such as NXDOMAIN or BADVERS). + if (rcode !== 0) + return { + question: null, + reason: `DNS response RCODE is nonzero (${rcode}); error response such as NXDOMAIN`, + }; + // OPCODE must be QUERY (0); unsupported opcodes (STATUS, IQUERY, NOTIFY, ...) + // do not establish ordinary successful-query address ownership. + if ((flags & 0x7800) !== 0) + return { + question: null, + reason: `DNS OPCODE is not QUERY (${(flags & 0x7800) >> 11})`, + }; + if (decodedQuestions.length !== 1 || question === undefined) + return { + question: null, + reason: "Question count does not establish Internet address ownership", + }; + if (![1, 255].includes(question.dnsClass)) + return { + question: null, + reason: "Question class does not establish Internet address ownership", + }; + if (![1, 28, 255].includes(question.type)) + return { + question: null, + reason: "Question type does not establish Internet address ownership", + }; + return { question, reason: null }; }; const attributeAddresses = ( questions: readonly DnsWireQuestion[], - question: WireQuestion | null, + eligibility: { question: WireQuestion | null; reason: string | null }, records: readonly RecordFact[], observed: readonly ObservedAddress[], ): DnsMessageFacts => { @@ -296,9 +329,11 @@ const attributeAddresses = ( }); if (observed.length === 0) return { questions, assigned: [], unattributed: [], incomplete: false }; + const { question } = eligibility; if (question === null) return unknown( - "Question count, response flag, type or class does not establish Internet address ownership", + eligibility.reason ?? + "Question count, response flag, type or class does not establish Internet address ownership", ); const links = new Map>(); for (const record of records) { @@ -424,10 +459,13 @@ export const parseDnsMessageRaw = ( })); const records = decodeRecords(message, at, classValue, observed); if (!records.ok) return unknown(records.error.reason); + const extendedRcode = records.value.extendedRcode; + const fullRcode = (extendedRcode << 4) | (flags & 0x000f); + const eligibility = eligibleQuestion(decodedQuestions, flags, fullRcode); const facts = attributeAddresses( questions, - eligibleQuestion(decodedQuestions, flags), - records.value, + eligibility, + records.value.records, observed, ); // A structurally complete packet can carry only part of the DNS response.