From 1fb9ec05daad38d94134049600dd40e339028cd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 11:31:26 +0000 Subject: [PATCH 1/3] fix(link): group the ELF archive block so ld can resolve back into stdlib (#8930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `bundled-streams` build died with `undefined reference to ::notify` out of `libperry_ext_http.a`, even though the `libperry_stdlib.a` on the same command line exports that symbol from its own `futures_channel` member. The mechanism is archive order, but not the obvious one. The wrapper archives already appear twice — once before perry-stdlib, once after — so at a glance every reference has somewhere to go. What the #8930 link map shows is that the FIRST listing pulls nothing at all: the user objects reference only `js_*` symbols that perry-stdlib provides, so nothing in the wrapper is undefined yet when `ld` walks it. All 92 wrapper members that end up in the executable are pulled from the SECOND listing, on references that stdlib's own members had just opened (`js_node_http_*`, …) — and the references those members carry back INTO stdlib have nowhere to go, because GNU `ld` scans each archive once and never revisits one it has passed. That was harmless while each wrapper bundled its own copy of everything it closed over. It becomes a hard error the moment `strip_bundled_shared_deps_from_well_known_lib` drops a bundled member because stdlib provides it — a correct decision by the archive index, but one that only holds if `ld` can still get back to stdlib. That is what `bundled-streams` changes: it is the feature (enabled by a `fs/promises` or `stream/web` import, per `stdlib_features::module_to_features`) that pulls futures_channel into perry-stdlib's own graph, so stdlib starts bundling a name-matching `futures_channel-*.rcgu.o`, so the wrapper's copy becomes eligible to drop. A stdlib built without it carries no futures_channel member at all, the wrapper keeps its copy, and the link stays self-contained. #8939 tightened the pruning rule to the name-matched stdlib member's own exports; here that member does export the symbol, so the rule fires correctly and the link still fails. The bug is on the link line, not in the pruning. Wrap the perry archive block in `-Wl,--start-group` / `-Wl,--end-group` on ELF targets so `ld` re-scans it to a fixed point — the guarantee a mutually recursive archive set needs. Repeating one archive (the codebase's usual "archive twice" trick) only covers a one-step cycle, and this graph is LTO-partitioned across hundreds of codegen units on both sides. Members are still pulled left to right, so the wrappers-before-stdlib and localized-runtime-last first-definition-wins ordering is unaffected; Mach-O `ld64` resolves archives to a fixed point already (and rejects the flag) and `lld-link` / MSVC have no group concept, so both are left alone. Verified against both reported repros in mb24 — `apps/landing` (hono plus a compiled `@skelpo/cms-client`) and `apps/api` (hono + ws + mysql2, four wrapper archives, no `perry.compilePackages` at all). Both fail before and link after; replaying either final link line with only the two group flags removed reproduces the exact undefined reference. A case that already linked (`packages/db/src/migrate.ts`) produces a byte-identical executable with and without the flags. The link/strip-dedup unit tests pass (68), as do the two archive-ordering integration tests — `issue_5920_wrapper_bundled_runtime_async_starvation` (the two-runtime-copies hazard) and `issue_6715_native_wrapper_precedence`. `native_link_cache` fails identically on the unpatched parent commit. --- changelog.d/8930-elf-archive-group.md | 15 +++ .../commands/compile/link/build_and_run.rs | 8 ++ crates/perry/src/commands/compile/link/mod.rs | 113 ++++++++++++++++++ 3 files changed, 136 insertions(+) create mode 100644 changelog.d/8930-elf-archive-group.md diff --git a/changelog.d/8930-elf-archive-group.md b/changelog.d/8930-elf-archive-group.md new file mode 100644 index 0000000000..3956ec184d --- /dev/null +++ b/changelog.d/8930-elf-archive-group.md @@ -0,0 +1,15 @@ +Fixed ELF links that died with `undefined reference to +::notify` out of a well-known wrapper +archive when a compiled package pulled in `bundled-streams`. + +GNU `ld` scans each archive on the command line exactly once, left to right, +and the perry archive block is mutually recursive: the wrapper archives listed +*before* perry-stdlib resolve nothing (the user objects reference only stdlib +symbols), every wrapper member is pulled from the repeat *after* stdlib on +references stdlib itself opened, and those members then reference back into +stdlib — which `ld` has already walked past. That stayed invisible until +shared-dependency pruning correctly dropped a bundled member because stdlib +exports it. The archive block is now wrapped in `-Wl,--start-group` / +`-Wl,--end-group` on ELF targets so the linker re-scans it to a fixed point. +Symbol precedence is unchanged and non-ELF link lines are untouched; a link +that already resolved produces a byte-identical executable. diff --git a/crates/perry/src/commands/compile/link/build_and_run.rs b/crates/perry/src/commands/compile/link/build_and_run.rs index 2c46590b3d..9fc5346d1a 100644 --- a/crates/perry/src/commands/compile/link/build_and_run.rs +++ b/crates/perry/src/commands/compile/link/build_and_run.rs @@ -441,6 +441,13 @@ pub(crate) fn build_and_run_link( // Multiple specifiers can route to the same native archive (`http` and // `https` both select perry_ext_http). They were de-duplicated before the // archive-preparation pass so the expensive transform also runs once. + // #8930: the archives below reference each other in BOTH directions and + // ELF `ld` scans each once — bracket them (see `ElfArchiveGroup`). + let archive_group = ElfArchiveGroup::open( + &mut cmd, + !is_windows && (is_linux || is_android || is_harmonyos), + !skip_runtime || ctx.needs_stdlib, + ); if !skip_runtime { if ctx.needs_stdlib || is_windows { // On Windows/MSVC, always try to link stdlib because codegen unconditionally @@ -554,6 +561,7 @@ pub(crate) fn build_and_run_link( eprintln!("Warning: stdlib required but libperry_stdlib.a not found"); } } + archive_group.close(&mut cmd); // Issue #76 — wasmi host runtime, opt-in via `--enable-wasm-runtime`. // Append after stdlib so the linker can resolve `perry_wasm_host_*` diff --git a/crates/perry/src/commands/compile/link/mod.rs b/crates/perry/src/commands/compile/link/mod.rs index 3c0b1d3413..a988b8b28e 100644 --- a/crates/perry/src/commands/compile/link/mod.rs +++ b/crates/perry/src/commands/compile/link/mod.rs @@ -648,6 +648,119 @@ fn rewrite_link_with_response_file(cmd: &Command, msvc: bool) -> Option<(Command Some((new_cmd, rsp)) } +/// `--start-group` / `--end-group` around the perry static-archive block on +/// ELF links (#8930). +/// +/// GNU `ld` scans each archive on the command line exactly once, left to +/// right, and pulls only the members that resolve symbols undefined *at that +/// moment*. The perry archive block is mutually recursive, and the direction +/// of that recursion is the opposite of the command-line order: +/// +/// * the user objects reference `js_*` symbols that **perry-stdlib** +/// provides, so the well-known wrapper archives listed BEFORE stdlib +/// resolve nothing at all — in the #8930 repro not one of the 92 +/// eventually-linked wrapper members was pulled from that first listing; +/// * every wrapper member came from the repeat AFTER stdlib, on references +/// that stdlib's own members had just opened (`js_node_http_*`, …); +/// * those wrapper members then reference back INTO stdlib, which `ld` has +/// already walked past and never revisits. +/// +/// While a wrapper archive bundled its own copy of everything it closed over, +/// the third step was invisible. It turns into a hard error the moment +/// `strip_bundled_shared_deps_from_well_known_lib` drops a bundled member +/// *because stdlib provides it* — a correct decision by the archive index, +/// but one that only holds if `ld` can still get back to stdlib. #8930 is +/// exactly that shape: `futures_channel::mpsc::SenderTask::notify`, exported +/// by the linked `libperry_stdlib.a`'s own `futures_channel` member and +/// referenced by a kept `perry_ext_http` codegen unit, with no path from the +/// one to the other. +/// +/// A group makes `ld` re-scan the block until it reaches a fixed point, which +/// is the guarantee a mutually recursive archive set needs. Repeating a single +/// archive (the codebase's usual "archive twice" trick — see the GTK4 stdlib +/// re-link) only covers a one-step cycle, and this graph is LTO-partitioned +/// across hundreds of codegen units on both sides. Symbol precedence is +/// unchanged: members are still pulled left to right, so the +/// wrappers-before-stdlib and localized-runtime-last first-definition-wins +/// ordering keeps working. +/// +/// ELF only. Mach-O's `ld64` already resolves archives to a fixed point (and +/// rejects the flag); `lld-link` / MSVC `link.exe` have no group concept. +pub(super) struct ElfArchiveGroup { + opened: bool, +} + +impl ElfArchiveGroup { + /// Emit `-Wl,--start-group` when this is an ELF link that will actually + /// receive archives. `has_archives` keeps an empty group off the command + /// line for the link shapes that add none. + pub(super) fn open(cmd: &mut Command, is_elf: bool, has_archives: bool) -> Self { + let opened = is_elf && has_archives; + if opened { + cmd.arg("-Wl,--start-group"); + } + Self { opened } + } + + /// Close a group opened by [`ElfArchiveGroup::open`]. Takes `self` by + /// value so the marker cannot be emitted twice. + pub(super) fn close(self, cmd: &mut Command) { + if self.opened { + cmd.arg("-Wl,--end-group"); + } + } +} + +#[cfg(test)] +mod elf_archive_group_tests { + use super::ElfArchiveGroup; + use std::process::Command; + + fn args(is_elf: bool, archives: &[&str]) -> Vec { + let mut cmd = Command::new("cc"); + let group = ElfArchiveGroup::open(&mut cmd, is_elf, !archives.is_empty()); + for archive in archives { + cmd.arg(archive); + } + group.close(&mut cmd); + cmd.get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect() + } + + /// #8930 — an ELF link brackets the archive block so `ld` re-scans it to a + /// fixed point instead of dying on a stdlib symbol it already walked past. + #[test] + fn elf_link_brackets_the_archive_block() { + assert_eq!( + args(true, &["wrapper.a", "libperry_stdlib.a", "wrapper.a"]), + vec![ + "-Wl,--start-group", + "wrapper.a", + "libperry_stdlib.a", + "wrapper.a", + "-Wl,--end-group", + ] + ); + } + + /// Mach-O's `ld64` rejects the flag and needs no group; `lld-link` / MSVC + /// have no group concept. Those command lines must come out unchanged. + #[test] + fn non_elf_link_is_untouched() { + assert_eq!( + args(false, &["libperry_stdlib.a"]), + vec!["libperry_stdlib.a"] + ); + } + + /// A link shape that contributes no archive must not emit an empty group. + #[test] + fn empty_archive_block_emits_no_group() { + assert!(args(true, &[]).is_empty()); + } +} + #[cfg(test)] mod optional_framework_dir_tests; From 8c06b30e1b04fb339f16dd95ac4d42f780cce7c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 14:40:12 +0000 Subject: [PATCH 2/3] fix(crypto): honor scrypt cost parameters (#9289) --- changelog.d/9289-scrypt-params.md | 6 + .../src/expr/calls/crypto_kdf.rs | 21 +- .../runtime_decls/stdlib_ffi/data_stores.rs | 5 +- .../src/runtime_decls/strings_part2.rs | 4 +- crates/perry-stdlib/src/crypto/kdf.rs | 326 +++++++++++++++--- crates/perry-stdlib/src/crypto/random.rs | 7 +- .../node-suite/crypto/scrypt/options.ts | 109 ++++++ 7 files changed, 405 insertions(+), 73 deletions(-) create mode 100644 changelog.d/9289-scrypt-params.md create mode 100644 test-parity/node-suite/crypto/scrypt/options.ts diff --git a/changelog.d/9289-scrypt-params.md b/changelog.d/9289-scrypt-params.md new file mode 100644 index 0000000000..95ce61a13d --- /dev/null +++ b/changelog.d/9289-scrypt-params.md @@ -0,0 +1,6 @@ +Fix a silent security downgrade in `node:crypto` scrypt: the callback form now +forwards `N`/`cost`, `r`/`blockSize`, `p`/`parallelization`, and `maxmem` +instead of always deriving with Node's defaults. Both callback and synchronous +forms now reject invalid combinations and insufficient `maxmem` with a +Node-compatible `RangeError`, rather than silently substituting the weaker +default work factor. diff --git a/crates/perry-codegen/src/expr/calls/crypto_kdf.rs b/crates/perry-codegen/src/expr/calls/crypto_kdf.rs index 6b5a7c568b..8fbc2e4f18 100644 --- a/crates/perry-codegen/src/expr/calls/crypto_kdf.rs +++ b/crates/perry-codegen/src/expr/calls/crypto_kdf.rs @@ -127,11 +127,13 @@ pub(crate) fn arm_crypto_scrypt( let pwd_box = lower_expr(ctx, &args[0])?; let salt_box = lower_expr(ctx, &args[1])?; let len_box = lower_expr(ctx, &args[2])?; - let cb_expr = if args.len() >= 5 { - let _ = lower_expr(ctx, &args[3])?; - &args[4] + let (opts_box, cb_expr) = if args.len() >= 5 { + (lower_expr(ctx, &args[3])?, &args[4]) } else { - &args[3] + ( + double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)), + &args[3], + ) }; let cb_box = lower_expr(ctx, cb_expr)?; let blk = ctx.block(); @@ -144,6 +146,7 @@ pub(crate) fn arm_crypto_scrypt( (I64, &pwd_handle), (I64, &salt_handle), (DOUBLE, &len_box), + (DOUBLE, &opts_box), (DOUBLE, &cb_box), ], )) @@ -243,19 +246,15 @@ pub(crate) fn arm_crypto_scrypt_sync( let salt_box = lower_expr(ctx, &args[1])?; let keylen_box = lower_expr(ctx, &args[2])?; let opts_box = if args.len() >= 4 { - Some(lower_expr(ctx, &args[3])?) + lower_expr(ctx, &args[3])? } else { - None + double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) }; // #2013/#3146: node validates keylen as an integer in [0, 2^31-1]. emit_validate_integer_arg(ctx, &keylen_box, "keylen", 0.0, i32::MAX as f64); let blk = ctx.block(); let pwd_handle = unbox_to_i64(blk, &pwd_box); let salt_handle = unbox_to_i64(blk, &salt_box); - let opts_handle = match &opts_box { - Some(b) => unbox_to_i64(blk, b), - None => "0".to_string(), - }; let buf_handle = blk.call( I64, "js_crypto_scrypt_bytes", @@ -263,7 +262,7 @@ pub(crate) fn arm_crypto_scrypt_sync( (I64, &pwd_handle), (I64, &salt_handle), (DOUBLE, &keylen_box), - (I64, &opts_handle), + (DOUBLE, &opts_box), ], ); Ok(nanbox_pointer_inline(blk, &buf_handle)) diff --git a/crates/perry-codegen/src/runtime_decls/stdlib_ffi/data_stores.rs b/crates/perry-codegen/src/runtime_decls/stdlib_ffi/data_stores.rs index cfdaa4c587..d9da94b533 100644 --- a/crates/perry-codegen/src/runtime_decls/stdlib_ffi/data_stores.rs +++ b/crates/perry-codegen/src/runtime_decls/stdlib_ffi/data_stores.rs @@ -274,8 +274,9 @@ pub(crate) fn declare_data_stores(module: &mut LlModule) { module.declare_function("js_crypto_random_nonce", I64, &[]); module.declare_function("js_crypto_scrypt", I64, &[I64, I64, DOUBLE]); // crypto.scryptSync(password, salt, keylen, options?) -> Buffer. The 4th - // arg is the NaN-unboxed options-object pointer (0 = none). - module.declare_function("js_crypto_scrypt_bytes", I64, &[I64, I64, DOUBLE, I64]); + // arg is the full NaN-boxed options value so validation can distinguish + // objects, primitives, and undefined. + module.declare_function("js_crypto_scrypt_bytes", I64, &[I64, I64, DOUBLE, DOUBLE]); // crypto.generateKeyPairSync(type, options) -> { publicKey, privateKey }. module.declare_function("js_crypto_generate_key_pair_sync", DOUBLE, &[I64, I64]); module.declare_function( diff --git a/crates/perry-codegen/src/runtime_decls/strings_part2.rs b/crates/perry-codegen/src/runtime_decls/strings_part2.rs index 44c013b7bb..e0736fb212 100644 --- a/crates/perry-codegen/src/runtime_decls/strings_part2.rs +++ b/crates/perry-codegen/src/runtime_decls/strings_part2.rs @@ -500,11 +500,11 @@ pub(crate) fn declare_phase_b_strings_part2(module: &mut LlModule) { DOUBLE, &[I64, I64, I64, I64, DOUBLE, DOUBLE], ); - module.declare_function("js_crypto_scrypt_bytes", I64, &[I64, I64, DOUBLE, I64]); + module.declare_function("js_crypto_scrypt_bytes", I64, &[I64, I64, DOUBLE, DOUBLE]); module.declare_function( "js_crypto_scrypt_async", DOUBLE, - &[I64, I64, DOUBLE, DOUBLE], + &[I64, I64, DOUBLE, DOUBLE, DOUBLE], ); module.declare_function("js_crypto_sign_rsa_sha256", I64, &[I64, I64, DOUBLE]); module.declare_function("js_crypto_sign_async", DOUBLE, &[I64, I64, DOUBLE, DOUBLE]); diff --git a/crates/perry-stdlib/src/crypto/kdf.rs b/crates/perry-stdlib/src/crypto/kdf.rs index 9194ff7788..b52b2a08d8 100644 --- a/crates/perry-stdlib/src/crypto/kdf.rs +++ b/crates/perry-stdlib/src/crypto/kdf.rs @@ -149,11 +149,10 @@ pub unsafe extern "C" fn js_crypto_scrypt_async( password_ptr: i64, salt_ptr: i64, keylen: f64, + options_bits: f64, callback_bits: f64, ) -> f64 { - // Routes to the 4-arg scryptSync (defined below) with no options - // object — same default cost parameters as Node. - let buf = js_crypto_scrypt_bytes(password_ptr, salt_ptr, keylen, 0); + let buf = js_crypto_scrypt_bytes(password_ptr, salt_ptr, keylen, options_bits); let value = if buf.is_null() { f64::from_bits(JSValue::undefined().bits()) } else { @@ -883,72 +882,209 @@ pub unsafe extern "C" fn js_crypto_scrypt_custom( js_string_from_bytes(hex_str.as_ptr(), hex_str.len() as u32) } +const SCRYPT_DEFAULT_N: u64 = 16_384; +const SCRYPT_DEFAULT_R: u32 = 8; +const SCRYPT_DEFAULT_P: u32 = 1; +const SCRYPT_DEFAULT_MAXMEM: u64 = 32 << 20; +const MAX_SAFE_INTEGER: u64 = 9_007_199_254_740_991; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ScryptParamError { + Invalid, + MemoryLimit, +} + +fn scrypt_numeric_value(value: JSValue, name: &str, max: u64) -> u64 { + let bits = f64::from_bits(value.bits()); + if !perry_runtime::fs::validate::is_numeric(value) { + let message = format!( + "The \"{name}\" argument must be of type number. Received {}", + perry_runtime::fs::validate::describe_received(bits) + ); + perry_runtime::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_TYPE"); + } + let number = if value.is_int32() { + value.as_int32() as f64 + } else { + value.as_number() + }; + if !number.is_finite() || number.fract() != 0.0 { + let message = format!( + "The value of \"{name}\" is out of range. It must be an integer. Received {}", + perry_runtime::fs::validate::format_received_number(number) + ); + perry_runtime::fs::validate::throw_range_error_with_code(&message); + } + if number < 0.0 || number > max as f64 { + let message = format!( + "The value of \"{name}\" is out of range. It must be >= 0 && <= {max}. Received {}", + perry_runtime::fs::validate::format_received_number(number) + ); + perry_runtime::fs::validate::throw_range_error_with_code(&message); + } + number as u64 +} + +unsafe fn scrypt_options_object(options_bits: f64) -> Option<*const ObjectHeader> { + let value = JSValue::from_bits(options_bits.to_bits()); + if !value.is_pointer() { + return None; + } + let ptr = value.as_pointer::(); + if ptr.is_null() || (ptr as usize) < perry_runtime::gc::GC_HEADER_SIZE + 0x1000 { + return None; + } + let header = + &*(ptr.sub(perry_runtime::gc::GC_HEADER_SIZE) as *const perry_runtime::gc::GcHeader); + if header.obj_type != perry_runtime::gc::GC_TYPE_OBJECT { + return None; + } + Some(ptr as *const ObjectHeader) +} + +unsafe fn read_scrypt_option(obj: *const ObjectHeader, name: &str) -> Option { + let key = js_string_from_bytes(name.as_ptr(), name.len() as u32); + let value = js_object_get_field_by_name(obj, key); + (!value.is_undefined()).then_some(value) +} + +fn throw_incompatible_scrypt_options(primary: &str, alias: &str) -> ! { + let message = + format!("Option \"{primary}\" cannot be used in combination with option \"{alias}\""); + perry_runtime::fs::validate::throw_type_error_with_code( + &message, + "ERR_INCOMPATIBLE_OPTION_PAIR", + ) +} + +unsafe fn read_scrypt_options(options_bits: f64) -> (u64, u32, u32, u64) { + let Some(obj) = scrypt_options_object(options_bits) else { + return ( + SCRYPT_DEFAULT_N, + SCRYPT_DEFAULT_R, + SCRYPT_DEFAULT_P, + SCRYPT_DEFAULT_MAXMEM, + ); + }; + + let n = read_scrypt_option(obj, "N"); + let cost = read_scrypt_option(obj, "cost"); + if n.is_some() && cost.is_some() { + throw_incompatible_scrypt_options("N", "cost"); + } + let r = read_scrypt_option(obj, "r"); + let block_size = read_scrypt_option(obj, "blockSize"); + if r.is_some() && block_size.is_some() { + throw_incompatible_scrypt_options("r", "blockSize"); + } + let p = read_scrypt_option(obj, "p"); + let parallelization = read_scrypt_option(obj, "parallelization"); + if p.is_some() && parallelization.is_some() { + throw_incompatible_scrypt_options("p", "parallelization"); + } + let maxmem = read_scrypt_option(obj, "maxmem"); + + let n_name = if n.is_some() { "N" } else { "cost" }; + let r_name = if r.is_some() { "r" } else { "blockSize" }; + let p_name = if p.is_some() { "p" } else { "parallelization" }; + let mut n = n + .or(cost) + .map(|value| scrypt_numeric_value(value, n_name, u32::MAX as u64)) + .unwrap_or(SCRYPT_DEFAULT_N); + let mut r = r + .or(block_size) + .map(|value| scrypt_numeric_value(value, r_name, u32::MAX as u64) as u32) + .unwrap_or(SCRYPT_DEFAULT_R); + let mut p = p + .or(parallelization) + .map(|value| scrypt_numeric_value(value, p_name, u32::MAX as u64) as u32) + .unwrap_or(SCRYPT_DEFAULT_P); + let mut maxmem = maxmem + .map(|value| scrypt_numeric_value(value, "maxmem", MAX_SAFE_INTEGER)) + .unwrap_or(SCRYPT_DEFAULT_MAXMEM); + + // Node treats explicit zeroes as requests for the corresponding default. + if n == 0 { + n = SCRYPT_DEFAULT_N; + } + if r == 0 { + r = SCRYPT_DEFAULT_R; + } + if p == 0 { + p = SCRYPT_DEFAULT_P; + } + if maxmem == 0 { + maxmem = SCRYPT_DEFAULT_MAXMEM; + } + (n, r, p, maxmem) +} + +fn checked_scrypt_params( + n: u64, + r: u32, + p: u32, + maxmem: u64, +) -> Result { + if n <= 1 || !n.is_power_of_two() { + return Err(ScryptParamError::Invalid); + } + let log_n = n.trailing_zeros() as u8; + // Params::len belongs to the crate's PHC-string facade, not its low-level + // `scrypt` function. Keep it valid independently of Node's requested + // output length, which may be any int32-sized byte count. + let params = scrypt::Params::new(log_n, r, p, 32).map_err(|_| ScryptParamError::Invalid)?; + + // OpenSSL's SCRYPT_MAX_MEM check includes B, V, and XY: + // 128 * r * (N + p + 2). This is stricter than the dominant 128*N*r + // term and matches Node at the exact acceptance boundary. + let workspace = 128u64 + .checked_mul(r as u64) + .and_then(|block| n.checked_add(p as u64)?.checked_add(2)?.checked_mul(block)) + .ok_or(ScryptParamError::Invalid)?; + if workspace > maxmem { + return Err(ScryptParamError::MemoryLimit); + } + Ok(params) +} + +fn throw_scrypt_param_error(error: ScryptParamError) -> ! { + let message = match error { + ScryptParamError::Invalid => "Invalid scrypt params", + ScryptParamError::MemoryLimit => { + "Invalid scrypt params: error:030000AC:digital envelope routines::memory limit exceeded" + } + }; + perry_runtime::fs::validate::throw_range_error_named( + message, + "ERR_CRYPTO_INVALID_SCRYPT_PARAMS", + ) +} + /// `crypto.scryptSync(password, salt, keylen[, options])` → Buffer. /// -/// Unlike `js_crypto_scrypt` (which returns a hex string), this returns a -/// Buffer to match Node's `scryptSync`, and reads password/salt via -/// `bytes_from_ptr` so Buffer inputs hash correctly. Optional cost -/// parameters are read from `options_ptr` (a NaN-unboxed object pointer, or -/// a null/sentinel for none): `N`/`cost`, `r`/`blockSize`, `p`/ -/// `parallelization`. Defaults match Node: N=16384, r=8, p=1. +/// The same helper backs the callback form. It receives the full NaN-boxed +/// options value, validates Node's primary/alias names, and never substitutes +/// defaults for an invalid or unsupported parameter combination. #[no_mangle] pub unsafe extern "C" fn js_crypto_scrypt_bytes( password_ptr: i64, salt_ptr: i64, key_length: f64, - options_ptr: i64, + options_bits: f64, ) -> *mut perry_runtime::buffer::BufferHeader { - use perry_runtime::{js_object_get_field_by_name, ObjectHeader}; let password = bytes_from_ptr(password_ptr); let salt = bytes_from_ptr(salt_ptr); - let klen = key_length as usize; - if klen == 0 || klen > 1024 { + let keylen_value = JSValue::from_bits(key_length.to_bits()); + let klen = scrypt_numeric_value(keylen_value, "keylen", i32::MAX as u64) as usize; + let (n, r, p, maxmem) = read_scrypt_options(options_bits); + let params = checked_scrypt_params(n, r, p, maxmem) + .unwrap_or_else(|error| throw_scrypt_param_error(error)); + if klen == 0 { return alloc_buffer_from_slice(&[]); } - // Node defaults: N=16384 (cost), r=8 (blockSize), p=1 (parallelization). - let (mut n, mut r, mut p) = (16384u64, 8u32, 1u32); - if (options_ptr as usize) >= 0x1000 { - let obj = options_ptr as *const ObjectHeader; - // Read a numeric option by primary or alias name; None if absent. - let read = |primary: &str, alias: &str| -> Option { - let pk = js_string_from_bytes(primary.as_ptr(), primary.len() as u32); - let v = js_object_get_field_by_name(obj, pk); - if !v.is_undefined() { - return Some(v.to_number()); - } - let ak = js_string_from_bytes(alias.as_ptr(), alias.len() as u32); - let v = js_object_get_field_by_name(obj, ak); - if v.is_undefined() { - None - } else { - Some(v.to_number()) - } - }; - if let Some(x) = read("N", "cost") { - if x >= 1.0 { - n = x as u64; - } - } - if let Some(x) = read("r", "blockSize") { - if x >= 1.0 { - r = x as u32; - } - } - if let Some(x) = read("p", "parallelization") { - if x >= 1.0 { - p = x as u32; - } - } - } - // `scrypt::Params` takes log2(N); Node requires N to be a power of two, - // so trailing_zeros gives the exact exponent. A non-power-of-two or an - // otherwise-invalid combo falls back to the Node defaults. - let log_n = n.trailing_zeros() as u8; - let params = scrypt::Params::new(log_n, r, p, klen) - .unwrap_or_else(|_| scrypt::Params::new(14, 8, 1, klen).unwrap()); let mut out = vec![0u8; klen]; if scrypt::scrypt(&password, &salt, ¶ms, &mut out).is_err() { - return alloc_buffer_from_slice(&[]); + throw_scrypt_param_error(ScryptParamError::Invalid); } alloc_buffer_from_slice(&out) } @@ -1100,3 +1236,83 @@ pub(super) unsafe fn build_key_pair_object(pub_pem: &str, priv_pem: &str) -> f64 js_object_set_keys(obj, keys); nanbox_pointer_f64(obj as usize) } + +#[cfg(test)] +mod scrypt_tests { + use super::*; + + const TEST_MAXMEM: u64 = 512 * 1024 * 1024; + + fn digest(n: u64, r: u32, p: u32, keylen: usize) -> String { + let password = b"correct horse battery staple"; + let salt = hex::decode("0123456789abcdef0123456789abcdef").unwrap(); + let params = checked_scrypt_params(n, r, p, TEST_MAXMEM).unwrap(); + let mut output = vec![0; keylen]; + scrypt::scrypt(password, &salt, ¶ms, &mut output).unwrap(); + hex::encode(output) + } + + #[test] + fn scrypt_cost_parameters_produce_exact_node_digests() { + let vectors = [ + ( + 1 << 12, + "05f47c22e65fc21d3e11a92222323c577271be35cea9b34b4a6970e2fa3ebf48", + ), + ( + 1 << 13, + "a2d6acfcc30e33aa067080845b9f4790427da3d8f992e651765c095abb7cd276", + ), + ( + 1 << 14, + "33e39503baad99447708713ceec3f39bb876329254d4f5cd93da92a65d983f01", + ), + ( + 1 << 15, + "cb18e56f62485654eba440e7cc2fcaff9f92102d944dab86b13642272dfeb1f4", + ), + ( + 1 << 16, + "9d12077809f9271ef0dd063bff62817d49d53c7f1acb8e42c67516c5b0287cf1", + ), + ( + 1 << 17, + "e5581239883361913bc8cd281ef2a9d7e5bf2171f4b6c8eb0292e99077483208", + ), + ]; + for (n, expected) in vectors { + assert_eq!(digest(n, 8, 1, 32), expected, "N={n}"); + } + assert_eq!( + digest(1 << 14, 4, 1, 32), + "cd4b3b2db0af02f65f71c999901e07abba66fd088d65268928a7e971a50651f0" + ); + assert_eq!( + digest(1 << 14, 8, 2, 32), + "77636ab3d38dd67285438f5694c64bfa9fe81144034a49e7599511136bcfb071" + ); + assert_eq!( + digest(1 << 12, 8, 1, 64), + "05f47c22e65fc21d3e11a92222323c577271be35cea9b34b4a6970e2fa3ebf48\ + 1399c1db6db41b7c49fcc677fbe03319ec8e9c37a73734dd9eec71bd1d9ed212" + .replace(' ', "") + ); + } + + #[test] + fn scrypt_rejects_invalid_parameters_and_enforces_exact_memory_boundary() { + assert_eq!( + checked_scrypt_params(3, 8, 1, TEST_MAXMEM).unwrap_err(), + ScryptParamError::Invalid + ); + assert_eq!( + checked_scrypt_params(1, 8, 1, TEST_MAXMEM).unwrap_err(), + ScryptParamError::Invalid + ); + assert_eq!( + checked_scrypt_params(4096, 8, 1, 4_197_375).unwrap_err(), + ScryptParamError::MemoryLimit + ); + assert!(checked_scrypt_params(4096, 8, 1, 4_197_376).is_ok()); + } +} diff --git a/crates/perry-stdlib/src/crypto/random.rs b/crates/perry-stdlib/src/crypto/random.rs index 8e9d0f94d5..3c846dc049 100644 --- a/crates/perry-stdlib/src/crypto/random.rs +++ b/crates/perry-stdlib/src/crypto/random.rs @@ -434,13 +434,14 @@ pub unsafe extern "C" fn js_crypto_native_dispatch( js_crypto_pbkdf2_async_alg(bytes_ptr(0), bytes_ptr(1), arg(2), arg(3), digest, callback) } "scrypt" => { + let options = if args_len >= 5 { arg(3) } else { undefined }; let callback = if args_len >= 5 { arg(4) } else { arg(3) }; - js_crypto_scrypt_async(bytes_ptr(0), bytes_ptr(1), arg(2), callback) + js_crypto_scrypt_async(bytes_ptr(0), bytes_ptr(1), arg(2), options, callback) } "scryptSync" => { - let options_ptr = if args_len >= 4 { bytes_ptr(3) } else { 0 }; + let options = if args_len >= 4 { arg(3) } else { undefined }; pointer_value( - js_crypto_scrypt_bytes(bytes_ptr(0), bytes_ptr(1), arg(2), options_ptr) as *mut u8, + js_crypto_scrypt_bytes(bytes_ptr(0), bytes_ptr(1), arg(2), options) as *mut u8, ) } // Node callback forms are randomInt(max, callback) and diff --git a/test-parity/node-suite/crypto/scrypt/options.ts b/test-parity/node-suite/crypto/scrypt/options.ts new file mode 100644 index 0000000000..dfb430db8c --- /dev/null +++ b/test-parity/node-suite/crypto/scrypt/options.ts @@ -0,0 +1,109 @@ +import * as crypto from "node:crypto"; +import { promisify } from "node:util"; + +const password = "correct horse battery staple"; +const salt = Buffer.from("0123456789abcdef0123456789abcdef", "hex"); +const maxmem = 512 * 1024 * 1024; +const vectors: Array<[number, string]> = [ + [1 << 12, "05f47c22e65fc21d3e11a92222323c577271be35cea9b34b4a6970e2fa3ebf48"], + [1 << 13, "a2d6acfcc30e33aa067080845b9f4790427da3d8f992e651765c095abb7cd276"], + [1 << 14, "33e39503baad99447708713ceec3f39bb876329254d4f5cd93da92a65d983f01"], + [1 << 15, "cb18e56f62485654eba440e7cc2fcaff9f92102d944dab86b13642272dfeb1f4"], + [1 << 16, "9d12077809f9271ef0dd063bff62817d49d53c7f1acb8e42c67516c5b0287cf1"], + [1 << 17, "e5581239883361913bc8cd281ef2a9d7e5bf2171f4b6c8eb0292e99077483208"], +]; + +function equal(actual: string, expected: string, label: string): void { + if (actual !== expected) { + throw new Error(label + ": expected " + expected + ", got " + actual); + } +} + +function requireScryptRangeError(error: any, fragment: string): void { + if (error?.name !== "RangeError") { + throw new Error("expected RangeError, got " + String(error)); + } + if (error?.code !== "ERR_CRYPTO_INVALID_SCRYPT_PARAMS") { + throw new Error("unexpected error code: " + String(error?.code)); + } + if (!String(error).includes(fragment)) { + throw new Error("unexpected error message: " + String(error)); + } +} + +for (const [N, expected] of vectors) { + equal( + crypto.scryptSync(password, salt, 32, { N, r: 8, p: 1, maxmem }).toString("hex"), + expected, + "scryptSync N=" + N, + ); +} +equal( + crypto.scryptSync(password, salt, 32, { N: 1 << 14, r: 4, p: 1, maxmem }).toString("hex"), + "cd4b3b2db0af02f65f71c999901e07abba66fd088d65268928a7e971a50651f0", + "scryptSync r=4", +); +equal( + crypto.scryptSync(password, salt, 32, { N: 1 << 14, r: 8, p: 2, maxmem }).toString("hex"), + "77636ab3d38dd67285438f5694c64bfa9fe81144034a49e7599511136bcfb071", + "scryptSync p=2", +); +equal( + crypto.scryptSync(password, salt, 64, { N: 1 << 12, r: 8, p: 1, maxmem }).toString("hex"), + "05f47c22e65fc21d3e11a92222323c577271be35cea9b34b4a6970e2fa3ebf48" + + "1399c1db6db41b7c49fcc677fbe03319ec8e9c37a73734dd9eec71bd1d9ed212", + "scryptSync keylen=64", +); +equal( + crypto.scryptSync(password, salt, 32, { + cost: 1 << 12, + blockSize: 8, + parallelization: 1, + maxmem, + }).toString("hex"), + vectors[0][1], + "scryptSync aliases", +); +console.log("scryptSync parameter vectors: ok"); + +const scrypt = promisify(crypto.scrypt); +for (const [N, expected] of vectors) { + const result: any = await scrypt(password, salt, 32, { N, r: 8, p: 1, maxmem }); + equal(result.toString("hex"), expected, "scrypt async N=" + N); +} +console.log("scrypt async parameter vectors: ok"); + +await new Promise((resolve, reject) => { + crypto.scrypt(password, salt, 32, { N: 1 << 14, r: 4, p: 1, maxmem }, (error, key) => { + if (error) { + reject(error); + return; + } + try { + equal( + key.toString("hex"), + "cd4b3b2db0af02f65f71c999901e07abba66fd088d65268928a7e971a50651f0", + "scrypt callback r=4", + ); + resolve(); + } catch (caught) { + reject(caught); + } + }); +}); +console.log("scrypt direct callback options: ok"); + +try { + crypto.scryptSync(password, salt, 32, { N: 3, r: 8, p: 1, maxmem }); + throw new Error("invalid N did not throw"); +} catch (error: any) { + requireScryptRangeError(error, "Invalid scrypt params"); +} + +try { + await scrypt(password, salt, 32, { N: 1 << 17, r: 8, p: 1, maxmem: 1024 }); + throw new Error("insufficient maxmem did not throw"); +} catch (error: any) { + requireScryptRangeError(error, "memory limit exceeded"); +} +console.log("scrypt invalid parameter errors: ok"); From 369e56a610aea5e884f2967f060cd2206c5b46de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 19:45:11 +0200 Subject: [PATCH 3/3] refactor(link): split the HarmonyOS native-object scan out of build_and_run.rs build_and_run.rs was at 1994 lines; #8930's archive-group call takes it over the 2000-line gate. The HarmonyOS block needs only cmd/target/format, so it moves to a sibling module unchanged. --- .../commands/compile/link/build_and_run.rs | 65 +--------------- .../compile/link/harmonyos_objects.rs | 78 +++++++++++++++++++ crates/perry/src/commands/compile/link/mod.rs | 1 + 3 files changed, 80 insertions(+), 64 deletions(-) create mode 100644 crates/perry/src/commands/compile/link/harmonyos_objects.rs diff --git a/crates/perry/src/commands/compile/link/build_and_run.rs b/crates/perry/src/commands/compile/link/build_and_run.rs index 9fc5346d1a..0b507130ff 100644 --- a/crates/perry/src/commands/compile/link/build_and_run.rs +++ b/crates/perry/src/commands/compile/link/build_and_run.rs @@ -176,70 +176,7 @@ pub(crate) fn build_and_run_link( // (mimalloc is the only C dep in perry-runtime's closure today) and // any that turn out unreferenced are dead-stripped via --gc-sections. if is_harmonyos { - let triple = super::rust_target_triple(target).unwrap_or("aarch64-unknown-linux-ohos"); - let build_roots: Vec = { - let mut roots: Vec = Vec::new(); - // auto_rebuild emits into a perry-auto- dir; the workspace's - // own target/ is a fallback for non-auto flows. - if let Ok(entries) = std::fs::read_dir("target") { - for entry in entries.flatten() { - let name = entry.file_name(); - let name_str = name.to_string_lossy(); - if name_str.starts_with("perry-auto-") || name_str == triple { - roots.push(entry.path()); - } - } - } - // When invoked from outside the workspace, auto_rebuild still - // lands under the perry source tree's target/. Add that. - if let Some(ws_root) = super::super::find_perry_workspace_root() { - let ws_target = ws_root.join("target"); - if let Ok(entries) = std::fs::read_dir(&ws_target) { - for entry in entries.flatten() { - let name = entry.file_name(); - let name_str = name.to_string_lossy(); - if name_str.starts_with("perry-auto-") { - roots.push(entry.path()); - } - } - } - } - roots - }; - let mut native_objs: Vec = Vec::new(); - for root in &build_roots { - let build_dir = root.join(triple).join("release").join("build"); - let entries = match std::fs::read_dir(&build_dir) { - Ok(e) => e, - Err(_) => continue, - }; - for crate_build in entries.flatten() { - let out_dir = crate_build.path().join("out"); - // Walk the out/ dir recursively (cc-rs can nest into source- - // mirror subdirs like c_src/mimalloc/v2/src/). - if let Ok(walker) = walkdir::WalkDir::new(&out_dir) - .into_iter() - .collect::, _>>() - { - for entry in walker { - if entry.file_type().is_file() - && entry.path().extension().and_then(|e| e.to_str()) == Some("o") - { - native_objs.push(entry.path().to_path_buf()); - } - } - } - } - } - if !native_objs.is_empty() && matches!(format, crate::OutputFormat::Text) { - println!( - " harmonyos: linking {} build.rs native object(s)", - native_objs.len() - ); - } - for obj in native_objs { - cmd.arg(obj); - } + super::harmonyos_objects::push_harmonyos_native_objects(&mut cmd, target, format); } // Dead code stripping — safe because compile_init() emits func_addr diff --git a/crates/perry/src/commands/compile/link/harmonyos_objects.rs b/crates/perry/src/commands/compile/link/harmonyos_objects.rs new file mode 100644 index 0000000000..32540d1657 --- /dev/null +++ b/crates/perry/src/commands/compile/link/harmonyos_objects.rs @@ -0,0 +1,78 @@ +//! HarmonyOS: collect the native objects `build.rs` scripts emit, split out of +//! `build_and_run.rs` to keep that file under the 2000-line gate. Behaviour is +//! unchanged — this is the block that used to sit inline behind `is_harmonyos`. + +use super::*; + +/// Append every `build.rs`-produced `.o` under the HarmonyOS build roots to the +/// link line. +/// +/// `auto_rebuild` emits into a `perry-auto-` directory; the workspace's +/// own `target/` is the fallback for non-auto flows, and a run invoked from +/// outside the workspace still lands under the perry source tree's `target/`. +pub(super) fn push_harmonyos_native_objects( + cmd: &mut std::process::Command, + target: Option<&str>, + format: crate::OutputFormat, +) { + let triple = super::rust_target_triple(target).unwrap_or("aarch64-unknown-linux-ohos"); + let build_roots: Vec = { + let mut roots: Vec = Vec::new(); + if let Ok(entries) = std::fs::read_dir("target") { + for entry in entries.flatten() { + let name = entry.file_name(); + let name_str = name.to_string_lossy(); + if name_str.starts_with("perry-auto-") || name_str == triple { + roots.push(entry.path()); + } + } + } + if let Some(ws_root) = super::super::find_perry_workspace_root() { + let ws_target = ws_root.join("target"); + if let Ok(entries) = std::fs::read_dir(&ws_target) { + for entry in entries.flatten() { + let name = entry.file_name(); + let name_str = name.to_string_lossy(); + if name_str.starts_with("perry-auto-") { + roots.push(entry.path()); + } + } + } + } + roots + }; + let mut native_objs: Vec = Vec::new(); + for root in &build_roots { + let build_dir = root.join(triple).join("release").join("build"); + let entries = match std::fs::read_dir(&build_dir) { + Ok(e) => e, + Err(_) => continue, + }; + for crate_build in entries.flatten() { + let out_dir = crate_build.path().join("out"); + // Walk the out/ dir recursively (cc-rs can nest into source-mirror + // subdirs like c_src/mimalloc/v2/src/). + if let Ok(walker) = walkdir::WalkDir::new(&out_dir) + .into_iter() + .collect::, _>>() + { + for entry in walker { + if entry.file_type().is_file() + && entry.path().extension().and_then(|e| e.to_str()) == Some("o") + { + native_objs.push(entry.path().to_path_buf()); + } + } + } + } + } + if !native_objs.is_empty() && matches!(format, crate::OutputFormat::Text) { + println!( + " harmonyos: linking {} build.rs native object(s)", + native_objs.len() + ); + } + for obj in native_objs { + cmd.arg(obj); + } +} diff --git a/crates/perry/src/commands/compile/link/mod.rs b/crates/perry/src/commands/compile/link/mod.rs index a988b8b28e..82e5fb6046 100644 --- a/crates/perry/src/commands/compile/link/mod.rs +++ b/crates/perry/src/commands/compile/link/mod.rs @@ -46,6 +46,7 @@ use super::{ mod archive_cache; mod build_and_run; +mod harmonyos_objects; mod link_cache; mod linux_dylib_libs; mod linux_ui_libs;