PVF: more filesystem sandboxing (#1373)

Author: mrcnski

Cargo.lock

@@ -8,6 +8,7 @@ license.workspace = true
 
 [dependencies]
 always-assert = "0.1"
+cfg-if = "1.0"
 futures = "0.3.21"
 futures-timer = "3.0.2"
 gum = { package = "tracing-gum", path = "../../gum" }

polkadot/node/core/pvf/Cargo.toml

@@ -7,6 +7,7 @@ edition.workspace = true
 license.workspace = true
 
 [dependencies]
+cfg-if = "1.0"
 cpu-time = "1.0.0"
 futures = "0.3.21"
 gum = { package = "tracing-gum", path = "../../../gum" }

polkadot/node/core/pvf/common/Cargo.toml

@@ -44,7 +44,17 @@ pub enum PrepareError {
 	/// The response from the worker is received, but the file cannot be renamed (moved) to the
 	/// final destination location. This state is reported by the validation host (not by the
 	/// worker).
-	RenameTmpFileErr(String),
+	RenameTmpFileErr {
+		err: String,
+		// Unfortunately `PathBuf` doesn't implement `Encode`/`Decode`, so we do a fallible
+		// conversion to `Option<String>`.
+		src: Option<String>,
+		dest: Option<String>,
+	},
+	/// The response from the worker is received, but the worker cache could not be cleared. The
+	/// worker has to be killed to avoid jobs having access to data from other jobs. This state is
+	/// reported by the validation host (not by the worker).
+	ClearWorkerDir(String),
 }
 
 impl PrepareError {
@@ -58,7 +68,11 @@ impl PrepareError {
 		use PrepareError::*;
 		match self {
 			Prevalidation(_) | Preparation(_) | Panic(_) => true,
-			TimedOut | IoErr(_) | CreateTmpFileErr(_) | RenameTmpFileErr(_) => false,
+			TimedOut |
+			IoErr(_) |
+			CreateTmpFileErr(_) |
+			RenameTmpFileErr { .. } |
+			ClearWorkerDir(_) => false,
 			// Can occur due to issues with the PVF, but also due to local errors.
 			RuntimeConstruction(_) => false,
 		}
@@ -76,7 +90,9 @@ impl fmt::Display for PrepareError {
 			TimedOut => write!(f, "prepare: timeout"),
 			IoErr(err) => write!(f, "prepare: io error while receiving response: {}", err),
 			CreateTmpFileErr(err) => write!(f, "prepare: error creating tmp file: {}", err),
-			RenameTmpFileErr(err) => write!(f, "prepare: error renaming tmp file: {}", err),
+			RenameTmpFileErr { err, src, dest } =>
+				write!(f, "prepare: error renaming tmp file ({:?} -> {:?}): {}", src, dest, err),
+			ClearWorkerDir(err) => write!(f, "prepare: error clearing worker cache: {}", err),
 		}
 	}
 }
@@ -89,8 +105,17 @@ impl fmt::Display for PrepareError {
 pub enum InternalValidationError {
 	/// Some communication error occurred with the host.
 	HostCommunication(String),
+	/// Host could not create a hard link to the artifact path.
+	CouldNotCreateLink(String),
 	/// Could not find or open compiled artifact file.
 	CouldNotOpenFile(String),
+	/// Host could not clear the worker cache after a job.
+	CouldNotClearWorkerDir {
+		err: String,
+		// Unfortunately `PathBuf` doesn't implement `Encode`/`Decode`, so we do a fallible
+		// conversion to `Option<String>`.
+		path: Option<String>,
+	},
 	/// An error occurred in the CPU time monitor thread. Should be totally unrelated to
 	/// validation.
 	CpuTimeMonitorThread(String),
@@ -104,8 +129,18 @@ impl fmt::Display for InternalValidationError {
 		match self {
 			HostCommunication(err) =>
 				write!(f, "validation: some communication error occurred with the host: {}", err),
+			CouldNotCreateLink(err) => write!(
+				f,
+				"validation: host could not create a hard link to the artifact path: {}",
+				err
+			),
 			CouldNotOpenFile(err) =>
 				write!(f, "validation: could not find or open compiled artifact file: {}", err),
+			CouldNotClearWorkerDir { err, path } => write!(
+				f,
+				"validation: host could not clear the worker cache ({:?}) after a job: {}",
+				path, err
+			),
 			CpuTimeMonitorThread(err) =>
 				write!(f, "validation: an error occurred in the CPU time monitor thread: {}", err),
 			NonDeterministicPrepareError(err) => write!(f, "validation: prepare: {}", err),

polkadot/node/core/pvf/common/src/error.rs

@@ -29,7 +29,7 @@ pub struct Handshake {
 }
 
 /// The response from an execution job on the worker.
-#[derive(Encode, Decode)]
+#[derive(Debug, Encode, Decode)]
 pub enum Response {
 	/// The job completed successfully.
 	Ok {

polkadot/node/core/pvf/common/src/execute.rs

@@ -22,6 +22,7 @@ pub mod executor_intf;
 pub mod prepare;
 pub mod pvf;
 pub mod worker;
+pub mod worker_dir;
 
 pub use cpu_time::ProcessTime;
 
@@ -30,8 +31,11 @@ pub use sp_tracing;
 
 const LOG_TARGET: &str = "parachain::pvf-common";
 
-use std::mem;
-use tokio::io::{self, AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _};
+use std::{
+	io::{Read, Write},
+	mem,
+};
+use tokio::io;
 
 #[cfg(feature = "test-utils")]
 pub mod tests {
@@ -41,20 +45,31 @@ pub mod tests {
 	pub const TEST_PREPARATION_TIMEOUT: Duration = Duration::from_secs(30);
 }
 
-/// Write some data prefixed by its length into `w`.
-pub async fn framed_send(w: &mut (impl AsyncWrite + Unpin), buf: &[u8]) -> io::Result<()> {
+/// Status of security features on the current system.
+#[derive(Debug, Clone, Default)]
+pub struct SecurityStatus {
+	/// Whether the landlock features we use are fully available on this system.
+	pub can_enable_landlock: bool,
+	// Whether we are able to unshare the user namespace and change the filesystem root.
+	pub can_unshare_user_namespace_and_change_root: bool,
+}
+
+/// Write some data prefixed by its length into `w`. Sync version of `framed_send` to avoid
+/// dependency on tokio.
+pub fn framed_send_blocking(w: &mut (impl Write + Unpin), buf: &[u8]) -> io::Result<()> {
 	let len_buf = buf.len().to_le_bytes();
-	w.write_all(&len_buf).await?;
-	w.write_all(buf).await?;
+	w.write_all(&len_buf)?;
+	w.write_all(buf)?;
 	Ok(())
 }
 
-/// Read some data prefixed by its length from `r`.
-pub async fn framed_recv(r: &mut (impl AsyncRead + Unpin)) -> io::Result<Vec<u8>> {
+/// Read some data prefixed by its length from `r`. Sync version of `framed_recv` to avoid
+/// dependency on tokio.
+pub fn framed_recv_blocking(r: &mut (impl Read + Unpin)) -> io::Result<Vec<u8>> {
 	let mut len_buf = [0u8; mem::size_of::<usize>()];
-	r.read_exact(&mut len_buf).await?;
+	r.read_exact(&mut len_buf)?;
 	let len = usize::from_le_bytes(len_buf);
 	let mut buf = vec![0; len];
-	r.read_exact(&mut buf).await?;
+	r.read_exact(&mut buf)?;
 	Ok(buf)
 }