Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

tls: veryify server's identity

  • Loading branch information...
commit 8ba189b8d32e3c6489e142101d3e37dcbc551179 1 parent 2b3ba3f
@indutny authored
View
14 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;
View
110 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);
View
5 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));
}
View
189 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));
+});
Please sign in to comment.
Something went wrong with that request. Please try again.