From 7bb717a30a710c9debf15e72d5ee929a51acb83c Mon Sep 17 00:00:00 2001 From: "Marcin S." Date: Thu, 9 Nov 2023 10:34:02 +0100 Subject: [PATCH 1/5] PVF host: Make unavailable security features print a warning The warning should say that this will soon be required to run securely, with a link to the announcement. See https://github.com/paritytech/polkadot-sdk/issues/1444 --- polkadot/node/core/pvf/src/host.rs | 1 + polkadot/node/core/pvf/src/security.rs | 133 +++++++++++++++---------- 2 files changed, 83 insertions(+), 51 deletions(-) diff --git a/polkadot/node/core/pvf/src/host.rs b/polkadot/node/core/pvf/src/host.rs index dd0bd8581985..e004b5608ac1 100644 --- a/polkadot/node/core/pvf/src/host.rs +++ b/polkadot/node/core/pvf/src/host.rs @@ -210,6 +210,7 @@ pub async fn start(config: Config, metrics: Metrics) -> (ValidationHost, impl Fu // Run checks for supported security features once per host startup. Warn here if not enabled. let security_status = { // TODO: add check that syslog is available and that seccomp violations are logged? + security::check_secure_mode_platform_requirement(); let (can_enable_landlock, can_enable_seccomp, can_unshare_user_namespace_and_change_root) = join!( security::check_landlock(&config.prepare_worker_program_path), security::check_seccomp(&config.prepare_worker_program_path), diff --git a/polkadot/node/core/pvf/src/security.rs b/polkadot/node/core/pvf/src/security.rs index decd321e415e..e044754a2c31 100644 --- a/polkadot/node/core/pvf/src/security.rs +++ b/polkadot/node/core/pvf/src/security.rs @@ -21,6 +21,40 @@ use tokio::{ io::{AsyncReadExt, AsyncSeekExt, SeekFrom}, }; +const SECURE_MODE_ANNOUNCEMENT: &'static str = + "In the next release this will be a hard error by default. More information: \ + https://github.com/w3f/polkadot-wiki/issues/4881"; + +/// Warns if a secure validator cannot be built for the target OS and architecture. +pub fn check_secure_mode_platform_requirement() { + cfg_if::cfg_if! { + if #[cfg(target_os = "linux")] { + cfg_if::cfg_if! { + if #[cfg(target_arch = "x86_64")] { + return () + } else { + let msg = "Secure validators are only supported on CPUs from the x86_64 family (usually Intel or AMD)."; + } + } + } else { + cfg_if::cfg_if! { + if #[cfg(target_arch = "x86_64")] { + let msg = "Secure validators are only supported on Linux."; + } else { + let msg = "Secure validators are only supported on Linux and on CPUs from the x86_64 family (usually Intel or AMD)."; + } + } + } + }; + + gum::warn!( + target: LOG_TARGET, + "{} {}", + msg, + SECURE_MODE_ANNOUNCEMENT + ); +} + /// Check if we can sandbox the root and emit a warning if not. /// /// We do this check by spawning a new process and trying to sandbox it. To get as close as possible @@ -32,25 +66,20 @@ pub async fn check_can_unshare_user_namespace_and_change_root( ) -> bool { cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { - match tokio::process::Command::new(prepare_worker_program_path) + let msg = match tokio::process::Command::new(prepare_worker_program_path) .arg("--check-can-unshare-user-namespace-and-change-root") .output() .await { - Ok(output) if output.status.success() => true, + Ok(output) if output.status.success() => return true, Ok(output) => { let stderr = std::str::from_utf8(&output.stderr) .expect("child process writes a UTF-8 string to stderr; qed") .trim(); - gum::warn!( - target: LOG_TARGET, - ?prepare_worker_program_path, - // Docs say to always print status using `Display` implementation. - status = %output.status, - %stderr, - "Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running with support for unsharing user namespaces for maximum security." - ); - false + format!( + "Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running with support for unsharing user namespaces for maximum security. Error: {}", + stderr + ) }, Err(err) => { gum::warn!( @@ -59,17 +88,21 @@ pub async fn check_can_unshare_user_namespace_and_change_root( "Could not start child process: {}", err ); - false + return false }, - } + }; } else { - gum::warn!( - target: LOG_TARGET, - "Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with support for unsharing user namespaces for maximum security." - ); - false + let msg = "Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with support for unsharing user namespaces for maximum security."; } } + + gum::warn!( + target: LOG_TARGET, + "{} {}", + msg, + SECURE_MODE_ANNOUNCEMENT + ); + false } /// Check if landlock is supported and emit a warning if not. @@ -83,23 +116,19 @@ pub async fn check_landlock( ) -> bool { cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { - match tokio::process::Command::new(prepare_worker_program_path) + let msg = match tokio::process::Command::new(prepare_worker_program_path) .arg("--check-can-enable-landlock") .status() .await { - Ok(status) if status.success() => true, + Ok(status) if status.success() => return true, Ok(status) => { let abi = polkadot_node_core_pvf_common::worker::security::landlock::LANDLOCK_ABI as u8; - gum::warn!( - target: LOG_TARGET, - ?prepare_worker_program_path, - ?status, - %abi, - "Cannot fully enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider upgrading the kernel version for maximum security." - ); - false + format!( + "Cannot fully enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider upgrading the kernel version for maximum security. Landlock ABI: {}", + abi + ) }, Err(err) => { gum::warn!( @@ -108,17 +137,21 @@ pub async fn check_landlock( "Could not start child process: {}", err ); - false + return false }, - } + }; } else { - gum::warn!( - target: LOG_TARGET, - "Cannot enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with landlock support for maximum security." - ); - false + let msg = "Cannot enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with landlock support for maximum security."; } } + + gum::warn!( + target: LOG_TARGET, + "{} {}", + msg, + SECURE_MODE_ANNOUNCEMENT + ); + false } /// Check if seccomp is supported and emit a warning if not. @@ -132,20 +165,14 @@ pub async fn check_seccomp( ) -> bool { cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { - match tokio::process::Command::new(prepare_worker_program_path) + let msg = match tokio::process::Command::new(prepare_worker_program_path) .arg("--check-can-enable-seccomp") .status() .await { - Ok(status) if status.success() => true, + Ok(status) if status.success() => return true, Ok(status) => { - gum::warn!( - target: LOG_TARGET, - ?prepare_worker_program_path, - ?status, - "Cannot fully enable seccomp, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider upgrading the kernel version for maximum security." - ); - false + "Cannot fully enable seccomp, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider upgrading the kernel version for maximum security." }, Err(err) => { gum::warn!( @@ -154,17 +181,21 @@ pub async fn check_seccomp( "Could not start child process: {}", err ); - false + return false }, - } + }; } else { - gum::warn!( - target: LOG_TARGET, - "Cannot enable seccomp, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with seccomp support for maximum security." - ); - false + let msg = "Cannot enable seccomp, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with seccomp support for maximum security."; } } + + gum::warn!( + target: LOG_TARGET, + "{} {}", + msg, + SECURE_MODE_ANNOUNCEMENT + ); + false } const AUDIT_LOG_PATH: &'static str = "/var/log/audit/audit.log"; From 1652d267b51afbcee4e8f56c6960cc34e89a3b47 Mon Sep 17 00:00:00 2001 From: "Marcin S." Date: Fri, 10 Nov 2023 10:58:12 +0100 Subject: [PATCH 2/5] Print all errors at once; update link; make landlock optional --- polkadot/node/core/pvf/src/host.rs | 20 +- polkadot/node/core/pvf/src/security.rs | 301 +++++++++++++++---------- 2 files changed, 185 insertions(+), 136 deletions(-) diff --git a/polkadot/node/core/pvf/src/host.rs b/polkadot/node/core/pvf/src/host.rs index e004b5608ac1..7b383e8034a7 100644 --- a/polkadot/node/core/pvf/src/host.rs +++ b/polkadot/node/core/pvf/src/host.rs @@ -29,12 +29,11 @@ use crate::{ use always_assert::never; use futures::{ channel::{mpsc, oneshot}, - join, Future, FutureExt, SinkExt, StreamExt, + Future, FutureExt, SinkExt, StreamExt, }; use polkadot_node_core_pvf_common::{ error::{PrepareError, PrepareResult}, pvf::PvfPrepData, - SecurityStatus, }; use polkadot_parachain_primitives::primitives::ValidationResult; use std::{ @@ -208,22 +207,7 @@ pub async fn start(config: Config, metrics: Metrics) -> (ValidationHost, impl Fu gum::debug!(target: LOG_TARGET, ?config, "starting PVF validation host"); // Run checks for supported security features once per host startup. Warn here if not enabled. - let security_status = { - // TODO: add check that syslog is available and that seccomp violations are logged? - security::check_secure_mode_platform_requirement(); - let (can_enable_landlock, can_enable_seccomp, can_unshare_user_namespace_and_change_root) = join!( - security::check_landlock(&config.prepare_worker_program_path), - security::check_seccomp(&config.prepare_worker_program_path), - security::check_can_unshare_user_namespace_and_change_root( - &config.prepare_worker_program_path - ) - ); - SecurityStatus { - can_enable_landlock, - can_enable_seccomp, - can_unshare_user_namespace_and_change_root, - } - }; + let security_status = security::check_security_status(&config).await; let (to_host_tx, to_host_rx) = mpsc::channel(10); diff --git a/polkadot/node/core/pvf/src/security.rs b/polkadot/node/core/pvf/src/security.rs index e044754a2c31..f004ce0ed0e9 100644 --- a/polkadot/node/core/pvf/src/security.rs +++ b/polkadot/node/core/pvf/src/security.rs @@ -14,188 +14,253 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use crate::LOG_TARGET; -use std::path::Path; +use crate::{Config, SecurityStatus, LOG_TARGET}; +use futures::join; +use std::{fmt, path::Path}; use tokio::{ fs::{File, OpenOptions}, io::{AsyncReadExt, AsyncSeekExt, SeekFrom}, }; const SECURE_MODE_ANNOUNCEMENT: &'static str = - "In the next release this will be a hard error by default. More information: \ - https://github.com/w3f/polkadot-wiki/issues/4881"; + "In the next release this will be a hard error by default. + \nMore information: https://wiki.polkadot.network/docs/maintain-guides-secure-validator#secure-validator-mode"; -/// Warns if a secure validator cannot be built for the target OS and architecture. -pub fn check_secure_mode_platform_requirement() { - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - cfg_if::cfg_if! { - if #[cfg(target_arch = "x86_64")] { - return () - } else { - let msg = "Secure validators are only supported on CPUs from the x86_64 family (usually Intel or AMD)."; - } - } - } else { - cfg_if::cfg_if! { - if #[cfg(target_arch = "x86_64")] { - let msg = "Secure validators are only supported on Linux."; - } else { - let msg = "Secure validators are only supported on Linux and on CPUs from the x86_64 family (usually Intel or AMD)."; - } - } - } - }; +/// Run checks for supported security features. +pub async fn check_security_status(config: &Config) -> SecurityStatus { + let Config { prepare_worker_program_path, .. } = config; + // TODO: Get this from `Config` once Secure Validator Mode is implemented. + // Right now we set it to `true`, but only display a warning if conditions are not met. + let secure_validator_mode = true; - gum::warn!( - target: LOG_TARGET, - "{} {}", - msg, - SECURE_MODE_ANNOUNCEMENT + // TODO: add check that syslog is available and that seccomp violations are logged? + let (landlock, seccomp, change_root) = join!( + check_landlock(prepare_worker_program_path), + check_seccomp(prepare_worker_program_path), + check_can_unshare_user_namespace_and_change_root(prepare_worker_program_path) ); + + let security_status = SecurityStatus { + can_enable_landlock: landlock.is_ok(), + can_enable_seccomp: seccomp.is_ok(), + can_unshare_user_namespace_and_change_root: change_root.is_ok(), + }; + + let errs: Vec = [landlock, seccomp, change_root] + .into_iter() + .filter_map(|result| result.err()) + .collect(); + let err_occurred = print_secure_mode_message(secure_validator_mode, errs); + if err_occurred { + gum::error!( + target: LOG_TARGET, + "{}", + SECURE_MODE_ANNOUNCEMENT, + ); + } + + security_status +} + +type SecureModeResult = std::result::Result<(), SecureModeError>; + +/// Errors related to enabling Secure Validator Mode. +#[derive(Debug)] +enum SecureModeError { + CannotEnableLandlock(String), + CannotEnableSeccomp(String), + CannotUnshareUserNamespaceAndChangeRoot(String), +} + +impl SecureModeError { + /// Whether this error is allowed with Secure Validator Mode enabled. + fn is_allowed_in_secure_mode(&self) -> bool { + use SecureModeError::*; + match self { + CannotEnableLandlock(_) => true, + CannotEnableSeccomp(_) => false, + CannotUnshareUserNamespaceAndChangeRoot(_) => false, + } + } +} + +impl fmt::Display for SecureModeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use SecureModeError::*; + let display = match self { + CannotEnableLandlock(err) => format!("Optional: Cannot enable landlock, a Linux 5.13+ kernel security feature: {}", err), + CannotEnableSeccomp(err) => format!("Cannot enable seccomp, a Linux-specific kernel security feature: {}", err), + CannotUnshareUserNamespaceAndChangeRoot(err) => format!("Cannot unshare user namespace and change root, which are Linux-specific kernel security features: {}", err), + }; + + write!(f, "{}", display) + } +} + +/// Errors if Secure Validator Mode and some mandatory errors occurred, warn otherwise. +/// +/// # Returns +/// +/// `true` if an error was printed, `false` otherwise. +fn print_secure_mode_message(secure_validator_mode: bool, errs: Vec) -> bool { + // Trying to run securely and some mandatory errors occurred. + const SECURE_MODE_ERROR: &'static str = "🚨 Your system cannot securely run a validator. \ + \nRunning validation of malicious PVF code has a higher risk of compromising this machine."; + // Some errors occurred when running insecurely, or some optional errors occurred when running + // securely. + const SECURE_MODE_WARNING: &'static str = "🚨 Some security issues have been detected. \ + \nRunning validation of malicious PVF code has a higher risk of compromising this machine."; + + if errs.is_empty() { + return false + } + + let errs_allowed = + !secure_validator_mode || errs.iter().all(|err| err.is_allowed_in_secure_mode()); + let errs_string: String = errs.iter().map(|err| format!("\n - {}", err)).collect(); + + if errs_allowed { + gum::warn!( + target: LOG_TARGET, + "{}{}", + SECURE_MODE_WARNING, + errs_string, + ); + false + } else { + gum::error!( + target: LOG_TARGET, + "{}{}", + SECURE_MODE_ERROR, + errs_string, + ); + true + } } -/// Check if we can sandbox the root and emit a warning if not. +/// Check if we can change root to a new, sandboxed root and return an error if not. /// /// We do this check by spawning a new process and trying to sandbox it. To get as close as possible /// to running the check in a worker, we try it... in a worker. The expected return status is 0 on /// success and -1 on failure. -pub async fn check_can_unshare_user_namespace_and_change_root( +async fn check_can_unshare_user_namespace_and_change_root( #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] prepare_worker_program_path: &Path, -) -> bool { +) -> SecureModeResult { cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { - let msg = match tokio::process::Command::new(prepare_worker_program_path) + match tokio::process::Command::new(prepare_worker_program_path) .arg("--check-can-unshare-user-namespace-and-change-root") .output() .await { - Ok(output) if output.status.success() => return true, + Ok(output) if output.status.success() => Ok(()), Ok(output) => { let stderr = std::str::from_utf8(&output.stderr) .expect("child process writes a UTF-8 string to stderr; qed") .trim(); - format!( - "Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running with support for unsharing user namespaces for maximum security. Error: {}", - stderr - ) + Err(SecureModeError::CannotUnshareUserNamespaceAndChangeRoot( + format!("not available: {}", stderr) + )) }, - Err(err) => { - gum::warn!( - target: LOG_TARGET, - ?prepare_worker_program_path, - "Could not start child process: {}", - err - ); - return false - }, - }; + Err(err) => + Err(SecureModeError::CannotUnshareUserNamespaceAndChangeRoot( + format!("could not start child process: {}", err) + )), + } } else { - let msg = "Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with support for unsharing user namespaces for maximum security."; + Err(SecureModeError::CannotUnshareUserNamespaceAndChangeRoot( + "only available on Linux".into() + )) } } - - gum::warn!( - target: LOG_TARGET, - "{} {}", - msg, - SECURE_MODE_ANNOUNCEMENT - ); - false } -/// Check if landlock is supported and emit a warning if not. +/// Check if landlock is supported and return an error if not. /// /// We do this check by spawning a new process and trying to sandbox it. To get as close as possible /// to running the check in a worker, we try it... in a worker. The expected return status is 0 on /// success and -1 on failure. -pub async fn check_landlock( +async fn check_landlock( #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] prepare_worker_program_path: &Path, -) -> bool { +) -> SecureModeResult { cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { - let msg = match tokio::process::Command::new(prepare_worker_program_path) + match tokio::process::Command::new(prepare_worker_program_path) .arg("--check-can-enable-landlock") .status() .await { - Ok(status) if status.success() => return true, - Ok(status) => { + Ok(status) if status.success() => Ok(()), + Ok(_status) => { let abi = polkadot_node_core_pvf_common::worker::security::landlock::LANDLOCK_ABI as u8; - format!( - "Cannot fully enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider upgrading the kernel version for maximum security. Landlock ABI: {}", - abi - ) + Err(SecureModeError::CannotEnableLandlock( + format!("landlock ABI {} not available", abi) + )) }, - Err(err) => { - gum::warn!( - target: LOG_TARGET, - ?prepare_worker_program_path, - "Could not start child process: {}", - err - ); - return false - }, - }; + Err(err) => + Err(SecureModeError::CannotEnableLandlock( + format!("could not start child process: {}", err) + )), + } } else { - let msg = "Cannot enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with landlock support for maximum security."; + Err(SecureModeError::CannotEnableLandlock( + "only available on Linux".into() + )) } } - - gum::warn!( - target: LOG_TARGET, - "{} {}", - msg, - SECURE_MODE_ANNOUNCEMENT - ); - false } -/// Check if seccomp is supported and emit a warning if not. +/// Check if seccomp is supported and return an error if not. /// /// We do this check by spawning a new process and trying to sandbox it. To get as close as possible /// to running the check in a worker, we try it... in a worker. The expected return status is 0 on /// success and -1 on failure. -pub async fn check_seccomp( +async fn check_seccomp( #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] prepare_worker_program_path: &Path, -) -> bool { +) -> SecureModeResult { cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { - let msg = match tokio::process::Command::new(prepare_worker_program_path) - .arg("--check-can-enable-seccomp") - .status() - .await - { - Ok(status) if status.success() => return true, - Ok(status) => { - "Cannot fully enable seccomp, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider upgrading the kernel version for maximum security." - }, - Err(err) => { - gum::warn!( - target: LOG_TARGET, - ?prepare_worker_program_path, - "Could not start child process: {}", - err - ); - return false - }, - }; + cfg_if::cfg_if! { + if #[cfg(target_arch = "x86_64")] { + match tokio::process::Command::new(prepare_worker_program_path) + .arg("--check-can-enable-seccomp") + .status() + .await + { + Ok(status) if status.success() => Ok(()), + Ok(_status) => + Err(SecureModeError::CannotEnableSeccomp( + "not available".into() + )), + Err(err) => + Err(SecureModeError::CannotEnableSeccomp( + format!("could not start child process: {}", err) + )), + } + } else { + Err(SecureModeError::CannotEnableSeccomp( + "only supported on CPUs from the x86_64 family (usually Intel or AMD)".into() + )) + } + } } else { - let msg = "Cannot enable seccomp, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with seccomp support for maximum security."; + cfg_if::cfg_if! { + if #[cfg(target_arch = "x86_64")] { + Err(SecureModeError::CannotEnableSeccomp( + "only supported on Linux".into() + )) + } else { + Err(SecureModeError::CannotEnableSeccomp( + "only supported on Linux and on CPUs from the x86_64 family (usually Intel or AMD).".into() + )) + } + } } } - - gum::warn!( - target: LOG_TARGET, - "{} {}", - msg, - SECURE_MODE_ANNOUNCEMENT - ); - false } const AUDIT_LOG_PATH: &'static str = "/var/log/audit/audit.log"; From 4e163b9526eb700f3e1192da722e1d013b328be2 Mon Sep 17 00:00:00 2001 From: "Marcin S." Date: Fri, 10 Nov 2023 11:48:20 +0100 Subject: [PATCH 3/5] Remove `secure_validator_mode` flag in helpers, we don't need it yet --- polkadot/node/core/pvf/src/security.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/polkadot/node/core/pvf/src/security.rs b/polkadot/node/core/pvf/src/security.rs index f004ce0ed0e9..feba11775833 100644 --- a/polkadot/node/core/pvf/src/security.rs +++ b/polkadot/node/core/pvf/src/security.rs @@ -29,9 +29,6 @@ const SECURE_MODE_ANNOUNCEMENT: &'static str = /// Run checks for supported security features. pub async fn check_security_status(config: &Config) -> SecurityStatus { let Config { prepare_worker_program_path, .. } = config; - // TODO: Get this from `Config` once Secure Validator Mode is implemented. - // Right now we set it to `true`, but only display a warning if conditions are not met. - let secure_validator_mode = true; // TODO: add check that syslog is available and that seccomp violations are logged? let (landlock, seccomp, change_root) = join!( @@ -50,7 +47,7 @@ pub async fn check_security_status(config: &Config) -> SecurityStatus { .into_iter() .filter_map(|result| result.err()) .collect(); - let err_occurred = print_secure_mode_message(secure_validator_mode, errs); + let err_occurred = print_secure_mode_message(errs); if err_occurred { gum::error!( target: LOG_TARGET, @@ -102,7 +99,7 @@ impl fmt::Display for SecureModeError { /// # Returns /// /// `true` if an error was printed, `false` otherwise. -fn print_secure_mode_message(secure_validator_mode: bool, errs: Vec) -> bool { +fn print_secure_mode_message(errs: Vec) -> bool { // Trying to run securely and some mandatory errors occurred. const SECURE_MODE_ERROR: &'static str = "🚨 Your system cannot securely run a validator. \ \nRunning validation of malicious PVF code has a higher risk of compromising this machine."; @@ -115,8 +112,7 @@ fn print_secure_mode_message(secure_validator_mode: bool, errs: Vec Date: Sat, 11 Nov 2023 17:15:46 +0100 Subject: [PATCH 4/5] Minor refactor of optionality --- polkadot/node/core/pvf/src/security.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/polkadot/node/core/pvf/src/security.rs b/polkadot/node/core/pvf/src/security.rs index feba11775833..3145c8c8770e 100644 --- a/polkadot/node/core/pvf/src/security.rs +++ b/polkadot/node/core/pvf/src/security.rs @@ -85,7 +85,7 @@ impl fmt::Display for SecureModeError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use SecureModeError::*; let display = match self { - CannotEnableLandlock(err) => format!("Optional: Cannot enable landlock, a Linux 5.13+ kernel security feature: {}", err), + CannotEnableLandlock(err) => format!("Cannot enable landlock, a Linux 5.13+ kernel security feature: {}", err), CannotEnableSeccomp(err) => format!("Cannot enable seccomp, a Linux-specific kernel security feature: {}", err), CannotUnshareUserNamespaceAndChangeRoot(err) => format!("Cannot unshare user namespace and change root, which are Linux-specific kernel security features: {}", err), }; @@ -113,7 +113,16 @@ fn print_secure_mode_message(errs: Vec) -> bool { } let errs_allowed = errs.iter().all(|err| err.is_allowed_in_secure_mode()); - let errs_string: String = errs.iter().map(|err| format!("\n - {}", err)).collect(); + let errs_string: String = errs + .iter() + .map(|err| { + format!( + "\n - {}{}", + if err.is_allowed_in_secure_mode() { "Optional: " } else { "" }, + err + ) + }) + .collect(); if errs_allowed { gum::warn!( From 516dcb470865f89f6ccb75c23f88b90f4a988167 Mon Sep 17 00:00:00 2001 From: Marcin S Date: Sun, 12 Nov 2023 20:51:06 +0100 Subject: [PATCH 5/5] Update polkadot/node/core/pvf/src/security.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- polkadot/node/core/pvf/src/security.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/polkadot/node/core/pvf/src/security.rs b/polkadot/node/core/pvf/src/security.rs index 3145c8c8770e..295dd7df94dd 100644 --- a/polkadot/node/core/pvf/src/security.rs +++ b/polkadot/node/core/pvf/src/security.rs @@ -84,13 +84,11 @@ impl SecureModeError { impl fmt::Display for SecureModeError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use SecureModeError::*; - let display = match self { - CannotEnableLandlock(err) => format!("Cannot enable landlock, a Linux 5.13+ kernel security feature: {}", err), - CannotEnableSeccomp(err) => format!("Cannot enable seccomp, a Linux-specific kernel security feature: {}", err), - CannotUnshareUserNamespaceAndChangeRoot(err) => format!("Cannot unshare user namespace and change root, which are Linux-specific kernel security features: {}", err), - }; - - write!(f, "{}", display) + match self { + CannotEnableLandlock(err) => write!(f, "Cannot enable landlock, a Linux 5.13+ kernel security feature: {err}"), + CannotEnableSeccomp(err) => write!(f, "Cannot enable seccomp, a Linux-specific kernel security feature: {err}"), + CannotUnshareUserNamespaceAndChangeRoot(err) => write!(f, "Cannot unshare user namespace and change root, which are Linux-specific kernel security features: {err}"), + } } }