Commit 74b00cc
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 745b329 commit 74b00cc
2 files changed
Lines changed: 28 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -214,19 +214,28 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { | |||
| 214 | 214 | let valid = false; | |
| 215 | 215 | let reason = 'Unknown reason'; | |
| 216 | 216 | ||
| 217 | + const hasAltNames = | ||
| 218 | + dnsNames.length > 0 || ips.length > 0 || uriNames.length > 0; | ||
| 219 | + | ||
| 220 | + hostname = unfqdn(hostname); // Remove trailing dot for error messages. | ||
| 221 | + | ||
| 217 | 222 | if (net.isIP(hostname)) { | |
| 218 | 223 | valid = ips.includes(canonicalizeIP(hostname)); | |
| 219 | 224 | if (!valid) | |
| 220 | 225 | reason = `IP: ${hostname} is not in the cert's list: ${ips.join(', ')}`; | |
| 221 | 226 | // TODO(bnoordhuis) Also check URI SANs that are IP addresses. | |
| 222 | - } else if (subject) { | ||
| 223 | - hostname = unfqdn(hostname); // Remove trailing dot for error messages. | ||
| 227 | + } else if (hasAltNames || subject) { | ||
| 224 | 228 | const hostParts = splitHost(hostname); | |
| 225 | 229 | const wildcard = (pattern) => check(hostParts, pattern, true); | |
| 226 | - const noWildcard = (pattern) => check(hostParts, pattern, false); | ||
| 227 | 230 | ||
| 228 | - // Match against Common Name only if no supported identifiers are present. | ||
| 229 | - if (dnsNames.length === 0 && ips.length === 0 && uriNames.length === 0) { | ||
| 231 | + if (hasAltNames) { | ||
| 232 | + const noWildcard = (pattern) => check(hostParts, pattern, false); | ||
| 233 | + valid = dnsNames.some(wildcard) || uriNames.some(noWildcard); | ||
| 234 | + if (!valid) | ||
| 235 | + reason = | ||
| 236 | + `Host: ${hostname}. is not in the cert's altnames: ${altNames}`; | ||
| 237 | + } else { | ||
| 238 | + // Match against Common Name only if no supported identifiers exist. | ||
| 230 | 239 | const cn = subject.CN; | |
| 231 | 240 | ||
| 232 | 241 | if (Array.isArray(cn)) | |
@@ -236,11 +245,6 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { | |||
| 236 | 245 | ||
| 237 | 246 | if (!valid) | |
| 238 | 247 | reason = `Host: ${hostname}. is not cert's CN: ${cn}`; | |
| 239 | - } else { | ||
| 240 | - valid = dnsNames.some(wildcard) || uriNames.some(noWildcard); | ||
| 241 | - if (!valid) | ||
| 242 | - reason = | ||
| 243 | - `Host: ${hostname}. is not in the cert's altnames: ${altNames}`; | ||
| 244 | 248 | } | |
| 245 | 249 | } else { | |
| 246 | 250 | reason = 'Cert is empty'; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -94,6 +94,20 @@ const tests = [ | |||
| 94 | 94 | error: 'Cert is empty' | |
| 95 | 95 | }, | |
| 96 | 96 | ||
| 97 | + // Empty Subject w/DNS name | ||
| 98 | + { | ||
| 99 | + host: 'a.com', cert: { | ||
| 100 | + subjectaltname: 'DNS:a.com', | ||
| 101 | + } | ||
| 102 | + }, | ||
| 103 | + | ||
| 104 | + // Empty Subject w/URI name | ||
| 105 | + { | ||
| 106 | + host: 'a.b.a.com', cert: { | ||
| 107 | + subjectaltname: 'URI:http://a.b.a.com/', | ||
| 108 | + } | ||
| 109 | + }, | ||
| 110 | + | ||
| 97 | 111 | // Multiple CN fields | |
| 98 | 112 | { | |
| 99 | 113 | host: 'foo.com', cert: { | |
0 commit comments