Skip to content

Commit

Permalink
tls: allow empty subject even with altNames defined
Browse files Browse the repository at this point in the history
Behavior described in #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: #11771

PR-URL: #22906
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
jasonmacgowan authored and MylesBorins committed Dec 17, 2019
1 parent 55e94cb commit 43cec65
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
24 changes: 14 additions & 10 deletions lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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';
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-tls-check-server-identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down

0 comments on commit 43cec65

Please sign in to comment.