Add basic impl for Node.js KeyObject equals

Since we're using CryptoKey under the covers of KeyObject, we
need to teach the various CryptoKey::Impl subclasses how to
compare their contents.

.github/secret_scanning.yml

@@ -0,0 +1,2 @@
+paths-ignore:
+  - "src/workerd/api/node/crypto_keys-test.js"

src/workerd/api/crypto-impl-aes.c++

@@ -7,6 +7,7 @@
 #include <cstdint>
 #include <openssl/aes.h>
 #include <openssl/bn.h>
+#include <openssl/crypto.h>
 #include <openssl/err.h>
 #include <workerd/io/io-context.h>
 
@@ -115,6 +116,15 @@ protected:
     return keyAlgorithm.name;
   }
 
+  bool equals(const CryptoKey::Impl& other) const override final {
+    return this == &other || (other.getType() == "secret"_kj && other.equals(keyData));
+  }
+
+  bool equals(const kj::Array<kj::byte>& other) const override final {
+    return keyData.size() == other.size() &&
+           CRYPTO_memcmp(keyData.begin(), other.begin(), keyData.size()) == 0;
+  }
+
 private:
   CryptoKey::AlgorithmVariant getAlgorithm() const override final { return keyAlgorithm; }
 

src/workerd/api/crypto-impl-asymmetric.c++

@@ -180,6 +180,17 @@ public:
 
   EVP_PKEY* getEvpPkey() const { return keyData.get(); }
 
+  bool equals(const CryptoKey::Impl& other) const override final {
+    if (this == &other) return true;
+    KJ_IF_MAYBE(otherImpl, kj::dynamicDowncastIfAvailable<const AsymmetricKey>(other)) {
+      // EVP_PKEY_cmp will return 1 if the inputs match, 0 if they don't match,
+      // -1 if the key types are different, and -2 if the operation is not supported.
+      // We only really care about the first two cases.
+      return EVP_PKEY_cmp(keyData.get(), otherImpl->keyData.get()) == 1;
+    }
+    return false;
+  }
+
 private:
   virtual SubtleCrypto::JsonWebKey exportJwk() const = 0;
   virtual kj::Array<kj::byte> exportRaw() const = 0;

src/workerd/api/crypto-impl-hkdf.c++

@@ -3,6 +3,7 @@
 //     https://opensource.org/licenses/Apache-2.0
 
 #include "crypto-impl.h"
+#include <openssl/crypto.h>
 #include <openssl/hkdf.h>
 
 namespace workerd::api {
@@ -52,6 +53,15 @@ private:
   kj::StringPtr getAlgorithmName() const override { return "HKDF"; }
   CryptoKey::AlgorithmVariant getAlgorithm() const override { return keyAlgorithm; }
 
+  bool equals(const CryptoKey::Impl& other) const override final {
+    return this == &other || (other.getType() == "secret"_kj && other.equals(keyData));
+  }
+
+  bool equals(const kj::Array<kj::byte>& other) const override final {
+    return keyData.size() == other.size() &&
+           CRYPTO_memcmp(keyData.begin(), other.begin(), keyData.size()) == 0;
+  }
+
   kj::Array<kj::byte> keyData;
   CryptoKey::KeyAlgorithm keyAlgorithm;
 };

src/workerd/api/crypto-impl-hmac.c++

@@ -84,6 +84,15 @@ private:
   kj::StringPtr getAlgorithmName() const override { return "HMAC"; }
   CryptoKey::AlgorithmVariant getAlgorithm() const override { return keyAlgorithm; }
 
+  bool equals(const CryptoKey::Impl& other) const override final {
+    return this == &other || (other.getType() == "secret"_kj && other.equals(keyData));
+  }
+
+  bool equals(const kj::Array<kj::byte>& other) const override final {
+    return keyData.size() == other.size() &&
+           CRYPTO_memcmp(keyData.begin(), other.begin(), keyData.size()) == 0;
+  }
+
   kj::Array<kj::byte> keyData;
   CryptoKey::HmacKeyAlgorithm keyAlgorithm;
 };

src/workerd/api/crypto-impl-pbkdf2.c++

@@ -3,6 +3,7 @@
 //     https://opensource.org/licenses/Apache-2.0
 
 #include "crypto-impl.h"
+#include <openssl/crypto.h>
 
 namespace workerd::api {
 namespace {
@@ -66,6 +67,15 @@ private:
   kj::StringPtr getAlgorithmName() const override { return "PBKDF2"; }
   CryptoKey::AlgorithmVariant getAlgorithm() const override { return keyAlgorithm; }
 
+  bool equals(const CryptoKey::Impl& other) const override final {
+    return this == &other || (other.getType() == "secret"_kj && other.equals(keyData));
+  }
+
+  bool equals(const kj::Array<kj::byte>& other) const override final {
+    return keyData.size() == other.size() &&
+           CRYPTO_memcmp(keyData.begin(), other.begin(), keyData.size()) == 0;
+  }
+
   kj::Array<kj::byte> keyData;
   CryptoKey::KeyAlgorithm keyAlgorithm;
 };

src/workerd/api/crypto-impl.c++

@@ -203,4 +203,8 @@ kj::EncodingResult<kj::Array<kj::byte>> decodeBase64Url(kj::String text) {
   return kj::decodeBase64(text);
 }
 
+bool CryptoKey::Impl::equals(const kj::Array<kj::byte>& other) const {
+  KJ_FAIL_REQUIRE("Unable to compare raw key material for this key");
+}
+
 }  // namespace workerd::api

src/workerd/api/crypto-impl.h

@@ -191,6 +191,9 @@ class CryptoKey::Impl {
   virtual AlgorithmVariant getAlgorithm() const = 0;
   virtual kj::StringPtr getType() const { return "secret"_kj; }
 
+  virtual bool equals(const Impl& other) const = 0;
+  virtual bool equals(const kj::Array<kj::byte>& other) const;
+
 private:
   const bool extractable;
   const CryptoKeyUsageSet usages;

src/workerd/api/crypto.c++

@@ -281,6 +281,15 @@ kj::Array<kj::StringPtr> CryptoKey::getUsages() const {
 }
 CryptoKeyUsageSet CryptoKey::getUsageSet() const { return impl->getUsages(); }
 
+bool CryptoKey::operator==(const CryptoKey& other) const {
+  // We check this first because we don't want any comparison to happen if
+  // either key is not extractable, even if they are the same object.
+  if (!getExtractable() || !other.getExtractable()) {
+    return false;
+  }
+  return this == &other || (getType() == other.getType() && impl->equals(*other.impl));
+}
+
 jsg::Promise<kj::Array<kj::byte>> SubtleCrypto::encrypt(
     jsg::Lock& js,
     kj::OneOf<kj::String, EncryptAlgorithm> algorithmParam,

src/workerd/api/crypto.h

@@ -225,6 +225,13 @@ class CryptoKey: public jsg::Object {
   explicit CryptoKey(kj::Own<Impl> impl);
   // Treat as private -- needs to be public for jsg::alloc<T>()...
 
+  bool operator==(const CryptoKey& other) const;
+  // Compare the contents of this key with the other. Will return false if
+  // either key is not extractable or if the keys are a different type.
+  // For secret keys, we will compare only the actual key material and not
+  // the algorithm parameters or the algorithm name. We will also ensure
+  // that a timing-safe comparison is used for the key material.
+
 private:
   kj::Own<Impl> impl;
 

src/workerd/api/node/crypto-keys.c++

@@ -10,7 +10,7 @@ kj::OneOf<kj::String, kj::Array<kj::byte>, SubtleCrypto::JsonWebKey> CryptoImpl:
 }
 
 bool CryptoImpl::equals(jsg::Lock& js, jsg::Ref<CryptoKey> key, jsg::Ref<CryptoKey> otherKey) {
-  KJ_UNIMPLEMENTED("not implemented");
+  return *key == *otherKey;
 }
 
 CryptoImpl::AsymmetricKeyDetails CryptoImpl::getAsymmetricKeyDetail(

src/workerd/api/node/crypto_keys-test.js

@@ -0,0 +1,156 @@
+
+import {
+  strictEqual,
+  ok,
+} from 'node:assert';
+import {
+  KeyObject,
+} from 'node:crypto';
+import {
+  Buffer,
+} from 'node:buffer';
+
+export const secret_key_equals_test = {
+  async test(ctrl, env, ctx) {
+    const secretKeyData1 = Buffer.from('abcdefghijklmnop');
+    const secretKeyData2 = Buffer.from('abcdefghijklmnop'.repeat(2));
+    const aes1 = await crypto.subtle.importKey('raw',
+        secretKeyData1,
+        { name: 'AES-CBC'},
+        true, ['encrypt', 'decrypt']);
+    const aes2 = await crypto.subtle.importKey('raw',
+        secretKeyData1,
+        { name: 'AES-CBC'},
+        true, ['encrypt', 'decrypt']);
+    const aes3 = await crypto.subtle.importKey('raw',
+        secretKeyData2,
+        { name: 'AES-CBC'},
+        true, ['encrypt', 'decrypt']);
+    const hmac = await crypto.subtle.importKey('raw',
+        secretKeyData1,
+        {
+          name: 'HMAC',
+          hash: 'SHA-256',
+        },
+        true, ['sign', 'verify']);
+    const hmac2 = await crypto.subtle.importKey('raw',
+        secretKeyData1,
+        {
+          name: 'HMAC',
+          hash: 'SHA-256',
+        },
+        false, ['sign', 'verify']);
+
+    const aes1_ko = KeyObject.from(aes1);
+    const aes2_ko = KeyObject.from(aes2);
+    const aes3_ko = KeyObject.from(aes3);
+    const hmac_ko = KeyObject.from(hmac);
+    const hmac2_ko = KeyObject.from(hmac2);
+
+    strictEqual(aes1_ko.type, 'secret');
+    strictEqual(aes2_ko.type, 'secret');
+    strictEqual(aes3_ko.type, 'secret');
+    strictEqual(hmac_ko.type, 'secret');
+
+    ok(aes1_ko.equals(aes1_ko));
+    ok(aes1_ko.equals(aes2_ko));
+    ok(aes1_ko.equals(hmac_ko));
+    ok(hmac_ko.equals(aes1_ko));
+    ok(hmac_ko.equals(aes2_ko));
+
+    ok(!aes1_ko.equals(aes3_ko));
+    ok(!hmac_ko.equals(aes3_ko));
+
+    // Unable to determine equality if either key is not extractable.
+    ok(!hmac_ko.equals(hmac2_ko));
+    ok(!hmac2_ko.equals(hmac_ko));
+  }
+};
+
+// In case anyone comes across these tests and wonders why there are
+// keys embedded... these are test keys only. They are not sensitive
+// or secret in any way. They are used only in the test here and are
+// pulled directly from MDN examples.
+
+const jwkEcKey = {
+  crv: "P-384",
+  d: "wouCtU7Nw4E8_7n5C1-xBjB4xqSb_liZhYMsy8MGgxUny6Q8NCoH9xSiviwLFfK_",
+  ext: true,
+  key_ops: ["sign"],
+  kty: "EC",
+  x: "SzrRXmyI8VWFJg1dPUNbFcc9jZvjZEfH7ulKI1UkXAltd7RGWrcfFxqyGPcwu6AQ",
+  y: "hHUag3OvDzEr0uUQND4PXHQTXP5IDGdYhJhL-WLKjnGjQAw0rNGy5V29-aV-yseW",
+};
+
+const pemEncodedKey =
+`
+MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAy3Xo3U13dc+xojwQYWoJLCbOQ5fOVY8LlnqcJm1W1BFtxIhOAJWohiHuIRMctv7d
+zx47TLlmARSKvTRjd0dF92jx/xY20Lz+DXp8YL5yUWAFgA3XkO3LSJ
+gEOex10NB8jfkmgSb7QIudTVvbbUDfd5fwIBmCtaCwWx7NyeWWDb7A
+9cFxj7EjRdrDaK3ux/ToMLHFXVLqSL341TkCf4ZQoz96RFPUGPPLOf
+vN0x66CM1PQCkdhzjE6U5XGE964ZkkYUPPsy6Dcie4obhW4vDjgUmL
+zv0z7UD010RLIneUgDE2FqBfY/C+uWigNPBPkkQ+Bv/UigS6dHqTCV
+eD5wgyBQIDAQAB
+`;
+
+export const asymmetric_key_equals_test = {
+  async test(ctrl, env, ctx) {
+    const jwk1_ec = await crypto.subtle.importKey(
+      'jwk',
+      jwkEcKey,
+      {
+        name: "ECDSA",
+        namedCurve: "P-384",
+      },
+      true,
+      ['sign']
+    );
+    const jwk2_ec = await crypto.subtle.importKey(
+      'jwk',
+      jwkEcKey,
+      {
+        name: "ECDSA",
+        namedCurve: "P-384",
+      },
+      true,
+      ['sign']
+    );
+    strictEqual(jwk1_ec.type, 'private');
+
+    // fetch the part of the PEM string between header and footer
+    const pemContent = Buffer.from(pemEncodedKey, 'base64');
+
+    const rsa = await crypto.subtle.importKey(
+      'spki',
+      pemContent,
+      {
+        name: "RSA-OAEP",
+        hash: "SHA-256",
+      },
+      true,
+      ["encrypt"]
+    );
+    const rsa2 = await crypto.subtle.importKey(
+      'spki',
+      pemContent,
+      {
+        name: "RSA-OAEP",
+        hash: "SHA-256",
+      },
+      false,
+      ["encrypt"]
+    );
+    strictEqual(rsa.type, 'public');
+
+    const jwk1_ko = KeyObject.from(jwk1_ec);
+    const jwk2_ko = KeyObject.from(jwk2_ec);
+    const rsa_ko = KeyObject.from(rsa);
+    const rsa2_ko = KeyObject.from(rsa2);
+
+    ok(jwk1_ko.equals(jwk1_ko));
+    ok(jwk1_ko.equals(jwk2_ko));
+    ok(!rsa_ko.equals(jwk1_ko));
+    ok(!jwk1_ko.equals(rsa_ko));
+    ok(!rsa_ko.equals(rsa2_ko));
+  }
+};

src/workerd/api/node/crypto_keys-test.wd-test

@@ -0,0 +1,15 @@
+using Workerd = import "/workerd/workerd.capnp";
+
+const unitTests :Workerd.Config = (
+  services = [
+    ( name = "crypto_keys-test",
+      worker = (
+        modules = [
+          (name = "worker", esModule = embed "crypto_keys-test.js")
+        ],
+        compatibilityDate = "2023-01-15",
+        compatibilityFlags = ["nodejs_compat", "experimental"]
+      )
+    ),
+  ],
+);