Skip to content

Commit

Permalink
PVF worker: switch on seccomp networking restrictions
Browse files Browse the repository at this point in the history
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 #2163
  • Loading branch information
mrcnski committed Nov 8, 2023
1 parent 4caa3d8 commit 1e068ed
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 38 deletions.
10 changes: 4 additions & 6 deletions polkadot/node/core/pvf/common/src/worker/security/seccomp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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);
Expand Down
6 changes: 0 additions & 6 deletions polkadot/node/core/pvf/execute-worker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
}
},
Expand Down
13 changes: 0 additions & 13 deletions polkadot/node/core/pvf/src/execute/worker_intf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 0 additions & 12 deletions polkadot/node/core/pvf/src/prepare/worker_intf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
2 changes: 1 addition & 1 deletion polkadot/node/core/pvf/src/security.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 1e068ed

Please sign in to comment.