From 185b5d5dde352fef9ed5b6977926e693bedbd2cb Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 15 Jul 2017 14:55:06 +0800 Subject: [PATCH 1/2] deps: cherry-pick 18ea996 from c-ares upstream Original commit message: ares_parse_naptr_reply: make buffer length check more accurate 9478908a490a6bf009ba58d81de8c1d06d50a117 introduced a length check for records parsed by `ares_parse_naptr_reply()`. However, that function is designed to parse replies which also contain non-NAPTR records; for A records, the `rr_len > 7` check will fail as there are only 4 bytes of payload. In particular, parsing ANY replies for NAPTR records was broken by that patch. Fix that by moving the check into the case in which it is already known that the record is a NAPTR record. Ref: https://github.com/c-ares/c-ares/commit/18ea99693d63f957ecb670045adbd2c1da8a4641 --- deps/cares/src/ares_parse_naptr_reply.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/deps/cares/src/ares_parse_naptr_reply.c b/deps/cares/src/ares_parse_naptr_reply.c index 717d355778117f..a14c226a9e7e25 100644 --- a/deps/cares/src/ares_parse_naptr_reply.c +++ b/deps/cares/src/ares_parse_naptr_reply.c @@ -110,18 +110,19 @@ ares_parse_naptr_reply (const unsigned char *abuf, int alen, status = ARES_EBADRESP; break; } - /* RR must contain at least 7 bytes = 2 x int16 + 3 x name */ - if (rr_len < 7) - { - status = ARES_EBADRESP; - break; - } /* Check if we are really looking at a NAPTR record */ if (rr_class == C_IN && rr_type == T_NAPTR) { /* parse the NAPTR record itself */ + /* RR must contain at least 7 bytes = 2 x int16 + 3 x name */ + if (rr_len < 7) + { + status = ARES_EBADRESP; + break; + } + /* Allocate storage for this NAPTR answer appending it to the list */ naptr_curr = ares_malloc_data(ARES_DATATYPE_NAPTR_REPLY); if (!naptr_curr) From 9c1e270912deabd24fe9b07fa18d6a6253119bc8 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 22 Jun 2017 22:52:03 +0200 Subject: [PATCH 2/2] test: add non-internet resolveAny tests This is a bit of a check to see how people feel about having this kind of test. Ref: https://github.com/nodejs/node/pull/13137 PR-URL: https://github.com/nodejs/node/pull/13883 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Refael Ackermann --- test/common/dns.js | 290 ++++++++++++++++++ .../test-dns-resolveany-bad-ancount.js | 35 +++ test/parallel/test-dns-resolveany.js | 53 ++++ 3 files changed, 378 insertions(+) create mode 100644 test/common/dns.js create mode 100644 test/parallel/test-dns-resolveany-bad-ancount.js create mode 100644 test/parallel/test-dns-resolveany.js diff --git a/test/common/dns.js b/test/common/dns.js new file mode 100644 index 00000000000000..fb2f36ee750d87 --- /dev/null +++ b/test/common/dns.js @@ -0,0 +1,290 @@ +/* eslint-disable required-modules */ +'use strict'; + +// Naïve DNS parser/serializer. + +const assert = require('assert'); +const os = require('os'); + +const types = { + A: 1, + AAAA: 28, + NS: 2, + CNAME: 5, + SOA: 6, + PTR: 12, + MX: 15, + TXT: 16, + ANY: 255 +}; + +const classes = { + IN: 1 +}; + +function readDomainFromPacket(buffer, offset) { + assert.ok(offset < buffer.length); + const length = buffer[offset]; + if (length === 0) { + return { nread: 1, domain: '' }; + } else if ((length & 0xC0) === 0) { + offset += 1; + const chunk = buffer.toString('ascii', offset, offset + length); + // Read the rest of the domain. + const { nread, domain } = readDomainFromPacket(buffer, offset + length); + return { + nread: 1 + length + nread, + domain: domain ? `${chunk}.${domain}` : chunk + }; + } else { + // Pointer to another part of the packet. + assert.strictEqual(length & 0xC0, 0xC0); + // eslint-disable-next-line + const pointeeOffset = buffer.readUInt16BE(offset) &~ 0xC000; + return { + nread: 2, + domain: readDomainFromPacket(buffer, pointeeOffset) + }; + } +} + +function parseDNSPacket(buffer) { + assert.ok(buffer.length > 12); + + const parsed = { + id: buffer.readUInt16BE(0), + flags: buffer.readUInt16BE(2), + }; + + const counts = [ + ['questions', buffer.readUInt16BE(4)], + ['answers', buffer.readUInt16BE(6)], + ['authorityAnswers', buffer.readUInt16BE(8)], + ['additionalRecords', buffer.readUInt16BE(10)] + ]; + + let offset = 12; + for (const [ sectionName, count ] of counts) { + parsed[sectionName] = []; + for (let i = 0; i < count; ++i) { + const { nread, domain } = readDomainFromPacket(buffer, offset); + offset += nread; + + const type = buffer.readUInt16BE(offset); + + const rr = { + domain, + cls: buffer.readUInt16BE(offset + 2), + }; + offset += 4; + + for (const name in types) { + if (types[name] === type) + rr.type = name; + } + + if (sectionName !== 'questions') { + rr.ttl = buffer.readInt32BE(offset); + const dataLength = buffer.readUInt16BE(offset); + offset += 6; + + switch (type) { + case types.A: + assert.strictEqual(dataLength, 4); + rr.address = `${buffer[offset + 0]}.${buffer[offset + 1]}.` + + `${buffer[offset + 2]}.${buffer[offset + 3]}`; + break; + case types.AAAA: + assert.strictEqual(dataLength, 16); + rr.address = buffer.toString('hex', offset, offset + 16) + .replace(/(.{4}(?!$))/g, '$1:'); + break; + case types.TXT: + { + let position = offset; + rr.entries = []; + while (position < offset + dataLength) { + const txtLength = buffer[offset]; + rr.entries.push(buffer.toString('utf8', + position + 1, + position + 1 + txtLength)); + position += 1 + txtLength; + } + assert.strictEqual(position, offset + dataLength); + break; + } + case types.MX: + { + rr.priority = buffer.readInt16BE(buffer, offset); + offset += 2; + const { nread, domain } = readDomainFromPacket(buffer, offset); + rr.exchange = domain; + assert.strictEqual(nread, dataLength); + break; + } + case types.NS: + case types.CNAME: + case types.PTR: + { + const { nread, domain } = readDomainFromPacket(buffer, offset); + rr.value = domain; + assert.strictEqual(nread, dataLength); + break; + } + case types.SOA: + { + const mname = readDomainFromPacket(buffer, offset); + const rname = readDomainFromPacket(buffer, offset + mname.nread); + rr.nsname = mname.domain; + rr.hostmaster = rname.domain; + const trailerOffset = offset + mname.nread + rname.nread; + rr.serial = buffer.readUInt32BE(trailerOffset); + rr.refresh = buffer.readUInt32BE(trailerOffset + 4); + rr.retry = buffer.readUInt32BE(trailerOffset + 8); + rr.expire = buffer.readUInt32BE(trailerOffset + 12); + rr.minttl = buffer.readUInt32BE(trailerOffset + 16); + + assert.strictEqual(trailerOffset + 20, dataLength); + break; + } + default: + throw new Error(`Unknown RR type ${rr.type}`); + } + offset += dataLength; + } + + parsed[sectionName].push(rr); + + assert.ok(offset <= buffer.length); + } + } + + assert.strictEqual(offset, buffer.length); + return parsed; +} + +function writeIPv6(ip) { + const parts = ip.replace(/^:|:$/g, '').split(':'); + const buf = Buffer.alloc(16); + + let offset = 0; + for (const part of parts) { + if (part === '') { + offset += 16 - 2 * (parts.length - 1); + } else { + buf.writeUInt16BE(parseInt(part, 16), offset); + offset += 2; + } + } + + return buf; +} + +function writeDomainName(domain) { + return Buffer.concat(domain.split('.').map((label) => { + assert(label.length < 64); + return Buffer.concat([ + Buffer.from([label.length]), + Buffer.from(label, 'ascii') + ]); + }).concat([Buffer.alloc(1)])); +} + +function writeDNSPacket(parsed) { + const buffers = []; + const kStandardResponseFlags = 0x8180; + + buffers.push(new Uint16Array([ + parsed.id, + parsed.flags === undefined ? kStandardResponseFlags : parsed.flags, + parsed.questions && parsed.questions.length, + parsed.answers && parsed.answers.length, + parsed.authorityAnswers && parsed.authorityAnswers.length, + parsed.additionalRecords && parsed.additionalRecords.length, + ])); + + for (const q of parsed.questions) { + assert(types[q.type]); + buffers.push(writeDomainName(q.domain)); + buffers.push(new Uint16Array([ + types[q.type], + q.cls === undefined ? classes.IN : q.cls + ])); + } + + for (const rr of [].concat(parsed.answers, + parsed.authorityAnswers, + parsed.additionalRecords)) { + if (!rr) continue; + + assert(types[rr.type]); + buffers.push(writeDomainName(rr.domain)); + buffers.push(new Uint16Array([ + types[rr.type], + rr.cls === undefined ? classes.IN : rr.cls + ])); + buffers.push(new Int32Array([rr.ttl])); + + const rdLengthBuf = new Uint16Array(1); + buffers.push(rdLengthBuf); + + switch (rr.type) { + case 'A': + rdLengthBuf[0] = 4; + buffers.push(new Uint8Array(rr.address.split('.'))); + break; + case 'AAAA': + rdLengthBuf[0] = 16; + buffers.push(writeIPv6(rr.address)); + break; + case 'TXT': + const total = rr.entries.map((s) => s.length).reduce((a, b) => a + b); + // Total length of all strings + 1 byte each for their lengths. + rdLengthBuf[0] = rr.entries.length + total; + for (const txt of rr.entries) { + buffers.push(new Uint8Array([Buffer.byteLength(txt)])); + buffers.push(Buffer.from(txt)); + } + break; + case 'MX': + rdLengthBuf[0] = 2; + buffers.push(new Uint16Array([rr.priority])); + // fall through + case 'NS': + case 'CNAME': + case 'PTR': + { + const domain = writeDomainName(rr.exchange || rr.value); + rdLengthBuf[0] += domain.length; + buffers.push(domain); + break; + } + case 'SOA': + { + const mname = writeDomainName(rr.nsname); + const rname = writeDomainName(rr.hostmaster); + rdLengthBuf[0] = mname.length + rname.length + 20; + buffers.push(mname, rname); + buffers.push(new Uint32Array([ + rr.serial, rr.refresh, rr.retry, rr.expire, rr.minttl + ])); + break; + } + default: + throw new Error(`Unknown RR type ${rr.type}`); + } + } + + return Buffer.concat(buffers.map((typedArray) => { + const buf = Buffer.from(typedArray.buffer, + typedArray.byteOffset, + typedArray.byteLength); + if (os.endianness() === 'LE') { + if (typedArray.BYTES_PER_ELEMENT === 2) buf.swap16(); + if (typedArray.BYTES_PER_ELEMENT === 4) buf.swap32(); + } + return buf; + })); +} + +module.exports = { types, classes, writeDNSPacket, parseDNSPacket }; diff --git a/test/parallel/test-dns-resolveany-bad-ancount.js b/test/parallel/test-dns-resolveany-bad-ancount.js new file mode 100644 index 00000000000000..25ce15935caad9 --- /dev/null +++ b/test/parallel/test-dns-resolveany-bad-ancount.js @@ -0,0 +1,35 @@ +'use strict'; +const common = require('../common'); +const dnstools = require('../common/dns'); +const dns = require('dns'); +const assert = require('assert'); +const dgram = require('dgram'); + +const server = dgram.createSocket('udp4'); + +server.on('message', common.mustCall((msg, { address, port }) => { + const parsed = dnstools.parseDNSPacket(msg); + const domain = parsed.questions[0].domain; + assert.strictEqual(domain, 'example.org'); + + const buf = dnstools.writeDNSPacket({ + id: parsed.id, + questions: parsed.questions, + answers: { type: 'A', address: '1.2.3.4', ttl: 123, domain }, + }); + // Overwrite the # of answers with 2, which is incorrect. + buf.writeUInt16LE(2, 6); + server.send(buf, port, address); +})); + +server.bind(0, common.mustCall(() => { + const address = server.address(); + dns.setServers([`127.0.0.1:${address.port}`]); + + dns.resolveAny('example.org', common.mustCall((err) => { + assert.strictEqual(err.code, 'EBADRESP'); + assert.strictEqual(err.syscall, 'queryAny'); + assert.strictEqual(err.hostname, 'example.org'); + server.close(); + })); +})); diff --git a/test/parallel/test-dns-resolveany.js b/test/parallel/test-dns-resolveany.js new file mode 100644 index 00000000000000..7ad0d3b0fa38d9 --- /dev/null +++ b/test/parallel/test-dns-resolveany.js @@ -0,0 +1,53 @@ +'use strict'; +const common = require('../common'); +const dnstools = require('../common/dns'); +const dns = require('dns'); +const assert = require('assert'); +const dgram = require('dgram'); + +const answers = [ + { type: 'A', address: '1.2.3.4', ttl: 123 }, + { type: 'AAAA', address: '::42', ttl: 123 }, + { type: 'MX', priority: 42, exchange: 'foobar.com', ttl: 124 }, + { type: 'NS', value: 'foobar.org', ttl: 457 }, + { type: 'TXT', entries: [ 'v=spf1 ~all', 'xyz' ] }, + { type: 'PTR', value: 'baz.org', ttl: 987 }, + { + type: 'SOA', + nsname: 'ns1.example.com', + hostmaster: 'admin.example.com', + serial: 156696742, + refresh: 900, + retry: 900, + expire: 1800, + minttl: 60 + }, +]; + +const server = dgram.createSocket('udp4'); + +server.on('message', common.mustCall((msg, { address, port }) => { + const parsed = dnstools.parseDNSPacket(msg); + const domain = parsed.questions[0].domain; + assert.strictEqual(domain, 'example.org'); + + server.send(dnstools.writeDNSPacket({ + id: parsed.id, + questions: parsed.questions, + answers: answers.map((answer) => Object.assign({ domain }, answer)), + }), port, address); +})); + +server.bind(0, common.mustCall(() => { + const address = server.address(); + dns.setServers([`127.0.0.1:${address.port}`]); + + dns.resolveAny('example.org', common.mustCall((err, res) => { + assert.ifError(err); + // Compare copies with ttl removed, c-ares fiddles with that value. + assert.deepStrictEqual( + res.map((r) => Object.assign({}, r, { ttl: null })), + answers.map((r) => Object.assign({}, r, { ttl: null }))); + server.close(); + })); +}));