diff --git a/CHANGELOG.md b/CHANGELOG.md index e5ddea9..971028e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ 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 - feat(client): add retryOverTCP option #117 diff --git a/packet.js b/packet.js index 1c41afd..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); }; /** @@ -399,9 +402,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..8410a86 100644 --- a/test/packet.js +++ b/test/packet.js @@ -671,6 +671,107 @@ test('Packet.RCODE is preserved through encode/parse round-trip', function() { } }); +test('Resource encode round-trips unknown type via raw data fallback', function() { + // 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(); + 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() { + // 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({ + 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.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;