tls: follow RFC6125 more stricly

indutny · isaacs · commit b4b750b6a52e · 2013-01-14T17:18:30.000-08:00
* Allow wildcards only in left-most part of hostname identifier.
* Do not match CN if altnames are present
diff --git 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
@@ -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
   },
 ];
 
