Permalink
Browse files

dns: use IDNA 2008 to encode non-ascii hostnames

Before this commit, Node.js left it up to the system resolver or c-ares.

Leaving it to the system resolver introduces platform differences
because:

* some support IDNA 2008
* some only IDNA 2003 (glibc until 2.28), and
* some don't support IDNA at all (musl libc)

c-ares doesn't support IDNA either although curl does, by virtue of
linking against libidn2. Upgrading from libidn1 to libidn2 in order
to get proper IDNA 2008 support was the fix for curl's CVE-2016-8625.

libidn2 is not an option (incompatible license) but ICU has an IDNA API
and we already use that in one place. For non-ICU builds, we fall back
to the bundled punycode.js that also supports IDNA 2008.

Fixes: nodejs-private/security#97
Fixes: #25558

PR-URL: #25679
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
  • Loading branch information...
bnoordhuis authored and addaleax committed Oct 10, 2018
1 parent 6d9af41 commit c82f2445e5f5e8b387992e4e0e0ed99ab3e93012
Showing with 51 additions and 9 deletions.
  1. +3 −2 lib/dns.js
  2. +3 −2 lib/internal/dns/promises.js
  3. +9 −0 lib/internal/idna.js
  4. +1 −4 lib/url.js
  5. +1 −0 node.gyp
  6. +33 −0 test/internet/test-dns-idna2008.js
  7. +1 −1 test/parallel/test-bootstrap-modules.js
@@ -22,6 +22,7 @@
'use strict';

const cares = internalBinding('cares_wrap');
const { toASCII } = require('internal/idna');
const { isIP, isIPv4, isLegalPort } = require('internal/net');
const { customPromisifyArgs } = require('internal/util');
const errors = require('internal/errors');
@@ -139,7 +140,7 @@ function lookup(hostname, options, callback) {
req.hostname = hostname;
req.oncomplete = all ? onlookupall : onlookup;

var err = cares.getaddrinfo(req, hostname, family, hints, verbatim);
var err = cares.getaddrinfo(req, toASCII(hostname), family, hints, verbatim);
if (err) {
process.nextTick(callback, dnsException(err, 'getaddrinfo', hostname));
return {};
@@ -219,7 +220,7 @@ function resolver(bindingName) {
req.hostname = name;
req.oncomplete = onresolve;
req.ttl = !!(options && options.ttl);
var err = this._handle[bindingName](req, name);
var err = this._handle[bindingName](req, toASCII(name));
if (err) throw dnsException(err, bindingName, name);
return req;
}
@@ -6,6 +6,7 @@ const {
emitInvalidHostnameWarning,
} = require('internal/dns/utils');
const { codes, dnsException } = require('internal/errors');
const { toASCII } = require('internal/idna');
const { isIP, isIPv4, isLegalPort } = require('internal/net');
const {
getaddrinfo,
@@ -86,7 +87,7 @@ function createLookupPromise(family, hostname, all, hints, verbatim) {
req.resolve = resolve;
req.reject = reject;

const err = getaddrinfo(req, hostname, family, hints, verbatim);
const err = getaddrinfo(req, toASCII(hostname), family, hints, verbatim);

if (err) {
reject(dnsException(err, 'getaddrinfo', hostname));
@@ -184,7 +185,7 @@ function createResolverPromise(resolver, bindingName, hostname, ttl) {
req.reject = reject;
req.ttl = ttl;

const err = resolver._handle[bindingName](req, hostname);
const err = resolver._handle[bindingName](req, toASCII(hostname));

if (err)
reject(dnsException(err, bindingName, hostname));
@@ -0,0 +1,9 @@
'use strict';

if (process.binding('config').hasIntl) {
const { toASCII, toUnicode } = internalBinding('icu');
module.exports = { toASCII, toUnicode };
} else {
const { toASCII, toUnicode } = require('punycode');
module.exports = { toASCII, toUnicode };
}
@@ -21,11 +21,8 @@

'use strict';

const { toASCII } = internalBinding('config').hasIntl ?
internalBinding('icu') : require('punycode');

const { toASCII } = require('internal/idna');
const { hexTable } = require('internal/querystring');

const { SafeSet } = require('internal/safe_globals');

const {
@@ -127,6 +127,7 @@
'lib/internal/fs/utils.js',
'lib/internal/fs/watchers.js',
'lib/internal/http.js',
'lib/internal/idna.js',
'lib/internal/inspector_async_hook.js',
'lib/internal/js_stream_socket.js',
'lib/internal/linkedlist.js',
@@ -0,0 +1,33 @@
'use strict';

// Verify that non-ASCII hostnames are handled correctly as IDNA 2008.
//
// * Tests will fail with NXDOMAIN when UTF-8 leaks through to a getaddrinfo()
// that doesn't support IDNA at all.
//
// * "straße.de" will resolve to the wrong address when the resolver supports
// only IDNA 2003 (e.g., glibc until 2.28) because it encodes it wrong.

const { mustCall } = require('../common');
const assert = require('assert');
const dns = require('dns');

const [host, expectedAddress] = ['straße.de', '81.169.145.78'];

dns.lookup(host, mustCall((err, address) => {
assert.ifError(err);
assert.strictEqual(address, expectedAddress);
}));

dns.promises.lookup(host).then(mustCall(({ address }) => {
assert.strictEqual(address, expectedAddress);
}));

dns.resolve4(host, mustCall((err, addresses) => {
assert.ifError(err);
assert.deepStrictEqual(addresses, [expectedAddress]);
}));

new dns.promises.Resolver().resolve4(host).then(mustCall((addresses) => {
assert.deepStrictEqual(addresses, [expectedAddress]);
}));
@@ -10,7 +10,7 @@ const assert = require('assert');

const isMainThread = common.isMainThread;
const kCoverageModuleCount = process.env.NODE_V8_COVERAGE ? 1 : 0;
const kMaxModuleCount = (isMainThread ? 64 : 84) + kCoverageModuleCount;
const kMaxModuleCount = (isMainThread ? 65 : 85) + kCoverageModuleCount;

assert(list.length <= kMaxModuleCount,
`Total length: ${list.length}\n` + list.join('\n')

0 comments on commit c82f244

Please sign in to comment.