PVF: add landlock sandboxing

node/core/pvf/common/Cargo.toml

@@ -22,5 +22,12 @@ sc-executor-wasmtime = { git = "https://github.com/paritytech/substrate", branch
 sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
 sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" }
 
+[target.'cfg(target_os = "linux")'.dependencies]
+landlock = "0.2.0"
+
+[dev-dependencies]
+assert_matches = "1.4.0"
+tempfile = "3.3.0"
+
 [build-dependencies]
 substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }

node/core/pvf/common/src/worker.rs → node/core/pvf/common/src/worker/mod.rs

@@ -16,6 +16,8 @@
 
 //! Functionality common to both prepare and execute workers.
 
+pub mod security;
+
 use crate::LOG_TARGET;
 use cpu_time::ProcessTime;
 use futures::never::Never;
@@ -203,7 +205,7 @@ pub mod thread {
 	};
 
 	/// Contains the outcome of waiting on threads, or `Pending` if none are ready.
-	#[derive(Clone, Copy)]
+	#[derive(Debug, Clone, Copy)]
 	pub enum WaitOutcome {
 		Finished,
 		TimedOut,
@@ -224,8 +226,9 @@ pub mod thread {
 		Arc::new((Mutex::new(WaitOutcome::Pending), Condvar::new()))
 	}
 
-	/// Runs a thread, afterwards notifying the threads waiting on the condvar. Catches panics and
-	/// resumes them after triggering the condvar, so that the waiting thread is notified on panics.
+	/// Runs a worker thread. Will first enable security features, and afterwards notify the threads waiting on the
+	/// condvar. Catches panics during execution and resumes the panics after triggering the condvar, so that the
+	/// waiting thread is notified on panics.
 	pub fn spawn_worker_thread<F, R>(
 		name: &str,
 		f: F,
@@ -237,9 +240,18 @@ pub mod thread {
 		F: Send + 'static + panic::UnwindSafe,
 		R: Send + 'static,
 	{
-		thread::Builder::new()
-			.name(name.into())
-			.spawn(move || cond_notify_on_done(f, cond, outcome))
+		thread::Builder::new().name(name.into()).spawn(move || {
+			cond_notify_on_done(
+				|| {
+					#[cfg(target_os = "linux")]
+					let _ = crate::worker::security::landlock::try_restrict_thread();
+
+					f()
+				},
+				cond,
+				outcome,
+			)
+		})
 	}
 
 	/// Runs a worker thread with the given stack size. See [`spawn_worker_thread`].

node/core/pvf/common/src/worker/security.rs

@@ -0,0 +1,137 @@
+// Copyright (C) Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Functionality for securing workers.
+//!
+//! This is needed because workers are used to compile and execute untrusted code (PVFs).
+
+/// The	[landlock] docs say it best:
+///
+/// > "Landlock is a security feature available since Linux 5.13. The goal is to enable to restrict
+/// ambient rights (e.g., global filesystem access) for a set of processes by creating safe security
+/// sandboxes as new security layers in addition to the existing system-wide access-controls. This
+/// kind of sandbox is expected to help mitigate the security impact of bugs, unexpected or
+/// malicious behaviors in applications. Landlock empowers any process, including unprivileged ones,
+/// to securely restrict themselves."
+///
+/// [landlock]: https://docs.rs/landlock/latest/landlock/index.html
+#[cfg(target_os = "linux")]
+pub mod landlock {
+	use landlock::{Access, AccessFs, Ruleset, RulesetAttr, RulesetError, RulesetStatus, ABI};
+
+	/// Version of landlock ABI. Use the latest version supported by our reference kernel version.
+	///
+	/// - Reference kernel version: 5.16+
+	/// - V1: 5.13
+	/// - V2: 5.19
+	///
+	/// Please update the above if it is out-of-date.
+	const LANDLOCK_ABI: ABI = ABI::V1;
+
+	// TODO: <https://github.com/landlock-lsm/rust-landlock/issues/36>
+	/// Returns to what degree landlock is enabled on the current Linux environment.
+	pub fn get_status() -> Result<RulesetStatus, Box<dyn std::error::Error>> {
+		match std::thread::spawn(|| try_restrict_thread()).join() {
+			Ok(Ok(status)) => Ok(status),
+			Ok(Err(ruleset_err)) => Err(ruleset_err.into()),
+			Err(_err) => Err("a panic occurred in try_restrict_thread".into()),
+		}
+	}
+
+	/// Returns a single bool indicating whether landlock is fully enabled on the current Linux
+	/// environment.
+	pub fn is_fully_enabled() -> bool {
+		matches!(get_status(), Ok(RulesetStatus::FullyEnforced))
+	}
+
+	/// Tries to restrict the current thread with the following landlock access controls:
+	///
+	/// 1. all global filesystem access
+	/// 2. ... more may be supported in the future.
+	///
+	/// If landlock is not supported in the current environment this is simply a noop.
+	///
+	/// # Returns
+	///
+	/// The status of the restriction (whether it was fully, partially, or not-at-all enforced).
+	pub fn try_restrict_thread() -> Result<RulesetStatus, RulesetError> {
+		let status = Ruleset::new()
+			.handle_access(AccessFs::from_all(LANDLOCK_ABI))?
+			.create()?
+			.restrict_self()?;
+		Ok(status.ruleset)
+	}
+
+	#[cfg(test)]
+	mod tests {
+		use super::*;
+		use std::{fs, io::ErrorKind, thread};
+
+		#[test]
+		fn restricted_thread_cannot_access_fs() {
+			// TODO: This would be nice: <https://github.com/rust-lang/rust/issues/68007>.
+			if !is_fully_enabled() {
+				return
+			}
+
+			// Restricted thread cannot read from FS.
+			let handle = thread::spawn(|| {
+				// Write to a tmp file, this should succeed before landlock is applied.
+				let text = "foo";
+				let tmpfile = tempfile::NamedTempFile::new().unwrap();
+				let path = tmpfile.path();
+				fs::write(path, text).unwrap();
+				let s = fs::read_to_string(path).unwrap();
+				assert_eq!(s, text);
+
+				let status = super::try_restrict_thread().unwrap();
+				if let RulesetStatus::NotEnforced = status {
+					panic!("Ruleset should be enforced since we checked if landlock is enabled");
+				}
+
+				// Try to read from the tmp file after landlock.
+				let result = fs::read_to_string(path);
+				assert!(matches!(
+					result,
+					Err(err) if matches!(err.kind(), ErrorKind::PermissionDenied)
+				));
+			});
+
+			assert!(handle.join().is_ok());
+
+			// Restricted thread cannot write to FS.
+			let handle = thread::spawn(|| {
+				let text = "foo";
+				let tmpfile = tempfile::NamedTempFile::new().unwrap();
+				let path = tmpfile.path();
+
+				let status = super::try_restrict_thread().unwrap();
+				if let RulesetStatus::NotEnforced = status {
+					panic!("Ruleset should be enforced since we checked if landlock is enabled");
+				}
+
+				// Try to write to the tmp file after landlock.
+				let result = fs::write(path, text);
+				assert!(matches!(
+					result,
+					Err(err) if matches!(err.kind(), ErrorKind::PermissionDenied)
+				));
+			});
+
+			assert!(handle.join().is_ok());
+		}
+	}
+}

roadmap/implementers-guide/src/SUMMARY.md

@@ -60,6 +60,7 @@
   - [Utility Subsystems](node/utility/README.md)
     - [Availability Store](node/utility/availability-store.md)
     - [Candidate Validation](node/utility/candidate-validation.md)
+      - [PVF Host and Workers](node/utility/pvf-host-and-workers.md)
     - [Provisioner](node/utility/provisioner.md)
     - [Network Bridge](node/utility/network-bridge.md)
     - [Gossip Support](node/utility/gossip-support.md)

roadmap/implementers-guide/src/node/utility/candidate-validation.md

@@ -44,86 +44,10 @@ Once we have all parameters, we can spin up a background task to perform the val
   * The collator signature is valid
   * The PoV provided matches the `pov_hash` field of the descriptor
 
+For more details please see [PVF Host and Workers](pvf-host-and-workers.md).
+
 ### Checking Validation Outputs
 
 If we can assume the presence of the relay-chain state (that is, during processing [`CandidateValidationMessage`][CVM]`::ValidateFromChainState`) we can run all the checks that the relay-chain would run at the inclusion time thus confirming that the candidate will be accepted.
 
-### PVF Host
-
-The PVF host is responsible for handling requests to prepare and execute PVF
-code blobs.
-
-One high-level goal is to make PVF operations as deterministic as possible, to
-reduce the rate of disputes. Disputes can happen due to e.g. a job timing out on
-one machine, but not another. While we do not yet have full determinism, there
-are some dispute reduction mechanisms in place right now.
-
-#### Retrying execution requests
-
-If the execution request fails during **preparation**, we will retry if it is
-possible that the preparation error was transient (e.g. if the error was a panic
-or time out). We will only retry preparation if another request comes in after
-15 minutes, to ensure any potential transient conditions had time to be
-resolved. We will retry up to 5 times.
-
-If the actual **execution** of the artifact fails, we will retry once if it was
-a possibly transient error, to allow the conditions that led to the error to
-hopefully resolve. We use a more brief delay here (1 second as opposed to 15
-minutes for preparation (see above)), because a successful execution must happen
-in a short amount of time.
-
-We currently know of the following specific cases that will lead to a retried
-execution request:
-
-1. **OOM:** The host might have been temporarily low on memory due to other
-   processes running on the same machine. **NOTE:** This case will lead to
-   voting against the candidate (and possibly a dispute) if the retry is still
-   not successful.
-2. **Artifact missing:** The prepared artifact might have been deleted due to
-   operator error or some bug in the system.
-3. **Panic:** The worker thread panicked for some indeterminate reason, which
-   may or may not be independent of the candidate or PVF.
-
-#### Preparation timeouts
-
-We use timeouts for both preparation and execution jobs to limit the amount of
-time they can take. As the time for a job can vary depending on the machine and
-load on the machine, this can potentially lead to disputes where some validators
-successfuly execute a PVF and others don't.
-
-One dispute mitigation we have in place is a more lenient timeout for
-preparation during execution than during pre-checking. The rationale is that the
-PVF has already passed pre-checking, so we know it should be valid, and we allow
-it to take longer than expected, as this is likely due to an issue with the
-machine and not the PVF.
-
-#### CPU clock timeouts
-
-Another timeout-related mitigation we employ is to measure the time taken by
-jobs using CPU time, rather than wall clock time. This is because the CPU time
-of a process is less variable under different system conditions. When the
-overall system is under heavy load, the wall clock time of a job is affected
-more than the CPU time.
-
-#### Internal errors
-
-In general, for errors not raising a dispute we have to be very careful. This is
-only sound, if we either:
-
-1. Ruled out that error in pre-checking. If something is not checked in
-   pre-checking, even if independent of the candidate and PVF, we must raise a
-   dispute.
-2. We are 100% confident that it is a hardware/local issue: Like corrupted file,
-   etc.
-
-Reasoning: Otherwise it would be possible to register a PVF where candidates can
-not be checked, but we don't get a dispute - so nobody gets punished. Second, we
-end up with a finality stall that is not going to resolve!
-
-There are some error conditions where we can't be sure whether the candidate is
-really invalid or some internal glitch occurred, e.g. panics. Whenever we are
-unsure, we can never treat an error as internal as we would abstain from voting.
-So we will first retry the candidate, and if the issue persists we are forced to
-vote invalid.
-
 [CVM]: ../../types/overseer-protocol.md#validationrequesttype