From 5d86dcf9dc0f592e16c3b013d577a7aec73da522 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 23 Sep 2021 12:34:46 +0200 Subject: [PATCH 1/3] fix: crash creating private key with unsupported algorithm --- patches/node/.patches | 3 ++ ...ivate_key_with_unsupported_algorithm.patch | 48 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 patches/node/fix_crash_creating_private_key_with_unsupported_algorithm.patch diff --git a/patches/node/.patches b/patches/node/.patches index fb143a8de5ddd..6451dfb17487d 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -27,3 +27,6 @@ fix_account_for_debugger_agent_race_condition.patch fix_use_new_v8_error_message_property_access_format.patch add_should_read_node_options_from_env_option_to_disable_node_options.patch repl_fix_crash_when_sharedarraybuffer_disabled.patch +fix_readbarrier_undefined_symbol_error_on_woa_arm64.patch +fix_-wunreachable-code-return.patch +fix_crash_creating_private_key_with_unsupported_algorithm.patch diff --git a/patches/node/fix_crash_creating_private_key_with_unsupported_algorithm.patch b/patches/node/fix_crash_creating_private_key_with_unsupported_algorithm.patch new file mode 100644 index 0000000000000..d8dd565ebcdad --- /dev/null +++ b/patches/node/fix_crash_creating_private_key_with_unsupported_algorithm.patch @@ -0,0 +1,48 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shelley Vohr +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; From 859872e59b69b06a881c1502c860cb4f0f3e6c7f Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 23 Sep 2021 12:39:59 +0200 Subject: [PATCH 2/3] test: add regression test --- patches/node/.patches | 2 -- spec/node-spec.js | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/patches/node/.patches b/patches/node/.patches index 6451dfb17487d..4a210077e91ec 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -27,6 +27,4 @@ fix_account_for_debugger_agent_race_condition.patch fix_use_new_v8_error_message_property_access_format.patch add_should_read_node_options_from_env_option_to_disable_node_options.patch repl_fix_crash_when_sharedarraybuffer_disabled.patch -fix_readbarrier_undefined_symbol_error_on_woa_arm64.patch -fix_-wunreachable-code-return.patch fix_crash_creating_private_key_with_unsupported_algorithm.patch diff --git a/spec/node-spec.js b/spec/node-spec.js index 56cf8c127d91f..4d06e39d445d4 100644 --- a/spec/node-spec.js +++ b/spec/node-spec.js @@ -317,6 +317,21 @@ describe('node feature', () => { // eslint-disable-next-line no-octal 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', () => { From 94ab3951be9b0c6f582edf7749886ff9447f2d9e Mon Sep 17 00:00:00 2001 From: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Date: Mon, 27 Sep 2021 21:07:34 +0000 Subject: [PATCH 3/3] chore: update patches --- ...rash_creating_private_key_with_unsupported_algorithm.patch | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/patches/node/fix_crash_creating_private_key_with_unsupported_algorithm.patch b/patches/node/fix_crash_creating_private_key_with_unsupported_algorithm.patch index d8dd565ebcdad..4fefe192c9b1a 100644 --- a/patches/node/fix_crash_creating_private_key_with_unsupported_algorithm.patch +++ b/patches/node/fix_crash_creating_private_key_with_unsupported_algorithm.patch @@ -19,10 +19,10 @@ 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 +index c24b2d14eb50019d442a32ba92560c8c0c58d2d5..2b120d3c264f44eff452a8f4f8e8da4443bdb6f3 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js -@@ -437,15 +437,19 @@ function getKeyObjectHandleFromJwk(key, ctx) { +@@ -459,15 +459,19 @@ function getKeyObjectHandleFromJwk(key, ctx) { const handle = new KeyObjectHandle(); if (isPublic) {