From 0d19c2045fc70220d17a767bb4dd5209f811aebe Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Thu, 10 Aug 2017 15:11:19 +0200 Subject: [PATCH 01/15] tls: handle ipv6 addresses in host-header When an http-client requests a url with the hostname specified as an IPv6 address, the Host: header will have the following format: Host: [::1]:3000 The servername in this case should be '::1'. Fixes: https://github.com/nodejs/node/issues/14736 --- lib/_http_agent.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index e71f0cb441f86e..5df9452f05eda6 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -157,7 +157,12 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/, options.servername = options.host; const hostHeader = req.getHeader('host'); if (hostHeader) { - options.servername = hostHeader.replace(/:.*$/, ''); + // abc => abc + // abc:123 => abc + // [::1] => ::1 + // [::1]:123 => ::1 + options.servername = hostHeader.replace(/:[^\]]+$/, '') + .replace(/^\[(.*)\]$/, '$1'); } } From 31b5eda5412b4415158269a3910471fb6cc8973f Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Thu, 10 Aug 2017 21:08:47 +0200 Subject: [PATCH 02/15] tls: compare san against ip in correct format When comparing IP addresses against addresses in the subjectAltName field of a certificate, format the address correctly before doing the string comparison. Fixes: https://github.com/nodejs/node/issues/14736 --- lib/tls.js | 30 +++++++++++++++++++++++++- test/internet/test-tls-canonical-ip.js | 18 ++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 test/internet/test-tls-canonical-ip.js diff --git a/lib/tls.js b/lib/tls.js index 2d1c539532f9bf..eee48a00ba8f14 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -164,6 +164,33 @@ function check(hostParts, pattern, wildcards) { return true; } +exports._canonicalIp = function(address) { + // Convert the ip address into the same format + // stored in certificates + if (net.isIPv6(address)) { + const b = ['0', '0', '0', '0', '0', '0', '0', '0']; + + const s = address.split('::'); + if (s.length === 2) { + const s1 = s[0].split(':'); + for (var n = 0; n < s1.length; n++) { + if (s1[n]) { + b[n] = s1[n].replace(/^0+(\d+)$/, '$1'); + } + } + const s2 = s[1].split(':'); + for (n = 0; n < s2.length; n++) { + if (s2[n]) { + b[8 - s2.length + n] = s2[n].replace(/^0+(\d+)$/, '$1'); + } + } + } + + return b.join(':'); + } else + return address.replace(/\b0+(\d)/g, '$1'); // Delete leading zeroes +}; + exports.checkServerIdentity = function checkServerIdentity(host, cert) { const subject = cert.subject; const altNames = cert.subjectaltname; @@ -190,7 +217,8 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { let reason = 'Unknown reason'; if (net.isIP(host)) { - valid = ips.includes(host); + const canonicalIp = exports._canonicalIp(host); + valid = ips.includes(canonicalIp); if (!valid) reason = `IP: ${host} is not in the cert's list: ${ips.join(', ')}`; // TODO(bnoordhuis) Also check URI SANs that are IP addresses. diff --git a/test/internet/test-tls-canonical-ip.js b/test/internet/test-tls-canonical-ip.js new file mode 100644 index 00000000000000..955ba5fba7f08a --- /dev/null +++ b/test/internet/test-tls-canonical-ip.js @@ -0,0 +1,18 @@ +'use strict'; +require('../common'); + +// Test conversion of IP addresses to the format returned +// for addresses in Subject Alternative Name section +// of a TLS certificate + +const assert = require('assert'); +const tls = require('tls'); + +assert.strictEqual(tls._canonicalIp('127.0.0.1'), '127.0.0.1'); +assert.strictEqual(tls._canonicalIp('010.001.0.1'), '10.1.0.1'); +assert.strictEqual(tls._canonicalIp('::1'), '0:0:0:0:0:0:0:1'); +assert.strictEqual(tls._canonicalIp('fe80::1'), 'fe80:0:0:0:0:0:0:1'); +assert.strictEqual(tls._canonicalIp('fe80::'), 'fe80:0:0:0:0:0:0:0'); +assert.strictEqual( + tls._canonicalIp('fe80::0000:0010:0001'), + 'fe80:0:0:0:0:0:10:1'); From f0332a10a4c12615b9833579f5fb1d32034bc3a8 Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Fri, 11 Aug 2017 22:31:32 +0200 Subject: [PATCH 03/15] http: optimize servername extraction --- lib/_http_agent.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 5df9452f05eda6..3e80305f534671 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -161,8 +161,11 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/, // abc:123 => abc // [::1] => ::1 // [::1]:123 => ::1 - options.servername = hostHeader.replace(/:[^\]]+$/, '') - .replace(/^\[(.*)\]$/, '$1'); + if (hostHeader.startsWith('[')) { + options.servername = hostHeader.split(']')[0].substr(1); + } else { + options.servername = hostHeader.split(':')[0]; + } } } From 328a02f230c1d25f9dd037b18510990202e7baac Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Mon, 14 Aug 2017 08:14:27 +0200 Subject: [PATCH 04/15] tls: move canonicalIp to internal/tls --- lib/internal/tls.js | 31 +++++++++++++++++++++++++- lib/tls.js | 31 ++------------------------ node.gyp | 1 + test/internet/test-tls-canonical-ip.js | 14 ++++++------ 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/lib/internal/tls.js b/lib/internal/tls.js index 6d367dbf285ff7..cd0d1935a9573a 100644 --- a/lib/internal/tls.js +++ b/lib/internal/tls.js @@ -1,4 +1,5 @@ 'use strict'; +const net = require('net'); // Example: // C=US\nST=CA\nL=SF\nO=Joyent\nOU=Node.js\nCN=ca1\nemailAddress=ry@clouds.org @@ -23,6 +24,34 @@ function parseCertString(s) { return out; } +function canonicalIp(address) { + // Convert the ip address into the same format + // stored in certificates + if (net.isIPv6(address)) { + const b = ['0', '0', '0', '0', '0', '0', '0', '0']; + + const s = address.split('::'); + if (s.length === 2) { + const s1 = s[0].split(':'); + for (var n = 0; n < s1.length; n++) { + if (s1[n]) { + b[n] = s1[n].replace(/^0+(\d+)$/, '$1'); + } + } + const s2 = s[1].split(':'); + for (n = 0; n < s2.length; n++) { + if (s2[n]) { + b[8 - s2.length + n] = s2[n].replace(/^0+(\d+)$/, '$1'); + } + } + } + + return b.join(':'); + } else + return address.replace(/\b0+(\d)/g, '$1'); // Delete leading zeroes +} + module.exports = { - parseCertString + parseCertString, + canonicalIp }; diff --git a/lib/tls.js b/lib/tls.js index eee48a00ba8f14..fa25780f78c45d 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -31,6 +31,7 @@ const net = require('net'); const url = require('url'); const binding = process.binding('crypto'); const Buffer = require('buffer').Buffer; +const canonicalIp = require('internal/tls').canonicalIp; // Allow {CLIENT_RENEG_LIMIT} client-initiated session renegotiations // every {CLIENT_RENEG_WINDOW} seconds. An error event is emitted if more @@ -164,33 +165,6 @@ function check(hostParts, pattern, wildcards) { return true; } -exports._canonicalIp = function(address) { - // Convert the ip address into the same format - // stored in certificates - if (net.isIPv6(address)) { - const b = ['0', '0', '0', '0', '0', '0', '0', '0']; - - const s = address.split('::'); - if (s.length === 2) { - const s1 = s[0].split(':'); - for (var n = 0; n < s1.length; n++) { - if (s1[n]) { - b[n] = s1[n].replace(/^0+(\d+)$/, '$1'); - } - } - const s2 = s[1].split(':'); - for (n = 0; n < s2.length; n++) { - if (s2[n]) { - b[8 - s2.length + n] = s2[n].replace(/^0+(\d+)$/, '$1'); - } - } - } - - return b.join(':'); - } else - return address.replace(/\b0+(\d)/g, '$1'); // Delete leading zeroes -}; - exports.checkServerIdentity = function checkServerIdentity(host, cert) { const subject = cert.subject; const altNames = cert.subjectaltname; @@ -217,8 +191,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { let reason = 'Unknown reason'; if (net.isIP(host)) { - const canonicalIp = exports._canonicalIp(host); - valid = ips.includes(canonicalIp); + valid = ips.includes(canonicalIp(host)); if (!valid) reason = `IP: ${host} is not in the cert's list: ${ips.join(', ')}`; // TODO(bnoordhuis) Also check URI SANs that are IP addresses. diff --git a/node.gyp b/node.gyp index 5ccc4ff423a59a..678aa1f86200ef 100644 --- a/node.gyp +++ b/node.gyp @@ -119,6 +119,7 @@ 'lib/internal/readline.js', 'lib/internal/repl.js', 'lib/internal/socket_list.js', + 'lib/internal/tls.js', 'lib/internal/test/unicode.js', 'lib/internal/tls.js', 'lib/internal/url.js', diff --git a/test/internet/test-tls-canonical-ip.js b/test/internet/test-tls-canonical-ip.js index 955ba5fba7f08a..1efa5290a69044 100644 --- a/test/internet/test-tls-canonical-ip.js +++ b/test/internet/test-tls-canonical-ip.js @@ -6,13 +6,13 @@ require('../common'); // of a TLS certificate const assert = require('assert'); -const tls = require('tls'); +const tls = require('internal/tls'); -assert.strictEqual(tls._canonicalIp('127.0.0.1'), '127.0.0.1'); -assert.strictEqual(tls._canonicalIp('010.001.0.1'), '10.1.0.1'); -assert.strictEqual(tls._canonicalIp('::1'), '0:0:0:0:0:0:0:1'); -assert.strictEqual(tls._canonicalIp('fe80::1'), 'fe80:0:0:0:0:0:0:1'); -assert.strictEqual(tls._canonicalIp('fe80::'), 'fe80:0:0:0:0:0:0:0'); +assert.strictEqual(tls.canonicalIp('127.0.0.1'), '127.0.0.1'); +assert.strictEqual(tls.canonicalIp('010.001.0.1'), '10.1.0.1'); +assert.strictEqual(tls.canonicalIp('::1'), '0:0:0:0:0:0:0:1'); +assert.strictEqual(tls.canonicalIp('fe80::1'), 'fe80:0:0:0:0:0:0:1'); +assert.strictEqual(tls.canonicalIp('fe80::'), 'fe80:0:0:0:0:0:0:0'); assert.strictEqual( - tls._canonicalIp('fe80::0000:0010:0001'), + tls.canonicalIp('fe80::0000:0010:0001'), 'fe80:0:0:0:0:0:10:1'); From be2b33c69958bf53e840b01811d96a6e9e42bd7c Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Tue, 29 Aug 2017 10:22:59 +0200 Subject: [PATCH 05/15] tls: handle complete ipv6 addresses --- lib/internal/tls.js | 13 +++++++------ test/internet/test-tls-canonical-ip.js | 7 +++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/internal/tls.js b/lib/internal/tls.js index cd0d1935a9573a..293e84ed5e8aab 100644 --- a/lib/internal/tls.js +++ b/lib/internal/tls.js @@ -31,13 +31,14 @@ function canonicalIp(address) { const b = ['0', '0', '0', '0', '0', '0', '0', '0']; const s = address.split('::'); - if (s.length === 2) { - const s1 = s[0].split(':'); - for (var n = 0; n < s1.length; n++) { - if (s1[n]) { - b[n] = s1[n].replace(/^0+(\d+)$/, '$1'); - } + const s1 = s[0].split(':'); + for (var n = 0; n < s1.length; n++) { + if (s1[n]) { + b[n] = s1[n].replace(/^0+(\d+)$/, '$1'); } + } + + if (s.length === 2) { const s2 = s[1].split(':'); for (n = 0; n < s2.length; n++) { if (s2[n]) { diff --git a/test/internet/test-tls-canonical-ip.js b/test/internet/test-tls-canonical-ip.js index 1efa5290a69044..e6071813ca04d2 100644 --- a/test/internet/test-tls-canonical-ip.js +++ b/test/internet/test-tls-canonical-ip.js @@ -16,3 +16,10 @@ assert.strictEqual(tls.canonicalIp('fe80::'), 'fe80:0:0:0:0:0:0:0'); assert.strictEqual( tls.canonicalIp('fe80::0000:0010:0001'), 'fe80:0:0:0:0:0:10:1'); +assert.strictEqual( + tls.canonicalIp('0001:2222:3333:4444:5555:6666:7777:0088'), + '1:2222:3333:4444:5555:6666:7777:88'); + +assert.strictEqual( + tls.canonicalIp('0001:2222:3333:4444:5555:6666::'), + '1:2222:3333:4444:5555:6666:0:0'); From 5384b4a2291b6f8f183e395fe9d68192d3400bb2 Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Thu, 7 Sep 2017 08:04:28 +0200 Subject: [PATCH 06/15] tls: force IPv6 addresses to lower case for comparison On some systems, IPv6 addresses in certificates are returned as upper case hex. Force them to lower case to be able to do string comparisons --- lib/internal/tls.js | 9 ++++++--- lib/tls.js | 2 +- test/internet/test-tls-canonical-ip.js | 4 ++++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/internal/tls.js b/lib/internal/tls.js index 293e84ed5e8aab..5e00bd038253da 100644 --- a/lib/internal/tls.js +++ b/lib/internal/tls.js @@ -30,11 +30,14 @@ function canonicalIp(address) { if (net.isIPv6(address)) { const b = ['0', '0', '0', '0', '0', '0', '0', '0']; - const s = address.split('::'); + const s = address + .toLowerCase() + .replace(/\b0+([0-9a-f])/g, '$1') + .split('::'); const s1 = s[0].split(':'); for (var n = 0; n < s1.length; n++) { if (s1[n]) { - b[n] = s1[n].replace(/^0+(\d+)$/, '$1'); + b[n] = s1[n]; } } @@ -42,7 +45,7 @@ function canonicalIp(address) { const s2 = s[1].split(':'); for (n = 0; n < s2.length; n++) { if (s2[n]) { - b[8 - s2.length + n] = s2[n].replace(/^0+(\d+)$/, '$1'); + b[8 - s2.length + n] = s2[n]; } } } diff --git a/lib/tls.js b/lib/tls.js index fa25780f78c45d..ee254449516722 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -182,7 +182,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { const uri = url.parse(name.slice(4)); uriNames.push(uri.hostname); // TODO(bnoordhuis) Also use scheme. } else if (name.startsWith('IP Address:')) { - ips.push(name.slice(11)); + ips.push(name.slice(11).toLowerCase()); } } } diff --git a/test/internet/test-tls-canonical-ip.js b/test/internet/test-tls-canonical-ip.js index e6071813ca04d2..55d8c18f6006d4 100644 --- a/test/internet/test-tls-canonical-ip.js +++ b/test/internet/test-tls-canonical-ip.js @@ -23,3 +23,7 @@ assert.strictEqual( assert.strictEqual( tls.canonicalIp('0001:2222:3333:4444:5555:6666::'), '1:2222:3333:4444:5555:6666:0:0'); + +assert.strictEqual( + tls.canonicalIp('a002:B12:00Ba:4444:5555:6666::'), + 'a002:b12:ba:4444:5555:6666:0:0'); From fbe4aa06f167a971b2cf6320b52cc64f2a96f683 Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Sun, 17 Sep 2017 19:13:40 +0200 Subject: [PATCH 07/15] tls: add expose-internals flag and move test --- test/{internet => parallel}/test-tls-canonical-ip.js | 1 + 1 file changed, 1 insertion(+) rename test/{internet => parallel}/test-tls-canonical-ip.js (97%) diff --git a/test/internet/test-tls-canonical-ip.js b/test/parallel/test-tls-canonical-ip.js similarity index 97% rename from test/internet/test-tls-canonical-ip.js rename to test/parallel/test-tls-canonical-ip.js index 55d8c18f6006d4..bbbf353569cab0 100644 --- a/test/internet/test-tls-canonical-ip.js +++ b/test/parallel/test-tls-canonical-ip.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; require('../common'); From 1fa645f1a252f23b3f971f45b814380fca53a4de Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Mon, 25 Sep 2017 19:49:19 +0200 Subject: [PATCH 08/15] net: canonicalIP-function using uv_inet_ntop/pton --- src/cares_wrap.cc | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index e800e0f2fee260..1527310f43ae52 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1935,6 +1935,32 @@ void IsIPv6(const FunctionCallbackInfo& args) { } } +void canonicalIP(const FunctionCallbackInfo& args) { + node::Utf8Value ip(args.GetIsolate(), args[0]); + char address_buffer[sizeof(struct in6_addr)]; + v8::Isolate* isolate = args.GetIsolate(); + + if (uv_inet_pton(AF_INET, *ip, &address_buffer) == 0) { + char canonical_ip[INET_ADDRSTRLEN]; + + int err = uv_inet_ntop(AF_INET, address_buffer, canonical_ip, + sizeof(canonical_ip)); + CHECK_EQ(err, 0); + + args.GetReturnValue().Set(String::NewFromUtf8(isolate, canonical_ip)); + } else if (uv_inet_pton(AF_INET6, *ip, &address_buffer) == 0) { + char canonical_ip[INET6_ADDRSTRLEN]; + + int err = uv_inet_ntop(AF_INET6, address_buffer, canonical_ip, + sizeof(canonical_ip)); + CHECK_EQ(err, 0); + + args.GetReturnValue().Set(String::NewFromUtf8(isolate, canonical_ip)); + } else { + args.GetReturnValue().Set(Undefined(isolate)); + } +} + void GetAddrInfo(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -2144,6 +2170,7 @@ void Initialize(Local target, env->SetMethod(target, "isIP", IsIP); env->SetMethod(target, "isIPv4", IsIPv4); env->SetMethod(target, "isIPv6", IsIPv6); + env->SetMethod(target, "canonicalIP", canonicalIP); env->SetMethod(target, "strerror", StrError); From 9d2cb6a9769af8fa2a8f81303e8f7c6f4e06a62f Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Mon, 25 Sep 2017 20:34:26 +0200 Subject: [PATCH 09/15] tls: use canonicalIP from cares_wrap --- lib/internal/tls.js | 35 +------------------------ lib/tls.js | 6 ++--- test/parallel/test-tls-canonical-ip.js | 36 ++++++++++++++++---------- 3 files changed, 26 insertions(+), 51 deletions(-) diff --git a/lib/internal/tls.js b/lib/internal/tls.js index 5e00bd038253da..6d367dbf285ff7 100644 --- a/lib/internal/tls.js +++ b/lib/internal/tls.js @@ -1,5 +1,4 @@ 'use strict'; -const net = require('net'); // Example: // C=US\nST=CA\nL=SF\nO=Joyent\nOU=Node.js\nCN=ca1\nemailAddress=ry@clouds.org @@ -24,38 +23,6 @@ function parseCertString(s) { return out; } -function canonicalIp(address) { - // Convert the ip address into the same format - // stored in certificates - if (net.isIPv6(address)) { - const b = ['0', '0', '0', '0', '0', '0', '0', '0']; - - const s = address - .toLowerCase() - .replace(/\b0+([0-9a-f])/g, '$1') - .split('::'); - const s1 = s[0].split(':'); - for (var n = 0; n < s1.length; n++) { - if (s1[n]) { - b[n] = s1[n]; - } - } - - if (s.length === 2) { - const s2 = s[1].split(':'); - for (n = 0; n < s2.length; n++) { - if (s2[n]) { - b[8 - s2.length + n] = s2[n]; - } - } - } - - return b.join(':'); - } else - return address.replace(/\b0+(\d)/g, '$1'); // Delete leading zeroes -} - module.exports = { - parseCertString, - canonicalIp + parseCertString }; diff --git a/lib/tls.js b/lib/tls.js index ee254449516722..1c9878cdae1202 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -31,7 +31,7 @@ const net = require('net'); const url = require('url'); const binding = process.binding('crypto'); const Buffer = require('buffer').Buffer; -const canonicalIp = require('internal/tls').canonicalIp; +const canonicalIP = process.binding('cares_wrap').canonicalIP; // Allow {CLIENT_RENEG_LIMIT} client-initiated session renegotiations // every {CLIENT_RENEG_WINDOW} seconds. An error event is emitted if more @@ -182,7 +182,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { const uri = url.parse(name.slice(4)); uriNames.push(uri.hostname); // TODO(bnoordhuis) Also use scheme. } else if (name.startsWith('IP Address:')) { - ips.push(name.slice(11).toLowerCase()); + ips.push(canonicalIP(name.slice(11))); } } } @@ -191,7 +191,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { let reason = 'Unknown reason'; if (net.isIP(host)) { - valid = ips.includes(canonicalIp(host)); + valid = ips.includes(canonicalIP(host)); if (!valid) reason = `IP: ${host} is not in the cert's list: ${ips.join(', ')}`; // TODO(bnoordhuis) Also check URI SANs that are IP addresses. diff --git a/test/parallel/test-tls-canonical-ip.js b/test/parallel/test-tls-canonical-ip.js index bbbf353569cab0..74e746a3c0302e 100644 --- a/test/parallel/test-tls-canonical-ip.js +++ b/test/parallel/test-tls-canonical-ip.js @@ -1,4 +1,3 @@ -// Flags: --expose-internals 'use strict'; require('../common'); @@ -7,24 +6,33 @@ require('../common'); // of a TLS certificate const assert = require('assert'); -const tls = require('internal/tls'); +const canonicalIP = process.binding('cares_wrap').canonicalIP; -assert.strictEqual(tls.canonicalIp('127.0.0.1'), '127.0.0.1'); -assert.strictEqual(tls.canonicalIp('010.001.0.1'), '10.1.0.1'); -assert.strictEqual(tls.canonicalIp('::1'), '0:0:0:0:0:0:0:1'); -assert.strictEqual(tls.canonicalIp('fe80::1'), 'fe80:0:0:0:0:0:0:1'); -assert.strictEqual(tls.canonicalIp('fe80::'), 'fe80:0:0:0:0:0:0:0'); +assert.strictEqual(canonicalIP('127.0.0.1'), '127.0.0.1'); +assert.strictEqual(canonicalIP('10.1.0.1'), '10.1.0.1'); +assert.strictEqual(canonicalIP('::1'), '::1'); +assert.strictEqual(canonicalIP('fe80:0:0:0:0:0:0:1'), 'fe80::1'); +assert.strictEqual(canonicalIP('fe80:0:0:0:0:0:0:0'), 'fe80::'); assert.strictEqual( - tls.canonicalIp('fe80::0000:0010:0001'), - 'fe80:0:0:0:0:0:10:1'); + canonicalIP('fe80::0000:0010:0001'), + 'fe80::10:1'); assert.strictEqual( - tls.canonicalIp('0001:2222:3333:4444:5555:6666:7777:0088'), + canonicalIP('0001:2222:3333:4444:5555:6666:7777:0088'), '1:2222:3333:4444:5555:6666:7777:88'); assert.strictEqual( - tls.canonicalIp('0001:2222:3333:4444:5555:6666::'), - '1:2222:3333:4444:5555:6666:0:0'); + canonicalIP('0001:2222:3333:4444:5555:6666::'), + '1:2222:3333:4444:5555:6666::'); assert.strictEqual( - tls.canonicalIp('a002:B12:00Ba:4444:5555:6666::'), - 'a002:b12:ba:4444:5555:6666:0:0'); + canonicalIP('a002:B12:00Ba:4444:5555:6666:0:0'), + 'a002:b12:ba:4444:5555:6666::'); + +// IPv4 address represented in IPv6 +assert.strictEqual( + canonicalIP('0:0:0:0:0:ffff:c0a8:101'), + '::ffff:192.168.1.1'); + + assert.strictEqual( + canonicalIP('::ffff:192.168.1.1'), + '::ffff:192.168.1.1'); From 897c99a67fdb598b1f195a64de101f0148149203 Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Wed, 27 Sep 2017 20:22:26 +0200 Subject: [PATCH 10/15] Remove unnecessary return value --- src/cares_wrap.cc | 2 -- test/parallel/test-tls-canonical-ip.js | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 1527310f43ae52..5cc9fa79fdc535 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1956,8 +1956,6 @@ void canonicalIP(const FunctionCallbackInfo& args) { CHECK_EQ(err, 0); args.GetReturnValue().Set(String::NewFromUtf8(isolate, canonical_ip)); - } else { - args.GetReturnValue().Set(Undefined(isolate)); } } diff --git a/test/parallel/test-tls-canonical-ip.js b/test/parallel/test-tls-canonical-ip.js index 74e746a3c0302e..3134548cb5f802 100644 --- a/test/parallel/test-tls-canonical-ip.js +++ b/test/parallel/test-tls-canonical-ip.js @@ -33,6 +33,6 @@ assert.strictEqual( canonicalIP('0:0:0:0:0:ffff:c0a8:101'), '::ffff:192.168.1.1'); - assert.strictEqual( - canonicalIP('::ffff:192.168.1.1'), - '::ffff:192.168.1.1'); +assert.strictEqual( + canonicalIP('::ffff:192.168.1.1'), + '::ffff:192.168.1.1'); From 19c96e4090483352936a9a8a20b1300b88d1e9ca Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Thu, 28 Sep 2017 14:48:11 +0200 Subject: [PATCH 11/15] http: speed up header splitting --- lib/_http_agent.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 3e80305f534671..87ebe70da7c710 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -162,7 +162,13 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/, // [::1] => ::1 // [::1]:123 => ::1 if (hostHeader.startsWith('[')) { - options.servername = hostHeader.split(']')[0].substr(1); + const index = hostHeader.indexOf(']'); + if (index === -1) { + // Leading '[', but no ']'. Need to do something... + options.serverName = hostHeader; + } else { + options.servername = hostHeader.substr(1, index - 1); + } } else { options.servername = hostHeader.split(':')[0]; } From a9dc19094b0f246f7a488fb9f775e1b169e9fb4d Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Thu, 28 Sep 2017 14:48:56 +0200 Subject: [PATCH 12/15] tls: rename canonicalIP as canonicalizeIP --- lib/tls.js | 6 ++-- src/cares_wrap.cc | 17 +++++------ test/parallel/test-tls-canonical-ip.js | 41 +++++++++++--------------- 3 files changed, 27 insertions(+), 37 deletions(-) diff --git a/lib/tls.js b/lib/tls.js index 1c9878cdae1202..a82535df618f99 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -31,7 +31,7 @@ const net = require('net'); const url = require('url'); const binding = process.binding('crypto'); const Buffer = require('buffer').Buffer; -const canonicalIP = process.binding('cares_wrap').canonicalIP; +const canonicalizeIP = process.binding('cares_wrap').canonicalizeIP; // Allow {CLIENT_RENEG_LIMIT} client-initiated session renegotiations // every {CLIENT_RENEG_WINDOW} seconds. An error event is emitted if more @@ -182,7 +182,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { const uri = url.parse(name.slice(4)); uriNames.push(uri.hostname); // TODO(bnoordhuis) Also use scheme. } else if (name.startsWith('IP Address:')) { - ips.push(canonicalIP(name.slice(11))); + ips.push(canonicalizeIP(name.slice(11))); } } } @@ -191,7 +191,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { let reason = 'Unknown reason'; if (net.isIP(host)) { - valid = ips.includes(canonicalIP(host)); + valid = ips.includes(canonicalizeIP(host)); if (!valid) reason = `IP: ${host} is not in the cert's list: ${ips.join(', ')}`; // TODO(bnoordhuis) Also check URI SANs that are IP addresses. diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 5cc9fa79fdc535..36b768d574db1d 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1935,24 +1935,21 @@ void IsIPv6(const FunctionCallbackInfo& args) { } } -void canonicalIP(const FunctionCallbackInfo& args) { - node::Utf8Value ip(args.GetIsolate(), args[0]); +void CanonicalizeIP(const FunctionCallbackInfo& args) { + v8::Isolate *isolate = args.GetIsolate(); + node::Utf8Value ip(isolate, args[0]); char address_buffer[sizeof(struct in6_addr)]; - v8::Isolate* isolate = args.GetIsolate(); + char canonical_ip[INET6_ADDRSTRLEN]; if (uv_inet_pton(AF_INET, *ip, &address_buffer) == 0) { - char canonical_ip[INET_ADDRSTRLEN]; - int err = uv_inet_ntop(AF_INET, address_buffer, canonical_ip, - sizeof(canonical_ip)); + sizeof(canonical_ip)); CHECK_EQ(err, 0); args.GetReturnValue().Set(String::NewFromUtf8(isolate, canonical_ip)); } else if (uv_inet_pton(AF_INET6, *ip, &address_buffer) == 0) { - char canonical_ip[INET6_ADDRSTRLEN]; - int err = uv_inet_ntop(AF_INET6, address_buffer, canonical_ip, - sizeof(canonical_ip)); + sizeof(canonical_ip)); CHECK_EQ(err, 0); args.GetReturnValue().Set(String::NewFromUtf8(isolate, canonical_ip)); @@ -2168,7 +2165,7 @@ void Initialize(Local target, env->SetMethod(target, "isIP", IsIP); env->SetMethod(target, "isIPv4", IsIPv4); env->SetMethod(target, "isIPv6", IsIPv6); - env->SetMethod(target, "canonicalIP", canonicalIP); + env->SetMethod(target, "canonicalizeIP", CanonicalizeIP); env->SetMethod(target, "strerror", StrError); diff --git a/test/parallel/test-tls-canonical-ip.js b/test/parallel/test-tls-canonical-ip.js index 3134548cb5f802..755f271a18ecfe 100644 --- a/test/parallel/test-tls-canonical-ip.js +++ b/test/parallel/test-tls-canonical-ip.js @@ -6,33 +6,26 @@ require('../common'); // of a TLS certificate const assert = require('assert'); -const canonicalIP = process.binding('cares_wrap').canonicalIP; +const canonicalizeIP = process.binding('cares_wrap').canonicalizeIP; -assert.strictEqual(canonicalIP('127.0.0.1'), '127.0.0.1'); -assert.strictEqual(canonicalIP('10.1.0.1'), '10.1.0.1'); -assert.strictEqual(canonicalIP('::1'), '::1'); -assert.strictEqual(canonicalIP('fe80:0:0:0:0:0:0:1'), 'fe80::1'); -assert.strictEqual(canonicalIP('fe80:0:0:0:0:0:0:0'), 'fe80::'); -assert.strictEqual( - canonicalIP('fe80::0000:0010:0001'), - 'fe80::10:1'); -assert.strictEqual( - canonicalIP('0001:2222:3333:4444:5555:6666:7777:0088'), - '1:2222:3333:4444:5555:6666:7777:88'); +assert.strictEqual(canonicalizeIP('127.0.0.1'), '127.0.0.1'); +assert.strictEqual(canonicalizeIP('10.1.0.1'), '10.1.0.1'); +assert.strictEqual(canonicalizeIP('::1'), '::1'); +assert.strictEqual(canonicalizeIP('fe80:0:0:0:0:0:0:1'), 'fe80::1'); +assert.strictEqual(canonicalizeIP('fe80:0:0:0:0:0:0:0'), 'fe80::'); +assert.strictEqual(canonicalizeIP('fe80::0000:0010:0001'), 'fe80::10:1'); +assert.strictEqual(canonicalizeIP('0001:2222:3333:4444:5555:6666:7777:0088'), + '1:2222:3333:4444:5555:6666:7777:88'); -assert.strictEqual( - canonicalIP('0001:2222:3333:4444:5555:6666::'), - '1:2222:3333:4444:5555:6666::'); +assert.strictEqual(canonicalizeIP('0001:2222:3333:4444:5555:6666::'), + '1:2222:3333:4444:5555:6666::'); -assert.strictEqual( - canonicalIP('a002:B12:00Ba:4444:5555:6666:0:0'), - 'a002:b12:ba:4444:5555:6666::'); +assert.strictEqual(canonicalizeIP('a002:B12:00Ba:4444:5555:6666:0:0'), + 'a002:b12:ba:4444:5555:6666::'); // IPv4 address represented in IPv6 -assert.strictEqual( - canonicalIP('0:0:0:0:0:ffff:c0a8:101'), - '::ffff:192.168.1.1'); +assert.strictEqual(canonicalizeIP('0:0:0:0:0:ffff:c0a8:101'), + '::ffff:192.168.1.1'); -assert.strictEqual( - canonicalIP('::ffff:192.168.1.1'), - '::ffff:192.168.1.1'); +assert.strictEqual(canonicalizeIP('::ffff:192.168.1.1'), + '::ffff:192.168.1.1'); From 34ef11ebab19409ffd94790215c349193eb06ab6 Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Fri, 29 Sep 2017 15:22:33 +0200 Subject: [PATCH 13/15] http: address review nits --- lib/_http_agent.js | 2 +- src/cares_wrap.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 87ebe70da7c710..f9fe60c8a79a0c 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -170,7 +170,7 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/, options.servername = hostHeader.substr(1, index - 1); } } else { - options.servername = hostHeader.split(':')[0]; + options.servername = hostHeader.split(':', 1)[0]; } } } diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 36b768d574db1d..112929e6638569 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1936,7 +1936,7 @@ void IsIPv6(const FunctionCallbackInfo& args) { } void CanonicalizeIP(const FunctionCallbackInfo& args) { - v8::Isolate *isolate = args.GetIsolate(); + v8::Isolate* isolate = args.GetIsolate(); node::Utf8Value ip(isolate, args[0]); char address_buffer[sizeof(struct in6_addr)]; char canonical_ip[INET6_ADDRSTRLEN]; From 067600bd754f4bd7c615e615fff10201cd4a28df Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Fri, 13 Oct 2017 17:07:44 +0200 Subject: [PATCH 14/15] http: set servername correctly in createSocket --- lib/_http_agent.js | 55 +++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index f9fe60c8a79a0c..2268dd813323b5 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -153,27 +153,8 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/, if (options.socketPath) options.path = options.socketPath; - if (!options.servername) { - options.servername = options.host; - const hostHeader = req.getHeader('host'); - if (hostHeader) { - // abc => abc - // abc:123 => abc - // [::1] => ::1 - // [::1]:123 => ::1 - if (hostHeader.startsWith('[')) { - const index = hostHeader.indexOf(']'); - if (index === -1) { - // Leading '[', but no ']'. Need to do something... - options.serverName = hostHeader; - } else { - options.servername = hostHeader.substr(1, index - 1); - } - } else { - options.servername = hostHeader.split(':', 1)[0]; - } - } - } + if (!options.servername) + options.servername = calculateServerName(options, req); var name = this.getName(options); if (!this.sockets[name]) { @@ -221,13 +202,8 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { if (options.socketPath) options.path = options.socketPath; - if (!options.servername) { - options.servername = options.host; - const hostHeader = req.getHeader('host'); - if (hostHeader) { - options.servername = hostHeader.replace(/:.*$/, ''); - } - } + if (!options.servername) + options.servername = calculateServerName(options, req); var name = self.getName(options); options._agentKey = name; @@ -255,6 +231,29 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { } }; +function calculateServerName(options, req) { + let servername = options.host; + const hostHeader = req.getHeader('host'); + if (hostHeader) { + // abc => abc + // abc:123 => abc + // [::1] => ::1 + // [::1]:123 => ::1 + if (hostHeader.startsWith('[')) { + const index = hostHeader.indexOf(']'); + if (index === -1) { + // Leading '[', but no ']'. Need to do something... + servername = hostHeader; + } else { + servername = hostHeader.substr(1, index - 1); + } + } else { + servername = hostHeader.split(':', 1)[0]; + } + } + return servername; +} + function installListeners(agent, s, options) { function onFree() { debug('CLIENT socket onFree'); From d5ecf17ddd9723b5f02d1269a9db7553b6127d80 Mon Sep 17 00:00:00 2001 From: Mattias Holmlund Date: Fri, 27 Oct 2017 19:29:37 +0200 Subject: [PATCH 15/15] tls: fix review comments --- node.gyp | 1 - src/cares_wrap.cc | 22 +++++++++++----------- test/parallel/test-tls-canonical-ip.js | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/node.gyp b/node.gyp index 678aa1f86200ef..5ccc4ff423a59a 100644 --- a/node.gyp +++ b/node.gyp @@ -119,7 +119,6 @@ 'lib/internal/readline.js', 'lib/internal/repl.js', 'lib/internal/socket_list.js', - 'lib/internal/tls.js', 'lib/internal/test/unicode.js', 'lib/internal/tls.js', 'lib/internal/url.js', diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 112929e6638569..bf97a0a10e904b 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1941,19 +1941,19 @@ void CanonicalizeIP(const FunctionCallbackInfo& args) { char address_buffer[sizeof(struct in6_addr)]; char canonical_ip[INET6_ADDRSTRLEN]; - if (uv_inet_pton(AF_INET, *ip, &address_buffer) == 0) { - int err = uv_inet_ntop(AF_INET, address_buffer, canonical_ip, - sizeof(canonical_ip)); - CHECK_EQ(err, 0); + int af; + if (uv_inet_pton(AF_INET, *ip, &address_buffer) == 0) + af = AF_INET; + else if (uv_inet_pton(AF_INET6, *ip, &address_buffer) == 0) + af = AF_INET6; + else + return; - args.GetReturnValue().Set(String::NewFromUtf8(isolate, canonical_ip)); - } else if (uv_inet_pton(AF_INET6, *ip, &address_buffer) == 0) { - int err = uv_inet_ntop(AF_INET6, address_buffer, canonical_ip, - sizeof(canonical_ip)); - CHECK_EQ(err, 0); + int err = uv_inet_ntop(af, address_buffer, canonical_ip, + sizeof(canonical_ip)); + CHECK_EQ(err, 0); - args.GetReturnValue().Set(String::NewFromUtf8(isolate, canonical_ip)); - } + args.GetReturnValue().Set(String::NewFromUtf8(isolate, canonical_ip)); } void GetAddrInfo(const FunctionCallbackInfo& args) { diff --git a/test/parallel/test-tls-canonical-ip.js b/test/parallel/test-tls-canonical-ip.js index 755f271a18ecfe..408948119c3501 100644 --- a/test/parallel/test-tls-canonical-ip.js +++ b/test/parallel/test-tls-canonical-ip.js @@ -6,7 +6,7 @@ require('../common'); // of a TLS certificate const assert = require('assert'); -const canonicalizeIP = process.binding('cares_wrap').canonicalizeIP; +const { canonicalizeIP } = process.binding('cares_wrap'); assert.strictEqual(canonicalizeIP('127.0.0.1'), '127.0.0.1'); assert.strictEqual(canonicalizeIP('10.1.0.1'), '10.1.0.1');