Permalink
Browse files

tls: follow RFC6125 more stricly

* Allow wildcards only in left-most part of hostname identifier.
* Do not match CN if altnames are present
  • Loading branch information...
1 parent 4dd70bb commit add5e12ab3bc940d73dd46eecbc9d6cf0a5ed6ea @indutny committed Jan 14, 2013
Showing with 36 additions and 12 deletions.
  1. +27 −8 lib/tls.js
  2. +9 −4 test/simple/test-tls-check-server-identity.js
View
@@ -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) {
@@ -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
},
];

0 comments on commit add5e12

Please sign in to comment.