From 43cec65d6f4409e4ffb1596904e94a95b07961b9 Mon Sep 17 00:00:00 2001
From: Jason Macgowan <jason.macgowan@icloud.com>
Date: Mon, 17 Sep 2018 11:31:24 -0400
Subject: [PATCH] tls: allow empty subject even with altNames defined

Behavior described in https://github.com/nodejs/node/issues/11771
is still true even though the issue is closed.

This PR is to allow DNS and URI names, even when there is not a subject.

Refs: https://github.com/nodejs/node/issues/11771

PR-URL: https://github.com/nodejs/node/pull/22906
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/tls.js                                    | 24 +++++++++++--------
 .../test-tls-check-server-identity.js         | 14 +++++++++++
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/lib/tls.js b/lib/tls.js
index 5372aadccd686d..c63c87a5dea6ae 100644
--- a/lib/tls.js
+++ b/lib/tls.js
@@ -243,19 +243,28 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
   let valid = false;
   let reason = 'Unknown reason';
 
+  const hasAltNames =
+    dnsNames.length > 0 || ips.length > 0 || uriNames.length > 0;
+
+  hostname = unfqdn(hostname);  // Remove trailing dot for error messages.
+
   if (net.isIP(hostname)) {
     valid = ips.includes(canonicalizeIP(hostname));
     if (!valid)
       reason = `IP: ${hostname} is not in the cert's list: ${ips.join(', ')}`;
     // TODO(bnoordhuis) Also check URI SANs that are IP addresses.
-  } else if (subject) {
-    hostname = unfqdn(hostname);  // Remove trailing dot for error messages.
+  } else if (hasAltNames || subject) {
     const hostParts = splitHost(hostname);
     const wildcard = (pattern) => check(hostParts, pattern, true);
-    const noWildcard = (pattern) => check(hostParts, pattern, false);
 
-    // Match against Common Name only if no supported identifiers are present.
-    if (dnsNames.length === 0 && ips.length === 0 && uriNames.length === 0) {
+    if (hasAltNames) {
+      const noWildcard = (pattern) => check(hostParts, pattern, false);
+      valid = dnsNames.some(wildcard) || uriNames.some(noWildcard);
+      if (!valid)
+        reason =
+          `Host: ${hostname}. is not in the cert's altnames: ${altNames}`;
+    } else {
+      // Match against Common Name only if no supported identifiers exist.
       const cn = subject.CN;
 
       if (Array.isArray(cn))
@@ -265,11 +274,6 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
 
       if (!valid)
         reason = `Host: ${hostname}. is not cert's CN: ${cn}`;
-    } else {
-      valid = dnsNames.some(wildcard) || uriNames.some(noWildcard);
-      if (!valid)
-        reason =
-          `Host: ${hostname}. is not in the cert's altnames: ${altNames}`;
     }
   } else {
     reason = 'Cert is empty';
diff --git a/test/parallel/test-tls-check-server-identity.js b/test/parallel/test-tls-check-server-identity.js
index afeb6c749928c3..04c77305eb3b89 100644
--- a/test/parallel/test-tls-check-server-identity.js
+++ b/test/parallel/test-tls-check-server-identity.js
@@ -143,6 +143,20 @@ const tests = [
     error: 'Cert is empty'
   },
 
+  // Empty Subject w/DNS name
+  {
+    host: 'a.com', cert: {
+      subjectaltname: 'DNS:a.com',
+    }
+  },
+
+  // Empty Subject w/URI name
+  {
+    host: 'a.b.a.com', cert: {
+      subjectaltname: 'URI:http://a.b.a.com/',
+    }
+  },
+
   // Multiple CN fields
   {
     host: 'foo.com', cert: {