Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 89e2c71

Browse filesBrowse 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 <jasnell@gmail.com>
1 parent 6322611 commit 89e2c71
Copy full SHA for 89e2c71

File tree

Expand file treeCollapse file tree

2 files changed

+28
-10
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+28
-10
lines changed
Open diff view settings
Collapse file

‎lib/tls.js‎

Copy file name to clipboardExpand all lines: lib/tls.js
+14-10Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -243,19 +243,28 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
243243
let valid = false;
244244
let reason = 'Unknown reason';
245245

246+
const hasAltNames =
247+
dnsNames.length > 0 || ips.length > 0 || uriNames.length > 0;
248+
249+
hostname = unfqdn(hostname); // Remove trailing dot for error messages.
250+
246251
if (net.isIP(hostname)) {
247252
valid = ips.includes(canonicalizeIP(hostname));
248253
if (!valid)
249254
reason = `IP: ${hostname} is not in the cert's list: ${ips.join(', ')}`;
250255
// TODO(bnoordhuis) Also check URI SANs that are IP addresses.
251-
} else if (subject) {
252-
hostname = unfqdn(hostname); // Remove trailing dot for error messages.
256+
} else if (hasAltNames || subject) {
253257
const hostParts = splitHost(hostname);
254258
const wildcard = (pattern) => check(hostParts, pattern, true);
255-
const noWildcard = (pattern) => check(hostParts, pattern, false);
256259

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

261270
if (Array.isArray(cn))
@@ -265,11 +274,6 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
265274

266275
if (!valid)
267276
reason = `Host: ${hostname}. is not cert's CN: ${cn}`;
268-
} else {
269-
valid = dnsNames.some(wildcard) || uriNames.some(noWildcard);
270-
if (!valid)
271-
reason =
272-
`Host: ${hostname}. is not in the cert's altnames: ${altNames}`;
273277
}
274278
} else {
275279
reason = 'Cert is empty';
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-tls-check-server-identity.js
+14Lines changed: 14 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.