Commit 89e2c71
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
2 files changed
Lines changed: 28 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -243,19 +243,28 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { | |||
| 243 | 243 | let valid = false; | |
| 244 | 244 | let reason = 'Unknown reason'; | |
| 245 | 245 | ||
| 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 | + | ||
| 246 | 251 | if (net.isIP(hostname)) { | |
| 247 | 252 | valid = ips.includes(canonicalizeIP(hostname)); | |
| 248 | 253 | if (!valid) | |
| 249 | 254 | reason = `IP: ${hostname} is not in the cert's list: ${ips.join(', ')}`; | |
| 250 | 255 | // 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) { | ||
| 253 | 257 | const hostParts = splitHost(hostname); | |
| 254 | 258 | const wildcard = (pattern) => check(hostParts, pattern, true); | |
| 255 | - const noWildcard = (pattern) => check(hostParts, pattern, false); | ||
| 256 | 259 | ||
| 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. | ||
| 259 | 268 | const cn = subject.CN; | |
| 260 | 269 | ||
| 261 | 270 | if (Array.isArray(cn)) | |
@@ -265,11 +274,6 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { | |||
| 265 | 274 | ||
| 266 | 275 | if (!valid) | |
| 267 | 276 | 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}`; | ||
| 273 | 277 | } | |
| 274 | 278 | } else { | |
| 275 | 279 | reason = 'Cert is empty'; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -143,6 +143,20 @@ const tests = [ | |||
| 143 | 143 | error: 'Cert is empty' | |
| 144 | 144 | }, | |
| 145 | 145 | ||
| 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 | + | ||
| 146 | 160 | // Multiple CN fields | |
| 147 | 161 | { | |
| 148 | 162 | host: 'foo.com', cert: { | |
0 commit comments