fix(crypto): KeyObject.from() must handle asymmetric and unregistered-buffer CryptoKeys (#6302)

crates/perry-runtime/src/buffer/header.rs

@@ -56,6 +56,32 @@ fn external_crypto_keys() -> &'static Mutex<HashMap<usize, CryptoKeyMeta>> {
     EXTERNAL_CRYPTO_KEY_META_REGISTRY.get_or_init(|| Mutex::new(HashMap::new()))
 }
 
+/// Called by the GC's buffer sweep when a CryptoKey-flagged `BufferHeader`
+/// dies, so perry-stdlib can drop the matching entry from its own
+/// `addr -> CryptoKeyMaterial` map. Registered by
+/// `js_set_crypto_key_death_hook` at startup; stays null when stdlib isn't
+/// linked. Must not allocate — it runs inside the sweep.
+pub type CryptoKeyDeathHookFn = extern "C" fn(usize);
+static CRYPTO_KEY_DEATH_HOOK: std::sync::atomic::AtomicPtr<()> =
+    std::sync::atomic::AtomicPtr::new(std::ptr::null_mut());
+
+/// Install the dead-CryptoKey callback (called by perry-stdlib at startup —
+/// this crate can't call into perry-stdlib, which depends on it). Same
+/// contract as the `js_set_native_*_dispatch` family in `value::handle`.
+#[no_mangle]
+pub extern "C" fn js_set_crypto_key_death_hook(func: CryptoKeyDeathHookFn) {
+    CRYPTO_KEY_DEATH_HOOK.store(func as *mut (), std::sync::atomic::Ordering::SeqCst);
+}
+
+fn notify_crypto_key_death(addr: usize) {
+    let ptr = CRYPTO_KEY_DEATH_HOOK.load(std::sync::atomic::Ordering::SeqCst);
+    if ptr.is_null() {
+        return;
+    }
+    let hook: CryptoKeyDeathHookFn = unsafe { std::mem::transmute(ptr) };
+    hook(addr);
+}
+
 pub type CryptoKeyMeta = (u8, u8, u8, bool, u32);
 
 thread_local! {
@@ -490,6 +516,50 @@ pub(crate) fn finalize_collected_dead_buffer(addr: usize) {
     BUFFER_AB_ALIAS.with(|r| {
         r.borrow_mut().remove(&addr);
     });
+    // The WebCrypto/KeyObject side tables were missing from this list. They are
+    // plain `addr -> metadata` maps that do not root the `BufferHeader`, so a
+    // collected CryptoKey/secret-key buffer left its entries behind forever.
+    // Two consequences, both real:
+    //
+    //  * an unbounded leak — every CryptoKey ever created kept an entry in the
+    //    thread-local map AND in the process-global one (a 60k-key run leaked
+    //    59,998 of them);
+    //  * the #6080 ABA class this very function exists to prevent: the old
+    //    arena resets a fully-empty block's offset to 0 while keeping its base
+    //    pointer (`arena_reset_empty_blocks` + the block-reuse forward scan in
+    //    `Arena::alloc`), so a recycled address inherits CryptoKey identity.
+    //    `crypto_key_meta`/`is_secret_key` gate `instanceof CryptoKey`,
+    //    `util.types.isCryptoKey`/`isKeyObject`, the `[object CryptoKey]` tag,
+    //    the `.algorithm`/`.type`/`.usages` property surface, `KeyObject.from`
+    //    and `.export()` — an unrelated fresh Buffer landing on a dead key's
+    //    address would answer to all of them.
+    CRYPTO_KEY_META_REGISTRY.with(|r| {
+        r.borrow_mut().remove(&addr);
+    });
+    SECRET_KEY_REGISTRY.with(|r| {
+        r.borrow_mut().remove(&addr);
+    });
+    UINT8ARRAY_FROM_CTOR.with(|r| {
+        r.borrow_mut().remove(&addr);
+    });
+    // `js_buffer_mark_as_crypto_key_external` writes all three global maps, and
+    // `is_registered_buffer`/`is_uint8array_buffer` consult them, so a dead
+    // external key buffer has to be dropped from every one of them.
+    if let Ok(mut r) = external_buffers().lock() {
+        r.remove(&addr);
+    }
+    if let Ok(mut r) = external_uint8arrays().lock() {
+        r.remove(&addr);
+    }
+    if let Ok(mut r) = external_crypto_keys().lock() {
+        r.remove(&addr);
+    }
+    // perry-stdlib keeps its own `addr -> CryptoKeyMaterial` map (the primary
+    // one `lookup_crypto_key` consults; the runtime table above is only its
+    // fallback), and this crate cannot call into perry-stdlib. Notify it
+    // through the hook it installs at startup. The callback only removes a
+    // HashMap entry — no allocation, so it is safe to run inside the sweep.
+    notify_crypto_key_death(addr);
     super::detach::remove_detached_entry_for_dead_buffer(addr);
     super::view::remove_entries_for_dead_buffer(addr);
 }

crates/perry-runtime/src/buffer/mod.rs

@@ -36,9 +36,10 @@ pub use header::{
     asymmetric_key_meta, buffer_ab_alias, buffer_alloc, buffer_backing_array_buffer,
     buffer_byte_offset, buffer_data, buffer_data_mut, crypto_key_meta, ensure_buffer_ab_alias,
     is_any_array_buffer, is_array_buffer, is_data_view, is_registered_buffer, is_secret_key,
-    is_shared_array_buffer, is_uint8array_buffer, mark_as_array_buffer, mark_as_asymmetric_key,
-    mark_as_crypto_key, mark_as_data_view, mark_as_secret_key, mark_as_shared_array_buffer,
-    mark_as_uint8array, register_buffer, resolve_buffer_ab_alias, set_buffer_ab_alias,
+    is_shared_array_buffer, is_uint8array_buffer, js_set_crypto_key_death_hook,
+    mark_as_array_buffer, mark_as_asymmetric_key, mark_as_crypto_key, mark_as_data_view,
+    mark_as_secret_key, mark_as_shared_array_buffer, mark_as_uint8array, register_buffer,
+    resolve_buffer_ab_alias, set_buffer_ab_alias, CryptoKeyDeathHookFn,
 };
 pub(crate) use header::{
     collect_dead_registered_buffers_post_trace, finalize_collected_dead_buffer,
@@ -146,6 +147,51 @@ pub use iter::{
 mod tests {
     use super::*;
 
+    /// The GC buffer sweep must drop the CryptoKey/secret-key side tables
+    /// along with the buffer identity ones. They are plain `addr -> metadata`
+    /// maps that never rooted the `BufferHeader`, so leaving them behind both
+    /// leaked an entry per key and let a recycled address inherit CryptoKey
+    /// identity (`crypto_key_meta`/`is_secret_key` gate `instanceof CryptoKey`,
+    /// `util.types.isCryptoKey`, `KeyObject.from`, `.export()` …) — the #6080
+    /// ABA class this finalizer exists to prevent.
+    #[test]
+    fn test_dead_buffer_finalize_prunes_crypto_key_side_tables() {
+        let buf = buffer_alloc(32);
+        assert!(!buf.is_null());
+        let addr = buf as usize;
+
+        // Shape a WebCrypto secret CryptoKey: HMAC / SHA-256 / secret.
+        mark_as_uint8array(addr);
+        mark_as_crypto_key(addr, 1, 2, 1);
+        mark_as_secret_key(addr);
+
+        assert!(crypto_key_meta(addr).is_some(), "meta registered");
+        assert!(is_secret_key(addr), "secret-key flag registered");
+        assert!(is_uint8array_buffer(addr), "uint8array flag registered");
+        assert!(is_registered_buffer(addr), "buffer registered");
+
+        // Exactly what the sweep subphase runs once the header is proven dead.
+        finalize_collected_dead_buffer(addr);
+
+        assert!(
+            crypto_key_meta(addr).is_none(),
+            "dead buffer must not keep CryptoKey metadata — a recycled address \
+             would answer to instanceof CryptoKey / KeyObject.from()"
+        );
+        assert!(
+            !is_secret_key(addr),
+            "dead buffer must not keep the secret-key flag"
+        );
+        assert!(
+            !is_uint8array_buffer(addr),
+            "dead buffer must not keep the uint8array flag"
+        );
+        assert!(
+            !is_registered_buffer(addr),
+            "dead buffer must not stay registered"
+        );
+    }
+
     #[test]
     fn test_small_buffer_slab_unique_addresses() {
         // Every allocation must occupy a distinct address (no overlap).

crates/perry-runtime/src/object/native_module_crypto_key_object.rs

@@ -17,28 +17,99 @@ fn invalid_key(value: f64) -> ! {
     crate::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_TYPE")
 }
 
+/// A CryptoKey Perry recognises but cannot turn into a KeyObject must still
+/// fail loudly. Returning `undefined` from a factory like this is what #6302
+/// was filed about.
+fn conversion_failed(detail: &str) -> ! {
+    let message = format!("KeyObject.from() failed: {detail}");
+    crate::fs::validate::throw_error_with_code(&message, "ERR_CRYPTO_OPERATION_FAILED")
+}
+
+/// `crypto.KeyObject.from(cryptoKey)` (#2565; asymmetric + unregistered-buffer
+/// keys #6302).
+///
+/// Node converts *any* CryptoKey — secret, public, or private — into the
+/// matching KeyObject, and throws `ERR_INVALID_ARG_TYPE` for everything else.
+/// Perry models the two KeyObject shapes differently:
+///
+/// * **secret** — the key bytes in a `BufferHeader` flagged by
+///   `mark_as_secret_key`, which is what the KeyObject property/method surface
+///   keys off (`type`, `symmetricKeySize`, `export`).
+/// * **asymmetric** — a PEM (RSA/EC) or internal Ed/X surrogate *string*
+///   flagged by `mark_as_asymmetric_key`.
+///
+/// The secret conversion is a buffer copy and happens here; the asymmetric one
+/// needs the key encoders (RSA PKCS#8/SPKI, P-256/384/521, Ed25519, X25519)
+/// that live in perry-stdlib, so it is routed through the WebCrypto dispatch
+/// hook — the same bridge `keyObject.toCryptoKey()` uses in the other
+/// direction (`native_call_method/string_methods.rs`).
+///
+/// Pre-#6302 this function demanded `kind == 1 && is_registered_buffer(addr)`
+/// and sent everything else to `invalid_key`, so a public/private CryptoKey —
+/// or a secret CryptoKey whose backing buffer was not in *this* thread's buffer
+/// registry — never produced a KeyObject at all.
 pub(super) unsafe fn key_object_from(value: f64) -> f64 {
     let addr = value_addr(value);
     let Some((_algo, _hash, kind, _extractable, _usages)) = crate::buffer::crypto_key_meta(addr)
     else {
         invalid_key(value);
     };
-    if kind != 1 || !crate::buffer::is_registered_buffer(addr) {
-        invalid_key(value);
+    match kind {
+        1 => secret_key_object(addr),
+        2 | 3 => asymmetric_key_object(addr),
+        _ => invalid_key(value),
     }
+}
 
+/// Copy the CryptoKey's bytes into a fresh secret-key Buffer.
+///
+/// The old `is_registered_buffer(addr)` precondition was wrong: CryptoKey
+/// metadata is only ever attached to a `BufferHeader` allocated by the
+/// WebCrypto key factories, and that metadata is *also* kept in a
+/// process-global registry, so the address is readable even when the current
+/// thread's (thread-local) buffer registry has no entry for it. Requiring the
+/// registry hit turned such keys into a silent `invalid_key` throw.
+unsafe fn secret_key_object(addr: usize) -> f64 {
     let src = addr as *const crate::buffer::BufferHeader;
     let len = (*src).length as usize;
     let out = crate::buffer::buffer_alloc(len as u32);
-    if !out.is_null() {
+    if out.is_null() {
+        conversion_failed("could not allocate the secret key buffer");
+    }
+    if len > 0 {
         std::ptr::copy_nonoverlapping(
             crate::buffer::buffer_data(src),
             crate::buffer::buffer_data_mut(out),
             len,
         );
-        (*out).length = len as u32;
-        crate::buffer::mark_as_uint8array(out as usize);
-        crate::buffer::mark_as_secret_key(out as usize);
     }
+    (*out).length = len as u32;
+    crate::buffer::mark_as_uint8array(out as usize);
+    crate::buffer::mark_as_secret_key(out as usize);
     f64::from_bits(JSValue::pointer(out as *const u8).bits())
 }
+
+/// Public/private CryptoKey → PEM/surrogate-string KeyObject, encoded by
+/// perry-stdlib's WebCrypto bridge (it owns the RSA/EC/Ed/X key writers and
+/// the CryptoKey material registry).
+unsafe fn asymmetric_key_object(addr: usize) -> f64 {
+    let ptr = crate::value::JS_NATIVE_WEBCRYPTO_DISPATCH.load(std::sync::atomic::Ordering::SeqCst);
+    if ptr.is_null() {
+        conversion_failed("the WebCrypto runtime is not available");
+    }
+    let dispatch: unsafe extern "C" fn(*const u8, usize, *const f64, usize) -> f64 =
+        std::mem::transmute(ptr);
+    // Hand the bridge a properly NaN-boxed pointer: a Buffer reaching
+    // `KeyObject.from` can arrive as a raw f64-bitcast pointer (module-level
+    // storage convention), which the stdlib pointer-strip helper would read as
+    // a tagged primitive and reject.
+    let args = [f64::from_bits(JSValue::pointer(addr as *const u8).bits())];
+    let method = "keyObjectFromCryptoKey";
+    let result = dispatch(method.as_ptr(), method.len(), args.as_ptr(), args.len());
+    // The bridge throws for key material it cannot model; a bare `undefined`
+    // return would be the exact silent failure #6302 is about.
+    if JSValue::from_bits(result.to_bits()).is_undefined() {
+        conversion_failed("unsupported asymmetric key type");
+    }
+    result
+}

crates/perry-stdlib/src/common/dispatch/init.rs

@@ -599,6 +599,11 @@ pub unsafe extern "C" fn js_stdlib_init_dispatch() {
     perry_runtime::js_set_native_crypto_dispatch(crate::crypto::js_crypto_native_dispatch);
     #[cfg(feature = "crypto")]
     perry_runtime::js_set_native_webcrypto_dispatch(crate::webcrypto::js_webcrypto_native_dispatch);
+    // Prune the stdlib CryptoKey-material map when the GC sweeps a key's
+    // backing buffer (otherwise it leaks an entry per key and a recycled
+    // address inherits the dead key's material).
+    #[cfg(feature = "crypto")]
+    perry_runtime::buffer::js_set_crypto_key_death_hook(crate::webcrypto::crypto_key_buffer_died);
     #[cfg(feature = "compression")]
     perry_runtime::js_set_native_zlib_dispatch(crate::zlib::js_zlib_native_dispatch);
     perry_runtime::js_set_native_querystring_dispatch(

crates/perry-stdlib/src/crypto/util.rs

@@ -183,14 +183,18 @@ pub(super) const ED25519_PUBLIC_PREFIX: &str = "PERRY-ED25519-PUBLIC:";
 pub(super) const X25519_PRIVATE_PREFIX: &str = "PERRY-X25519-PRIVATE:";
 pub(super) const X25519_PUBLIC_PREFIX: &str = "PERRY-X25519-PUBLIC:";
 
-pub(super) fn ed25519_private_surrogate(key: &ed25519_dalek::SigningKey) -> String {
+/// `pub(crate)` (not `pub(super)`): the WebCrypto bridge builds the very same
+/// surrogate strings when `KeyObject.from()` converts an asymmetric CryptoKey
+/// back into a KeyObject (#6302), and both sides must agree byte-for-byte or
+/// the `parse_*_surrogate` readers below reject the result.
+pub(crate) fn ed25519_private_surrogate(key: &ed25519_dalek::SigningKey) -> String {
     let secret = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(key.to_bytes());
     let public =
         base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(key.verifying_key().to_bytes());
     format!("{ED25519_PRIVATE_PREFIX}{secret}.{public}")
 }
 
-pub(super) fn ed25519_public_surrogate(key: &ed25519_dalek::VerifyingKey) -> String {
+pub(crate) fn ed25519_public_surrogate(key: &ed25519_dalek::VerifyingKey) -> String {
     let public = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(key.to_bytes());
     format!("{ED25519_PUBLIC_PREFIX}{public}")
 }
@@ -217,12 +221,12 @@ pub(crate) fn parse_ed25519_public_surrogate(value: &str) -> Option<ed25519_dale
     ed25519_dalek::VerifyingKey::from_bytes(&public).ok()
 }
 
-pub(super) fn x25519_private_surrogate(secret: &[u8; 32]) -> String {
+pub(crate) fn x25519_private_surrogate(secret: &[u8; 32]) -> String {
     let encoded = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(secret);
     format!("{X25519_PRIVATE_PREFIX}{encoded}")
 }
 
-pub(super) fn x25519_public_surrogate(public: &[u8; 32]) -> String {
+pub(crate) fn x25519_public_surrogate(public: &[u8; 32]) -> String {
     let encoded = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(public);
     format!("{X25519_PUBLIC_PREFIX}{encoded}")
 }

crates/perry-stdlib/src/webcrypto.rs

@@ -31,6 +31,10 @@ pub use self::{
     aes::*, digest::*, encapsulation::*, hmac::*, jwk::*, kdf::*, keys::*, supports::*, wrap::*,
 };
 
+// GC buffer-sweep callback for dead CryptoKey buffers, installed by
+// `js_stdlib_init_dispatch`. Re-exported here so `util` stays a private module.
+pub(crate) use self::util::crypto_key_buffer_died;
+
 /// Dispatcher for captured/dynamic `crypto.subtle.*` calls. Static
 /// `crypto.subtle.method(...)` call sites still lower directly in codegen;
 /// this keeps namespace property reads such as `const subtle = crypto.subtle`
@@ -106,6 +110,12 @@ pub unsafe extern "C" fn js_webcrypto_native_dispatch(
         "keyObjectToCryptoKey" if args_len >= 4 => {
             js_webcrypto_key_object_to_crypto_key(arg(0), arg(1), arg(2), arg(3))
         }
+        // Internal bridge for `crypto.KeyObject.from(<asymmetric CryptoKey>)`
+        // (#6302) — the runtime owns the secret-key shape but the asymmetric
+        // key encoders live here. Not a `crypto.subtle` method name.
+        "keyObjectFromCryptoKey" if args_len >= 1 => {
+            js_webcrypto_key_object_from_crypto_key(arg(0))
+        }
         "supports" if args_len >= 2 => js_webcrypto_supports(arg(0), arg(1), arg(2)),
         "encapsulateBits" => promise_to_value(js_webcrypto_encapsulate_bits(args_ptr, args_len)),
         "decapsulateBits" => promise_to_value(js_webcrypto_decapsulate_bits(args_ptr, args_len)),