tls: veryify server's identity

4 files changed, 306 insertions, 13 deletions

diff --git a/lib/http.js b/lib/http.js
index eccea9944..5e7c0ba16 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -1123,13 +1123,11 @@ Agent.prototype.removeSocket = function(s, name, host, port, localAddress) {
   }
   if (this.requests[name] && this.requests[name].length) {
     // If we have pending requests and a socket gets closed a new one
-    this.createSocket(
-      name,
-      host,
-      port,
-      localAddress,
-      this.requests[name][0]
-    ).emit('free');
+    this.createSocket(name,
+                      host,
+                      port,
+                      localAddress,
+                      this.requests[name][0]).emit('free');
   }
 };
 
@@ -1141,7 +1139,7 @@ function ClientRequest(options, cb) {
   var self = this;
   OutgoingMessage.call(self);
 
-  this.options = options;
+  this.options = util._extend({}, options);
   self.agent = options.agent === undefined ? globalAgent : options.agent;
 
   var defaultPort = options.defaultPort || 80;
diff --git a/lib/tls.js b/lib/tls.js
index 9f3a42b51..640328ec4 100644
--- a/lib/tls.js
+++ b/lib/tls.js
@@ -22,6 +22,7 @@
 var crypto = require('crypto');
 var util = require('util');
 var net = require('net');
+var url = require('url');
 var events = require('events');
 var Stream = require('stream');
 var END_OF_FILE = 42;
@@ -77,6 +78,99 @@ function convertNPNProtocols(NPNProtocols, out) {
   }
 }
 
+
+function checkServerIdentity(host, cert) {
+  // Create regexp to much hostnames
+  function regexpify(host, wildcards) {
+    // Add trailing dot (make hostnames uniform)
+    if (!/\.$/.test(host)) host += '.';
+
+    // Host names with less than one dots are considered too broad,
+    // and should not be allowed
+    if (!/^.+\..+$/.test(host)) return /$./;
+
+    // The same applies to hostname with more than one wildcard,
+    // if hostname has wildcard when wildcards are not allowed,
+    // or if there are less than two dots after wildcard (i.e. *.com or *d.com)
+    if (/\*.*\*/.test(host) || !wildcards && /\*/.test(host) ||
+        /\*/.test(host) && !/\*.*\..+\..+/.test(host)) {
+      return /$./;
+    }
+
+    // Replace wildcard chars with regexp's wildcard and
+    // escape all characters that have special meaning in regexps
+    // (i.e. '.', '[', '{', '*', and others)
+    var re = host.replace(
+      /\*([a-z0-9\\-_\.])|[\.,\-\\\^\$+?*\[\]\(\):!\|{}]/g,
+      function(all, sub) {
+        if (sub) return '[a-z0-9\\-_]*' + (sub === '-' ? '\\-' : sub);
+        return '\\' + all;
+      }
+    );
+
+    return new RegExp('^' + re + '$', 'i');
+  }
+
+  var dnsNames = [],
+      uriNames = [],
+      ips = [],
+      valid = false;
+
+  // There're several names to perform check against:
+  // CN and altnames in certificate extension
+  // (DNS names, IP addresses, and URIs)
+  //
+  // Walk through altnames and generate lists of those names
+  if (cert.subjectaltname) {
+    cert.subjectaltname.split(/, /g).forEach(function(altname) {
+      if (/^DNS:/.test(altname)) {
+        dnsNames.push(altname.slice(4));
+      } else if (/^IP Address:/.test(altname)) {
+        ips.push(altname.slice(11));
+      } else if (/^URI:/.test(altname)) {
+        var uri = url.parse(altname.slice(4));
+        if (uri) uriNames.push(uri.hostname);
+      }
+    });
+  }
+
+  // If hostname is an IP address, it should be present in the list of IP
+  // addresses.
+  if (net.isIP(host)) {
+    valid = ips.some(function(ip) {
+      return ip === host;
+    });
+  } else {
+    // Transform hostname to canonical form
+    if (!/\.$/.test(host)) host += '.';
+
+    // Otherwise check all DNS/URI records from certificate
+    // (with allowed wildcards)
+    dnsNames = dnsNames.map(function(name) {
+      return regexpify(name, true);
+    });
+
+    // Wildcards ain't allowed in URI names
+    uriNames = uriNames.map(function(name) {
+      return regexpify(name, false);
+    });
+
+    dnsNames = dnsNames.concat(uriNames);
+
+    // And only after check if hostname matches CN
+    // (because CN is deprecated, but should be used for compatiblity anyway)
+    dnsNames.push(regexpify(cert.subject.CN, false));
+
+    valid = dnsNames.some(function(re) {
+      return re.test(host);
+    });
+  }
+
+  return valid;
+};
+exports.checkServerIdentity = checkServerIdentity;
+
+
 // Base class of both CleartextStream and EncryptedStream
 function CryptoStream(pair) {
   Stream.call(this);
@@ -1118,11 +1212,12 @@ exports.connect = function(/* [port, host], options, cb */) {
   var sslcontext = crypto.createCredentials(options);
 
   convertNPNProtocols(options.NPNProtocols, this);
-  var pair = new SecurePair(sslcontext, false, true,
+  var hostname = options.servername || options.host,
+      pair = new SecurePair(sslcontext, false, true,
                             options.rejectUnauthorized === true ? true : false,
                             {
                               NPNProtocols: this.NPNProtocols,
-                              servername: options.servername || options.host
+                              servername: hostname
                             });
 
   if (options.session) {
@@ -1147,9 +1242,19 @@ exports.connect = function(/* [port, host], options, cb */) {
 
     cleartext.npnProtocol = pair.npnProtocol;
 
+    // Verify that server's identity matches it's certificate's names
+    if (!verifyError) {
+      var validCert = checkServerIdentity(hostname,
+                                          pair.cleartext.getPeerCertificate());
+      if (!validCert) {
+        verifyError = new Error('Hostname/IP doesn\'t match certificate\'s ' +
+                                'altnames');
+      }
+    }
+
     if (verifyError) {
       cleartext.authorized = false;
-      cleartext.authorizationError = verifyError;
+      cleartext.authorizationError = verifyError.message;
 
       if (pair._rejectUnauthorized) {
         cleartext.emit('error', verifyError);
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 8b21825a2..5b21e55a8 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -1547,7 +1547,8 @@ Handle<Value> Connection::VerifyError(const Arguments& args) {
     // We requested a certificate and they did not send us one.
     // Definitely an error.
     // XXX is this the right error message?
-    return scope.Close(String::New("UNABLE_TO_GET_ISSUER_CERT"));
+    return scope.Close(Exception::Error(
+          String::New("UNABLE_TO_GET_ISSUER_CERT")));
   }
   X509_free(peer_cert);
 
@@ -1673,7 +1674,7 @@ Handle<Value> Connection::VerifyError(const Arguments& args) {
       break;
   }
 
-  return scope.Close(s);
+  return scope.Close(Exception::Error(s));
 }
 
 
diff --git a/test/simple/test-tls-check-server-identity.js b/test/simple/test-tls-check-server-identity.js
new file mode 100644
index 000000000..f79823b84
--- /dev/null
+++ b/test/simple/test-tls-check-server-identity.js
@@ -0,0 +1,189 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var util = require('util');
+var tls = require('tls');
+
+var tests = [
+  // Basic CN handling
+  { host: 'a.com', cert: { subject: { CN: 'a.com' } }, result: true },
+  { host: 'a.com', cert: { subject: { CN: 'A.COM' } }, result: true },
+  { host: 'a.com', cert: { subject: { CN: 'b.com' } }, result: false },
+  { host: 'a.com', cert: { subject: { CN: 'a.com.' } }, result: true },
+
+  // No wildcards in CN
+  { host: 'b.a.com', cert: { subject: { CN: '*.a.com' } }, result: false },
+
+  // DNS names and CN
+  {
+    host: 'a.com', cert: {
+      subjectaltname: 'DNS:*',
+      subject: { CN: 'b.com' }
+    },
+    result: false
+  },
+  {
+    host: 'a.com', cert: {
+      subjectaltname: 'DNS:*.com',
+      subject: { CN: 'b.com' }
+    },
+    result: false
+  },
+  {
+    host: 'a.co.uk', cert: {
+      subjectaltname: 'DNS:*.co.uk',
+      subject: { CN: 'b.com' }
+    },
+    result: true
+  },
+  {
+    host: 'a.com', cert: {
+      subjectaltname: 'DNS:*.a.com',
+      subject: { CN: 'a.com' }
+    },
+    result: true
+  },
+  {
+    host: 'a.com', cert: {
+      subjectaltname: 'DNS:*.a.com',
+      subject: { CN: 'b.com' }
+    },
+    result: false
+  },
+  {
+    host: 'a.com', cert: {
+      subjectaltname: 'DNS:a.com',
+      subject: { CN: 'b.com' }
+    },
+    result: true
+  },
+  {
+    host: 'a.com', cert: {
+      subjectaltname: 'DNS:A.COM',
+      subject: { CN: 'b.com' }
+    },
+    result: true
+  },
+
+  // DNS names
+  {
+    host: 'a.com', cert: {
+      subjectaltname: 'DNS:*.a.com',
+      subject: {}
+    },
+    result: false
+  },
+  {
+    host: 'b.a.com', cert: {
+      subjectaltname: 'DNS:*.a.com',
+      subject: {}
+    },
+    result: true
+  },
+  {
+    host: 'c.b.a.com', cert: {
+      subjectaltname: 'DNS:*.a.com',
+      subject: {}
+    },
+    result: false
+  },
+  {
+    host: 'b.a.com', cert: {
+      subjectaltname: 'DNS:*b.a.com',
+      subject: {}
+    },
+    result: true
+  },
+  {
+    host: 'a-cb.a.com', cert: {
+      subjectaltname: 'DNS:*b.a.com',
+      subject: {}
+    },
+    result: true
+  },
+  {
+    host: 'a.b.a.com', cert: {
+      subjectaltname: 'DNS:*b.a.com',
+      subject: {}
+    },
+    result: false
+  },
+  // Mutliple DNS names
+  {
+    host: 'a.b.a.com', cert: {
+      subjectaltname: 'DNS:*b.a.com, DNS:a.b.a.com',
+      subject: {}
+    },
+    result: true
+  },
+  // URI names
+  {
+    host: 'a.b.a.com', cert: {
+      subjectaltname: 'URI:http://a.b.a.com/',
+      subject: {}
+    },
+    result: true
+  },
+  {
+    host: 'a.b.a.com', cert: {
+      subjectaltname: 'URI:http://*.b.a.com/',
+      subject: {}
+    },
+    result: false
+  },
+  // IP addresses
+  {
+    host: 'a.b.a.com', cert: {
+      subjectaltname: 'IP Address:127.0.0.1',
+      subject: {}
+    },
+    result: false
+  },
+  {
+    host: '127.0.0.1', cert: {
+      subjectaltname: 'IP Address:127.0.0.1',
+      subject: {}
+    },
+    result: true
+  },
+  {
+    host: '127.0.0.2', cert: {
+      subjectaltname: 'IP Address:127.0.0.1',
+      subject: {}
+    },
+    result: false
+  },
+  {
+    host: '127.0.0.1', cert: {
+      subjectaltname: 'DNS:a.com',
+      subject: {}
+    },
+    result: false
+  },
+];
+
+tests.forEach(function(test, i) {
+  assert.equal(tls.checkServerIdentity(test.host, test.cert),
+               test.result,
+               'Test#' + i + ' failed: ' + util.inspect(test));
+});