Skip to content

Commit

Permalink
fix: crash creating private key with unsupported algorithm (#31137)
Browse files Browse the repository at this point in the history
* fix: crash creating private key with unsupported algorithm

* test: add regression test

* chore: update patches

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Sep 29, 2021
1 parent da132e0 commit 2cbfd19
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/node/.patches
Expand Up @@ -27,3 +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_crash_creating_private_key_with_unsupported_algorithm.patch
@@ -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 c24b2d14eb50019d442a32ba92560c8c0c58d2d5..2b120d3c264f44eff452a8f4f8e8da4443bdb6f3 100644
--- a/lib/internal/crypto/keys.js
+++ b/lib/internal/crypto/keys.js
@@ -459,15 +459,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;
15 changes: 15 additions & 0 deletions spec/node-spec.js
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 2cbfd19

Please sign in to comment.