From b4b750b6a52ebb893c9c8af93bdaca5238236547 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 15 Jan 2013 03:29:46 +0400 Subject: [PATCH] tls: follow RFC6125 more stricly * Allow wildcards only in left-most part of hostname identifier. * Do not match CN if altnames are present --- lib/tls.js | 35 +++++++++++++++++++++------ test/simple/test-tls-check-server-identity.js | 13 +++++++--- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/lib/tls.js b/lib/tls.js index faacb09..0222fa9 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -91,7 +91,14 @@ function checkServerIdentity(host, cert) { // 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) || + // + // 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 /$./; } @@ -112,6 +119,7 @@ function checkServerIdentity(host, cert) { var dnsNames = [], uriNames = [], ips = [], + matchCN = true, valid = false; // There're several names to perform check against: @@ -120,6 +128,7 @@ function checkServerIdentity(host, cert) { // // Walk through altnames and generate lists of those names if (cert.subjectaltname) { + matchCN = false; cert.subjectaltname.split(/, /g).forEach(function(altname) { if (/^DNS:/.test(altname)) { dnsNames.push(altname.slice(4)); @@ -155,14 +164,24 @@ function checkServerIdentity(host, cert) { dnsNames = dnsNames.concat(uriNames); - // And only after check if hostname matches CN - 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)); + if (dnsNames.length > 0) matchCN = false; + + // Match against Common Name (CN) only if altnames are not 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)); } - } else { - dnsNames.push(regexpify(commonNames, true)); } valid = dnsNames.some(function(re) { diff --git a/test/simple/test-tls-check-server-identity.js b/test/simple/test-tls-check-server-identity.js index 39093a7..487f162 100644 --- a/test/simple/test-tls-check-server-identity.js +++ b/test/simple/test-tls-check-server-identity.js @@ -31,8 +31,13 @@ var tests = [ { 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 }, + // Wildcards in CN + { host: 'b.a.com', cert: { subject: { CN: '*.a.com' } }, result: true }, + { host: 'b.a.com', cert: { + subjectaltname: 'DNS:omg.com', + subject: { CN: '*.a.com' } }, + result: false + }, // Multiple CN fields { @@ -69,7 +74,7 @@ var tests = [ subjectaltname: 'DNS:*.a.com', subject: { CN: 'a.com' } }, - result: true + result: false }, { host: 'a.com', cert: { @@ -193,7 +198,7 @@ var tests = [ subjectaltname: 'DNS:a.com', subject: { CN: 'localhost' } }, - result: true + result: false }, ]; -- 2.7.4