From 85fe9cc74e6c78d61959648095c652f65069dd95 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Fri, 21 Nov 2025 12:09:53 +0100 Subject: [PATCH 01/14] verify workers at start up --- Cargo.lock | 1 + .../node/core/pvf/common/src/worker/mod.rs | 2 +- polkadot/node/service/Cargo.toml | 1 + polkadot/node/service/src/workers.rs | 22 +++++++++++++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index d42e29dd885d0..1ef65887b2e8f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17090,6 +17090,7 @@ dependencies = [ "polkadot-node-core-provisioner", "polkadot-node-core-pvf", "polkadot-node-core-pvf-checker", + "polkadot-node-core-pvf-common", "polkadot-node-core-runtime-api", "polkadot-node-network-protocol", "polkadot-node-primitives", diff --git a/polkadot/node/core/pvf/common/src/worker/mod.rs b/polkadot/node/core/pvf/common/src/worker/mod.rs index 70dcf055a2628..a301048668416 100644 --- a/polkadot/node/core/pvf/common/src/worker/mod.rs +++ b/polkadot/node/core/pvf/common/src/worker/mod.rs @@ -458,7 +458,7 @@ pub fn run_worker( } /// Provide a consistent message on unexpected worker shutdown. -fn worker_shutdown(worker_info: WorkerInfo, err: &str) -> ! { +pub fn worker_shutdown(worker_info: WorkerInfo, err: &str) -> ! { gum::warn!(target: LOG_TARGET, ?worker_info, "quitting pvf worker ({}): {}", worker_info.kind, err); std::process::exit(1); } diff --git a/polkadot/node/service/Cargo.toml b/polkadot/node/service/Cargo.toml index 50bb0dc698669..f319a2d75f768 100644 --- a/polkadot/node/service/Cargo.toml +++ b/polkadot/node/service/Cargo.toml @@ -89,6 +89,7 @@ thiserror = { workspace = true } # Polkadot polkadot-core-primitives = { workspace = true, default-features = true } polkadot-node-core-parachains-inherent = { workspace = true, default-features = true } +polkadot-node-core-pvf-common = { workspace = true, default-features = true } polkadot-node-network-protocol = { workspace = true, default-features = true } polkadot-node-primitives = { workspace = true, default-features = true } polkadot-node-subsystem = { workspace = true, default-features = true } diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index 73c3aa466608f..37614a2cc3f9e 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -19,6 +19,8 @@ use super::Error; use is_executable::IsExecutable; use std::path::PathBuf; +use polkadot_node_core_pvf_common::worker::{WorkerInfo, WorkerKind, worker_shutdown}; +use polkadot_node_core_pvf_common::worker::security; #[cfg(test)] thread_local! { @@ -97,6 +99,26 @@ pub fn determine_workers_paths( log::warn!("Skipping node/worker version checks. This could result in incorrect behavior in PVF workers."); } + { + let worker_version = polkadot_node_core_pvf::get_worker_version(&prep_worker_path)?; + let mut worker_dir = prep_worker_path.clone(); + let _ = worker_dir.pop(); + + let worker_info = WorkerInfo { + pid: std::process::id(), + kind: WorkerKind::Prepare, + version: Some(worker_version), + worker_dir_path: worker_dir, + }; + + if !security::check_env_vars_were_cleared(&worker_info) { + let err = "Not all env vars were cleared when spawning the process."; + log::warn!("{}", err); + + worker_shutdown(worker_info, err); + } + } + Ok((prep_worker_path, exec_worker_path)) } From d7e0d109bdba2030173e3ce6ba9c66fec8d98ca7 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Sun, 23 Nov 2025 11:10:51 +0100 Subject: [PATCH 02/14] add verify_on_start cli flag --- polkadot/cli/src/cli.rs | 4 ++++ polkadot/cli/src/command.rs | 1 + polkadot/node/service/src/builder/mod.rs | 4 ++++ polkadot/node/service/src/workers.rs | 3 ++- 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/polkadot/cli/src/cli.rs b/polkadot/cli/src/cli.rs index fa8595cc7c57c..fae1427d87f00 100644 --- a/polkadot/cli/src/cli.rs +++ b/polkadot/cli/src/cli.rs @@ -97,6 +97,10 @@ pub struct RunCmd { #[arg(long = "insecure-validator-i-know-what-i-do", requires = "validator")] pub insecure_validator: bool, + /// Verify validator on start. Checks that env vars were cleared on node startup. + #[arg(long, requires = "validator")] + pub verify_on_start: bool, + /// Enable the block authoring backoff that is triggered when finality is lagging. #[arg(long)] pub force_authoring_backoff: bool, diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs index b9366d07f6c54..d6ae3a33e2457 100644 --- a/polkadot/cli/src/command.rs +++ b/polkadot/cli/src/command.rs @@ -278,6 +278,7 @@ where telemetry_worker_handle: None, node_version, secure_validator_mode, + verify_on_start: cli.run.verify_on_start, workers_path: cli.run.workers_path, workers_names: None, overseer_gen, diff --git a/polkadot/node/service/src/builder/mod.rs b/polkadot/node/service/src/builder/mod.rs index 9d52617cc8da9..f0de4366b6a00 100644 --- a/polkadot/node/service/src/builder/mod.rs +++ b/polkadot/node/service/src/builder/mod.rs @@ -76,6 +76,8 @@ pub struct NewFullParams { pub node_version: Option, /// Whether the node is attempting to run as a secure validator. pub secure_validator_mode: bool, + /// Whether validator should be verified on start. For example, check that env vars were cleared. + pub verify_on_start: bool, /// An optional path to a directory containing the workers. pub workers_path: Option, /// Optional custom names for the prepare and execute workers. @@ -197,6 +199,7 @@ where telemetry_worker_handle: _, node_version, secure_validator_mode, + verify_on_start, workers_path, workers_names, overseer_gen, @@ -377,6 +380,7 @@ where let parachains_db = open_database(&config.database)?; let candidate_validation_config = if role.is_authority() { let (prep_worker_path, exec_worker_path) = workers::determine_workers_paths( + verify_on_start, workers_path, workers_names, node_version.clone(), diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index 37614a2cc3f9e..563bee4219fcb 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -55,6 +55,7 @@ fn workers_lib_path_override() -> Option { /// /// 6. At this point the final set of paths should be good to use. pub fn determine_workers_paths( + verify_on_start: bool, given_workers_path: Option, workers_names: Option<(String, String)>, node_version: Option, @@ -99,7 +100,7 @@ pub fn determine_workers_paths( log::warn!("Skipping node/worker version checks. This could result in incorrect behavior in PVF workers."); } - { + if verify_on_start { let worker_version = polkadot_node_core_pvf::get_worker_version(&prep_worker_path)?; let mut worker_dir = prep_worker_path.clone(); let _ = worker_dir.pop(); From f55fc13d73e6f8391ea1fc5dfcc0d3d173ea48c7 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Sun, 23 Nov 2025 12:36:54 +0100 Subject: [PATCH 03/14] add running_node_verify_on_start_fails test --- .../tests/running_the_node_and_interrupt.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/polkadot/tests/running_the_node_and_interrupt.rs b/polkadot/tests/running_the_node_and_interrupt.rs index 053acc9667964..22555004ab1de 100644 --- a/polkadot/tests/running_the_node_and_interrupt.rs +++ b/polkadot/tests/running_the_node_and_interrupt.rs @@ -17,6 +17,10 @@ use assert_cmd::cargo::cargo_bin; use std::process::{self, Command}; use tempfile::tempdir; +use nix::{ + sys::signal::{kill, Signal::SIGINT}, + unistd::Pid, +}; pub mod common; @@ -59,3 +63,22 @@ async fn running_the_node_works_and_can_be_interrupted() { run_command_and_kill(SIGINT).await; run_command_and_kill(SIGTERM).await; } + +#[test] +fn running_node_verify_on_start_fails() { + let tmp_dir = tempdir().expect("could not create a temp dir"); + let base_path = tmp_dir.path(); + let mut cmd = Command::new(cargo_bin("polkadot")) + .stdout(process::Stdio::piped()) + .stderr(process::Stdio::piped()) + .args([ + "--chain", + "westend", + "--verify-on-start", + ]) + .arg("-d") + .arg(base_path) + .arg("--no-hardware-benchmarks") + .spawn() + .expect("Not all env vars were cleared when spawning the process."); +} \ No newline at end of file From c4bbbec0f093d71ce6fb038e581705cf5e31de3e Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Sun, 23 Nov 2025 12:43:25 +0100 Subject: [PATCH 04/14] add newline --- polkadot/tests/running_the_node_and_interrupt.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/tests/running_the_node_and_interrupt.rs b/polkadot/tests/running_the_node_and_interrupt.rs index 22555004ab1de..6fc3b228c5adc 100644 --- a/polkadot/tests/running_the_node_and_interrupt.rs +++ b/polkadot/tests/running_the_node_and_interrupt.rs @@ -81,4 +81,4 @@ fn running_node_verify_on_start_fails() { .arg("--no-hardware-benchmarks") .spawn() .expect("Not all env vars were cleared when spawning the process."); -} \ No newline at end of file +} From b378ea487b445d5b8b41d2e30e36785f86aa1115 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Sun, 23 Nov 2025 18:49:10 +0100 Subject: [PATCH 05/14] add assertion to test --- polkadot/tests/running_the_node_and_interrupt.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/polkadot/tests/running_the_node_and_interrupt.rs b/polkadot/tests/running_the_node_and_interrupt.rs index 6fc3b228c5adc..6223a192aa558 100644 --- a/polkadot/tests/running_the_node_and_interrupt.rs +++ b/polkadot/tests/running_the_node_and_interrupt.rs @@ -17,10 +17,7 @@ use assert_cmd::cargo::cargo_bin; use std::process::{self, Command}; use tempfile::tempdir; -use nix::{ - sys::signal::{kill, Signal::SIGINT}, - unistd::Pid, -}; +use assert_cmd::assert::OutputAssertExt; pub mod common; @@ -68,17 +65,21 @@ async fn running_the_node_works_and_can_be_interrupted() { fn running_node_verify_on_start_fails() { let tmp_dir = tempdir().expect("could not create a temp dir"); let base_path = tmp_dir.path(); - let mut cmd = Command::new(cargo_bin("polkadot")) + Command::new(cargo_bin("polkadot")) .stdout(process::Stdio::piped()) .stderr(process::Stdio::piped()) .args([ "--chain", "westend", + "--force-authoring", + "--alice", + "--unsafe-force-node-key-generation", + "--validator", "--verify-on-start", ]) .arg("-d") .arg(base_path) .arg("--no-hardware-benchmarks") - .spawn() - .expect("Not all env vars were cleared when spawning the process."); + .assert() + .failure(); } From d4cea42abf7341f2e2d9f9e1b5521f2adc922d27 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 08:19:41 +0100 Subject: [PATCH 06/14] remove earlier impl --- polkadot/cli/src/cli.rs | 4 ---- polkadot/cli/src/command.rs | 1 - polkadot/node/service/src/builder/mod.rs | 4 ---- polkadot/node/service/src/workers.rs | 21 ----------------- .../tests/running_the_node_and_interrupt.rs | 23 ------------------- 5 files changed, 53 deletions(-) diff --git a/polkadot/cli/src/cli.rs b/polkadot/cli/src/cli.rs index fae1427d87f00..fa8595cc7c57c 100644 --- a/polkadot/cli/src/cli.rs +++ b/polkadot/cli/src/cli.rs @@ -97,10 +97,6 @@ pub struct RunCmd { #[arg(long = "insecure-validator-i-know-what-i-do", requires = "validator")] pub insecure_validator: bool, - /// Verify validator on start. Checks that env vars were cleared on node startup. - #[arg(long, requires = "validator")] - pub verify_on_start: bool, - /// Enable the block authoring backoff that is triggered when finality is lagging. #[arg(long)] pub force_authoring_backoff: bool, diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs index d6ae3a33e2457..b9366d07f6c54 100644 --- a/polkadot/cli/src/command.rs +++ b/polkadot/cli/src/command.rs @@ -278,7 +278,6 @@ where telemetry_worker_handle: None, node_version, secure_validator_mode, - verify_on_start: cli.run.verify_on_start, workers_path: cli.run.workers_path, workers_names: None, overseer_gen, diff --git a/polkadot/node/service/src/builder/mod.rs b/polkadot/node/service/src/builder/mod.rs index f0de4366b6a00..9d52617cc8da9 100644 --- a/polkadot/node/service/src/builder/mod.rs +++ b/polkadot/node/service/src/builder/mod.rs @@ -76,8 +76,6 @@ pub struct NewFullParams { pub node_version: Option, /// Whether the node is attempting to run as a secure validator. pub secure_validator_mode: bool, - /// Whether validator should be verified on start. For example, check that env vars were cleared. - pub verify_on_start: bool, /// An optional path to a directory containing the workers. pub workers_path: Option, /// Optional custom names for the prepare and execute workers. @@ -199,7 +197,6 @@ where telemetry_worker_handle: _, node_version, secure_validator_mode, - verify_on_start, workers_path, workers_names, overseer_gen, @@ -380,7 +377,6 @@ where let parachains_db = open_database(&config.database)?; let candidate_validation_config = if role.is_authority() { let (prep_worker_path, exec_worker_path) = workers::determine_workers_paths( - verify_on_start, workers_path, workers_names, node_version.clone(), diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index 563bee4219fcb..e9c80fc4b54a0 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -55,7 +55,6 @@ fn workers_lib_path_override() -> Option { /// /// 6. At this point the final set of paths should be good to use. pub fn determine_workers_paths( - verify_on_start: bool, given_workers_path: Option, workers_names: Option<(String, String)>, node_version: Option, @@ -100,26 +99,6 @@ pub fn determine_workers_paths( log::warn!("Skipping node/worker version checks. This could result in incorrect behavior in PVF workers."); } - if verify_on_start { - let worker_version = polkadot_node_core_pvf::get_worker_version(&prep_worker_path)?; - let mut worker_dir = prep_worker_path.clone(); - let _ = worker_dir.pop(); - - let worker_info = WorkerInfo { - pid: std::process::id(), - kind: WorkerKind::Prepare, - version: Some(worker_version), - worker_dir_path: worker_dir, - }; - - if !security::check_env_vars_were_cleared(&worker_info) { - let err = "Not all env vars were cleared when spawning the process."; - log::warn!("{}", err); - - worker_shutdown(worker_info, err); - } - } - Ok((prep_worker_path, exec_worker_path)) } diff --git a/polkadot/tests/running_the_node_and_interrupt.rs b/polkadot/tests/running_the_node_and_interrupt.rs index 6223a192aa558..6104a5c22cfe2 100644 --- a/polkadot/tests/running_the_node_and_interrupt.rs +++ b/polkadot/tests/running_the_node_and_interrupt.rs @@ -60,26 +60,3 @@ async fn running_the_node_works_and_can_be_interrupted() { run_command_and_kill(SIGINT).await; run_command_and_kill(SIGTERM).await; } - -#[test] -fn running_node_verify_on_start_fails() { - let tmp_dir = tempdir().expect("could not create a temp dir"); - let base_path = tmp_dir.path(); - Command::new(cargo_bin("polkadot")) - .stdout(process::Stdio::piped()) - .stderr(process::Stdio::piped()) - .args([ - "--chain", - "westend", - "--force-authoring", - "--alice", - "--unsafe-force-node-key-generation", - "--validator", - "--verify-on-start", - ]) - .arg("-d") - .arg(base_path) - .arg("--no-hardware-benchmarks") - .assert() - .failure(); -} From c1b3ba39bb008cb2a9e8c422db8856d829e67a19 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 09:15:24 +0100 Subject: [PATCH 07/14] add --check-all flag for worker and integrate check into determine_workers_paths --- .../node/core/pvf/common/src/worker/mod.rs | 35 ++++++++++++++++++- polkadot/node/service/src/workers.rs | 24 ++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/polkadot/node/core/pvf/common/src/worker/mod.rs b/polkadot/node/core/pvf/common/src/worker/mod.rs index a301048668416..a045a06235a5d 100644 --- a/polkadot/node/core/pvf/common/src/worker/mod.rs +++ b/polkadot/node/core/pvf/common/src/worker/mod.rs @@ -81,7 +81,40 @@ macro_rules! decl_worker_main { println!("{}", get_full_version()); return }, - + "--check-all" => { + #[cfg(target_os = "linux")] + let mut status = 0; + #[cfg(target_os = "linux")] + if let Err(err) = security::landlock::check_can_fully_enable() { + eprintln!("{}", err); + status = -1; + } + #[cfg(all(target_os = "linux", target_arch = "x86_64"))] + if let Err(err) = security::seccomp::check_can_fully_enable() { + eprintln!("{}", err); + status = -1; + } + #[cfg(target_os = "linux")] + { + let cache_path_tempdir = std::path::Path::new(&args[2]); + if let Err(err) = + security::change_root::check_can_fully_enable(&cache_path_tempdir) + { + eprintln!("{}", err); + status = -1; + } + } + #[cfg(target_os = "linux")] + // SAFETY: new process is spawned within a single threaded process. This + // invariant is enforced by tests. + if let Err(err) = unsafe { security::clone::check_can_fully_clone() } { + eprintln!("{}", err); + status = -1; + } + #[cfg(not(target_os = "linux"))] + let status = -1; + std::process::exit(status) + }, "--check-can-enable-landlock" => { #[cfg(target_os = "linux")] let status = if let Err(err) = security::landlock::check_can_fully_enable() { diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index e9c80fc4b54a0..b9e737da4d73e 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -20,7 +20,7 @@ use super::Error; use is_executable::IsExecutable; use std::path::PathBuf; use polkadot_node_core_pvf_common::worker::{WorkerInfo, WorkerKind, worker_shutdown}; -use polkadot_node_core_pvf_common::worker::security; +use std::process::Command; #[cfg(test)] thread_local! { @@ -99,6 +99,28 @@ pub fn determine_workers_paths( log::warn!("Skipping node/worker version checks. This could result in incorrect behavior in PVF workers."); } + let worker_version = polkadot_node_core_pvf::get_worker_version(&prep_worker_path)?; + let mut worker_dir = prep_worker_path.clone(); + let _ = worker_dir.pop(); + + let worker_info = WorkerInfo { + pid: std::process::id(), + kind: WorkerKind::Prepare, + version: Some(worker_version), + worker_dir_path: worker_dir, + }; + + let exit_status = Command::new(&exec_worker_path) + .arg("--check-all") + .status() + .unwrap(); + + if !exit_status.success() { + let err = "Not all env vars were cleared when spawning the process."; + log::warn!("{}", err); + worker_shutdown(worker_info, err); + } + Ok((prep_worker_path, exec_worker_path)) } From 311a088034412e689e096ba145c947eddb901fa3 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 09:22:48 +0100 Subject: [PATCH 08/14] remove assert_cmd --- polkadot/tests/running_the_node_and_interrupt.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/polkadot/tests/running_the_node_and_interrupt.rs b/polkadot/tests/running_the_node_and_interrupt.rs index 6104a5c22cfe2..053acc9667964 100644 --- a/polkadot/tests/running_the_node_and_interrupt.rs +++ b/polkadot/tests/running_the_node_and_interrupt.rs @@ -17,7 +17,6 @@ use assert_cmd::cargo::cargo_bin; use std::process::{self, Command}; use tempfile::tempdir; -use assert_cmd::assert::OutputAssertExt; pub mod common; From 6c83437e4c34e9a4252aa99aa01e7c4b58afadf0 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 10:09:53 +0100 Subject: [PATCH 09/14] add target_os --- polkadot/node/service/src/workers.rs | 43 ++++++++++++++++------------ 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index b9e737da4d73e..25b5cd0934b0a 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -19,7 +19,9 @@ use super::Error; use is_executable::IsExecutable; use std::path::PathBuf; +#[cfg(target_os = "linux")] use polkadot_node_core_pvf_common::worker::{WorkerInfo, WorkerKind, worker_shutdown}; +#[cfg(target_os = "linux")] use std::process::Command; #[cfg(test)] @@ -99,26 +101,29 @@ pub fn determine_workers_paths( log::warn!("Skipping node/worker version checks. This could result in incorrect behavior in PVF workers."); } - let worker_version = polkadot_node_core_pvf::get_worker_version(&prep_worker_path)?; - let mut worker_dir = prep_worker_path.clone(); - let _ = worker_dir.pop(); - - let worker_info = WorkerInfo { - pid: std::process::id(), - kind: WorkerKind::Prepare, - version: Some(worker_version), - worker_dir_path: worker_dir, - }; - - let exit_status = Command::new(&exec_worker_path) - .arg("--check-all") - .status() - .unwrap(); + #[cfg(target_os = "linux")] + { + let worker_version = polkadot_node_core_pvf::get_worker_version(&prep_worker_path)?; + let mut worker_dir = prep_worker_path.clone(); + let _ = worker_dir.pop(); + + let worker_info = WorkerInfo { + pid: std::process::id(), + kind: WorkerKind::Prepare, + version: Some(worker_version), + worker_dir_path: worker_dir, + }; + + let exit_status = Command::new(&exec_worker_path) + .arg("--check-all") + .status() + .unwrap(); - if !exit_status.success() { - let err = "Not all env vars were cleared when spawning the process."; - log::warn!("{}", err); - worker_shutdown(worker_info, err); + if !exit_status.success() { + let err = "Not all env vars were cleared when spawning the process."; + log::warn!("{}", err); + worker_shutdown(worker_info, err); + } } Ok((prep_worker_path, exec_worker_path)) From cf0740d8f7ca65ea217e5386851f72ae1f9f3bd9 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 10:22:07 +0100 Subject: [PATCH 10/14] update error message --- polkadot/node/service/src/workers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index 25b5cd0934b0a..cc1e7c46db6b2 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -120,7 +120,7 @@ pub fn determine_workers_paths( .unwrap(); if !exit_status.success() { - let err = "Not all env vars were cleared when spawning the process."; + let err = "Worker security checks failed."; log::warn!("{}", err); worker_shutdown(worker_info, err); } From 37561e7189225e05039b88bf29130f2af7753ea4 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 13:00:27 +0100 Subject: [PATCH 11/14] refactor macro --- .../node/core/pvf/common/src/worker/mod.rs | 78 +++++++++---------- polkadot/node/service/src/workers.rs | 31 +------- polkadot/src/bin/execute-worker.rs | 1 + polkadot/src/bin/prepare-worker.rs | 1 + 4 files changed, 44 insertions(+), 67 deletions(-) diff --git a/polkadot/node/core/pvf/common/src/worker/mod.rs b/polkadot/node/core/pvf/common/src/worker/mod.rs index a045a06235a5d..d7aa597172adb 100644 --- a/polkadot/node/core/pvf/common/src/worker/mod.rs +++ b/polkadot/node/core/pvf/common/src/worker/mod.rs @@ -43,7 +43,7 @@ use std::{ /// spawning the desired worker. #[macro_export] macro_rules! decl_worker_main { - ($expected_command:expr, $entrypoint:expr, $worker_version:expr, $worker_version_hash:expr $(,)*) => { + ($worker_kind:expr, $expected_command:expr, $entrypoint:expr, $worker_version:expr, $worker_version_hash:expr $(,)*) => { fn get_full_version() -> String { format!("{}-{}", $worker_version, $worker_version_hash) } @@ -67,6 +67,7 @@ macro_rules! decl_worker_main { return } + let mut do_security_checks = false; match args[1].as_ref() { "--help" | "-h" => { print_help($expected_command); @@ -82,38 +83,7 @@ macro_rules! decl_worker_main { return }, "--check-all" => { - #[cfg(target_os = "linux")] - let mut status = 0; - #[cfg(target_os = "linux")] - if let Err(err) = security::landlock::check_can_fully_enable() { - eprintln!("{}", err); - status = -1; - } - #[cfg(all(target_os = "linux", target_arch = "x86_64"))] - if let Err(err) = security::seccomp::check_can_fully_enable() { - eprintln!("{}", err); - status = -1; - } - #[cfg(target_os = "linux")] - { - let cache_path_tempdir = std::path::Path::new(&args[2]); - if let Err(err) = - security::change_root::check_can_fully_enable(&cache_path_tempdir) - { - eprintln!("{}", err); - status = -1; - } - } - #[cfg(target_os = "linux")] - // SAFETY: new process is spawned within a single threaded process. This - // invariant is enforced by tests. - if let Err(err) = unsafe { security::clone::check_can_fully_clone() } { - eprintln!("{}", err); - status = -1; - } - #[cfg(not(target_os = "linux"))] - let status = -1; - std::process::exit(status) + do_security_checks = true; }, "--check-can-enable-landlock" => { #[cfg(target_os = "linux")] @@ -220,6 +190,16 @@ macro_rules! decl_worker_main { let socket_path = std::path::Path::new(socket_path).to_owned(); let worker_dir_path = std::path::Path::new(worker_dir_path).to_owned(); + if do_security_checks { + $crate::worker::do_security_checks( + $worker_kind, + socket_path.clone(), + worker_dir_path.clone(), + node_version, + None + ); + } + $entrypoint(socket_path, worker_dir_path, node_version, Some($worker_version)); } }; @@ -350,6 +330,29 @@ pub fn run_worker( ) where F: FnMut(UnixStream, &WorkerInfo, SecurityStatus) -> io::Result, { + let ( stream, worker_info, security_status ) = do_security_checks( + worker_kind, + socket_path, + worker_dir_path.clone(), + node_version, + worker_version, + ); + + // Run the main worker loop. + let err = event_loop(stream, &worker_info, security_status) + // It's never `Ok` because it's `Ok(Never)`. + .unwrap_err(); + + worker_shutdown(worker_info, &err.to_string()); +} + +pub fn do_security_checks( + worker_kind: WorkerKind, + socket_path: PathBuf, + worker_dir_path: PathBuf, + node_version: Option<&str>, + worker_version: Option<&str>, +) -> (UnixStream, WorkerInfo, SecurityStatus) { #[cfg_attr(not(target_os = "linux"), allow(unused_mut))] let mut worker_info = WorkerInfo { pid: std::process::id(), @@ -482,16 +485,11 @@ pub fn run_worker( } } - // Run the main worker loop. - let err = event_loop(stream, &worker_info, security_status) - // It's never `Ok` because it's `Ok(Never)`. - .unwrap_err(); - - worker_shutdown(worker_info, &err.to_string()); + ( stream, worker_info, security_status ) } /// Provide a consistent message on unexpected worker shutdown. -pub fn worker_shutdown(worker_info: WorkerInfo, err: &str) -> ! { +fn worker_shutdown(worker_info: WorkerInfo, err: &str) -> ! { gum::warn!(target: LOG_TARGET, ?worker_info, "quitting pvf worker ({}): {}", worker_info.kind, err); std::process::exit(1); } diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index cc1e7c46db6b2..ef1879aa5aa5f 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -19,9 +19,6 @@ use super::Error; use is_executable::IsExecutable; use std::path::PathBuf; -#[cfg(target_os = "linux")] -use polkadot_node_core_pvf_common::worker::{WorkerInfo, WorkerKind, worker_shutdown}; -#[cfg(target_os = "linux")] use std::process::Command; #[cfg(test)] @@ -101,30 +98,10 @@ pub fn determine_workers_paths( log::warn!("Skipping node/worker version checks. This could result in incorrect behavior in PVF workers."); } - #[cfg(target_os = "linux")] - { - let worker_version = polkadot_node_core_pvf::get_worker_version(&prep_worker_path)?; - let mut worker_dir = prep_worker_path.clone(); - let _ = worker_dir.pop(); - - let worker_info = WorkerInfo { - pid: std::process::id(), - kind: WorkerKind::Prepare, - version: Some(worker_version), - worker_dir_path: worker_dir, - }; - - let exit_status = Command::new(&exec_worker_path) - .arg("--check-all") - .status() - .unwrap(); - - if !exit_status.success() { - let err = "Worker security checks failed."; - log::warn!("{}", err); - worker_shutdown(worker_info, err); - } - } + let _exit_status = Command::new(&exec_worker_path) + .arg("--check-all") + .status() + .unwrap(); Ok((prep_worker_path, exec_worker_path)) } diff --git a/polkadot/src/bin/execute-worker.rs b/polkadot/src/bin/execute-worker.rs index c39a5a3c89de7..63d205b7d9fff 100644 --- a/polkadot/src/bin/execute-worker.rs +++ b/polkadot/src/bin/execute-worker.rs @@ -17,6 +17,7 @@ //! Execute worker. polkadot_node_core_pvf_common::decl_worker_main!( + polkadot_node_core_pvf_common::worker::WorkerKind::Execute, "execute-worker", polkadot_node_core_pvf_execute_worker::worker_entrypoint, polkadot_cli::NODE_VERSION, diff --git a/polkadot/src/bin/prepare-worker.rs b/polkadot/src/bin/prepare-worker.rs index 3b42991f18baa..62bbfa9141b06 100644 --- a/polkadot/src/bin/prepare-worker.rs +++ b/polkadot/src/bin/prepare-worker.rs @@ -17,6 +17,7 @@ //! Prepare worker. polkadot_node_core_pvf_common::decl_worker_main!( + polkadot_node_core_pvf_common::worker::WorkerKind::Prepare, "prepare-worker", polkadot_node_core_pvf_prepare_worker::worker_entrypoint, polkadot_cli::NODE_VERSION, From aa70c92e27e4f04bed5a08901ec0ad4d70647d38 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 14:48:57 +0100 Subject: [PATCH 12/14] add and return error types --- polkadot/node/service/src/lib.rs | 12 ++++++++++++ polkadot/node/service/src/workers.rs | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/polkadot/node/service/src/lib.rs b/polkadot/node/service/src/lib.rs index 349facb7dcf28..15518251b61a5 100644 --- a/polkadot/node/service/src/lib.rs +++ b/polkadot/node/service/src/lib.rs @@ -239,6 +239,18 @@ pub enum Error { node_version: String, worker_path: PathBuf, }, + + #[cfg(feature = "full-node")] + #[error("Execute binary failed security checks, path: {exec_worker_path:?}")] + ExecuteWorkerFailedSecurityChecks { + exec_worker_path: PathBuf, + }, + + #[cfg(feature = "full-node")] + #[error("Prepare binary failed security checks, path: {prep_worker_path:?}")] + PrepareWorkerFailedSecurityChecks { + prep_worker_path: PathBuf, + }, } /// Identifies the variant of the chain. diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index ef1879aa5aa5f..c885e0eb6b3a3 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -98,11 +98,24 @@ pub fn determine_workers_paths( log::warn!("Skipping node/worker version checks. This could result in incorrect behavior in PVF workers."); } - let _exit_status = Command::new(&exec_worker_path) + let exit_status = Command::new(&exec_worker_path) .arg("--check-all") .status() .unwrap(); + if exit_status.success() == false { + return Err(Error::ExecuteWorkerFailedSecurityChecks { exec_worker_path }); + } + + let exit_status = Command::new(&prep_worker_path) + .arg("--check-all") + .status() + .unwrap(); + + if exit_status.success() == false { + return Err(Error::PrepareWorkerFailedSecurityChecks { prep_worker_path }); + } + Ok((prep_worker_path, exec_worker_path)) } From b18daf1225831f9c321609edd1a0e16f2ba936cb Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 15:14:28 +0100 Subject: [PATCH 13/14] add separate tests for execute and prepare binaries --- polkadot/node/service/src/lib.rs | 8 +++++--- polkadot/node/service/src/workers.rs | 20 ++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/polkadot/node/service/src/lib.rs b/polkadot/node/service/src/lib.rs index 15518251b61a5..e026cfbbd3db2 100644 --- a/polkadot/node/service/src/lib.rs +++ b/polkadot/node/service/src/lib.rs @@ -241,15 +241,17 @@ pub enum Error { }, #[cfg(feature = "full-node")] - #[error("Execute binary failed security checks, path: {exec_worker_path:?}")] + #[error("Execute binary failed security checks, execute binary: {exec_worker_path:?}, directory path: {exec_worker_dir_path:?}")] ExecuteWorkerFailedSecurityChecks { - exec_worker_path: PathBuf, + exec_worker_path: PathBuf, + exec_worker_dir_path: PathBuf, }, #[cfg(feature = "full-node")] - #[error("Prepare binary failed security checks, path: {prep_worker_path:?}")] + #[error("Prepare binary failed security checks, prepare binary: {prep_worker_path:?}, path: {prep_worker_dir_path:?}")] PrepareWorkerFailedSecurityChecks { prep_worker_path: PathBuf, + prep_worker_dir_path: PathBuf, }, } diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index c885e0eb6b3a3..92945d771052a 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -98,22 +98,38 @@ pub fn determine_workers_paths( log::warn!("Skipping node/worker version checks. This could result in incorrect behavior in PVF workers."); } + let mut exec_worker_dir_path = exec_worker_path.clone(); + let _ = exec_worker_dir_path.pop(); let exit_status = Command::new(&exec_worker_path) + // .arg("--socket-path") + // .arg(socket_path.as_ref().as_os_str()) + .arg("--worker-dir-path") + .arg(exec_worker_dir_path.as_os_str()) .arg("--check-all") .status() .unwrap(); if exit_status.success() == false { - return Err(Error::ExecuteWorkerFailedSecurityChecks { exec_worker_path }); + return Err(Error::ExecuteWorkerFailedSecurityChecks { + exec_worker_path, + exec_worker_dir_path } + ); } + let mut prep_worker_dir_path = prep_worker_path.clone(); + let _ = prep_worker_dir_path.pop(); let exit_status = Command::new(&prep_worker_path) + .arg("--worker-dir-path") + .arg(prep_worker_dir_path.as_os_str()) .arg("--check-all") .status() .unwrap(); if exit_status.success() == false { - return Err(Error::PrepareWorkerFailedSecurityChecks { prep_worker_path }); + return Err(Error::PrepareWorkerFailedSecurityChecks { + prep_worker_path, + prep_worker_dir_path + }); } Ok((prep_worker_path, exec_worker_path)) From bee6989e8f665c4a21b23de43a06f62303b67e87 Mon Sep 17 00:00:00 2001 From: Nnamdi Aninye Date: Tue, 25 Nov 2025 15:40:33 +0100 Subject: [PATCH 14/14] add commented out section --- polkadot/node/service/src/workers.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/polkadot/node/service/src/workers.rs b/polkadot/node/service/src/workers.rs index 92945d771052a..ccbf65992c029 100644 --- a/polkadot/node/service/src/workers.rs +++ b/polkadot/node/service/src/workers.rs @@ -119,6 +119,8 @@ pub fn determine_workers_paths( let mut prep_worker_dir_path = prep_worker_path.clone(); let _ = prep_worker_dir_path.pop(); let exit_status = Command::new(&prep_worker_path) + // .arg("--socket-path") + // .arg(socket_path.as_ref().as_os_str()) .arg("--worker-dir-path") .arg(prep_worker_dir_path.as_os_str()) .arg("--check-all")