diff --git a/Cargo.lock b/Cargo.lock index 2a091ce6817d..382406dd8c92 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12462,6 +12462,7 @@ dependencies = [ "polkadot-node-core-pvf-prepare-worker", "polkadot-node-metrics", "polkadot-node-primitives", + "polkadot-node-subsystem", "polkadot-parachain-primitives", "polkadot-primitives", "procfs", diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index 89ea02728840..4232e5f1cdd8 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -150,7 +150,7 @@ async fn run( ), pvf_metrics, ) - .await; + .await?; ctx.spawn_blocking("pvf-validation-host", task.boxed())?; loop { diff --git a/polkadot/node/core/pvf/Cargo.toml b/polkadot/node/core/pvf/Cargo.toml index 430f7cd5e8ef..3e72ca9e5326 100644 --- a/polkadot/node/core/pvf/Cargo.toml +++ b/polkadot/node/core/pvf/Cargo.toml @@ -27,6 +27,7 @@ polkadot-core-primitives = { path = "../../../core-primitives" } polkadot-node-core-pvf-common = { path = "common" } polkadot-node-metrics = { path = "../../metrics" } polkadot-node-primitives = { path = "../../primitives" } +polkadot-node-subsystem = { path = "../../subsystem" } polkadot-primitives = { path = "../../../primitives" } sp-core = { path = "../../../../substrate/primitives/core" } diff --git a/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs b/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs index acd80526262c..d0cefae6cdbf 100644 --- a/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs +++ b/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs @@ -47,7 +47,7 @@ impl TestHost { execute_worker_path, ); f(&mut config); - let (host, task) = start(config, Metrics::default()).await; + let (host, task) = start(config, Metrics::default()).await.unwrap(); let _ = handle.spawn(task); Self { host: Mutex::new(host) } } diff --git a/polkadot/node/core/pvf/common/src/worker/mod.rs b/polkadot/node/core/pvf/common/src/worker/mod.rs index 274a2fc80397..f6a67b98321a 100644 --- a/polkadot/node/core/pvf/common/src/worker/mod.rs +++ b/polkadot/node/core/pvf/common/src/worker/mod.rs @@ -92,13 +92,13 @@ macro_rules! decl_worker_main { std::process::exit(status) }, "--check-can-unshare-user-namespace-and-change-root" => { + #[cfg(target_os = "linux")] + let cache_path_tempdir = std::path::Path::new(&args[2]); #[cfg(target_os = "linux")] let status = if let Err(err) = security::unshare_user_namespace_and_change_root( $crate::worker::WorkerKind::CheckPivotRoot, worker_pid, - // We're not accessing any files, so we can try to pivot_root in the temp - // dir without conflicts with other processes. - &std::env::temp_dir(), + &cache_path_tempdir, ) { // Write the error to stderr, log it on the host-side. eprintln!("{}", err); diff --git a/polkadot/node/core/pvf/src/host.rs b/polkadot/node/core/pvf/src/host.rs index 7b383e8034a7..5919b9ba32c9 100644 --- a/polkadot/node/core/pvf/src/host.rs +++ b/polkadot/node/core/pvf/src/host.rs @@ -35,6 +35,7 @@ use polkadot_node_core_pvf_common::{ error::{PrepareError, PrepareResult}, pvf::PvfPrepData, }; +use polkadot_node_subsystem::SubsystemResult; use polkadot_parachain_primitives::primitives::ValidationResult; use std::{ collections::HashMap, @@ -203,7 +204,10 @@ impl Config { /// The future should not return normally but if it does then that indicates an unrecoverable error. /// In that case all pending requests will be canceled, dropping the result senders and new ones /// will be rejected. -pub async fn start(config: Config, metrics: Metrics) -> (ValidationHost, impl Future) { +pub async fn start( + config: Config, + metrics: Metrics, +) -> SubsystemResult<(ValidationHost, impl Future)> { gum::debug!(target: LOG_TARGET, ?config, "starting PVF validation host"); // Run checks for supported security features once per host startup. Warn here if not enabled. @@ -273,7 +277,7 @@ pub async fn start(config: Config, metrics: Metrics) -> (ValidationHost, impl Fu }; }; - (validation_host, task) + Ok((validation_host, task)) } /// A mapping from an artifact ID which is in preparation state to the list of pending execution diff --git a/polkadot/node/core/pvf/src/security.rs b/polkadot/node/core/pvf/src/security.rs index 295dd7df94dd..0c0c5f401663 100644 --- a/polkadot/node/core/pvf/src/security.rs +++ b/polkadot/node/core/pvf/src/security.rs @@ -27,14 +27,19 @@ const SECURE_MODE_ANNOUNCEMENT: &'static str = \nMore information: https://wiki.polkadot.network/docs/maintain-guides-secure-validator#secure-validator-mode"; /// Run checks for supported security features. +/// +/// # Return +/// +/// Returns the set of security features that we were able to enable. If an error occurs while +/// enabling a security feature we set the corresponding status to `false`. pub async fn check_security_status(config: &Config) -> SecurityStatus { - let Config { prepare_worker_program_path, .. } = config; + let Config { prepare_worker_program_path, cache_path, .. } = config; // TODO: add check that syslog is available and that seccomp violations are logged? let (landlock, seccomp, change_root) = join!( check_landlock(prepare_worker_program_path), check_seccomp(prepare_worker_program_path), - check_can_unshare_user_namespace_and_change_root(prepare_worker_program_path) + check_can_unshare_user_namespace_and_change_root(prepare_worker_program_path, cache_path) ); let security_status = SecurityStatus { @@ -149,11 +154,22 @@ fn print_secure_mode_message(errs: Vec) -> bool { async fn check_can_unshare_user_namespace_and_change_root( #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] prepare_worker_program_path: &Path, + #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] cache_path: &Path, ) -> SecureModeResult { cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { + let cache_dir_tempdir = + crate::worker_intf::tmppath_in("check-can-unshare", cache_path) + .await + .map_err( + |err| + SecureModeError::CannotUnshareUserNamespaceAndChangeRoot( + format!("could not create a temporary directory in {:?}: {}", cache_path, err) + ) + )?; match tokio::process::Command::new(prepare_worker_program_path) .arg("--check-can-unshare-user-namespace-and-change-root") + .arg(cache_dir_tempdir) .output() .await { diff --git a/polkadot/node/core/pvf/tests/it/main.rs b/polkadot/node/core/pvf/tests/it/main.rs index f4fd7f802f5e..801b60884fa3 100644 --- a/polkadot/node/core/pvf/tests/it/main.rs +++ b/polkadot/node/core/pvf/tests/it/main.rs @@ -61,7 +61,7 @@ impl TestHost { execute_worker_path, ); f(&mut config); - let (host, task) = start(config, Metrics::default()).await; + let (host, task) = start(config, Metrics::default()).await.unwrap(); let _ = tokio::task::spawn(task); Self { cache_dir, host: Mutex::new(host) } }