Skip to content

Commit 9a0a189

Browse files
mhdawsonkumarak
authored andcommittedJan 10, 2022
src: add cve reverts and associated tests
Signed-off-by: Michael Dawson <mdawson@devrus.com> Co-authored-by: Akshay K <iit.akshay@gmail.com> Backport-PR-URL: nodejs-private/node-private#304 PR-URL: nodejs-private/node-private#300 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 965536f commit 9a0a189

6 files changed

+515
-3
lines changed
 

‎lib/tls.js

+19-3
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const net = require('net');
6060
const { getOptionValue } = require('internal/options');
6161
const { getRootCertificates, getSSLCiphers } = internalBinding('crypto');
6262
const { Buffer } = require('buffer');
63+
const { URL } = require('internal/url'); // Only used for Security Revert
6364
const { canonicalizeIP } = internalBinding('cares_wrap');
6465
const _tls_common = require('_tls_common');
6566
const _tls_wrap = require('_tls_wrap');
@@ -274,6 +275,7 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
274275
const subject = cert.subject;
275276
const altNames = cert.subjectaltname;
276277
const dnsNames = [];
278+
const uriNames = [];
277279
const ips = [];
278280

279281
hostname = '' + hostname;
@@ -285,6 +287,12 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
285287
ArrayPrototypeForEach(splitAltNames, (name) => {
286288
if (StringPrototypeStartsWith(name, 'DNS:')) {
287289
ArrayPrototypePush(dnsNames, StringPrototypeSlice(name, 4));
290+
} else if (process.REVERT_CVE_2021_44531 &&
291+
StringPrototypeStartsWith(name, 'URI:')) {
292+
const uri = new URL(StringPrototypeSlice(name, 4));
293+
294+
// TODO(bnoordhuis) Also use scheme.
295+
ArrayPrototypePush(uriNames, uri.hostname);
288296
} else if (StringPrototypeStartsWith(name, 'IP Address:')) {
289297
ArrayPrototypePush(ips, canonicalizeIP(StringPrototypeSlice(name, 11)));
290298
}
@@ -294,19 +302,27 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
294302
let valid = false;
295303
let reason = 'Unknown reason';
296304

305+
const hasAltNames =
306+
dnsNames.length > 0 || ips.length > 0 || uriNames.length > 0;
307+
297308
hostname = unfqdn(hostname); // Remove trailing dot for error messages.
298309

299310
if (net.isIP(hostname)) {
300311
valid = ArrayPrototypeIncludes(ips, canonicalizeIP(hostname));
301312
if (!valid)
302313
reason = `IP: ${hostname} is not in the cert's list: ` +
303314
ArrayPrototypeJoin(ips, ', ');
304-
} else if (dnsNames.length > 0 || subject?.CN) {
315+
// TODO(bnoordhuis) Also check URI SANs that are IP addresses.
316+
} else if ((process.REVERT_CVE_2021_44531 && (hasAltNames || subject)) ||
317+
(dnsNames.length > 0 || subject?.CN)) {
305318
const hostParts = splitHost(hostname);
306319
const wildcard = (pattern) => check(hostParts, pattern, true);
307320

308-
if (dnsNames.length > 0) {
309-
valid = ArrayPrototypeSome(dnsNames, wildcard);
321+
if ((process.REVERT_CVE_2021_44531 && hasAltNames) ||
322+
(dnsNames.length > 0)) {
323+
const noWildcard = (pattern) => check(hostParts, pattern, false);
324+
valid = ArrayPrototypeSome(dnsNames, wildcard) ||
325+
ArrayPrototypeSome(uriNames, noWildcard);
310326
if (!valid)
311327
reason =
312328
`Host: ${hostname}. is not in the cert's altnames: ${altNames}`;

‎src/crypto/crypto_common.cc

+47
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "crypto/crypto_common.h"
77
#include "node.h"
88
#include "node_internals.h"
9+
#include "node_revert.h"
910
#include "node_url.h"
1011
#include "string_bytes.h"
1112
#include "memory_tracker-inl.h"
@@ -620,6 +621,44 @@ MaybeLocal<Value> GetValidFrom(
620621
return ToV8Value(env, bio);
621622
}
622623

624+
// deprecated, only used for security revert
625+
bool SafeX509ExtPrint(const BIOPointer& out, X509_EXTENSION* ext) {
626+
const X509V3_EXT_METHOD* method = X509V3_EXT_get(ext);
627+
628+
if (method != X509V3_EXT_get_nid(NID_subject_alt_name))
629+
return false;
630+
631+
GENERAL_NAMES* names = static_cast<GENERAL_NAMES*>(X509V3_EXT_d2i(ext));
632+
if (names == nullptr)
633+
return false;
634+
635+
for (int i = 0; i < sk_GENERAL_NAME_num(names); i++) {
636+
GENERAL_NAME* gen = sk_GENERAL_NAME_value(names, i);
637+
638+
if (i != 0)
639+
BIO_write(out.get(), ", ", 2);
640+
641+
if (gen->type == GEN_DNS) {
642+
ASN1_IA5STRING* name = gen->d.dNSName;
643+
644+
BIO_write(out.get(), "DNS:", 4);
645+
BIO_write(out.get(), name->data, name->length);
646+
} else {
647+
STACK_OF(CONF_VALUE)* nval = i2v_GENERAL_NAME(
648+
const_cast<X509V3_EXT_METHOD*>(method), gen, nullptr);
649+
if (nval == nullptr) {
650+
sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
651+
return false;
652+
}
653+
X509V3_EXT_val_prn(out.get(), nval, 0, 0);
654+
sk_CONF_VALUE_pop_free(nval, X509V3_conf_free);
655+
}
656+
}
657+
sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
658+
659+
return true;
660+
}
661+
623662
static inline bool IsSafeAltName(const char* name, size_t length, bool utf8) {
624663
for (size_t i = 0; i < length; i++) {
625664
char c = name[i];
@@ -844,6 +883,10 @@ bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext) {
844883
const X509V3_EXT_METHOD* method = X509V3_EXT_get(ext);
845884
CHECK(method == X509V3_EXT_get_nid(NID_subject_alt_name));
846885

886+
if (IsReverted(SECURITY_REVERT_CVE_2021_44532)) {
887+
return SafeX509ExtPrint(out, ext);
888+
}
889+
847890
GENERAL_NAMES* names = static_cast<GENERAL_NAMES*>(X509V3_EXT_d2i(ext));
848891
if (names == nullptr)
849892
return false;
@@ -869,6 +912,10 @@ bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext) {
869912
const X509V3_EXT_METHOD* method = X509V3_EXT_get(ext);
870913
CHECK(method == X509V3_EXT_get_nid(NID_info_access));
871914

915+
if (IsReverted(SECURITY_REVERT_CVE_2021_44532)) {
916+
return (X509V3_EXT_print(out.get(), ext, 0, 0) == 1);
917+
}
918+
872919
AUTHORITY_INFO_ACCESS* descs =
873920
static_cast<AUTHORITY_INFO_ACCESS*>(X509V3_EXT_d2i(ext));
874921
if (descs == nullptr)

‎src/node_revert.h

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
namespace node {
1717

1818
#define SECURITY_REVERSIONS(XX) \
19+
XX(CVE_2021_44531, "CVE-2021-44531", "Cert Verif Bypass via URI SAN") \
20+
XX(CVE_2021_44532, "CVE-2021-44532", "Cert Verif Bypass via Str Inject") \
1921
// XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title")
2022

2123
enum reversion {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Flags: --security-revert=CVE-2021-44531
2+
'use strict';
3+
const common = require('../common');
4+
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
8+
const assert = require('assert');
9+
const util = require('util');
10+
11+
const tls = require('tls');
12+
13+
const tests = [
14+
// Likewise for "URI:" Subject Alternative Names.
15+
// See also https://github.com/nodejs/node/issues/8108.
16+
{
17+
host: '8.8.8.8',
18+
cert: { subject: { CN: '8.8.8.8' }, subjectaltname: 'URI:http://8.8.8.8/' },
19+
error: 'IP: 8.8.8.8 is not in the cert\'s list: '
20+
},
21+
// Empty Subject w/URI name
22+
{
23+
host: 'a.b.a.com', cert: {
24+
subjectaltname: 'URI:http://a.b.a.com/',
25+
}
26+
},
27+
// URI names
28+
{
29+
host: 'a.b.a.com', cert: {
30+
subjectaltname: 'URI:http://a.b.a.com/',
31+
subject: {}
32+
}
33+
},
34+
{
35+
host: 'a.b.a.com', cert: {
36+
subjectaltname: 'URI:http://*.b.a.com/',
37+
subject: {}
38+
},
39+
error: 'Host: a.b.a.com. is not in the cert\'s altnames: ' +
40+
'URI:http://*.b.a.com/'
41+
},
42+
];
43+
44+
tests.forEach(function(test, i) {
45+
const err = tls.checkServerIdentity(test.host, test.cert);
46+
assert.strictEqual(err && err.reason,
47+
test.error,
48+
`Test# ${i} failed: ${util.inspect(test)} \n` +
49+
`${test.error} != ${(err && err.reason)}`);
50+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
// Flags: --security-revert=CVE-2021-44532
22+
'use strict';
23+
const common = require('../common');
24+
if (!common.hasCrypto)
25+
common.skip('missing crypto');
26+
27+
// Check getPeerCertificate can properly handle '\0' for fix CVE-2009-2408.
28+
29+
const assert = require('assert');
30+
const tls = require('tls');
31+
const fixtures = require('../common/fixtures');
32+
33+
const server = tls.createServer({
34+
key: fixtures.readSync(['0-dns', '0-dns-key.pem']),
35+
cert: fixtures.readSync(['0-dns', '0-dns-cert.pem'])
36+
}, common.mustCall((c) => {
37+
c.once('data', common.mustCall(() => {
38+
c.destroy();
39+
server.close();
40+
}));
41+
})).listen(0, common.mustCall(() => {
42+
const c = tls.connect(server.address().port, {
43+
rejectUnauthorized: false
44+
}, common.mustCall(() => {
45+
const cert = c.getPeerCertificate();
46+
assert.strictEqual(cert.subjectaltname,
47+
'DNS:good.example.org\0.evil.example.com, ' +
48+
'DNS:just-another.example.com, ' +
49+
'IP Address:8.8.8.8, ' +
50+
'IP Address:8.8.4.4, ' +
51+
'DNS:last.example.com');
52+
c.write('ok');
53+
}));
54+
}));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,343 @@
1+
// Flags: --security-revert=CVE-2021-44532
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
8+
const assert = require('assert');
9+
const { X509Certificate } = require('crypto');
10+
const tls = require('tls');
11+
const fixtures = require('../common/fixtures');
12+
13+
const { hasOpenSSL3 } = common;
14+
15+
// Test escaping rules for subject alternative names.
16+
{
17+
const expectedSANs = [
18+
'DNS:good.example.com, DNS:evil.example.com',
19+
// URIs should not require escaping.
20+
'URI:http://example.com/',
21+
'URI:http://example.com/?a=b&c=d',
22+
// Unless they contain commas.
23+
'URI:http://example.com/a,b',
24+
// Percent encoding should not require escaping.
25+
'URI:http://example.com/a%2Cb',
26+
// Malicious attempts should be escaped.
27+
'URI:http://example.com/a, DNS:good.example.com',
28+
// Non-ASCII characters in DNS names should be treated as Latin-1.
29+
'DNS:ex�mple.com',
30+
// It should not be possible to cause unescaping without escaping.
31+
'DNS:"evil.example.com"',
32+
// IPv4 addresses should be represented as usual.
33+
'IP Address:8.8.8.8',
34+
'IP Address:8.8.4.4',
35+
// For backward-compatibility, include invalid IP address lengths.
36+
hasOpenSSL3 ? 'IP Address:<invalid length=5>' : 'IP Address:<invalid>',
37+
hasOpenSSL3 ? 'IP Address:<invalid length=6>' : 'IP Address:<invalid>',
38+
// IPv6 addresses are represented as OpenSSL does.
39+
'IP Address:A0B:C0D:E0F:0:0:0:7A7B:7C7D',
40+
// Regular email addresses don't require escaping.
41+
'email:foo@example.com',
42+
// ... but should be escaped if they contain commas.
43+
'email:foo@example.com, DNS:good.example.com',
44+
'DirName:/C=DE/L=Hannover',
45+
// TODO(tniessen): support UTF8 in DirName
46+
'DirName:/C=DE/L=M\\xC3\\xBCnchen',
47+
'DirName:/C=DE/L=Berlin, DNS:good.example.com',
48+
'DirName:/C=DE/L=Berlin, DNS:good.example.com\\x00' +
49+
'evil.example.com',
50+
'DirName:/C=DE/L=Berlin, DNS:good.example.com\\\\x00' +
51+
'evil.example.com',
52+
// These next two tests might be surprising. OpenSSL applies its own rules
53+
// first, which introduce backslashes, which activate node's escaping.
54+
// Unfortunately, there are also differences between OpenSSL 1.1.1 and 3.0.
55+
'DirName:/C=DE/L=Berlin\\x0D\\x0A',
56+
hasOpenSSL3 ?
57+
'DirName:/C=DE/L=Berlin\\/CN=good.example.com' :
58+
'DirName:/C=DE/L=Berlin/CN=good.example.com',
59+
// TODO(tniessen): even OIDs that are well-known (such as the following,
60+
// which is sha256WithRSAEncryption) should be represented numerically only.
61+
'Registered ID:sha256WithRSAEncryption',
62+
// This is an OID that will likely never be assigned to anything, thus
63+
// OpenSSL hould not know it.
64+
'Registered ID:1.3.9999.12.34',
65+
hasOpenSSL3 ?
66+
'othername: XmppAddr::abc123' :
67+
'othername:<unsupported>',
68+
hasOpenSSL3 ?
69+
'othername: XmppAddr::abc123, DNS:good.example.com' :
70+
'othername:<unsupported>',
71+
hasOpenSSL3 ?
72+
null :
73+
'othername:<unsupported>',
74+
// This is unsupported because the OID is not recognized.
75+
hasOpenSSL3 ?
76+
'othername: 1.3.9999.12.34::abc123' :
77+
'othername:<unsupported>',
78+
hasOpenSSL3 ? 'othername: SRVName::abc123' : 'othername:<unsupported>',
79+
// This is unsupported because it is an SRVName with a UTF8String value,
80+
// which is not allowed for SRVName.
81+
hasOpenSSL3 ?
82+
null : 'othername:<unsupported>',
83+
hasOpenSSL3 ?
84+
null :
85+
'othername:<unsupported>',
86+
];
87+
88+
const serverKey = fixtures.readSync('x509-escaping/server-key.pem', 'utf8');
89+
90+
for (let i = 0; i < expectedSANs.length; i++) {
91+
const pem = fixtures.readSync(`x509-escaping/alt-${i}-cert.pem`, 'utf8');
92+
93+
// Test the subjectAltName property of the X509Certificate API.
94+
const cert = new X509Certificate(pem);
95+
assert.strictEqual(cert.subjectAltName, expectedSANs[i]);
96+
97+
// Test that the certificate obtained by checkServerIdentity has the correct
98+
// subjectaltname property.
99+
const server = tls.createServer({
100+
key: serverKey,
101+
cert: pem,
102+
}, common.mustCall((conn) => {
103+
conn.destroy();
104+
server.close();
105+
})).listen(common.mustCall(() => {
106+
const { port } = server.address();
107+
tls.connect(port, {
108+
ca: pem,
109+
servername: 'example.com',
110+
checkServerIdentity: (hostname, peerCert) => {
111+
assert.strictEqual(hostname, 'example.com');
112+
assert.strictEqual(peerCert.subjectaltname, expectedSANs[i]);
113+
},
114+
}, common.mustCall());
115+
}));
116+
}
117+
}
118+
119+
// Test escaping rules for authority info access.
120+
{
121+
const expectedInfoAccess = [
122+
{
123+
text: 'OCSP - URI:http://good.example.com/\n' +
124+
'OCSP - URI:http://evil.example.com/',
125+
legacy: {
126+
'OCSP - URI': [
127+
'http://good.example.com/',
128+
'http://evil.example.com/',
129+
],
130+
},
131+
},
132+
{
133+
text: 'CA Issuers - URI:http://ca.example.com/\n' +
134+
'OCSP - URI:http://evil.example.com\n' +
135+
'OCSP - DNS:good.example.com\n' +
136+
'OCSP - URI:http://ca.nodejs.org/ca.cert',
137+
legacy: {
138+
'CA Issuers - URI': [
139+
'http://ca.example.com/',
140+
],
141+
'OCSP - DNS': [
142+
'good.example.com',
143+
],
144+
'OCSP - URI': [
145+
'http://evil.example.com',
146+
'http://ca.nodejs.org/ca.cert',
147+
],
148+
},
149+
},
150+
{
151+
text: '1.3.9999.12.34 - URI:http://ca.example.com/',
152+
legacy: {
153+
'1.3.9999.12.34 - URI': [
154+
'http://ca.example.com/',
155+
],
156+
},
157+
},
158+
hasOpenSSL3 ? {
159+
text: 'OCSP - othername: XmppAddr::good.example.com\n' +
160+
'OCSP - othername: 1.3.9999.12.34::abc123\n' +
161+
'OCSP - othername: SRVName::abc123',
162+
legacy: {
163+
'OCSP - othername': [
164+
' XmppAddr::good.example.com',
165+
' 1.3.9999.12.34::abc123',
166+
' SRVName::abc123',
167+
],
168+
},
169+
} : {
170+
text: 'OCSP - othername:<unsupported>\n' +
171+
'OCSP - othername:<unsupported>\n' +
172+
'OCSP - othername:<unsupported>',
173+
legacy: {
174+
'OCSP - othername': [
175+
'<unsupported>',
176+
'<unsupported>',
177+
'<unsupported>',
178+
],
179+
},
180+
},
181+
hasOpenSSL3 ? {
182+
text: null,
183+
legacy: null,
184+
} : {
185+
text: 'OCSP - othername:<unsupported>',
186+
legacy: {
187+
'OCSP - othername': [
188+
'<unsupported>',
189+
]
190+
},
191+
},
192+
];
193+
194+
const serverKey = fixtures.readSync('x509-escaping/server-key.pem', 'utf8');
195+
196+
for (let i = 0; i < expectedInfoAccess.length; i++) {
197+
const pem = fixtures.readSync(`x509-escaping/info-${i}-cert.pem`, 'utf8');
198+
const expected = expectedInfoAccess[i];
199+
200+
// Test the subjectAltName property of the X509Certificate API.
201+
const cert = new X509Certificate(pem);
202+
assert.strictEqual(cert.infoAccess, expected.text ?
203+
`${expected.text}${hasOpenSSL3 ? '' : '\n'}` :
204+
expected.text);
205+
206+
// Test that the certificate obtained by checkServerIdentity has the correct
207+
// subjectaltname property.
208+
const server = tls.createServer({
209+
key: serverKey,
210+
cert: pem,
211+
}, common.mustCall((conn) => {
212+
conn.destroy();
213+
server.close();
214+
})).listen(common.mustCall(() => {
215+
const { port } = server.address();
216+
tls.connect(port, {
217+
ca: pem,
218+
servername: 'example.com',
219+
checkServerIdentity: (hostname, peerCert) => {
220+
assert.strictEqual(hostname, 'example.com');
221+
assert.deepStrictEqual(peerCert.infoAccess,
222+
expected.legacy ?
223+
Object.assign(Object.create(null),
224+
expected.legacy) :
225+
expected.legacy);
226+
},
227+
}, common.mustCall());
228+
}));
229+
}
230+
}
231+
232+
// The internal parsing logic must match the JSON specification exactly.
233+
{
234+
// This list is partially based on V8's own JSON tests.
235+
const invalidJSON = [
236+
'"\\a invalid escape"',
237+
'"\\v invalid escape"',
238+
'"\\\' invalid escape"',
239+
'"\\x42 invalid escape"',
240+
'"\\u202 invalid escape"',
241+
'"\\012 invalid escape"',
242+
'"Unterminated string',
243+
'"Unterminated string\\"',
244+
'"Unterminated string\\\\\\"',
245+
'"\u0000 control character"',
246+
'"\u001e control character"',
247+
'"\u001f control character"',
248+
];
249+
250+
for (const invalidStringLiteral of invalidJSON) {
251+
// Usually, checkServerIdentity returns an error upon verification failure.
252+
// In this case, however, it should throw an error since this is not a
253+
// verification error. Node.js itself will never produce invalid JSON string
254+
// literals, so this can only happen when users construct invalid subject
255+
// alternative name strings (that do not follow escaping rules).
256+
assert.throws(() => {
257+
tls.checkServerIdentity('example.com', {
258+
subjectaltname: `DNS:${invalidStringLiteral}`,
259+
});
260+
}, {
261+
code: 'ERR_TLS_CERT_ALTNAME_FORMAT',
262+
message: 'Invalid subject alternative name string'
263+
});
264+
}
265+
}
266+
267+
// While node does not produce commas within SAN entries, it should parse them
268+
// correctly (i.e., not simply split at commas).
269+
{
270+
// Regardless of the quotes, splitting this SAN string at commas would
271+
// cause checkServerIdentity to see 'DNS:b.example.com' and thus to accept
272+
// the certificate for b.example.com.
273+
const san = 'DNS:"a.example.com, DNS:b.example.com, DNS:c.example.com"';
274+
275+
// This is what node used to do, and which is not correct!
276+
const hostname = 'b.example.com';
277+
assert.strictEqual(san.split(', ')[1], `DNS:${hostname}`);
278+
279+
// The new implementation should parse the string correctly.
280+
const err = tls.checkServerIdentity(hostname, { subjectaltname: san });
281+
assert(err);
282+
assert.strictEqual(err.code, 'ERR_TLS_CERT_ALTNAME_INVALID');
283+
assert.strictEqual(err.message, 'Hostname/IP does not match certificate\'s ' +
284+
'altnames: Host: b.example.com. is not in ' +
285+
'the cert\'s altnames: DNS:"a.example.com, ' +
286+
'DNS:b.example.com, DNS:c.example.com"');
287+
}
288+
289+
// The subject MUST be ignored if a dNSName subject alternative name exists.
290+
{
291+
const key = fixtures.readKey('incorrect_san_correct_subject-key.pem');
292+
const cert = fixtures.readKey('incorrect_san_correct_subject-cert.pem');
293+
294+
// The hostname is the CN, but not a SAN entry.
295+
const servername = 'good.example.com';
296+
const certX509 = new X509Certificate(cert);
297+
assert.strictEqual(certX509.subject, `CN=${servername}`);
298+
assert.strictEqual(certX509.subjectAltName, 'DNS:evil.example.com');
299+
300+
// Try connecting to a server that uses the self-signed certificate.
301+
const server = tls.createServer({ key, cert }, common.mustNotCall());
302+
server.listen(common.mustCall(() => {
303+
const { port } = server.address();
304+
const socket = tls.connect(port, {
305+
ca: cert,
306+
servername,
307+
}, common.mustNotCall());
308+
socket.on('error', common.mustCall((err) => {
309+
assert.strictEqual(err.code, 'ERR_TLS_CERT_ALTNAME_INVALID');
310+
assert.strictEqual(err.message, 'Hostname/IP does not match ' +
311+
"certificate's altnames: Host: " +
312+
"good.example.com. is not in the cert's" +
313+
' altnames: DNS:evil.example.com');
314+
}));
315+
})).unref();
316+
}
317+
318+
// The subject MUST NOT be ignored if no dNSName subject alternative name
319+
// exists, even if other subject alternative names exist.
320+
{
321+
const key = fixtures.readKey('irrelevant_san_correct_subject-key.pem');
322+
const cert = fixtures.readKey('irrelevant_san_correct_subject-cert.pem');
323+
324+
// The hostname is the CN, but there is no dNSName SAN entry.
325+
const servername = 'good.example.com';
326+
const certX509 = new X509Certificate(cert);
327+
assert.strictEqual(certX509.subject, `CN=${servername}`);
328+
assert.strictEqual(certX509.subjectAltName, 'IP Address:1.2.3.4');
329+
330+
// Connect to a server that uses the self-signed certificate.
331+
const server = tls.createServer({ key, cert }, common.mustCall((socket) => {
332+
socket.destroy();
333+
server.close();
334+
})).listen(common.mustCall(() => {
335+
const { port } = server.address();
336+
tls.connect(port, {
337+
ca: cert,
338+
servername,
339+
}, common.mustCall(() => {
340+
// Do nothing, the server will close the connection.
341+
}));
342+
}));
343+
}

0 commit comments

Comments
 (0)
Please sign in to comment.