lukechilds
/
node
mirror of https://github.com/lukechilds/node.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

lib: make tls.checkServerIdentity() more strict 

Incorporates changes from commit e345253 ("tls: better error reporting
at cert validation") to test/simple/test-tls-check-server-identity.js
to make back-porting the patch easier.

CVE-2016-7099

PR-URL: https://github.com/nodejs/node-private/pull/62
Reviewed-By: Rod Vagg <rod@vagg.org>
v0.10
Ben Noordhuis 9 years ago
committed by Rod Vagg
parent
3614a173d0
commit
0d7e21ee7b
2 changed files with 248 additions and 138 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 238
      lib/tls.js
  2. 148
      test/simple/test-tls-check-server-identity.js

238
lib/tls.js
View File

@ -96,115 +96,163 @@ function convertNPNProtocols(NPNProtocols, out) {
}
}
function unfqdn(host) {
return host.replace(/[.]$/, '');
}
function checkServerIdentity(host, cert) {
// Create regexp to much hostnames
function regexpify(host, wildcards) {
// Add trailing dot (make hostnames uniform)
if (!/\.$/.test(host)) host += '.';
// 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)
//
// also
//
// "The client SHOULD NOT attempt to match a presented identifier in
// which the wildcard character comprises a label other than the
// left-most label (e.g., do not match bar.*.example.net)."
// RFC6125
if (!wildcards && /\*/.test(host) || /[\.\*].*\*/.test(host) ||
/\*/.test(host) && !/\*.*\..+\..+/.test(host)) {
return /$./;
}
function splitHost(host) {
// String#toLowerCase() is locale-sensitive so we use
// a conservative version that only lowercases A-Z.
function replacer(c) {
return String.fromCharCode(32 + c.charCodeAt(0));
};
return unfqdn(host).replace(/[A-Z]/g, replacer).split('.');
}
// 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');
function check(hostParts, pattern, wildcards) {
// Empty strings, null, undefined, etc. never match.
if (!pattern)
return false;
var patternParts = splitHost(pattern);
if (hostParts.length !== patternParts.length)
return false;
// Pattern has empty components, e.g. "bad..example.com".
if (patternParts.indexOf('') !== -1)
return false;
// RFC 6125 allows IDNA U-labels (Unicode) in names but we have no
// good way to detect their encoding or normalize them so we simply
// reject them. Control characters and blanks are rejected as well
// because nothing good can come from accepting them.
function isBad(s) {
return /[^\u0021-\u007F]/.test(s);
}
var dnsNames = [],
uriNames = [],
ips = [],
matchCN = true,
valid = false;
if (patternParts.some(isBad))
return 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);
// Check host parts from right to left first.
for (var i = hostParts.length - 1; i > 0; i -= 1)
if (hostParts[i] !== patternParts[i])
return false;
var hostSubdomain = hostParts[0];
var patternSubdomain = patternParts[0];
var patternSubdomainParts = patternSubdomain.split('*');
// Short-circuit when the subdomain does not contain a wildcard.
// RFC 6125 does not allow wildcard substitution for components
// containing IDNA A-labels (Punycode) so match those verbatim.
if (patternSubdomainParts.length === 1 ||
patternSubdomain.indexOf('xn--') !== -1) {
return hostSubdomain === patternSubdomain;
}
if (!wildcards)
return false;
// More than one wildcard is always wrong.
if (patternSubdomainParts.length > 2)
return false;
// *.tld wildcards are not allowed.
if (patternParts.length <= 2)
return false;
var prefix = patternSubdomainParts[0];
var suffix = patternSubdomainParts[1];
if (prefix.length + suffix.length > hostSubdomain.length)
return false;
if (prefix.length > 0 && hostSubdomain.slice(0, prefix.length) !== prefix)
return false;
if (suffix.length > 0 && hostSubdomain.slice(-suffix.length) !== suffix)
return false;
return true;
}
function _checkServerIdentity(host, cert) {
var subject = cert.subject;
var altNames = cert.subjectaltname;
var dnsNames = [];
var uriNames = [];
var ips = [];
host = '' + host;
if (altNames) {
altNames.split(', ').forEach(function(name) {
if (/^DNS:/.test(name)) {
dnsNames.push(name.slice(4));
} else if (/^URI:/.test(name)) {
var uri = url.parse(name.slice(4));
uriNames.push(uri.hostname); // TODO(bnoordhuis) Also use scheme.
} else if (/^IP Address:/.test(name)) {
ips.push(name.slice(11));
}
});
}
// If hostname is an IP address, it should be present in the list of IP
// addresses.
var valid = false;
var reason = 'Unknown reason';
if (net.isIP(host)) {
valid = ips.some(function(ip) {
return ip === host;
});
} else {
// Transform hostname to canonical form
if (!/\.$/.test(host)) host += '.';
valid = ips.indexOf(host) !== -1;
if (!valid)
reason = 'IP: ' + host + ' is not in the cert\'s list: ' + ips.join(', ');
// TODO(bnoordhuis) Also check URI SANs that are IP addresses.
} else if (subject) {
host = unfqdn(host); // Remove trailing dot for error messages.
var hostParts = splitHost(host);
function wildcard(pattern) {
return check(hostParts, pattern, true);
}
// Otherwise check all DNS/URI records from certificate
// (with allowed wildcards)
dnsNames = dnsNames.map(function(name) {
return regexpify(name, true);
});
function noWildcard(pattern) {
return check(hostParts, pattern, false);
}
// Wildcards ain't allowed in URI names
uriNames = uriNames.map(function(name) {
return regexpify(name, false);
});
// Match against Common Name only if no supported identifiers are present.
if (dnsNames.length === 0 && ips.length === 0 && uriNames.length === 0) {
var cn = subject.CN;
dnsNames = dnsNames.concat(uriNames);
if (dnsNames.length > 0) matchCN = false;
// Match against Common Name (CN) only if no supported identifiers are
// present.
//
// "As noted, a client MUST NOT seek a match for a reference identifier
// of CN-ID if the presented identifiers include a DNS-ID, SRV-ID,
// URI-ID, or any application-specific identifier types supported by the
// client."
// RFC6125
if (matchCN) {
var commonNames = cert.subject.CN;
if (Array.isArray(commonNames)) {
for (var i = 0, k = commonNames.length; i < k; ++i) {
dnsNames.push(regexpify(commonNames[i], true));
}
} else {
dnsNames.push(regexpify(commonNames, true));
if (Array.isArray(cn))
valid = cn.some(wildcard);
else if (cn)
valid = wildcard(cn);
if (!valid)
reason = 'Host: ' + host + '. is not cert\'s CN: ' + cn;
} else {
valid = dnsNames.some(wildcard) || uriNames.some(noWildcard);
if (!valid) {
reason =
'Host: ' + host + '. is not in the cert\'s altnames: ' + altNames;
}
}
} else {
reason = 'Cert is empty';
}
valid = dnsNames.some(function(re) {
return re.test(host);
});
if (!valid) {
var err = new Error('Hostname/IP doesn\'t match certificate\'s altnames');
err.reason = reason;
err.host = host;
err.cert = cert;
return err;
}
}
exports._checkServerIdentity = _checkServerIdentity;
return valid;
function checkServerIdentity(host, cert) {
return !!_checkServerIdentity(host, cert);
}
exports.checkServerIdentity = checkServerIdentity;
@ -1384,12 +1432,8 @@ exports.connect = function(/* [port, host], options, cb */) {
// 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');
}
verifyError = _checkServerIdentity(hostname,
pair.cleartext.getPeerCertificate());
}
if (verifyError) {

148
test/simple/test-tls-check-server-identity.js
View File

@ -25,26 +25,74 @@ var util = require('util');
var tls = require('tls');
var tests = [
// False-y values.
{
host: false,
cert: { subject: { CN: 'a.com' } },
error: 'Host: false. is not cert\'s CN: a.com'
},
{
host: null,
cert: { subject: { CN: 'a.com' } },
error: 'Host: null. is not cert\'s CN: a.com'
},
{
host: undefined,
cert: { subject: { CN: 'a.com' } },
error: 'Host: undefined. is not cert\'s CN: a.com'
},
// 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.' } } },
{
host: 'a.com',
cert: { subject: { CN: '.a.com' } },
error: 'Host: a.com. is not cert\'s 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: 'ba.com',
cert: { subject: { CN: '*.a.com' } },
error: 'Host: ba.com. is not cert\'s CN: *.a.com'
},
{
host: '\n.b.com',
cert: { subject: { CN: '*n.b.com' } },
error: 'Host: \n.b.com. is not cert\'s CN: *n.b.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'
},
{
host: 'b.a.com',
cert: { subject: { CN: 'b*b.a.com' } },
error: 'Host: b.a.com. is not cert\'s CN: b*b.a.com'
},
// Empty Cert
{
host: 'a.com',
cert: { },
error: 'Cert is empty'
},
// 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 +101,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 +153,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 +218,58 @@ 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'
},
// IDNA
{
host: 'xn--bcher-kva.example.com',
cert: { subject: { CN: '*.example.com' } },
},
// RFC 6125, section 6.4.3: "[...] the client SHOULD NOT attempt to match
// a presented identifier where the wildcard character is embedded within
// an A-label [...]"
{
host: 'xn--bcher-kva.example.com',
cert: { subject: { CN: 'xn--*.example.com' } },
error: 'Host: xn--bcher-kva.example.com. is not cert\'s CN: ' +
'xn--*.example.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));
});

Write Preview
Loading…
Cancel
Save