Browse Source

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

* fix: crash creating private key with unsupported algorithm

* test: add regression test

* chore: update patches

Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
trop[bot] 3 years ago
parent
commit
2cbfd19d94

+ 1 - 0
patches/node/.patches

@@ -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

+ 48 - 0
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 <[email protected]>
+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 - 0
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', () => {