From e34525356201654bfe35b4bb7edec6f5b47b6374 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 14 Apr 2014 20:08:38 +0400 Subject: [PATCH] tls: better error reporting at cert validation fix #7417 Signed-off-by: Fedor Indutny --- lib/_tls_wrap.js | 6 +- lib/tls.js | 30 ++++++- test/simple/test-https-strict.js | 12 ++- test/simple/test-tls-check-server-identity.js | 90 ++++++++++--------- test/simple/test-tls-client-verify.js | 4 +- 5 files changed, 88 insertions(+), 54 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 5a921f750a4..e3ddb49fb87 100644 --- 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 9345632353b..b698cc11e9d 100644 --- 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 720b0a6db74..9cd763f7ae3 100644 --- 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 487f16292a6..598dac0e00b 100644 --- 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 cc4e21572e8..590dfc6e63c 100644 --- 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);