From 1e068edc636c322a6c188dd99118d534f16b4498 Mon Sep 17 00:00:00 2001
From: "Marcin S." <marcin@realemail.net>
Date: Wed, 8 Nov 2023 10:54:52 +0100
Subject: [PATCH] PVF worker: switch on seccomp networking restrictions

We've merged #2009 which logs seccomp violations for networking, but does not
switch on full seccomp restrictions (voting against blocks on violations).

We should monitor releases and once #2009 has been released, we can switch this
on for the next release.

Closes https://github.com/paritytech/polkadot-sdk/issues/2163
---
 .../core/pvf/common/src/worker/security/seccomp.rs  | 10 ++++------
 polkadot/node/core/pvf/execute-worker/src/lib.rs    |  6 ------
 polkadot/node/core/pvf/src/execute/worker_intf.rs   | 13 -------------
 polkadot/node/core/pvf/src/prepare/worker_intf.rs   | 12 ------------
 polkadot/node/core/pvf/src/security.rs              |  2 +-
 5 files changed, 5 insertions(+), 38 deletions(-)

diff --git a/polkadot/node/core/pvf/common/src/worker/security/seccomp.rs b/polkadot/node/core/pvf/common/src/worker/security/seccomp.rs
index 5539ad284400..c3822d3c4c69 100644
--- a/polkadot/node/core/pvf/common/src/worker/security/seccomp.rs
+++ b/polkadot/node/core/pvf/common/src/worker/security/seccomp.rs
@@ -67,11 +67,9 @@
 //!
 //! # Action on syscall violations
 //!
-//! On syscall violations we currently only log, to make sure this works correctly before enforcing.
-//!
-//! In the future, when a forbidden syscall is attempted we immediately kill the process in order to
-//! prevent the attacker from doing anything else. In execution, this will result in voting against
-//! the candidate.
+//! When a forbidden syscall is attempted we immediately kill the process in order to prevent the
+//! attacker from doing anything else. In execution, this will result in voting against the
+//! candidate.
 
 use crate::{
 	worker::{stringify_panic_payload, WorkerKind},
@@ -82,7 +80,7 @@ use std::{collections::BTreeMap, path::Path};
 
 /// The action to take on caught syscalls.
 #[cfg(not(test))]
-const CAUGHT_ACTION: SeccompAction = SeccompAction::Log;
+const CAUGHT_ACTION: SeccompAction = SeccompAction::KillProcess;
 /// Don't kill the process when testing.
 #[cfg(test)]
 const CAUGHT_ACTION: SeccompAction = SeccompAction::Errno(libc::EACCES as u32);
diff --git a/polkadot/node/core/pvf/execute-worker/src/lib.rs b/polkadot/node/core/pvf/execute-worker/src/lib.rs
index 8872f9bc8dd3..5c88030aa1eb 100644
--- a/polkadot/node/core/pvf/execute-worker/src/lib.rs
+++ b/polkadot/node/core/pvf/execute-worker/src/lib.rs
@@ -243,12 +243,6 @@ pub fn worker_entrypoint(
 					),
 				};
 
-				gum::trace!(
-					target: LOG_TARGET,
-					%worker_pid,
-					"worker: sending response to host: {:?}",
-					response
-				);
 				send_response(&mut stream, response)?;
 			}
 		},
diff --git a/polkadot/node/core/pvf/src/execute/worker_intf.rs b/polkadot/node/core/pvf/src/execute/worker_intf.rs
index 61264f7d517d..b8e3f57b996e 100644
--- a/polkadot/node/core/pvf/src/execute/worker_intf.rs
+++ b/polkadot/node/core/pvf/src/execute/worker_intf.rs
@@ -176,19 +176,6 @@ pub async fn start_work(
 						return Outcome::IoErr
 					},
 					Ok(response) => {
-						// Check if any syscall violations occurred during the job. For now this is
-						// only informative, as we are not enforcing the seccomp policy yet.
-						for syscall in security::check_seccomp_violations_for_worker(audit_log_file, pid).await {
-							gum::error!(
-								target: LOG_TARGET,
-								worker_pid = %pid,
-								%syscall,
-								validation_code_hash = ?artifact.id.code_hash,
-								?artifact_path,
-								"A forbidden syscall was attempted! This is a violation of our seccomp security policy. Report an issue ASAP!"
-							);
-						}
-
 						if let Response::Ok{duration, ..} = response {
 							if duration > execution_timeout {
 								// The job didn't complete within the timeout.
diff --git a/polkadot/node/core/pvf/src/prepare/worker_intf.rs b/polkadot/node/core/pvf/src/prepare/worker_intf.rs
index 0e50caf1feb5..1584805c2411 100644
--- a/polkadot/node/core/pvf/src/prepare/worker_intf.rs
+++ b/polkadot/node/core/pvf/src/prepare/worker_intf.rs
@@ -156,18 +156,6 @@ pub async fn start_work(
 			match result {
 				// Received bytes from worker within the time limit.
 				Ok(Ok(prepare_result)) => {
-					// Check if any syscall violations occurred during the job. For now this is only
-					// informative, as we are not enforcing the seccomp policy yet.
-					for syscall in security::check_seccomp_violations_for_worker(audit_log_file, pid).await {
-						gum::error!(
-							target: LOG_TARGET,
-							worker_pid = %pid,
-							%syscall,
-							?pvf,
-							"A forbidden syscall was attempted! This is a violation of our seccomp security policy. Report an issue ASAP!"
-						);
-					}
-
 					handle_response(
 						metrics,
 						IdleWorker { stream, pid, worker_dir },
diff --git a/polkadot/node/core/pvf/src/security.rs b/polkadot/node/core/pvf/src/security.rs
index decd321e415e..86308ae98236 100644
--- a/polkadot/node/core/pvf/src/security.rs
+++ b/polkadot/node/core/pvf/src/security.rs
@@ -221,7 +221,7 @@ pub async fn check_seccomp_violations_for_worker(
 
 	let audit_log_file = match audit_log_file {
 		Some(file) => {
-			gum::debug!(
+			gum::trace!(
 				target: LOG_TARGET,
 				%worker_pid,
 				audit_log_path = ?file.path,