Skip to content

Commit 9aa379e

Browse files
authored
tls: reject hostnames with forbidden code points
domainToASCII() is implemented with the URL host parser, which stops at path, query and fragment delimiters and percent-decodes. Passing a hostname through it can therefore silently shorten the name, so checkServerIdentity() validates a certificate against a host the application never asked for: name resolution and SNI use the original string, while the certificate is matched after normalization. Reject hostnames containing a WHATWG forbidden host code point before matching. None of these code points is valid in a DNS hostname, so failing closed is safe. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #67254 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>
1 parent f344e7f commit 9aa379e

2 files changed

Lines changed: 48 additions & 1 deletion

File tree

‎lib/tls.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,17 @@ function splitEscapedAltNames(altNames) {
427427
return result;
428428
}
429429

430+
// domainToASCII() is implemented on top of the URL host parser, which stops at
431+
// path, query and fragment delimiters ("/", "?", "#" and backslash),
432+
// percent-decodes and drops a few code points. Passing a hostname through it
433+
// can therefore yield a different host than the one the application actually
434+
// connects to (the original string is used for name resolution and SNI), so
435+
// the certificate would be checked against a host the application never asked
436+
// for. Every one of these code points (the WHATWG "forbidden host code point"
437+
// set) is invalid in a DNS hostname, so failing closed is safe.
438+
// eslint-disable-next-line no-control-regex
439+
const forbiddenHostCodePoint = /[\u0000\u0009\u000A\u000D\u0020#%/:<>?@[\]\\^|]/;
440+
430441
exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
431442
const subject = cert.subject;
432443
const altNames = cert.subjectaltname;
@@ -442,6 +453,17 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
442453
const hostnameIsIP = net.isIP(hostname);
443454
const hostnameASCIIIsIP = net.isIP(hostnameASCIIWithoutFQDN);
444455

456+
// The application's hostname is only meaningful for the exact string it
457+
// supplied, but domainToASCII() can silently shorten it. Reject hostnames
458+
// containing a forbidden host code point so the certificate is not validated
459+
// against a host that was never requested (e.g. "agent1/.bank.example" must
460+
// not be verified as "agent1"). IP literals are exempt because IPv6 uses
461+
// "[", "]" and ":" legitimately.
462+
if (!hostnameIsIP && forbiddenHostCodePoint.test(hostname)) {
463+
return new ERR_TLS_CERT_ALTNAME_INVALID(
464+
'Host contains a forbidden host code point', hostname, cert);
465+
}
466+
445467
if (altNames) {
446468
const splitAltNames = altNames.includes('"') ?
447469
splitEscapedAltNames(altNames) :

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,13 @@ const tests = [
138138
cert: { subject: { CN: '*.a.com' } },
139139
error: 'Host: ba.com. is not cert\'s CN: *.a.com'
140140
},
141+
// A hostname containing a forbidden host code point (here a line feed) is
142+
// rejected instead of being silently normalized and matched: domainToASCII()
143+
// would drop the control character, producing a host that was never asked for.
141144
{
142145
host: '\n.b.com',
143146
cert: { subject: { CN: '*n.b.com' } },
144-
error: 'Host: \n.b.com. is not cert\'s CN: *n.b.com'
147+
error: 'Host contains a forbidden host code point'
145148
},
146149
{ host: 'b.a.com',
147150
cert: {
@@ -432,6 +435,28 @@ const tests = [
432435
subject: {},
433436
},
434437
},
438+
// domainToASCII() is implemented on top of the URL host parser, which stops at
439+
// path/query/fragment delimiters and percent-decodes. Reject hostnames that
440+
// would be silently truncated or otherwise transformed, so the certificate is
441+
// never validated against a host the application did not ask for (e.g.
442+
// "agent1/.bank.example" must not be verified as "agent1").
443+
...[
444+
['agent1/.bank.example'],
445+
['agent1?.bank.example'],
446+
['agent1#.bank.example'],
447+
['agent1\\.bank.example'],
448+
['ag%65nt1'],
449+
['ag@ent1'],
450+
['ag:ent1'],
451+
['ag ent1'],
452+
].map(([host]) => ({
453+
host,
454+
cert: {
455+
subjectaltname: 'DNS:agent1',
456+
subject: {},
457+
},
458+
error: 'Host contains a forbidden host code point',
459+
})),
435460
{
436461
host: 'localhost', cert: {
437462
subjectaltname: 'DNS:a.com',

0 commit comments

Comments
 (0)