PVF: fix detection of unshare-and-change-root security capability (#2304

)

polkadot/node/core/candidate-validation/src/lib.rs

@@ -150,7 +150,7 @@ async fn run<Context>(
 		),
 		pvf_metrics,
 	)
-	.await;
+	.await?;
 	ctx.spawn_blocking("pvf-validation-host", task.boxed())?;
 
 	loop {

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" }

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) }
 	}

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);

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<Output = ()>) {
+pub async fn start(
+	config: Config,
+	metrics: Metrics,
+) -> SubsystemResult<(ValidationHost, impl Future<Output = ()>)> {
 	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

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<SecureModeError>) -> 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
 			{

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) }
 	}