summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--doc/api/tls.md12
-rw-r--r--lib/tls.js21
-rw-r--r--test/fixtures/keys/Makefile14
-rw-r--r--test/fixtures/keys/irrelevant_san_correct_subject-cert.pem11
-rw-r--r--test/fixtures/keys/irrelevant_san_correct_subject-key.pem5
-rw-r--r--test/parallel/test-tls-check-server-identity.js14
-rw-r--r--test/parallel/test-x509-escaping.js31
7 files changed, 80 insertions, 28 deletions
diff --git a/doc/api/tls.md b/doc/api/tls.md
index c45c778da7..cdd678c56e 100644
--- a/doc/api/tls.md
+++ b/doc/api/tls.md
@@ -1460,6 +1460,11 @@ decrease overall server throughput.
<!-- YAML
added: v0.8.4
+changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs-private/node-private/pull/300
+ description: Support for `uniformResourceIdentifier` subject alternative
+ names has been disabled in response to CVE-2021-44531.
-->
* `hostname` {string} The host name or IP address to verify the certificate
@@ -1480,6 +1485,12 @@ the checks done with additional verification.
This function is only called if the certificate passed all other checks, such as
being issued by trusted CA (`options.ca`).
+Earlier versions of Node.js incorrectly accepted certificates for a given
+`hostname` if a matching `uniformResourceIdentifier` subject alternative name
+was present (see [CVE-2021-44531][]). Applications that wish to accept
+`uniformResourceIdentifier` subject alternative names can use a custom
+`options.checkServerIdentity` function that implements the desired behavior.
+
## `tls.connect(options[, callback])`
<!-- YAML
@@ -2143,6 +2154,7 @@ added: v11.4.0
`'TLSv1.3'`. If multiple of the options are provided, the lowest minimum is
used.
+[CVE-2021-44531]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-44531
[Chrome's 'modern cryptography' setting]: https://www.chromium.org/Home/chromium-security/education/tls#TOC-Cipher-Suites
[DHE]: https://en.wikipedia.org/wiki/Diffie%E2%80%93Hellman_key_exchange
[ECDHE]: https://en.wikipedia.org/wiki/Elliptic_curve_Diffie%E2%80%93Hellman
diff --git a/lib/tls.js b/lib/tls.js
index eaba340d57..f17a2fd0c5 100644
--- a/lib/tls.js
+++ b/lib/tls.js
@@ -60,7 +60,6 @@ const net = require('net');
const { getOptionValue } = require('internal/options');
const { getRootCertificates, getSSLCiphers } = internalBinding('crypto');
const { Buffer } = require('buffer');
-const { URL } = require('internal/url');
const { canonicalizeIP } = internalBinding('cares_wrap');
const _tls_common = require('_tls_common');
const _tls_wrap = require('_tls_wrap');
@@ -275,7 +274,6 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
const subject = cert.subject;
const altNames = cert.subjectaltname;
const dnsNames = [];
- const uriNames = [];
const ips = [];
hostname = '' + hostname;
@@ -287,11 +285,6 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
ArrayPrototypeForEach(splitAltNames, (name) => {
if (StringPrototypeStartsWith(name, 'DNS:')) {
ArrayPrototypePush(dnsNames, StringPrototypeSlice(name, 4));
- } else if (StringPrototypeStartsWith(name, 'URI:')) {
- const uri = new URL(StringPrototypeSlice(name, 4));
-
- // TODO(bnoordhuis) Also use scheme.
- ArrayPrototypePush(uriNames, uri.hostname);
} else if (StringPrototypeStartsWith(name, 'IP Address:')) {
ArrayPrototypePush(ips, canonicalizeIP(StringPrototypeSlice(name, 11)));
}
@@ -301,9 +294,6 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
let valid = false;
let reason = 'Unknown reason';
- const hasAltNames =
- dnsNames.length > 0 || ips.length > 0 || uriNames.length > 0;
-
hostname = unfqdn(hostname); // Remove trailing dot for error messages.
if (net.isIP(hostname)) {
@@ -311,15 +301,12 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
if (!valid)
reason = `IP: ${hostname} is not in the cert's list: ` +
ArrayPrototypeJoin(ips, ', ');
- // TODO(bnoordhuis) Also check URI SANs that are IP addresses.
- } else if (hasAltNames || subject) {
+ } else if (dnsNames.length > 0 || subject?.CN) {
const hostParts = splitHost(hostname);
const wildcard = (pattern) => check(hostParts, pattern, true);
- if (hasAltNames) {
- const noWildcard = (pattern) => check(hostParts, pattern, false);
- valid = ArrayPrototypeSome(dnsNames, wildcard) ||
- ArrayPrototypeSome(uriNames, noWildcard);
+ if (dnsNames.length > 0) {
+ valid = ArrayPrototypeSome(dnsNames, wildcard);
if (!valid)
reason =
`Host: ${hostname}. is not in the cert's altnames: ${altNames}`;
@@ -336,7 +323,7 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) {
reason = `Host: ${hostname}. is not cert's CN: ${cn}`;
}
} else {
- reason = 'Cert is empty';
+ reason = 'Cert does not contain a DNS name';
}
if (!valid) {
diff --git a/test/fixtures/keys/Makefile b/test/fixtures/keys/Makefile
index e5dab9e046..71bc36aaa3 100644
--- a/test/fixtures/keys/Makefile
+++ b/test/fixtures/keys/Makefile
@@ -87,6 +87,8 @@ all: \
ec_secp256k1_public.pem \
incorrect_san_correct_subject-cert.pem \
incorrect_san_correct_subject-key.pem \
+ irrelevant_san_correct_subject-cert.pem \
+ irrelevant_san_correct_subject-key.pem \
#
# Create Certificate Authority: ca1
@@ -795,6 +797,18 @@ incorrect_san_correct_subject-cert.pem: incorrect_san_correct_subject-key.pem
incorrect_san_correct_subject-key.pem:
openssl ecparam -name prime256v1 -genkey -noout -out incorrect_san_correct_subject-key.pem
+irrelevant_san_correct_subject-cert.pem: irrelevant_san_correct_subject-key.pem
+ openssl req -x509 \
+ -key irrelevant_san_correct_subject-key.pem \
+ -out irrelevant_san_correct_subject-cert.pem \
+ -sha256 \
+ -days 3650 \
+ -subj "/CN=good.example.com" \
+ -addext "subjectAltName = IP:1.2.3.4"
+
+irrelevant_san_correct_subject-key.pem:
+ openssl ecparam -name prime256v1 -genkey -noout -out irrelevant_san_correct_subject-key.pem
+
clean:
rm -f *.pfx *.pem *.srl ca2-database.txt ca2-serial fake-startcom-root-serial *.print *.old fake-startcom-root-issued-certs/*.pem
@> fake-startcom-root-database.txt
diff --git a/test/fixtures/keys/irrelevant_san_correct_subject-cert.pem b/test/fixtures/keys/irrelevant_san_correct_subject-cert.pem
new file mode 100644
index 0000000000..cdb74b7de3
--- /dev/null
+++ b/test/fixtures/keys/irrelevant_san_correct_subject-cert.pem
@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBnTCCAUKgAwIBAgIUa28EJmmQ7yZOq3WWNP3SLiJnzcAwCgYIKoZIzj0EAwIw
+GzEZMBcGA1UEAwwQZ29vZC5leGFtcGxlLmNvbTAeFw0yMTEyMTExNzE0NDVaFw0z
+MTEyMDkxNzE0NDVaMBsxGTAXBgNVBAMMEGdvb2QuZXhhbXBsZS5jb20wWTATBgcq
+hkjOPQIBBggqhkjOPQMBBwNCAATEKoJfDvKQ6dD+yvc4DaeH0ZlG8VuGJUVi6iIb
+ugY3dKHdmXUIuwwUScgztLc6W8FfvbTxfTF2q90ZBJlr/Klvo2QwYjAdBgNVHQ4E
+FgQUu55oRZI5tdQDmViwAvPEbzZuY2owHwYDVR0jBBgwFoAUu55oRZI5tdQDmViw
+AvPEbzZuY2owDwYDVR0TAQH/BAUwAwEB/zAPBgNVHREECDAGhwQBAgMEMAoGCCqG
+SM49BAMCA0kAMEYCIQDw8z8d7ToB14yxMJxEDF1dhUqMReJFFwPVnvzkr174igIh
+AKJ9XL+02sGOE7xZd5C0KqUXeHoIE9shnejnhm3WBrB/
+-----END CERTIFICATE-----
diff --git a/test/fixtures/keys/irrelevant_san_correct_subject-key.pem b/test/fixtures/keys/irrelevant_san_correct_subject-key.pem
new file mode 100644
index 0000000000..b0a96659c6
--- /dev/null
+++ b/test/fixtures/keys/irrelevant_san_correct_subject-key.pem
@@ -0,0 +1,5 @@
+-----BEGIN EC PRIVATE KEY-----
+MHcCAQEEIDsijdVlHMNTvJ4eqeUbpjMMnl72+HLtEIEcbauckCP6oAoGCCqGSM49
+AwEHoUQDQgAExCqCXw7ykOnQ/sr3OA2nh9GZRvFbhiVFYuoiG7oGN3Sh3Zl1CLsM
+FEnIM7S3OlvBX7208X0xdqvdGQSZa/ypbw==
+-----END EC PRIVATE KEY-----
diff --git a/test/parallel/test-tls-check-server-identity.js b/test/parallel/test-tls-check-server-identity.js
index ad79d93a3d..fe81fc5285 100644
--- a/test/parallel/test-tls-check-server-identity.js
+++ b/test/parallel/test-tls-check-server-identity.js
@@ -134,7 +134,7 @@ const tests = [
{
host: 'a.com',
cert: { },
- error: 'Cert is empty'
+ error: 'Cert does not contain a DNS name'
},
// Empty Subject w/DNS name
@@ -148,7 +148,8 @@ const tests = [
{
host: 'a.b.a.com', cert: {
subjectaltname: 'URI:http://a.b.a.com/',
- }
+ },
+ error: 'Cert does not contain a DNS name'
},
// Multiple CN fields
@@ -265,15 +266,15 @@ const tests = [
host: 'a.b.a.com', cert: {
subjectaltname: 'URI:http://a.b.a.com/',
subject: {}
- }
+ },
+ error: 'Cert does not contain a DNS name'
},
{
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/'
+ error: 'Cert does not contain a DNS name'
},
// IP addresses
{
@@ -281,8 +282,7 @@ const tests = [
subjectaltname: 'IP Address:127.0.0.1',
subject: {}
},
- error: 'Host: a.b.a.com. is not in the cert\'s altnames: ' +
- 'IP Address:127.0.0.1'
+ error: 'Cert does not contain a DNS name'
},
{
host: '127.0.0.1', cert: {
diff --git a/test/parallel/test-x509-escaping.js b/test/parallel/test-x509-escaping.js
index 5904e01e43..5970f24347 100644
--- a/test/parallel/test-x509-escaping.js
+++ b/test/parallel/test-x509-escaping.js
@@ -20,10 +20,6 @@ const { hasOpenSSL3 } = common;
const numLeaves = 5;
for (let i = 0; i < numLeaves; i++) {
- // TODO(tniessen): this test case requires proper handling of URI SANs,
- // which node currently does not implement.
- if (i === 3) continue;
-
const name = `x509-escaping/google/leaf${i}.pem`;
const leafPEM = fixtures.readSync(name, 'utf8');
@@ -336,3 +332,30 @@ const { hasOpenSSL3 } = common;
}));
})).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';
+ 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.
+ }));
+ }));
+}
View Lorry Controller Status