From 74b00cca64e480a74bf1792576db46ef31824f2f Mon Sep 17 00:00:00 2001 From: Jason Macgowan 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 --- 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 b28ffa3d666b9b..fc9d0c96e59607 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -214,19 +214,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)) @@ -236,11 +245,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 1623e70a2af2ec..bacbf598727cae 100644 --- a/test/parallel/test-tls-check-server-identity.js +++ b/test/parallel/test-tls-check-server-identity.js @@ -94,6 +94,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: {