diff options
| -rw-r--r-- | lib/http.js | 14 | ||||
| -rw-r--r-- | lib/tls.js | 110 | ||||
| -rw-r--r-- | src/node_crypto.cc | 5 | ||||
| -rw-r--r-- | test/simple/test-tls-check-server-identity.js | 189 |
4 files changed, 305 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 5b8f179ba..be24283f4 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; @@ -78,6 +79,98 @@ 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; + + function SlabBuffer() { this.create(); } @@ -1149,11 +1242,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) { @@ -1178,9 +1272,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)); +}); |