tls: better error reporting at cert validation

fix #7417

Signed-off-by: Fedor Indutny <fedor@indutny.com>

diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index 5a921f7..e3ddb49 100644 (file)
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -910,11 +910,7 @@ exports.connect = function(/* [port, host], options, cb */) {
       // Verify that server's identity matches it's certificate's names
       if (!verifyError) {
         var cert = result.getPeerCertificate();
-        var validCert = tls.checkServerIdentity(hostname, cert);
-        if (!validCert) {
-          verifyError = new Error('Hostname/IP doesn\'t match certificate\'s ' +
-                                  'altnames');
-        }
+        verifyError = tls.checkServerIdentity(hostname, cert);
       }
 
       if (verifyError) {

diff --git a/lib/tls.js b/lib/tls.js
index 9345632..b698cc1 100644 (file)
--- a/lib/tls.js
+++ b/lib/tls.js
@@ -113,7 +113,8 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) {
       uriNames = [],
       ips = [],
       matchCN = true,
-      valid = false;
+      valid = false,
+      reason = 'Unknown reason';
 
   // There're several names to perform check against:
   // CN and altnames in certificate extension
@@ -142,6 +143,11 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) {
     valid = ips.some(function(ip) {
       return ip === host;
     });
+    if (!valid) {
+      reason = util.format('IP: %s is not in the cert\'s list: %s',
+                           host,
+                           ips.join(', '));
+    }
   } else {
     // Transform hostname to canonical form
     if (!/\.$/.test(host)) host += '.';
@@ -183,9 +189,29 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) {
     valid = dnsNames.some(function(re) {
       return re.test(host);
     });
+
+    if (!valid) {
+      if (cert.subjectaltname) {
+        reason = util.format('Host: %s is not in the cert\'s altnames: %s',
+                             host,
+                             cert.subjectaltname);
+      } else {
+        reason = util.format('Host: %s is not cert\'s CN: %s',
+                             host,
+                             cert.subject.CN);
+      }
+    }
   }
 
-  return valid;
+  if (!valid) {
+    var err = new Error(
+      util.format('Hostname/IP doesn\'t match certificate\'s altnames: %j',
+                  reason));
+    err.reason = reason;
+    err.host = host;
+    err.cert = cert;
+    return err;
+  }
 };
 
 // Example:

diff --git a/test/simple/test-https-strict.js b/test/simple/test-https-strict.js
index 720b0a6..9cd763f 100644 (file)
--- a/test/simple/test-https-strict.js
+++ b/test/simple/test-https-strict.js
@@ -180,10 +180,12 @@ function allListening() {
   // server1: host 'agent1', signed by ca1
   makeReq('/inv1', port1, 'UNABLE_TO_VERIFY_LEAF_SIGNATURE');
   makeReq('/inv1-ca1', port1,
-          'Hostname/IP doesn\'t match certificate\'s altnames',
+          'Hostname/IP doesn\'t match certificate\'s altnames: ' +
+              '"Host: localhost. is not cert\'s CN: agent1"',
           null, ca1);
   makeReq('/inv1-ca1ca2', port1,
-          'Hostname/IP doesn\'t match certificate\'s altnames',
+          'Hostname/IP doesn\'t match certificate\'s altnames: ' +
+              '"Host: localhost. is not cert\'s CN: agent1"',
           null, [ca1, ca2]);
   makeReq('/val1-ca1', port1, null, 'agent1', ca1);
   makeReq('/val1-ca1ca2', port1, null, 'agent1', [ca1, ca2]);
@@ -201,10 +203,12 @@ function allListening() {
   // server3: host 'agent3', signed by ca2
   makeReq('/inv3', port3, 'UNABLE_TO_VERIFY_LEAF_SIGNATURE');
   makeReq('/inv3-ca2', port3,
-          'Hostname/IP doesn\'t match certificate\'s altnames',
+          'Hostname/IP doesn\'t match certificate\'s altnames: ' +
+              '"Host: localhost. is not cert\'s CN: agent3"',
           null, ca2);
   makeReq('/inv3-ca1ca2', port3,
-          'Hostname/IP doesn\'t match certificate\'s altnames',
+          'Hostname/IP doesn\'t match certificate\'s altnames: ' +
+              '"Host: localhost. is not cert\'s CN: agent3"',
           null, [ca1, ca2]);
   makeReq('/val3-ca2', port3, null, 'agent3', ca2);
   makeReq('/val3-ca1ca2', port3, null, 'agent3', [ca1, ca2]);

diff --git a/test/simple/test-tls-check-server-identity.js b/test/simple/test-tls-check-server-identity.js
index 487f162..598dac0 100644 (file)
--- a/test/simple/test-tls-check-server-identity.js
+++ b/test/simple/test-tls-check-server-identity.js
@@ -26,25 +26,29 @@ 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 },
+  { host: 'a.com', cert: { subject: { CN: 'a.com' } } },
+  { host: 'a.com', cert: { subject: { CN: 'A.COM' } } },
+  {
+    host: 'a.com',
+    cert: { subject: { CN: 'b.com' } },
+    error: 'Host: a.com. is not cert\'s CN: b.com'
+  },
+  { host: 'a.com', cert: { subject: { CN: 'a.com.' } } },
 
   // Wildcards in CN
-  { host: 'b.a.com', cert: { subject: { CN: '*.a.com' } }, result: true },
+  { host: 'b.a.com', cert: { subject: { CN: '*.a.com' } } },
   { host: 'b.a.com', cert: {
     subjectaltname: 'DNS:omg.com',
     subject: { CN: '*.a.com' } },
-    result: false
+    error: 'Host: b.a.com. is not in the cert\'s altnames: ' +
+           'DNS:omg.com'
   },
 
   // Multiple CN fields
   {
     host: 'foo.com', cert: {
       subject: { CN: ['foo.com', 'bar.com'] } // CN=foo.com; CN=bar.com;
-    },
-    result: true
+    }
   },
 
   // DNS names and CN
@@ -53,49 +57,50 @@ var tests = [
       subjectaltname: 'DNS:*',
       subject: { CN: 'b.com' }
     },
-    result: false
+    error: 'Host: a.com. is not in the cert\'s altnames: ' +
+           'DNS:*'
   },
   {
     host: 'a.com', cert: {
       subjectaltname: 'DNS:*.com',
       subject: { CN: 'b.com' }
     },
-    result: false
+    error: 'Host: a.com. is not in the cert\'s altnames: ' +
+           'DNS:*.com'
   },
   {
     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: false
+    error: 'Host: a.com. is not in the cert\'s altnames: ' +
+           'DNS:*.a.com'
   },
   {
     host: 'a.com', cert: {
       subjectaltname: 'DNS:*.a.com',
       subject: { CN: 'b.com' }
     },
-    result: false
+    error: 'Host: a.com. is not in the cert\'s altnames: ' +
+           'DNS:*.a.com'
   },
   {
     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
@@ -104,65 +109,64 @@ var tests = [
       subjectaltname: 'DNS:*.a.com',
       subject: {}
     },
-    result: false
+    error: 'Host: a.com. is not in the cert\'s altnames: ' +
+           'DNS:*.a.com'
   },
   {
     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
+    error: 'Host: c.b.a.com. is not in the cert\'s altnames: ' +
+           'DNS:*.a.com'
   },
   {
     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
+    error: 'Host: a.b.a.com. is not in the cert\'s altnames: ' +
+           'DNS:*b.a.com'
   },
   // 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
+    error: 'Host: a.b.a.com. is not in the cert\'s altnames: ' +
+           'URI:http://*.b.a.com/'
   },
   // IP addresses
   {
@@ -170,40 +174,44 @@ var tests = [
       subjectaltname: 'IP Address:127.0.0.1',
       subject: {}
     },
-    result: false
+    error: 'Host: a.b.a.com. is not in the cert\'s altnames: ' +
+           'IP Address:127.0.0.1'
   },
   {
     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
+    error: 'IP: 127.0.0.2 is not in the cert\'s list: ' +
+           '127.0.0.1'
   },
   {
     host: '127.0.0.1', cert: {
       subjectaltname: 'DNS:a.com',
       subject: {}
     },
-    result: false
+    error: 'IP: 127.0.0.1 is not in the cert\'s list: '
   },
   {
     host: 'localhost', cert: {
       subjectaltname: 'DNS:a.com',
       subject: { CN: 'localhost' }
     },
-    result: false
+    error: 'Host: localhost. is not in the cert\'s altnames: ' +
+           'DNS:a.com'
   },
 ];
 
 tests.forEach(function(test, i) {
-  assert.equal(tls.checkServerIdentity(test.host, test.cert),
-               test.result,
-               'Test#' + i + ' failed: ' + util.inspect(test));
+  var err = tls.checkServerIdentity(test.host, test.cert);
+  assert.equal(err && err.reason,
+               test.error,
+               'Test#' + i + ' failed: ' + util.inspect(test) + '\n' +
+               test.error + ' != ' + (err && err.reason));
 });

diff --git a/test/simple/test-tls-client-verify.js b/test/simple/test-tls-client-verify.js
index cc4e215..590dfc6 100644 (file)
--- a/test/simple/test-tls-client-verify.js
+++ b/test/simple/test-tls-client-verify.js
@@ -25,7 +25,7 @@ if (!process.versions.openssl) {
 }
 
 
-var hosterr = 'Hostname/IP doesn\'t match certificate\'s altnames';
+var hosterr = /Hostname\/IP doesn\'t match certificate\'s altnames/g;
 var testCases =
     [{ ca: ['ca1-cert'],
        key: 'agent2-key',
@@ -103,7 +103,7 @@ function testServers(index, servers, clientOptions, cb) {
     console.error('connecting...');
     var client = tls.connect(clientOptions, function() {
       var authorized = client.authorized ||
-                       client.authorizationError === hosterr;
+                       hosterr.test(client.authorizationError);
 
       console.error('expected: ' + ok + ' authed: ' + authorized);