Skip to content

Commit ff48009

Browse files
jasonmacgowanaddaleax
authored andcommitted
tls: allow empty subject even with altNames defined
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 <[email protected]>
1 parent 3e79c00 commit ff48009

File tree

2 files changed

+28
-10
lines changed

2 files changed

+28
-10
lines changed

lib/tls.js

+14-10
Original file line numberDiff line numberDiff line change
@@ -248,19 +248,28 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
248248
let valid = false;
249249
let reason = 'Unknown reason';
250250

251+
const hasAltNames =
252+
dnsNames.length > 0 || ips.length > 0 || uriNames.length > 0;
253+
254+
hostname = unfqdn(hostname); // Remove trailing dot for error messages.
255+
251256
if (net.isIP(hostname)) {
252257
valid = ips.includes(canonicalizeIP(hostname));
253258
if (!valid)
254259
reason = `IP: ${hostname} is not in the cert's list: ${ips.join(', ')}`;
255260
// TODO(bnoordhuis) Also check URI SANs that are IP addresses.
256-
} else if (subject) {
257-
hostname = unfqdn(hostname); // Remove trailing dot for error messages.
261+
} else if (hasAltNames || subject) {
258262
const hostParts = splitHost(hostname);
259263
const wildcard = (pattern) => check(hostParts, pattern, true);
260-
const noWildcard = (pattern) => check(hostParts, pattern, false);
261264

262-
// Match against Common Name only if no supported identifiers are present.
263-
if (dnsNames.length === 0 && ips.length === 0 && uriNames.length === 0) {
265+
if (hasAltNames) {
266+
const noWildcard = (pattern) => check(hostParts, pattern, false);
267+
valid = dnsNames.some(wildcard) || uriNames.some(noWildcard);
268+
if (!valid)
269+
reason =
270+
`Host: ${hostname}. is not in the cert's altnames: ${altNames}`;
271+
} else {
272+
// Match against Common Name only if no supported identifiers exist.
264273
const cn = subject.CN;
265274

266275
if (ArrayIsArray(cn))
@@ -270,11 +279,6 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
270279

271280
if (!valid)
272281
reason = `Host: ${hostname}. is not cert's CN: ${cn}`;
273-
} else {
274-
valid = dnsNames.some(wildcard) || uriNames.some(noWildcard);
275-
if (!valid)
276-
reason =
277-
`Host: ${hostname}. is not in the cert's altnames: ${altNames}`;
278282
}
279283
} else {
280284
reason = 'Cert is empty';

test/parallel/test-tls-check-server-identity.js

+14
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,20 @@ const tests = [
143143
error: 'Cert is empty'
144144
},
145145

146+
// Empty Subject w/DNS name
147+
{
148+
host: 'a.com', cert: {
149+
subjectaltname: 'DNS:a.com',
150+
}
151+
},
152+
153+
// Empty Subject w/URI name
154+
{
155+
host: 'a.b.a.com', cert: {
156+
subjectaltname: 'URI:http://a.b.a.com/',
157+
}
158+
},
159+
146160
// Multiple CN fields
147161
{
148162
host: 'foo.com', cert: {

0 commit comments

Comments
 (0)