fix: crash creating private key with unsupported algorithm (#31087)

* fix: crash creating private key with unsupported algorithm

* test: add regression test
This commit is contained in:
Shelley Vohr 2021-09-27 15:02:13 +02:00 committed by GitHub
parent 2360012cad
commit 25d0963d9b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 64 additions and 0 deletions

View file

@ -25,3 +25,4 @@ add_should_read_node_options_from_env_option_to_disable_node_options.patch
repl_fix_crash_when_sharedarraybuffer_disabled.patch repl_fix_crash_when_sharedarraybuffer_disabled.patch
fix_readbarrier_undefined_symbol_error_on_woa_arm64.patch fix_readbarrier_undefined_symbol_error_on_woa_arm64.patch
fix_-wunreachable-code-return.patch fix_-wunreachable-code-return.patch
fix_crash_creating_private_key_with_unsupported_algorithm.patch

View file

@ -0,0 +1,48 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Thu, 23 Sep 2021 12:29:23 +0200
Subject: fix: crash creating private key with unsupported algorithm
This patch fixes an issue where some calls to crypto.createPrivateKey
made with algorithms unsupported by BoringSSL cause a crash when invoking
methods on their return values. This was happening because BoringSSL
shims some algorithms but doesn't implement them and so attempted to
created keys with them will fail (see https://source.chromium.org/chromium/chromium/src/+/main:third_party/boringssl/src/include/openssl/evp.h;l=835-837?q=ed448&ss=chromium)
Node.js returned false in initEdRaw (see: https://github.com/nodejs/node/blob/20cf47004e7801ede1588d2de8785c0100f6ab38/src/crypto/crypto_keys.cc#L1106)
but then did nothing with the return value, meaning that if no pkey was
created successfully that a key object was still returned but attempts
to use the data_ field would crash the process as data_ was never
assigned. This is fixed by checking the return value of initEdRaw at the
JavaScript level and throwing an error if the function returns false.
This patch will be upstreamed in some form.
diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
index 4d5545ab1aae1ad392ac1fea878b887f4a8736c4..bde806e71a7b299ec2855de8e78c3e9ab449f508 100644
--- a/lib/internal/crypto/keys.js
+++ b/lib/internal/crypto/keys.js
@@ -437,15 +437,19 @@ function getKeyObjectHandleFromJwk(key, ctx) {
const handle = new KeyObjectHandle();
if (isPublic) {
- handle.initEDRaw(
+ if (!handle.initEDRaw(
`NODE-${key.crv.toUpperCase()}`,
keyData,
- kKeyTypePublic);
+ kKeyTypePublic)) {
+ throw new Error('Failed to create key - unsupported algorithm');
+ }
} else {
- handle.initEDRaw(
+ if (!handle.initEDRaw(
`NODE-${key.crv.toUpperCase()}`,
keyData,
- kKeyTypePrivate);
+ kKeyTypePrivate)) {
+ throw new Error('Failed to create key - unsupported algorithm');
+ }
}
return handle;

View file

@ -317,6 +317,21 @@ describe('node feature', () => {
// eslint-disable-next-line no-octal // eslint-disable-next-line no-octal
crypto.createDiffieHellman('abc', '123'); crypto.createDiffieHellman('abc', '123');
}); });
it('does not crash when calling crypto.createPrivateKey() with an unsupported algorithm', () => {
const crypto = require('crypto');
const ed448 = {
crv: 'Ed448',
x: 'KYWcaDwgH77xdAwcbzOgvCVcGMy9I6prRQBhQTTdKXUcr-VquTz7Fd5adJO0wT2VHysF3bk3kBoA',
d: 'UhC3-vN5vp_g9PnTknXZgfXUez7Xvw-OfuJ0pYkuwzpYkcTvacqoFkV_O05WMHpyXkzH9q2wzx5n',
kty: 'OKP'
};
expect(() => {
crypto.createPrivateKey({ key: ed448, format: 'jwk' });
}).to.throw(/Failed to create key - unsupported algorithm/);
});
}); });
describe('process.stdout', () => { describe('process.stdout', () => {