Skip to content
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
);
Comment on lines -225 to -230
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was producing a bit too much noise in the log output.

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!(
Comment on lines -186 to -189
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still do this check elsewhere in this file, but only if the worker died.

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