http, tls: better support for IPv6 addresses

- Properly handle IPv6 in Host header when setting servername.
- When comparing IP addresses against addresses in the subjectAltName
  field of a certificate, format the address correctly before
  doing the string comparison.

PR-URL: nodejs#14772
Fixes: nodejs#14736
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: mattiasholmlund

lib/_http_agent.js

@@ -151,13 +151,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) {
-      options.servername = hostHeader.replace(/:.*$/, '');
-    }
-  }
+  if (!options.servername)
+    options.servername = calculateServerName(options, req);
 
   var name = this.getName(options);
   if (!this.sockets[name]) {
@@ -204,13 +199,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 = this.getName(options);
   options._agentKey = name;
@@ -239,6 +229,29 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) {
     oncreate(null, newSocket);
 };
 
+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');

lib/tls.js

@@ -29,6 +29,7 @@ const net = require('net');
 const url = require('url');
 const binding = process.binding('crypto');
 const Buffer = require('buffer').Buffer;
+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
@@ -179,7 +180,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(canonicalizeIP(name.slice(11)));
       }
     }
   }
@@ -188,7 +189,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) {
   let reason = 'Unknown reason';
 
   if (net.isIP(host)) {
-    valid = ips.includes(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.

src/cares_wrap.cc

@@ -1943,6 +1943,27 @@ void IsIPv6(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+void CanonicalizeIP(const FunctionCallbackInfo<Value>& args) {
+  v8::Isolate* isolate = args.GetIsolate();
+  node::Utf8Value ip(isolate, args[0]);
+  char address_buffer[sizeof(struct in6_addr)];
+  char canonical_ip[INET6_ADDRSTRLEN];
+
+  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;
+
+  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));
+}
+
 void GetAddrInfo(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
@@ -2163,6 +2184,7 @@ void Initialize(Local<Object> target,
   env->SetMethod(target, "isIP", IsIP);
   env->SetMethod(target, "isIPv4", IsIPv4);
   env->SetMethod(target, "isIPv6", IsIPv6);
+  env->SetMethod(target, "canonicalizeIP", CanonicalizeIP);
 
   env->SetMethod(target, "strerror", StrError);
 

test/parallel/test-tls-canonical-ip.js

@@ -0,0 +1,31 @@
+'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 { 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');
+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(canonicalizeIP('0001:2222:3333:4444:5555:6666::'),
+                   '1:2222:3333: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(canonicalizeIP('0:0:0:0:0:ffff:c0a8:101'),
+                   '::ffff:192.168.1.1');
+
+assert.strictEqual(canonicalizeIP('::ffff:192.168.1.1'),
+                   '::ffff:192.168.1.1');