crypto: ensure valid point on elliptic curve in SubtleCrypto.importKey

PR-URL: #50234 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Nov 11, 2023 · 773320e · 773320e
1 parent a589a1a
commit 773320e
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 10 deletions.
diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js
@@ -264,6 +264,10 @@ async function ecImportKey(
       break;
   }
 
+  if (!keyObject[kHandle].checkEcKeyData()) {
+    throw lazyDOMException('Invalid keyData', 'DataError');
+  }
+
   const {
     namedCurve: checkNamedCurve,
   } = keyObject[kHandle].keyDetail({});

diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc
@@ -907,6 +907,8 @@ v8::Local<v8::Function> KeyObjectHandle::Initialize(Environment* env) {
         isolate, templ, "getSymmetricKeySize", GetSymmetricKeySize);
     SetProtoMethodNoSideEffect(
         isolate, templ, "getAsymmetricKeyType", GetAsymmetricKeyType);
+    SetProtoMethodNoSideEffect(
+        isolate, templ, "checkEcKeyData", CheckEcKeyData);
     SetProtoMethod(isolate, templ, "export", Export);
     SetProtoMethod(isolate, templ, "exportJwk", ExportJWK);
     SetProtoMethod(isolate, templ, "initECRaw", InitECRaw);
@@ -926,6 +928,7 @@ void KeyObjectHandle::RegisterExternalReferences(
   registry->Register(Init);
   registry->Register(GetSymmetricKeySize);
   registry->Register(GetAsymmetricKeyType);
+  registry->Register(CheckEcKeyData);
   registry->Register(Export);
   registry->Register(ExportJWK);
   registry->Register(InitECRaw);
@@ -1237,6 +1240,34 @@ void KeyObjectHandle::GetAsymmetricKeyType(
   args.GetReturnValue().Set(key->GetAsymmetricKeyType());
 }
 
+bool KeyObjectHandle::CheckEcKeyData() const {
+  MarkPopErrorOnReturn mark_pop_error_on_return;
+
+  const ManagedEVPPKey& key = data_->GetAsymmetricKey();
+  KeyType type = data_->GetKeyType();
+  CHECK_NE(type, kKeyTypeSecret);
+  EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(key.get(), nullptr));
+  CHECK(ctx);
+  CHECK_EQ(EVP_PKEY_id(key.get()), EVP_PKEY_EC);
+
+  if (type == kKeyTypePrivate) {
+    return EVP_PKEY_check(ctx.get()) == 1;
+  }
+
+#if OPENSSL_VERSION_MAJOR >= 3
+  return EVP_PKEY_public_check_quick(ctx.get()) == 1;
+#else
+  return EVP_PKEY_public_check(ctx.get()) == 1;
+#endif
+}
+
+void KeyObjectHandle::CheckEcKeyData(const FunctionCallbackInfo<Value>& args) {
+  KeyObjectHandle* key;
+  ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder());
+
+  args.GetReturnValue().Set(key->CheckEcKeyData());
+}
+
 void KeyObjectHandle::GetSymmetricKeySize(
     const FunctionCallbackInfo<Value>& args) {
   KeyObjectHandle* key;

diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h
@@ -193,6 +193,9 @@ class KeyObjectHandle : public BaseObject {
       const v8::FunctionCallbackInfo<v8::Value>& args);
   v8::Local<v8::Value> GetAsymmetricKeyType() const;
 
+  static void CheckEcKeyData(const v8::FunctionCallbackInfo<v8::Value>& args);
+  bool CheckEcKeyData() const;
+
   static void GetSymmetricKeySize(
       const v8::FunctionCallbackInfo<v8::Value>& args);
 

diff --git a/test/parallel/test-webcrypto-export-import-ec.js b/test/parallel/test-webcrypto-export-import-ec.js
@@ -400,15 +400,81 @@ async function testImportRaw({ name, publicUsages }, namedCurve) {
     ['ECDSA', ['verify'], ['sign']],
     ['ECDH', [], ['deriveBits', 'deriveBits']],
   ]) {
-    assert.rejects(subtle.importKey(
-      'spki',
-      rsaPublic.export({ format: 'der', type: 'spki' }),
-      { name, hash: 'SHA-256', namedCurve: 'P-256' },
-      true, publicUsages), { message: /Invalid key type/ });
-    assert.rejects(subtle.importKey(
-      'pkcs8',
-      rsaPrivate.export({ format: 'der', type: 'pkcs8' }),
-      { name, hash: 'SHA-256', namedCurve: 'P-256' },
-      true, privateUsages), { message: /Invalid key type/ });
+    assert.rejects(
+      subtle.importKey(
+        'spki',
+        rsaPublic.export({ format: 'der', type: 'spki' }),
+        { name, hash: 'SHA-256', namedCurve: 'P-256' },
+        true, publicUsages), { message: /Invalid key type/ },
+    ).then(common.mustCall());
+    assert.rejects(
+      subtle.importKey(
+        'pkcs8',
+        rsaPrivate.export({ format: 'der', type: 'pkcs8' }),
+        { name, hash: 'SHA-256', namedCurve: 'P-256' },
+        true, privateUsages), { message: /Invalid key type/ },
+    ).then(common.mustCall());
+  }
+}
+
+// Bad private keys
+{
+  for (const { namedCurve, key: pkcs8 } of [
+    // The private key is exactly equal to the order, and the public key is
+    // private key * order.
+    {
+      namedCurve: 'P-256',
+      key: Buffer.from(
+        '3066020100301306072a8648ce3d020106082a8648ce3d030107044c304a0201' +
+        '010420ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc' +
+        '632551a12303210000ffffff00000000ffffffffffffffffbce6faada7179e84' +
+        'f3b9cac2fc632551', 'hex'),
+    },
+    // The private key is exactly equal to the order, and the public key is
+    // omitted.
+    {
+      namedCurve: 'P-256',
+      key: Buffer.from(
+        '3041020100301306072a8648ce3d020106082a8648ce3d030107042730250201' +
+        '010420ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc' +
+        '632551', 'hex'),
+    },
+    // The private key is exactly equal to the order + 11, and the public key is
+    // private key * order.
+    {
+      namedCurve: 'P-521',
+      key: Buffer.from(
+        '3081ee020100301006072a8648ce3d020106052b810400230481d63081d30201' +
+        '01044201ffffffffffffffffffffffffffffffffffffffffffffffffffffffff' +
+        'fffffffffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb7' +
+        '1e91386414a181890381860004008a75841259fdedff546f1a39573b4315cfed' +
+        '5dc7ed7c17849543ef2c54f2991652f3dbc5332663da1bd19b1aebe319108501' +
+        '5c024fa4c9a902ecc0e02dda0cdb9a0096fb303fcbba2129849d0ca877054fb2' +
+        '293add566210bd0493ed2e95d4e0b9b82b1bc8a90e8b42a4ab3892331914a953' +
+        '36dcac80e3f4819b5d58874f92ce48c808', 'hex'),
+    },
+    // The private key is exactly equal to the order + 11, and the public key is
+    // omitted.
+    {
+      namedCurve: 'P-521',
+      key: Buffer.from(
+        '3060020100301006072a8648ce3d020106052b81040023044930470201010442' +
+        '01ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' +
+        'fffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb71e9138' +
+        '6414', 'hex'),
+    },
+  ]) {
+    for (const [name, privateUsages] of [
+      ['ECDSA', ['sign']],
+      ['ECDH', ['deriveBits', 'deriveBits']],
+    ]) {
+      assert.rejects(
+        subtle.importKey(
+          'pkcs8',
+          pkcs8,
+          { name, hash: 'SHA-256', namedCurve },
+          true, privateUsages), { name: 'DataError', message: /Invalid keyData/ },
+      ).then(common.mustCall());
+    }
   }
 }