node:assert: compare KeyObject and CryptoKey material in deep equality

src/jsc/bindings/bindings.cpp

@@ -134,6 +134,8 @@
 #include "FetchHeaders.h"
 #include "DOMURL.h"
 #include "JSDOMURL.h"
+#include "node/crypto/JSKeyObject.h"
+#include "webcrypto/JSCryptoKey.h"
 
 #include <string_view>
 #include <bun-uws/src/App.h>
@@ -1381,6 +1383,41 @@ static constexpr DeepEqualsMode deepEqualsMode {
     checkPrototypes ? &nonIndexOwnPropertiesEqual<isStrict, enableAsymmetricMatchers, checkPrototypes, skipPrototypeIdentity> : nullptr,
 };
 
+// KeyObject and CryptoKey keep their key material in C++ and have no own
+// properties, so the generic own-property walk would call any two keys equal.
+// Follow node's comparisons.js: a KeyObject compares by type and key material,
+// a CryptoKey also by extractable, algorithm and usages. Returns std::nullopt
+// when neither side is a key, or when the keys match and the own enumerable
+// properties still have to be compared.
+static std::optional<bool> cryptoKeysDequal(const DeepEqualsMode& mode, JSC::JSGlobalObject* globalObject, MarkedArgumentBuffer& gcBuffer, Vector<std::pair<JSC::JSValue, JSC::JSValue>, 16>& stack, ThrowScope& scope, JSC::JSObject* o1, JSC::JSObject* o2)
+{
+    if (auto* keyObject1 = dynamicDowncast<Bun::JSKeyObject>(o1)) {
+        auto* keyObject2 = dynamicDowncast<Bun::JSKeyObject>(o2);
+        if (!keyObject2)
+            return false;
+        bool equal = keyObject1->handle().deepEquals(globalObject, scope, keyObject2->handle());
+        RETURN_IF_EXCEPTION(scope, {});
+        if (!equal)
+            return false;
+        return std::nullopt;
+    }
+    if (auto* cryptoKey1 = dynamicDowncast<WebCore::JSCryptoKey>(o1)) {
+        auto* cryptoKey2 = dynamicDowncast<WebCore::JSCryptoKey>(o2);
+        if (!cryptoKey2)
+            return false;
+        bool equal = Bun::KeyObject::cryptoKeysDeepEqual(globalObject, scope, cryptoKey1, cryptoKey2, [&](JSValue algorithm1, JSValue algorithm2) {
+            return mode.deepEquals(globalObject, algorithm1, algorithm2, gcBuffer, stack, scope, true);
+        });
+        RETURN_IF_EXCEPTION(scope, {});
+        if (!equal)
+            return false;
+        return std::nullopt;
+    }
+    if (o2->inherits<Bun::JSKeyObject>() || o2->inherits<WebCore::JSCryptoKey>())
+        return false;
+    return std::nullopt;
+}
+
 // The per-type comparisons (Map, Set, Date, typed arrays, ...) are compiled once
 // and take the mode at runtime; only the dispatch and the plain-object tail in
 // `specialObjectsDequal` below stay specialised per mode.
@@ -2124,6 +2161,11 @@ std::optional<bool> specialObjectsDequal(JSC::JSGlobalObject* globalObject, Mark
             if (temporalEqual.has_value())
                 return temporalEqual;
 
+            std::optional<bool> keysEqual = cryptoKeysDequal(deepEqualsMode<isStrict, enableAsymmetricMatchers, checkPrototypes, skipPrototypeIdentity>, globalObject, gcBuffer, stack, scope, obj1, obj2);
+            RETURN_IF_EXCEPTION(scope, {});
+            if (keysEqual.has_value())
+                return keysEqual;
+
             const bool isSymbol1 = obj1->inherits<SymbolObject>();
             const bool isBigInt1 = obj1->inherits<BigIntObject>();
             if (isSymbol1 || isBigInt1) {

src/jsc/bindings/node/crypto/KeyObject.cpp

@@ -920,6 +920,46 @@ std::optional<bool> KeyObject::equals(const KeyObject& other) const
     }
 }
 
+bool KeyObject::deepEquals(JSGlobalObject* globalObject, ThrowScope& scope, const KeyObject& other) const
+{
+    std::optional<bool> result = equals(other);
+    if (!result.has_value()) {
+        ERR::CRYPTO_UNSUPPORTED_OPERATION(scope, globalObject);
+        return false;
+    }
+    return *result;
+}
+
+bool KeyObject::cryptoKeysDeepEqual(JSGlobalObject* globalObject, ThrowScope& scope, WebCore::JSCryptoKey* cryptoKey1, WebCore::JSCryptoKey* cryptoKey2, const WTF::Function<bool(JSValue, JSValue)>& algorithmsEqual)
+{
+    auto& key1 = cryptoKey1->wrapped();
+    auto& key2 = cryptoKey2->wrapped();
+    if (key1.type() != key2.type() || key1.extractable() != key2.extractable() || key1.usagesBitmap() != key2.usagesBitmap())
+        return false;
+
+    const Identifier algorithmName = Identifier::fromString(globalObject->vm(), "algorithm"_s);
+    JSValue algorithm1 = cryptoKey1->get(globalObject, algorithmName);
+    RETURN_IF_EXCEPTION(scope, false);
+    JSValue algorithm2 = cryptoKey2->get(globalObject, algorithmName);
+    RETURN_IF_EXCEPTION(scope, false);
+    bool sameAlgorithm = algorithmsEqual(algorithm1, algorithm2);
+    RETURN_IF_EXCEPTION(scope, false);
+    if (!sameAlgorithm)
+        return false;
+
+    auto handle1 = create(key1);
+    if (handle1.hasException()) {
+        WebCore::propagateException(*globalObject, scope, handle1.releaseException());
+        return false;
+    }
+    auto handle2 = create(key2);
+    if (handle2.hasException()) {
+        WebCore::propagateException(*globalObject, scope, handle2.releaseException());
+        return false;
+    }
+    return handle1.returnValue().deepEquals(globalObject, scope, handle2.returnValue());
+}
+
 static std::optional<Vector<uint8_t>> marshalAsymmetricKey(const ncrypto::EVPKeyPointer& pkey, bool isPublic)
 {
     return WebCore::marshalEVPKey(pkey.get(), isPublic);

src/jsc/bindings/node/crypto/KeyObject.h

@@ -8,6 +8,7 @@
 
 namespace WebCore {
 class CryptoKey;
+class JSCryptoKey;
 }
 
 namespace Bun {
@@ -110,6 +111,12 @@ class KeyObject {
     JSC::JSObject* asymmetricKeyDetails(JSC::JSGlobalObject*, JSC::ThrowScope&);
 
     std::optional<bool> equals(const KeyObject& other) const;
+    // `equals` for node's deep equality: throws ERR_CRYPTO_UNSUPPORTED_OPERATION
+    // when the key material cannot be compared, like KeyObject.prototype.equals.
+    bool deepEquals(JSC::JSGlobalObject*, JSC::ThrowScope&, const KeyObject& other) const;
+    // node's deep equality for two CryptoKeys: type, extractable, `algorithm`
+    // (compared by the caller's recursion), usages, and key material.
+    static bool cryptoKeysDeepEqual(JSC::JSGlobalObject*, JSC::ThrowScope&, WebCore::JSCryptoKey*, WebCore::JSCryptoKey*, const WTF::Function<bool(JSC::JSValue, JSC::JSValue)>& algorithmsEqual);
     JSC::JSValue toCryptoKey(JSC::JSGlobalObject*, JSC::ThrowScope&,
         JSC::JSValue algorithmValue, JSC::JSValue extractableValue, JSC::JSValue keyUsagesValue);
 

src/jsc/modules/NodeUtilTypesModule.cpp

@@ -729,18 +729,27 @@ static bool compareBranch(JSC::JSGlobalObject* globalObject, JSC::MarkedArgument
     if (actualIsKeyObject || expectedIsKeyObject) {
         if (!actualIsKeyObject || !expectedIsKeyObject)
             return false;
-        // KeyObject.prototype.equals, called like the JS implementation did.
-        JSC::JSObject* actualObject = actual.getObject();
-        JSValue equalsFunction = actualObject->get(globalObject, JSC::Identifier::fromString(vm, "equals"_s));
+        auto& actualHandle = uncheckedDowncast<Bun::JSKeyObject>(actual.asCell())->handle();
+        auto& expectedHandle = uncheckedDowncast<Bun::JSKeyObject>(expected.asCell())->handle();
+        bool equal = actualHandle.deepEquals(globalObject, scope, expectedHandle);
         RETURN_IF_EXCEPTION(scope, false);
-        auto callData = JSC::getCallData(equalsFunction);
-        if (callData.type == JSC::CallData::Type::None)
+        if (!equal)
             return false;
-        JSC::MarkedArgumentBuffer args;
-        args.append(expected);
-        JSValue result = JSC::call(globalObject, equalsFunction, callData, actual, args);
+        return withCycleGuard(globalObject, gcBuffer, cycles, scope, actual, expected, objectSubset);
+    }
+
+    const bool actualIsCryptoKey = actual.isCell() && actual.asCell()->inherits<WebCore::JSCryptoKey>();
+    const bool expectedIsCryptoKey = expected.isCell() && expected.asCell()->inherits<WebCore::JSCryptoKey>();
+    if (actualIsCryptoKey || expectedIsCryptoKey) {
+        if (!actualIsCryptoKey || !expectedIsCryptoKey)
+            return false;
+        auto* actualKey = uncheckedDowncast<WebCore::JSCryptoKey>(actual.asCell());
+        auto* expectedKey = uncheckedDowncast<WebCore::JSCryptoKey>(expected.asCell());
+        bool equal = Bun::KeyObject::cryptoKeysDeepEqual(globalObject, scope, actualKey, expectedKey, [&](JSValue actualAlgorithm, JSValue expectedAlgorithm) {
+            return compareBranch(globalObject, gcBuffer, cycles, scope, actualAlgorithm, expectedAlgorithm);
+        });
         RETURN_IF_EXCEPTION(scope, false);
-        if (!result.toBoolean(globalObject))
+        if (!equal)
             return false;
         return withCycleGuard(globalObject, gcBuffer, cycles, scope, actual, expected, objectSubset);
     }

test/js/bun/bun-object/deep-equals.test.ts

@@ -1,4 +1,5 @@
 import { bunEnv, bunExe, isASAN, isWindows } from "harness";
+import { createSecretKey } from "node:crypto";
 import vm from "node:vm";
 
 describe.each([true, false])("Bun.deepEquals(a, b, strict: %p)", strict => {
@@ -49,6 +50,35 @@ describe.each([true, false])("Bun.deepEquals(a, b, strict: %p)", strict => {
     expect(deepEquals(b, a)).toBe(false);
   });
 
+  // KeyObject and CryptoKey have no own properties; compare the key material,
+  // like node's assert and util do.
+  it("KeyObjects and CryptoKeys compare by key material", () => {
+    const keyA = createSecretKey(Buffer.from("secret-A"));
+    const keyA2 = createSecretKey(Buffer.from("secret-A"));
+    const keyB = createSecretKey(Buffer.from("secret-B"));
+    expect(deepEquals(keyA, keyA2)).toBe(true);
+    expect(deepEquals(keyA, keyB)).toBe(false);
+    expect(deepEquals({ k: keyA }, { k: keyB })).toBe(false);
+    expect(deepEquals(keyA, {})).toBe(false);
+    expect(deepEquals({}, keyA)).toBe(false);
+    expect(keyA).toEqual(keyA2);
+    expect(keyA).not.toEqual(keyB);
+    expect(keyA).not.toStrictEqual(keyB);
+
+    const hmac = { name: "HMAC", hash: "SHA-256" };
+    const cryptoKeyA = keyA.toCryptoKey(hmac, true, ["sign"]);
+    const cryptoKeyA2 = keyA2.toCryptoKey(hmac, true, ["sign"]);
+    const cryptoKeyB = keyB.toCryptoKey(hmac, true, ["sign"]);
+    expect(deepEquals(cryptoKeyA, cryptoKeyA2)).toBe(true);
+    expect(deepEquals(cryptoKeyA, cryptoKeyB)).toBe(false);
+    expect(deepEquals(cryptoKeyA, keyA.toCryptoKey(hmac, false, ["sign"]))).toBe(false);
+    expect(deepEquals(cryptoKeyA, keyA.toCryptoKey(hmac, true, ["verify"]))).toBe(false);
+    expect(deepEquals(cryptoKeyA, keyA.toCryptoKey({ name: "HMAC", hash: "SHA-512" }, true, ["sign"]))).toBe(false);
+    expect(deepEquals(cryptoKeyA, keyA)).toBe(false);
+    expect(cryptoKeyA).toEqual(cryptoKeyA2);
+    expect(cryptoKeyA).not.toEqual(cryptoKeyB);
+  });
+
   // we may change this in the future
   it("functions that are not reference-equal are never equal", () => {
     function foo() {}

test/js/node/assert/deep-equal.test.ts

@@ -3,6 +3,7 @@
 // cross-checked against Node.js. Cases Bun gets wrong today are marked `test.failing`.
 import { describe, expect, test } from "bun:test";
 import assert from "node:assert";
+import { createPublicKey, createSecretKey, generateKeyPairSync } from "node:crypto";
 import util from "node:util";
 
 type Thunk = () => unknown;
@@ -25,6 +26,20 @@ interface Case {
 const sym = Symbol("shared");
 const sharedArrayBuffer = new ArrayBuffer(4);
 
+const secretKeyA = createSecretKey(Buffer.from("secret-A"));
+const secretKeyA2 = createSecretKey(Buffer.from("secret-A"));
+const secretKeyB = createSecretKey(Buffer.from("secret-B"));
+const ecPairA = generateKeyPairSync("ec", { namedCurve: "P-256" });
+const ecPairB = generateKeyPairSync("ec", { namedCurve: "P-256" });
+const ecPublicA2 = createPublicKey(ecPairA.publicKey.export({ type: "spki", format: "pem" }));
+const hmac256 = { name: "HMAC", hash: "SHA-256" };
+const cryptoKeyA = secretKeyA.toCryptoKey(hmac256, true, ["sign"]);
+const cryptoKeyA2 = secretKeyA2.toCryptoKey(hmac256, true, ["sign"]);
+const cryptoKeyB = secretKeyB.toCryptoKey(hmac256, true, ["sign"]);
+const cryptoKeyANotExtractable = secretKeyA.toCryptoKey(hmac256, false, ["sign"]);
+const cryptoKeyAVerify = secretKeyA.toCryptoKey(hmac256, true, ["verify"]);
+const cryptoKeyASha512 = secretKeyA.toCryptoKey({ name: "HMAC", hash: "SHA-512" }, true, ["sign"]);
+
 function float64WithNaNPayload(bits: bigint) {
   const arr = new Float64Array(1);
   new BigUint64Array(arr.buffer)[0] = bits;
@@ -943,6 +958,102 @@ const cases: Case[] = [
     strict: true,
     loose: true,
   },
+
+  // KeyObject and CryptoKey: node compares the key material (comparisons.js isKeyObject /
+  // isCryptoKey arms), not the (empty) set of own properties.
+  {
+    name: "two secret keys with the same material",
+    a: () => secretKeyA,
+    b: () => secretKeyA2,
+    strict: true,
+    loose: true,
+  },
+  {
+    name: "two secret keys with different material",
+    a: () => secretKeyA,
+    b: () => secretKeyB,
+    strict: false,
+    loose: false,
+  },
+  {
+    name: "objects holding secret keys with different material",
+    a: () => ({ k: secretKeyA }),
+    b: () => ({ k: secretKeyB }),
+    strict: false,
+    loose: false,
+  },
+  {
+    name: "a secret key with an extra own property and a plain one",
+    a: () => Object.assign(createSecretKey(Buffer.from("secret-A")), { x: 1 }),
+    b: () => secretKeyA,
+    strict: false,
+    loose: false,
+  },
+  { name: "a secret key and an empty object", a: () => secretKeyA, b: () => ({}), strict: false, loose: false },
+  {
+    name: "two public keys with the same material",
+    a: () => ecPairA.publicKey,
+    b: () => ecPublicA2,
+    strict: true,
+    loose: true,
+  },
+  {
+    name: "two public keys from different key pairs",
+    a: () => ecPairA.publicKey,
+    b: () => ecPairB.publicKey,
+    strict: false,
+    loose: false,
+  },
+  {
+    name: "two private keys from different key pairs",
+    a: () => ecPairA.privateKey,
+    b: () => ecPairB.privateKey,
+    strict: false,
+    loose: false,
+  },
+  {
+    name: "a public key and the private key of the same pair",
+    a: () => ecPairA.publicKey,
+    b: () => ecPairA.privateKey,
+    strict: false,
+    loose: false,
+  },
+  {
+    name: "two CryptoKeys with the same material",
+    a: () => cryptoKeyA,
+    b: () => cryptoKeyA2,
+    strict: true,
+    loose: true,
+  },
+  {
+    name: "two CryptoKeys with different material",
+    a: () => cryptoKeyA,
+    b: () => cryptoKeyB,
+    strict: false,
+    loose: false,
+  },
+  {
+    name: "two CryptoKeys that differ only in extractable",
+    a: () => cryptoKeyA,
+    b: () => cryptoKeyANotExtractable,
+    strict: false,
+    loose: false,
+  },
+  {
+    name: "two CryptoKeys that differ only in usages",
+    a: () => cryptoKeyA,
+    b: () => cryptoKeyAVerify,
+    strict: false,
+    loose: false,
+  },
+  {
+    name: "two CryptoKeys that differ only in algorithm",
+    a: () => cryptoKeyA,
+    b: () => cryptoKeyASha512,
+    strict: false,
+    loose: false,
+  },
+  { name: "a CryptoKey and a KeyObject", a: () => cryptoKeyA, b: () => secretKeyA, strict: false, loose: false },
 ];
 
 function caught(fn: () => void): (Error & { code?: string }) | null {
@@ -1119,6 +1230,15 @@ describe("detached ArrayBuffer", () => {
     ).not.toThrow();
   });
 
+  test("assert.partialDeepStrictEqual compares CryptoKey material, algorithm, usages and extractable", () => {
+    expect(() => assert.partialDeepStrictEqual(cryptoKeyA, cryptoKeyA2)).not.toThrow();
+    expect(() => assert.partialDeepStrictEqual(cryptoKeyA, cryptoKeyB)).toThrow(assert.AssertionError);
+    expect(() => assert.partialDeepStrictEqual(cryptoKeyA, cryptoKeyASha512)).toThrow(assert.AssertionError);
+    expect(() => assert.partialDeepStrictEqual(cryptoKeyA, cryptoKeyAVerify)).toThrow(assert.AssertionError);
+    expect(() => assert.partialDeepStrictEqual(cryptoKeyA, cryptoKeyANotExtractable)).toThrow(assert.AssertionError);
+    expect(() => assert.partialDeepStrictEqual(cryptoKeyA, secretKeyA)).toThrow(assert.AssertionError);
+  });
+
   test("error matches Node's message", () => {
     const error = caught(() => assert.deepStrictEqual(detached(), new ArrayBuffer(0)));
     expect(error).toBeInstanceOf(TypeError);