This PR adds support for encrypted private key import/export in Deno's Node.js crypto compatibility layer. While the functionality is a welcome addition, there are several significant issues: a dangerously low PBKDF2 iteration count (2048 vs. recommended 600,000+), the cipher parameter being silently ignored during PKCS#8 export (always uses AES-256-CBC), a double .js extension in a test config entry, and key decryption happening in JS-land rather than native code. The cipher-ignored issue and low iteration count are particularly concerning for security and Node.js compatibility.

[HIGH] ext/node_crypto/keys.rs:3392: PBKDF2 iteration count of 2048 is dangerously low for key encryption. Modern recommendations (NIST SP 800-132, OWASP) require at least 600,000 iterations for PBKDF2-SHA256. Node.js uses much higher iteration counts. This makes encrypted private keys exported by Deno trivially brute-forceable, which is especially problematic since these keys are typically persisted to disk.

Suggestion: Use at least 600,000 iterations for PBKDF2-SHA256, matching modern security standards. If scrypt is considered too slow as noted in the comment, a high PBKDF2 iteration count is the appropriate alternative.

[HIGH] ext/node_crypto/keys.rs:3533: The cipher parameter is completely ignored for PKCS#8 exports — encrypt_pkcs8_der always uses AES-256-CBC regardless of what the user specifies (e.g., 'aes-128-cbc', 'des-ede3-cbc'). The same issue exists in op_node_export_private_key_der at line 3593. This silently violates the user's request and breaks Node.js compatibility.

Suggestion: Either honor the cipher parameter by selecting the appropriate PBES2 algorithm in encrypt_pkcs8_der, or validate that only 'aes-256-cbc' is accepted and return an explicit error for unsupported ciphers.

[MEDIUM] tests/node_compat/config.jsonc:395: Test config entry has a double .js extension: 'test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js.js'. The test file will not be found/run.

Suggestion: Rename to 'parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js' (remove the extra .js).

[MEDIUM] ext/node/polyfills/internal/crypto/cipher.ts:690: The condition passphrase != null && format != null will fail to decrypt encrypted private keys when format is not explicitly provided. In Node.js, { key: pemString, passphrase: 'secret' } works without specifying format. Here, missing format causes the code to fall through to the plain path, passing encrypted PEM as-is.

Suggestion: Default format to 'pem' when key data is a string and passphrase is provided, matching Node.js auto-detection behavior.

[MEDIUM] ext/node/polyfills/internal/crypto/cipher.ts:693: Private key decryption happens in JS-land: the key is decrypted and re-exported as unencrypted PEM, which then lives as a JavaScript string without zeroing guarantees. Consider passing the passphrase to the native layer instead to keep plaintext key material in native code.

Suggestion: Pass the passphrase down to the native crypto operations rather than decrypting at the JS layer and re-exporting as plaintext PEM.

[MEDIUM] ext/node_crypto/keys.rs:1382: Public key creation from PEM uses a simple contains("Proc-Type: 4,ENCRYPTED") substring search on the full input, which could match against base64-encoded data that happens to contain this string, causing misrouting.

Suggestion: Use a more precise check — verify the string appears as a header line between the BEGIN marker and the base64 body, rather than a simple contains() on the entire PEM.

[LOW] ext/node_crypto/keys.rs:926: When a passphrase is provided for DER PKCS#8 import, the code silently falls back to unencrypted parsing on decryption failure. This means a wrong passphrase on an actually-encrypted key won't produce a decryption error, instead giving a misleading generic parse error.

Suggestion: Check if the DER data is an EncryptedPrivateKeyInfo before falling back to unencrypted parsing. Only fall back if it's genuinely not encrypted.

[LOW] ext/node_crypto/keys.rs:3254: In parse_legacy_encrypted_pem, salt.copy_from_slice(&iv[..8]) will panic if the decoded IV is shorter than 8 bytes (e.g., from a malformed PEM with a short IV hex string).

Suggestion: Add a check that iv.len() >= 8 before the slice operation, and validate that the IV length matches the expected block size for the cipher.

This PR adds support for encrypted private key import/export in Deno's Node.js crypto compatibility layer. While the functionality works, there are several security-relevant concerns: a dangerously low PBKDF2 iteration count for key encryption, decrypted key material unnecessarily exposed in JavaScript strings, and fragile PEM detection logic that could misroute key parsing.

[HIGH] ext/node_crypto/keys.rs:3408: PBKDF2 iteration count of 2,048 is critically weak for password-based key derivation. OWASP recommends at least 600,000 iterations for PBKDF2-SHA256. Node.js uses much higher counts. Encrypted keys exported with this count are trivially brute-forceable offline.

Suggestion: Increase the iteration count to at least 600,000. If scrypt is too slow for tests, that's a test configuration issue, not a reason to ship insecure defaults to users.

[MEDIUM] ext/node/polyfills/internal/crypto/cipher.ts:690: prepareKey() decrypts the private key and re-exports it as unencrypted PKCS#8 PEM in JavaScript. This means plaintext private key material sits in a JS string that cannot be securely zeroed, increasing the window for key material exposure in memory. The original design kept encrypted material encrypted until the native layer.

Suggestion: Handle passphrase-protected keys entirely in the Rust layer rather than round-tripping through plaintext PEM in JavaScript.

[MEDIUM] ext/node_crypto/keys.rs:1369: Public key import uses a raw string search for 'Proc-Type: 4,ENCRYPTED' on the entire PEM input without first validating the PEM label. This could cause any PEM content (including public keys) containing this string to be incorrectly routed through the private key import path. The PEM label should be validated first.

Suggestion: Parse the PEM structure and validate the label (e.g., 'RSA PRIVATE KEY', 'EC PRIVATE KEY', 'PRIVATE KEY') before checking for Proc-Type headers and routing to the private key import path.

[MEDIUM] ext/node_crypto/keys.rs:922: For DER PKCS#8 format with a passphrase, the code tries encrypted decryption first, then silently falls back to treating the data as unencrypted, ignoring the passphrase. This means a wrong passphrase on an encrypted key would silently try to parse raw encrypted bytes as an unencrypted key rather than reporting 'bad decrypt'.

Suggestion: Check if the data is an EncryptedPrivateKeyInfo first (as done in the no-passphrase branch). Only fall back to unencrypted parsing if the data doesn't appear to be encrypted.

[LOW] ext/node_crypto/keys.rs:3290: parse_legacy_encrypted_pem assumes Proc-Type header appears on the exact line after BEGIN. While unlikely to be an issue in practice, more robust PEM header parsing would handle edge cases better.

Suggestion: Iterate through header lines until an empty line is found, collecting all headers before checking for Proc-Type and DEK-Info.

This PR adds passphrase support for encrypted private key PEM/DER import and export in Deno's Node.js crypto compatibility layer. While the implementation is generally solid, there are several issues ranging from a likely typo in test config to inconsistencies in cipher support and fragile PEM detection logic.

[HIGH] tests/node_compat/config.jsonc:397: The filename has a double .js.js extension which is almost certainly a typo. This test entry won't match any actual test file and the test will silently not run.

Suggestion: Fix to parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js (single .js extension).

[HIGH] ext/node/polyfills/internal/crypto/keys.ts:710: createPublicKey passes res.passphrase as 4th argument to op_node_create_public_key when the key is a private key type (~line 710), but further down (~line 720) for the direct public key path, op_node_create_public_key is still called with only 3 arguments (no passphrase). This means passing an encrypted private key PEM directly to createPublicKey (without it being detected as a private key type first) will fail to decrypt it.

Suggestion: Ensure both call sites of op_node_create_public_key consistently pass the passphrase parameter.

[MEDIUM] ext/node_crypto/keys.rs:1312: The PEM format path in new_asymmetric_public_key_from_js uses raw pem.contains("-----BEGIN RSA PRIVATE KEY-----") and pem.contains("Proc-Type: 4,ENCRYPTED") for detection. These substring checks on the entire PEM string are fragile — a crafted PEM could contain these strings in base64 data or after the END marker, triggering the wrong parsing path. The parse_legacy_encrypted_pem function already does proper structural parsing.

Suggestion: Use structured PEM parsing (parse label first, then check headers in the proper section) rather than raw string contains on the entire input.

[MEDIUM] ext/node_crypto/keys.rs:3440: encrypt_pkcs8_der only supports aes-128-cbc and aes-256-cbc, but the decrypt path supports DES-EDE3-CBC and AES-192-CBC. This creates an asymmetry — a user who imports a key encrypted with aes-192-cbc or des-ede3-cbc cannot re-export it with the same cipher. This may break Node.js compatibility.

Suggestion: Add support for at least aes-192-cbc and des-ede3-cbc in encrypt_pkcs8_der, or document the limitation.

[MEDIUM] ext/node_crypto/keys.rs:848: When a passphrase is provided but PKCS#8 encrypted parsing fails, the code falls back to parsing as unencrypted PEM and silently ignores the passphrase. This could mask real errors (wrong passphrase, unrecognized encrypted format) and has security implications — replacing an encrypted PEM with an unencrypted one would not produce an error.

Suggestion: Consider checking whether the PEM actually looks encrypted before falling back to unencrypted parsing, or at minimum document this as intentional Node.js compatibility behavior.

[MEDIUM] ext/node/polyfills/internal/crypto/cipher.ts:691: The prepareKey function decrypts a passphrase-protected private key and re-exports it as unencrypted PEM, creating an additional plaintext copy of sensitive key material in JavaScript heap memory that cannot be zeroed. The decrypted key handle could potentially be passed directly to the encrypt/decrypt ops.

Suggestion: Consider passing the decrypted key handle directly to crypto ops instead of re-serializing as PEM to minimize sensitive key material copies in memory.

[LOW] ext/node_crypto/keys.rs:3287: parse_legacy_encrypted_pem assumes fixed line positions for Proc-Type and DEK-Info headers. Per RFC 1421, other headers could appear between them, though this is unlikely in practice.

Suggestion: Consider searching for Proc-Type and DEK-Info headers rather than assuming fixed line positions.

This PR adds support for legacy encrypted PEM keys and improves crypto key handling. The main concerns are: (1) a test config entry with a double .js.js extension that will silently fail to match any test file, (2) fragile string-based detection of legacy encrypted PEMs that could cause false positives, (3) silent error swallowing in the TypeScript prepareKey function that masks wrong-passphrase errors, and (4) missing aes-192-cbc cipher support in PKCS#8 encryption.

[HIGH] tests/node_compat/config.jsonc:397: Test filename has double .js.js extension. This won't match any actual test file and the test entry will be silently ignored.

Suggestion: Rename to parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js (single .js extension).

[MEDIUM] ext/node/polyfills/internal/crypto/cipher.ts:689: The catch block silently swallows ALL exceptions when passphrase-based decryption fails, then passes the still-encrypted key data to subsequent operations. A wrong passphrase will never produce a clear error — instead users get confusing errors from operations trying to use encrypted data as a plaintext key.

Suggestion: Consider re-throwing passphrase-specific errors (e.g., wrong passphrase / decryption failure) rather than silently falling through for all error types. Only fall through for cases where the key might not actually be encrypted.

[MEDIUM] ext/node_crypto/keys.rs:1312: String-based detection of legacy encrypted PEM in the public key import path uses pem.lines().any(|l| l.starts_with("Proc-Type: 4,ENCRYPTED")) which checks ALL lines in the file. This could cause false positives if the string appears in base64-encoded data or concatenated PEMs, misrouting to private key import.

Suggestion: Check that the Proc-Type header appears immediately after a BEGIN line (within the PEM header section) rather than using .any() which scans the entire content.

[MEDIUM] ext/node_crypto/keys.rs:3440: encrypt_pkcs8_der does not support aes-192-cbc, which Node.js supports. Users will get an unhelpful 'Unsupported cipher for PKCS#8 encryption' error.

Suggestion: Either implement aes-192-cbc support or return a clear error message indicating this specific cipher is not yet supported.

[LOW] ext/node_crypto/keys.rs:3290: parse_legacy_encrypted_pem assumes the line immediately after -----BEGIN ...----- is the Proc-Type line. RFC 1421 allows for other header ordering or whitespace, which would cause parsing to fail with an unhelpful InvalidPemPrivateKey error.

Suggestion: Consider scanning the header block (lines between BEGIN and the first blank line) for both Proc-Type and DEK-Info rather than assuming fixed line positions.

[LOW] ext/node/polyfills/internal/crypto/keygen.ts:1137: The 'explicit' paramEncoding for EC key generation is silently treated as 'named'. Applications that specifically request explicit encoding (embedding full curve parameters) will silently get different behavior.

Suggestion: Consider logging a warning that 'explicit' is treated as 'named', or document this deviation from Node.js behavior.

[LOW] ext/node_crypto/keys.rs:3266: The legacy PEM key derivation uses OpenSSL's EVP_BytesToKey with only 8 bytes of salt from the IV and a single MD5 iteration. This is inherently weak but correct for legacy compatibility.

Suggestion: Add a code comment noting this is a legacy compatibility feature with known cryptographic weaknesses (single MD5 iteration, 8-byte salt).

Found 2 issue(s) in this PR.

[MEDIUM] ext/node/polyfills/internal/crypto/cipher.ts:707: Silent catch-all swallows all errors during passphrase-protected key decryption, including potentially important errors (e.g., unsupported cipher, malformed key structure). The empty catch block falls through to return raw encrypted data, which will produce a confusing/incorrect error downstream rather than the actual root cause.

Suggestion: At minimum, catch only specific expected error types (wrong passphrase, corrupt key) and re-throw unexpected errors. Alternatively, log or preserve the original error information so debugging is possible.

[LOW] ext/node_crypto/keys.rs:1370: Legacy encrypted PEM detection is inconsistent between public key import and parse_legacy_encrypted_pem. The public key path uses pem.lines().any(|l| l.starts_with("Proc-Type: 4,ENCRYPTED")) (line-start check) while parse_legacy_encrypted_pem uses pem.contains("Proc-Type: 4,ENCRYPTED") (substring search). The contains variant is slightly more permissive and could theoretically match content embedded elsewhere in the PEM.

Suggestion: Consolidate the legacy encrypted PEM detection into a single reusable function used by both code paths, preferring the stricter line-based check.

The config file contains a filename with a double .js.js extension, which means the test entry will never match an actual test file and the test won't be discovered or run as intended.

[HIGH] tests/node_compat/config.jsonc:396: Double .js extension in test config filename: parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js.js. This will never match an actual test file, so the test won't be discovered or run as intended.

Suggestion: Rename to parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js

This PR adds passphrase support for encrypted private/public key operations in Deno's Node.js crypto polyfill. The review identified a critical bug where createPublicKey cannot actually use passphrases due to prepareAsymmetricKey never propagating the passphrase field, several medium-severity issues around silent error swallowing and inconsistent PEM parsing, and minor issues with test configuration and documentation.

[CRITICAL] ext/node/polyfills/internal/crypto/keys.ts:710: createPublicKey passes res.passphrase to op_node_create_public_key, but prepareAsymmetricKey never includes a passphrase property in its return value. It returns { data, format, type } without passphrase, so res.passphrase is always undefined. The new passphrase-based public key creation path for encrypted private keys will never work from the JS side.

Suggestion: Ensure prepareAsymmetricKey propagates the passphrase field from the input key object into its return value, e.g., include passphrase in the returned object so it's available to pass to the native op.

[MEDIUM] ext/node/polyfills/internal/crypto/cipher.ts:689: When passphrase-based key decryption fails in prepareKey, the catch block silently falls through and returns the raw (still encrypted) key data. This means a wrong passphrase won't produce a clear error — instead, the encrypted blob is passed to encrypt/decrypt operations producing confusing downstream errors (e.g., 'invalid key' rather than 'bad passphrase'). This could also mask security-relevant errors.

Suggestion: Distinguish between 'wrong passphrase' errors and other errors. Re-throw passphrase-related errors immediately rather than falling through with encrypted key data that will inevitably fail in a confusing way.

[MEDIUM] tests/node_compat/config.jsonc:400: Test filename has a double extension: parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js.js. Even though the test is marked as ignored, the wrong filename means the test runner would never find the corresponding test file if it were enabled.

Suggestion: Rename to parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js

[MEDIUM] ext/node_crypto/keys.rs:3261: parse_legacy_encrypted_pem uses pem.contains("Proc-Type: 4,ENCRYPTED") which is not line-bounded and could match content embedded within the PEM base64 body. Meanwhile, the public key creation path at line 1370 uses a line-bounded check. This inconsistency could lead to false positives in parse_legacy_encrypted_pem for specially crafted input.

Suggestion: Use line-bounded checking consistently: pem.lines().any(|l| l.starts_with("Proc-Type: 4,ENCRYPTED")) in parse_legacy_encrypted_pem as well.

[MEDIUM] ext/node_crypto/keys.rs:3437: encrypt_pkcs8_der only supports aes-128-cbc and aes-256-cbc but not aes-192-cbc, while the PEM export path and legacy PEM decryption both support AES-192-CBC. This asymmetry means round-tripping a key encrypted with aes-192-cbc through DER export will fail with an opaque "Unsupported cipher" error.

Suggestion: Document this limitation in the error message or implement aes-192-cbc support for PKCS#8 DER encryption.

[LOW] ext/node_crypto/keys.rs:3295: parse_legacy_encrypted_pem skips exactly one line after DEK-Info assuming it's the blank separator. A malformed PEM with no blank line would silently discard the first line of base64 data, leading to a confusing decryption error rather than a clear parse error.

Suggestion: Verify the skipped line is actually empty/blank and return a descriptive error if it isn't.

[LOW] ext/node/polyfills/internal/crypto/keygen.ts:1137: paramEncoding: 'explicit' is silently treated as 'named' with no warning. Explicit encoding embeds full curve parameters, which some systems may rely on for interoperability. Silently substituting could produce keys rejected by systems expecting explicit parameters.

Suggestion: Consider logging a warning that explicit encoding is not supported and named encoding is used instead.

Reviewed the changes with full repo context. No issues found.

This PR adds support for encrypted private key import/export in Deno's Node.js crypto polyfill. While the implementation is largely functional, there are several issues worth addressing: a fragile implicit argument omission in an op call, a double .js extension typo in test config, silent error swallowing that masks wrong-passphrase errors, and an incomplete cipher support matrix for PKCS#8 export.

[HIGH] ext/node/polyfills/internal/crypto/keys.ts:939: op_node_export_private_key_der is called with only 2 arguments but the Rust op now expects 4 parameters (handle, typ, cipher, passphrase). While Deno's op layer may coerce missing args to None for Option<String>, this is fragile and inconsistent with the explicit cipher ?? null, passphrase != null ? passphrase.toString() : null pattern used in the export() method.

Suggestion: Change to: op_node_export_private_key_der(this[kHandle], "pkcs8", null, null)

[MEDIUM] tests/node_compat/config.jsonc:397: The test file entry has a double .js.js extension: parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js.js. This means the test won't be found and won't run.

Suggestion: Rename to parallel/test-crypto-keygen-async-explicit-elliptic-curve-encrypted.js

[MEDIUM] ext/node/polyfills/internal/crypto/cipher.ts:689: The empty catch block (line 710) silently swallows all passphrase decryption errors. If an encrypted private key is provided with a wrong passphrase, the raw encrypted PEM data is passed through to the subsequent operation, producing a misleading error (e.g., 'invalid key') instead of a clear 'wrong passphrase' error. The comment claims 'the subsequent op will produce the appropriate Node-compatible error' but this is not guaranteed since the op will see encrypted/corrupted data.

Suggestion: Only catch errors indicating the data is not an encrypted key. Re-throw errors related to wrong passphrases or decryption failures so users get actionable error messages.

[MEDIUM] ext/node_crypto/keys.rs:3250: encrypt_pkcs8_der only supports aes-128-cbc and aes-256-cbc, but the legacy PEM encryption path supports additional ciphers like aes-192-cbc and des-ede3-cbc. Exporting a PKCS#8 key in PEM format with aes-192-cbc will hit this path and return an UnsupportedCipher error, even though Node.js supports it.

Suggestion: Add support for aes-192-cbc (and ideally des-ede3-cbc) in encrypt_pkcs8_der, or document the limitation.

[MEDIUM] ext/node_crypto/keys.rs:3260: In parse_legacy_encrypted_pem(), the check pem.contains("Proc-Type: 4,ENCRYPTED") is a substring search on the entire PEM string rather than a line-based check. While unlikely due to base64 charset constraints, this could theoretically match content within the base64-encoded body.

Suggestion: Use pem.lines().any(|l| l == "Proc-Type: 4,ENCRYPTED") for consistency with the check at the public key import path.

[MEDIUM] ext/node/polyfills/internal/crypto/keygen.ts:1137: paramEncoding: 'explicit' is silently accepted but treated identically to 'named'. EC keys will always use named curve OIDs, never explicit curve parameters. Code depending on explicit parameter encoding for interoperability will silently get incorrect behavior.

Suggestion: Either implement explicit parameter encoding or emit a warning/throw that it's not supported.

[MEDIUM] ext/node_crypto/keys.rs:860: When PKCS#8 encrypted PEM decryption fails with a passphrase, the code falls back to treating the PEM as unencrypted (lines 865-883). This means a wrong passphrase for an encrypted key could potentially load a different key silently if the encrypted PEM happens to parse as valid unencrypted PEM in another format.

Suggestion: Before falling back, check whether the PEM actually has encrypted markers (ENCRYPTED PRIVATE KEY label or Proc-Type headers). Only fall back for genuinely unencrypted PEMs.

Substance scan on the encrypted-key path looks reasonable — the fallback flow in keys.rs matches Node's tolerant behavior (passphrase-with-unencrypted-key is silently accepted), and the new ERR_MISSING_PASSPHRASE error code on EncryptedPrivateKeyRequiresPassphraseToDecrypt matches Node's error code. Holding at COMMENT instead of APPROVE for two reasons:

1. CI is red. All 7 test node_compat debug shards (linux-x86_64, linux-aarch64, macos-x86_64, macos-aarch64, windows-x86_64, windows-aarch64, plus release linux-x86_64) fail at HEAD d29e15c9. The 8 failing tests on linux-x86_64 are:

   test-child-process-fork-timeout-kill-signal.js
   test-util-parse-env.js
   test-net-autoselectfamily-attempt-timeout-default-value.js
   test-process-features.js
   test-child-process-fork.js
   test-child-process-spawnsync-validation-errors.js
   test-child-process-spawn-timeout-kill-signal.js
   test-child-process-spawn-typeerror.js
   

   None of these are crypto/keys related, and none are enabled by this PR's tests/node_compat/config.jsonc diff (which only touches test-crypto-keygen-async-*). Most likely cause: the branch is dirty against main from 2026-03-18, so these tests were either flaky-then or have been disabled/fixed on main since. A rebase would re-run CI with the current base and almost certainly clear them. If any persist after rebase, they'd be the ones to dig into.

2. Two questions inline on the substance — one on the legacy-PEM path (re-checking the no-passphrase case), one on the DER fallback ordering.