From be2e660ed45f4b2e4be13b77415fae2ac9f6a76e Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Mon, 25 May 2026 16:01:29 -0700 Subject: [PATCH 1/2] fix(packet): preserve RDLENGTH+RDATA for unknown RR --- CHANGELOG.md | 1 + packet.js | 13 +++++++-- test/packet.js | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5ddea9..ea1e792 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### 2.2.0 - 2026-05-25 +- fix(packet): preserve RDLENGTH+RDATA for unknown RR types - feat(client): add retryOverTCP option #117 - feat(client): support `dns` argument, fix docs #116 - feat: add resolveSOA #115 diff --git a/packet.js b/packet.js index 1c41afd..abb0963 100644 --- a/packet.js +++ b/packet.js @@ -399,9 +399,18 @@ Packet.Resource.encode = function(resource, writer) { })[0]; if (encoder in Packet.Resource && Packet.Resource[encoder].encode) { return Packet.Resource[encoder].encode(resource, writer); - } else { - debug('node-dns > unknown encoder %s(%j)', encoder, resource.type); } + debug('node-dns > unknown encoder %s(%j)', encoder, resource.type); + // Fallback for unknown / decoder-only types: round-trip the raw RDATA the + // decoder preserved as `resource.data`. Without this, RDLENGTH and RDATA + // would be omitted entirely, truncating the wire format and corrupting any + // records that follow. + const data = Buffer.isBuffer(resource.data) ? resource.data : Buffer.alloc(0); + writer.write(data.length, 16); + for (const byte of data) { + writer.write(byte, 8); + } + return writer.toBuffer(); }; /** * [parse description] diff --git a/test/packet.js b/test/packet.js index a794212..59ad2f9 100644 --- a/test/packet.js +++ b/test/packet.js @@ -671,6 +671,82 @@ test('Packet.RCODE is preserved through encode/parse round-trip', function() { } }); +test('Resource encode round-trips unknown type via raw data fallback', function() { + // C1 (AUDIT-RFC.md): the encoder must write RDLENGTH+RDATA for types it + // doesn't know how to serialize, otherwise the wire format is truncated. + // 0xABCD is intentionally not in Packet.TYPE. + const rdata = Buffer.from([ 0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01 ]); + const packet = new Packet(); + packet.header.qr = 1; + packet.answers.push({ + name : 'unknown.example', + type : 0xABCD, + class : Packet.CLASS.IN, + ttl : 60, + data : rdata, + }); + const parsed = Packet.parse(packet.toBuffer()); + assert.equal(parsed.answers.length, 1); + assert.equal(parsed.answers[0].type, 0xABCD); + assert.equal(parsed.answers[0].class, Packet.CLASS.IN); + assert.equal(parsed.answers[0].ttl, 60); + assert.ok(Buffer.isBuffer(parsed.answers[0].data)); + assert.deepEqual(parsed.answers[0].data, rdata); +}); + +test('Resource encode of unknown type does not corrupt following records', function() { + // The strongest signal for the C1 fix: without it, the missing RDLENGTH + // would make the parser interpret the next record's bytes as RDATA, and + // the A record below would never appear in `answers`. + const packet = new Packet(); + packet.header.qr = 1; + packet.answers.push({ + name : 'unknown.example', + type : 0xABCD, + class : Packet.CLASS.IN, + ttl : 60, + data : Buffer.from([ 0x01, 0x02, 0x03 ]), + }); + packet.answers.push({ + name : 'after.example', + type : Packet.TYPE.A, + class : Packet.CLASS.IN, + ttl : 30, + address : '203.0.113.9', + }); + const parsed = Packet.parse(packet.toBuffer()); + assert.equal(parsed.answers.length, 2); + assert.equal(parsed.answers[0].type, 0xABCD); + assert.equal(parsed.answers[1].type, Packet.TYPE.A); + assert.equal(parsed.answers[1].name, 'after.example'); + assert.equal(parsed.answers[1].address, '203.0.113.9'); +}); + +test('Resource encode of unknown type with no data writes empty RDATA', function() { + // When an unknown-type record has no `data`, encode should still emit a + // valid RDLENGTH=0 block so the packet remains parseable. + const packet = new Packet(); + packet.header.qr = 1; + packet.answers.push({ + name : 'bare.example', + type : 0xABCD, + class : Packet.CLASS.IN, + ttl : 0, + }); + packet.answers.push({ + name : 'follow.example', + type : Packet.TYPE.A, + class : Packet.CLASS.IN, + ttl : 30, + address : '198.51.100.1', + }); + const parsed = Packet.parse(packet.toBuffer()); + assert.equal(parsed.answers.length, 2); + assert.equal(parsed.answers[0].type, 0xABCD); + assert.equal(parsed.answers[0].data.length, 0); + assert.equal(parsed.answers[1].address, '198.51.100.1'); +}); + test('Packet.parse tolerates multiple questions', function() { const request = new Packet(); request.header.id = 0x9999; From de48d8e0f319fa239e5c61ce1b2ef047f7da8a70 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Mon, 25 May 2026 16:06:36 -0700 Subject: [PATCH 2/2] fix: improve query ID randomness and anti-spoofing --- CHANGELOG.md | 4 +++- packet.js | 9 ++++++--- test/packet.js | 35 ++++++++++++++++++++++++++++++----- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea1e792..971028e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Unreleased +- fix(packet): use crypto.randomInt for Packet.uuid (RFC 5452) +- fix(packet): preserve RDLENGTH+RDATA for unknown RR types + ### 2.2.0 - 2026-05-25 -- fix(packet): preserve RDLENGTH+RDATA for unknown RR types - feat(client): add retryOverTCP option #117 - feat(client): support `dns` argument, fix docs #116 - feat: add resolveSOA #115 diff --git a/packet.js b/packet.js index abb0963..cdc1f15 100644 --- a/packet.js +++ b/packet.js @@ -1,4 +1,5 @@ const { debuglog } = require('node:util'); +const { randomInt } = require('node:crypto'); const BufferReader = require('./lib/reader'); const BufferWriter = require('./lib/writer'); @@ -145,11 +146,13 @@ Packet.EDNS_OPTION_CODE = { }; /** - * [uuid description] - * @return {[type]} [description] + * Generate a cryptographically random 16-bit DNS transaction ID. + * RFC 5452 §3 — the full 16-bit space must be used from a CSPRNG to make + * response forgery / cache poisoning impractical. + * @return {number} integer in [0, 0xFFFF] */ Packet.uuid = function() { - return Math.floor(Math.random() * 1e5); + return randomInt(0x10000); }; /** diff --git a/test/packet.js b/test/packet.js index 59ad2f9..8410a86 100644 --- a/test/packet.js +++ b/test/packet.js @@ -672,8 +672,8 @@ test('Packet.RCODE is preserved through encode/parse round-trip', function() { }); test('Resource encode round-trips unknown type via raw data fallback', function() { - // C1 (AUDIT-RFC.md): the encoder must write RDLENGTH+RDATA for types it - // doesn't know how to serialize, otherwise the wire format is truncated. + // the encoder must write RDLENGTH+RDATA for types it doesn't know how + // to serialize, else the wire format is truncated. // 0xABCD is intentionally not in Packet.TYPE. const rdata = Buffer.from([ 0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01 ]); const packet = new Packet(); @@ -695,9 +695,8 @@ test('Resource encode round-trips unknown type via raw data fallback', function( }); test('Resource encode of unknown type does not corrupt following records', function() { - // The strongest signal for the C1 fix: without it, the missing RDLENGTH - // would make the parser interpret the next record's bytes as RDATA, and - // the A record below would never appear in `answers`. + // without the fix, the missing RDLENGTH would make the parser interpret the + // next record's bytes as RDATA, and the A record would never appear in `answers`. const packet = new Packet(); packet.header.qr = 1; packet.answers.push({ @@ -747,6 +746,32 @@ test('Resource encode of unknown type with no data writes empty RDATA', function assert.equal(parsed.answers[1].address, '198.51.100.1'); }); +test('Packet.uuid returns a 16-bit integer', function() { + // must use the full 16-bit space, not Math.random()*1e5. + for (let i = 0; i < 1000; i++) { + const id = Packet.uuid(); + assert.ok(Number.isInteger(id), `not an integer: ${id}`); + assert.ok(id >= 0 && id <= 0xFFFF, `out of range: ${id}`); + } +}); + +test('Packet.uuid exercises the full 16-bit range with high diversity', function() { + // Sample size large enough that a CSPRNG over [0, 0xFFFF] almost certainly + // produces values in every quartile of the range. Catches regressions to a + // constant, low-entropy, or capped implementation. + const samples = new Set(); + const quartile = [ 0, 0, 0, 0 ]; + for (let i = 0; i < 5000; i++) { + const id = Packet.uuid(); + samples.add(id); + quartile[Math.floor((id / 0x10000) * 4)]++; + } + assert.ok(samples.size > 4000, `expected high diversity, got ${samples.size}`); + for (let q = 0; q < 4; q++) { + assert.ok(quartile[q] > 200, `quartile ${q} underrepresented (${quartile[q]}/5000)`); + } +}); + test('Packet.parse tolerates multiple questions', function() { const request = new Packet(); request.header.id = 0x9999;