diff options
author | Michael Dawson <michael_dawson@ca.ibm.com> | 2021-12-30 06:06:39 +0300 |
---|---|---|
committer | Richard Lau <rlau@redhat.com> | 2022-01-07 18:57:11 +0300 |
commit | 9f2c52617f7910eebe194981d7caf9824a4aba44 (patch) | |
tree | c17bfaf4f189a40c15f4b7cc6f69816752278adb /test | |
parent | 83d8f880bbbde610659740d15f5ba6a33555c4b3 (diff) |
src: add cve reverts and associated tests
Co-authored-by: Akshay K <iit.akshay@gmail.com>
Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/305
PR-URL: https://github.com/nodejs-private/node-private/pull/300
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Diffstat (limited to 'test')
3 files changed, 478 insertions, 0 deletions
diff --git a/test/parallel/test-revert-CVE-2021-44531.js b/test/parallel/test-revert-CVE-2021-44531.js new file mode 100644 index 00000000000..56571730d4a --- /dev/null +++ b/test/parallel/test-revert-CVE-2021-44531.js @@ -0,0 +1,64 @@ +// Flags: --security-revert=CVE-2021-44531 +'use strict'; +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const util = require('util'); + +const tls = require('tls'); + +common.expectWarning('DeprecationWarning', [ + ['The URI http://[a.b.a.com]/ found in cert.subjectaltname ' + + 'is not a valid URI, and is supported in the tls module ' + + 'solely for compatibility.', + 'DEP0109'], +]); + +const tests = [ + // Likewise for "URI:" Subject Alternative Names. + // See also https://github.com/nodejs/node/issues/8108. + { + host: '8.8.8.8', + cert: { subject: { CN: '8.8.8.8' }, subjectaltname: 'URI:http://8.8.8.8/' }, + error: 'IP: 8.8.8.8 is not in the cert\'s list: ' + }, + // Empty Subject w/URI name + { + host: 'a.b.a.com', cert: { + subjectaltname: 'URI:http://a.b.a.com/', + } + }, + // URI names + { + host: 'a.b.a.com', cert: { + subjectaltname: 'URI:http://a.b.a.com/', + subject: {} + } + }, + { + host: 'a.b.a.com', cert: { + subjectaltname: 'URI:http://*.b.a.com/', + subject: {} + }, + error: 'Host: a.b.a.com. is not in the cert\'s altnames: ' + + 'URI:http://*.b.a.com/' + }, + // Invalid URI + { + host: 'a.b.a.com', cert: { + subjectaltname: 'URI:http://[a.b.a.com]/', + subject: {} + } + }, +]; + +tests.forEach(function(test, i) { + const err = tls.checkServerIdentity(test.host, test.cert); + assert.strictEqual(err && err.reason, + test.error, + `Test# ${i} failed: ${util.inspect(test)} \n` + + `${test.error} != ${(err && err.reason)}`); +}); diff --git a/test/parallel/test-tls-0-dns-altname-revert-CVE-2021-44532.js b/test/parallel/test-tls-0-dns-altname-revert-CVE-2021-44532.js new file mode 100644 index 00000000000..8624c46f827 --- /dev/null +++ b/test/parallel/test-tls-0-dns-altname-revert-CVE-2021-44532.js @@ -0,0 +1,54 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. +// Flags: --security-revert=CVE-2021-44532 +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// Check getPeerCertificate can properly handle '\0' for fix CVE-2009-2408. + +const assert = require('assert'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +const server = tls.createServer({ + key: fixtures.readSync(['0-dns', '0-dns-key.pem']), + cert: fixtures.readSync(['0-dns', '0-dns-cert.pem']) +}, common.mustCall((c) => { + c.once('data', common.mustCall(() => { + c.destroy(); + server.close(); + })); +})).listen(0, common.mustCall(() => { + const c = tls.connect(server.address().port, { + rejectUnauthorized: false + }, common.mustCall(() => { + const cert = c.getPeerCertificate(); + assert.strictEqual(cert.subjectaltname, + 'DNS:good.example.org\0.evil.example.com, ' + + 'DNS:just-another.example.com, ' + + 'IP Address:8.8.8.8, ' + + 'IP Address:8.8.4.4, ' + + 'DNS:last.example.com'); + c.write('ok'); + })); +})); diff --git a/test/parallel/test-x509-escaping-revert-CVE-2021-44532.js b/test/parallel/test-x509-escaping-revert-CVE-2021-44532.js new file mode 100644 index 00000000000..c07edca86d8 --- /dev/null +++ b/test/parallel/test-x509-escaping-revert-CVE-2021-44532.js @@ -0,0 +1,360 @@ +// Flags: --security-revert=CVE-2021-44532 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +const { hasOpenSSL3 } = common; + +// Test escaping rules for subject alternative names. +{ + const expectedSANs = [ + 'DNS:good.example.com, DNS:evil.example.com', + // URIs should not require escaping. + 'URI:http://example.com/', + 'URI:http://example.com/?a=b&c=d', + // Unless they contain commas. + 'URI:http://example.com/a,b', + // Percent encoding should not require escaping. + 'URI:http://example.com/a%2Cb', + // Malicious attempts should be escaped. + 'URI:http://example.com/a, DNS:good.example.com', + // Non-ASCII characters in DNS names should be treated as Latin-1. + 'DNS:ex�mple.com', + // It should not be possible to cause unescaping without escaping. + 'DNS:"evil.example.com"', + // IPv4 addresses should be represented as usual. + 'IP Address:8.8.8.8', + 'IP Address:8.8.4.4', + // For backward-compatibility, include invalid IP address lengths. + hasOpenSSL3 ? 'IP Address:<invalid length=5>' : 'IP Address:<invalid>', + hasOpenSSL3 ? 'IP Address:<invalid length=6>' : 'IP Address:<invalid>', + // IPv6 addresses are represented as OpenSSL does. + 'IP Address:A0B:C0D:E0F:0:0:0:7A7B:7C7D', + // Regular email addresses don't require escaping. + 'email:foo@example.com', + // ... but should be escaped if they contain commas. + 'email:foo@example.com, DNS:good.example.com', + 'DirName:/C=DE/L=Hannover', + // TODO(tniessen): support UTF8 in DirName + 'DirName:/C=DE/L=M\\xC3\\xBCnchen', + 'DirName:/C=DE/L=Berlin, DNS:good.example.com', + 'DirName:/C=DE/L=Berlin, DNS:good.example.com\\x00' + + 'evil.example.com', + 'DirName:/C=DE/L=Berlin, DNS:good.example.com\\\\x00' + + 'evil.example.com', + // These next two tests might be surprising. OpenSSL applies its own rules + // first, which introduce backslashes, which activate node's escaping. + // Unfortunately, there are also differences between OpenSSL 1.1.1 and 3.0. + 'DirName:/C=DE/L=Berlin\\x0D\\x0A', + hasOpenSSL3 ? + 'DirName:/C=DE/L=Berlin\\/CN=good.example.com' : + 'DirName:/C=DE/L=Berlin/CN=good.example.com', + // TODO(tniessen): even OIDs that are well-known (such as the following, + // which is sha256WithRSAEncryption) should be represented numerically only. + 'Registered ID:sha256WithRSAEncryption', + // This is an OID that will likely never be assigned to anything, thus + // OpenSSL hould not know it. + 'Registered ID:1.3.9999.12.34', + hasOpenSSL3 ? + 'othername: XmppAddr::abc123' : + 'othername:<unsupported>', + hasOpenSSL3 ? + 'othername: XmppAddr::abc123, DNS:good.example.com' : + 'othername:<unsupported>', + hasOpenSSL3 ? + null : + 'othername:<unsupported>', + // This is unsupported because the OID is not recognized. + hasOpenSSL3 ? + 'othername: 1.3.9999.12.34::abc123' : + 'othername:<unsupported>', + hasOpenSSL3 ? 'othername: SRVName::abc123' : 'othername:<unsupported>', + // This is unsupported because it is an SRVName with a UTF8String value, + // which is not allowed for SRVName. + hasOpenSSL3 ? + null : 'othername:<unsupported>', + hasOpenSSL3 ? + null : + 'othername:<unsupported>', + ]; + + const serverKey = fixtures.readSync('x509-escaping/server-key.pem', 'utf8'); + + for (let i = 0; i < expectedSANs.length; i++) { + const pem = fixtures.readSync(`x509-escaping/alt-${i}-cert.pem`, 'utf8'); + + // X509Certificate interface is not supported in v12.x & v14.x. Disable + // checks for subjectAltName with expectedSANs. The testcase is ported + // from v17.x + // + // Test the subjectAltName property of the X509Certificate API. + // const cert = new X509Certificate(pem); + // assert.strictEqual(cert.subjectAltName, expectedSANs[i]); + + // Test that the certificate obtained by checkServerIdentity has the correct + // subjectaltname property. + const server = tls.createServer({ + key: serverKey, + cert: pem, + }, common.mustCall((conn) => { + conn.destroy(); + server.close(); + })).listen(common.mustCall(() => { + const { port } = server.address(); + tls.connect(port, { + ca: pem, + servername: 'example.com', + checkServerIdentity: (hostname, peerCert) => { + assert.strictEqual(hostname, 'example.com'); + assert.strictEqual(peerCert.subjectaltname, expectedSANs[i]); + }, + }, common.mustCall()); + })); + } +} + +// Test escaping rules for authority info access. +{ + const expectedInfoAccess = [ + { + text: 'OCSP - URI:http://good.example.com/\n' + + 'OCSP - URI:http://evil.example.com/', + legacy: { + 'OCSP - URI': [ + 'http://good.example.com/', + 'http://evil.example.com/', + ], + }, + }, + { + text: 'CA Issuers - URI:http://ca.example.com/\n' + + 'OCSP - URI:http://evil.example.com\n' + + 'OCSP - DNS:good.example.com\n' + + 'OCSP - URI:http://ca.nodejs.org/ca.cert', + legacy: { + 'CA Issuers - URI': [ + 'http://ca.example.com/', + ], + 'OCSP - DNS': [ + 'good.example.com', + ], + 'OCSP - URI': [ + 'http://evil.example.com', + 'http://ca.nodejs.org/ca.cert', + ], + }, + }, + { + text: '1.3.9999.12.34 - URI:http://ca.example.com/', + legacy: { + '1.3.9999.12.34 - URI': [ + 'http://ca.example.com/', + ], + }, + }, + hasOpenSSL3 ? { + text: 'OCSP - othername: XmppAddr::good.example.com\n' + + 'OCSP - othername: 1.3.9999.12.34::abc123\n' + + 'OCSP - othername: SRVName::abc123', + legacy: { + 'OCSP - othername': [ + ' XmppAddr::good.example.com', + ' 1.3.9999.12.34::abc123', + ' SRVName::abc123', + ], + }, + } : { + text: 'OCSP - othername:<unsupported>\n' + + 'OCSP - othername:<unsupported>\n' + + 'OCSP - othername:<unsupported>', + legacy: { + 'OCSP - othername': [ + '<unsupported>', + '<unsupported>', + '<unsupported>', + ], + }, + }, + hasOpenSSL3 ? { + text: null, + legacy: null, + } : { + text: 'OCSP - othername:<unsupported>', + legacy: { + 'OCSP - othername': [ + '<unsupported>', + ] + }, + }, + ]; + + const serverKey = fixtures.readSync('x509-escaping/server-key.pem', 'utf8'); + + for (let i = 0; i < expectedInfoAccess.length; i++) { + const pem = fixtures.readSync(`x509-escaping/info-${i}-cert.pem`, 'utf8'); + const expected = expectedInfoAccess[i]; + + // X509Certificate interface is not supported in v12.x & v14.x. Disable + // checks for cert.infoAccess with expected text. The testcase is ported + // from v17.x + // + // Test the subjectAltName property of the X509Certificate API. + // const cert = new X509Certificate(pem); + // assert.strictEqual(cert.infoAccess, expected.text ? + // `${expected.text}${hasOpenSSL3 ? '' : '\n'}` : + // expected.text); + + // Test that the certificate obtained by checkServerIdentity has the correct + // subjectaltname property. + const server = tls.createServer({ + key: serverKey, + cert: pem, + }, common.mustCall((conn) => { + conn.destroy(); + server.close(); + })).listen(common.mustCall(() => { + const { port } = server.address(); + tls.connect(port, { + ca: pem, + servername: 'example.com', + checkServerIdentity: (hostname, peerCert) => { + assert.strictEqual(hostname, 'example.com'); + assert.deepStrictEqual(peerCert.infoAccess, + expected.legacy ? + Object.assign(Object.create(null), + expected.legacy) : + expected.legacy); + }, + }, common.mustCall()); + })); + } +} + +// The internal parsing logic must match the JSON specification exactly. +{ + // This list is partially based on V8's own JSON tests. + const invalidJSON = [ + '"\\a invalid escape"', + '"\\v invalid escape"', + '"\\\' invalid escape"', + '"\\x42 invalid escape"', + '"\\u202 invalid escape"', + '"\\012 invalid escape"', + '"Unterminated string', + '"Unterminated string\\"', + '"Unterminated string\\\\\\"', + '"\u0000 control character"', + '"\u001e control character"', + '"\u001f control character"', + ]; + + for (const invalidStringLiteral of invalidJSON) { + // Usually, checkServerIdentity returns an error upon verification failure. + // In this case, however, it should throw an error since this is not a + // verification error. Node.js itself will never produce invalid JSON string + // literals, so this can only happen when users construct invalid subject + // alternative name strings (that do not follow escaping rules). + assert.throws(() => { + tls.checkServerIdentity('example.com', { + subjectaltname: `DNS:${invalidStringLiteral}`, + }); + }, { + code: 'ERR_TLS_CERT_ALTNAME_FORMAT', + message: 'Invalid subject alternative name string' + }); + } +} + +// While node does not produce commas within SAN entries, it should parse them +// correctly (i.e., not simply split at commas). +{ + // Regardless of the quotes, splitting this SAN string at commas would + // cause checkServerIdentity to see 'DNS:b.example.com' and thus to accept + // the certificate for b.example.com. + const san = 'DNS:"a.example.com, DNS:b.example.com, DNS:c.example.com"'; + + // This is what node used to do, and which is not correct! + const hostname = 'b.example.com'; + assert.strictEqual(san.split(', ')[1], `DNS:${hostname}`); + + // The new implementation should parse the string correctly. + const err = tls.checkServerIdentity(hostname, { subjectaltname: san }); + assert(err); + assert.strictEqual(err.code, 'ERR_TLS_CERT_ALTNAME_INVALID'); + assert.strictEqual(err.message, 'Hostname/IP does not match certificate\'s ' + + 'altnames: Host: b.example.com. is not in ' + + 'the cert\'s altnames: DNS:"a.example.com, ' + + 'DNS:b.example.com, DNS:c.example.com"'); +} + +// The subject MUST be ignored if a dNSName subject alternative name exists. +{ + const key = fixtures.readKey('incorrect_san_correct_subject-key.pem'); + const cert = fixtures.readKey('incorrect_san_correct_subject-cert.pem'); + + // The hostname is the CN, but not a SAN entry. + const servername = 'good.example.com'; + + // X509Certificate interface is not supported in v12.x & v14.x. Disable + // checks for certX509.subject and certX509.subjectAltName with expected + // value. The testcase is ported from v17.x + // + // const certX509 = new X509Certificate(cert); + // assert.strictEqual(certX509.subject, `CN=${servername}`); + // assert.strictEqual(certX509.subjectAltName, 'DNS:evil.example.com'); + + // Try connecting to a server that uses the self-signed certificate. + const server = tls.createServer({ key, cert }, common.mustNotCall()); + server.listen(common.mustCall(() => { + const { port } = server.address(); + const socket = tls.connect(port, { + ca: cert, + servername, + }, common.mustNotCall()); + socket.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_TLS_CERT_ALTNAME_INVALID'); + assert.strictEqual(err.message, 'Hostname/IP does not match ' + + "certificate's altnames: Host: " + + "good.example.com. is not in the cert's" + + ' altnames: DNS:evil.example.com'); + })); + })).unref(); +} + +// The subject MUST NOT be ignored if no dNSName subject alternative name +// exists, even if other subject alternative names exist. +{ + const key = fixtures.readKey('irrelevant_san_correct_subject-key.pem'); + const cert = fixtures.readKey('irrelevant_san_correct_subject-cert.pem'); + + // The hostname is the CN, but there is no dNSName SAN entry. + const servername = 'good.example.com'; + + // X509Certificate interface is not supported in v12.x & v14.x. Disable + // checks for certX509.subject and certX509.subjectAltName with expected + // value. The testcase is ported from v17.x + // + // const certX509 = new X509Certificate(cert); + // assert.strictEqual(certX509.subject, `CN=${servername}`); + // assert.strictEqual(certX509.subjectAltName, 'IP Address:1.2.3.4'); + + // Connect to a server that uses the self-signed certificate. + const server = tls.createServer({ key, cert }, common.mustCall((socket) => { + socket.destroy(); + server.close(); + })).listen(common.mustCall(() => { + const { port } = server.address(); + tls.connect(port, { + ca: cert, + servername, + }, common.mustCall(() => { + // Do nothing, the server will close the connection. + })); + })); +} |