From 55fe1e054b50a3494c6a6bc9936bcc6bf6b35a03 Mon Sep 17 00:00:00 2001 From: Maksim Ryndin Date: Wed, 24 Jan 2024 11:20:16 +0000 Subject: [PATCH 1/4] refactor pvf security module --- polkadot/node/core/pvf/README.md | 6 +- polkadot/node/core/pvf/common/Cargo.toml | 2 + polkadot/node/core/pvf/src/host.rs | 25 +++- polkadot/node/core/pvf/src/lib.rs | 20 +++ polkadot/node/core/pvf/src/security.rs | 160 ++++++++--------------- 5 files changed, 101 insertions(+), 112 deletions(-) diff --git a/polkadot/node/core/pvf/README.md b/polkadot/node/core/pvf/README.md index 796e17c05faa..5304b0720b2d 100644 --- a/polkadot/node/core/pvf/README.md +++ b/polkadot/node/core/pvf/README.md @@ -13,7 +13,7 @@ See also: Running `cargo test` in the `pvf/` directory will run unit and integration tests. -**Note:** some tests run only under Linux, amd64, and/or with the +**Note:** some tests run only under Linux, x86-64, and/or with the `ci-only-tests` feature enabled. See the general [Testing][testing] instructions for more information on @@ -34,8 +34,8 @@ RUST_LOG=parachain::pvf=trace zombienet --provider=native spawn zombienet_tests/ ## Testing on Linux Some of the PVF functionality, especially related to security, is Linux-only, -and some is amd64-only. If you touch anything security-related, make sure to -test on Linux amd64! If you're on a Mac, you can either run a VM or you can hire +and some is x86-64-only. If you touch anything security-related, make sure to +test on Linux x86-64! If you're on a Mac, you can either run a VM or you can hire a VPS and use the open-source tool [EternalTerminal][et] to connect to it.[^et] [^et]: Unlike ssh, ET preserves your session across disconnects, and unlike diff --git a/polkadot/node/core/pvf/common/Cargo.toml b/polkadot/node/core/pvf/common/Cargo.toml index 1f75e1190c50..095ad7faf709 100644 --- a/polkadot/node/core/pvf/common/Cargo.toml +++ b/polkadot/node/core/pvf/common/Cargo.toml @@ -35,6 +35,8 @@ sp-tracing = { path = "../../../../../substrate/primitives/tracing" } [target.'cfg(target_os = "linux")'.dependencies] landlock = "0.3.0" nix = { version = "0.27.1", features = ["sched"] } + +[target.'cfg(all(target_os = "linux", target_arch = "x86_64"))'.dependencies] seccompiler = "0.4.0" [dev-dependencies] diff --git a/polkadot/node/core/pvf/src/host.rs b/polkadot/node/core/pvf/src/host.rs index e215f11b91d3..279313c5755a 100644 --- a/polkadot/node/core/pvf/src/host.rs +++ b/polkadot/node/core/pvf/src/host.rs @@ -24,7 +24,7 @@ use crate::{ artifacts::{ArtifactId, ArtifactPathId, ArtifactState, Artifacts}, execute::{self, PendingExecutionRequest}, metrics::Metrics, - prepare, security, Priority, SecurityStatus, ValidationError, LOG_TARGET, + prepare, Priority, SecurityStatus, ValidationError, LOG_TARGET, }; use always_assert::never; use futures::{ @@ -225,10 +225,31 @@ pub async fn start( // Run checks for supported security features once per host startup. If some checks fail, warn // if Secure Validator Mode is disabled and return an error otherwise. - let security_status = match security::check_security_status(&config).await { + #[cfg(target_os = "linux")] + let security_status = match crate::security::check_security_status(&config).await { Ok(ok) => ok, Err(err) => return Err(SubsystemError::Context(err)), }; + #[cfg(not(target_os = "linux"))] + let security_status = if config.secure_validator_mode { + gum::error!( + target: LOG_TARGET, + "{}{}{}", + crate::SECURE_MODE_ERROR, + crate::SECURE_LINUX_NOTE, + crate::IGNORE_SECURE_MODE_TIP + ); + return Err(SubsystemError::Context( + "could not enable Secure Validator Mode for non-Linux; check logs".into(), + )); + } else { + gum::warn!( + target: LOG_TARGET, + "{}{}", + crate::SECURE_MODE_WARNING, + crate::SECURE_LINUX_NOTE, + ); + }; let (to_host_tx, to_host_rx) = mpsc::channel(HOST_MESSAGE_QUEUE_SIZE); diff --git a/polkadot/node/core/pvf/src/lib.rs b/polkadot/node/core/pvf/src/lib.rs index 92263281eeab..462498fa8f6b 100644 --- a/polkadot/node/core/pvf/src/lib.rs +++ b/polkadot/node/core/pvf/src/lib.rs @@ -97,6 +97,7 @@ mod host; mod metrics; mod prepare; mod priority; +#[cfg(target_os = "linux")] mod security; mod worker_interface; @@ -135,3 +136,22 @@ pub fn get_worker_version(worker_path: &Path) -> std::io::Result { .trim() .to_string()) } + +// Trying to run securely and some mandatory errors occurred. +pub(crate) 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. +pub(crate) 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."; +// Message to be printed only when running securely and mandatory errors occurred. +pub(crate) const IGNORE_SECURE_MODE_TIP: &'static str = +"\nYou can ignore this error with the `--insecure-validator-i-know-what-i-do` \ +command line argument if you understand and accept the risks of running insecurely. \ +With this flag, security features are enabled on a best-effort basis, but not mandatory. \ +\nMore information: https://wiki.polkadot.network/docs/maintain-guides-secure-validator#secure-validator-mode"; +// Only Linux supports security features +#[cfg(not(target_os = "linux"))] +pub(crate) const SECURE_LINUX_NOTE: &'static str = "\nSecure mode is enabled only for Linux \ +\nand a full secure mode is enabled only for Linux x86-64."; diff --git a/polkadot/node/core/pvf/src/security.rs b/polkadot/node/core/pvf/src/security.rs index f62a232abf27..733ef18bcadb 100644 --- a/polkadot/node/core/pvf/src/security.rs +++ b/polkadot/node/core/pvf/src/security.rs @@ -169,20 +169,6 @@ impl fmt::Display for SecureModeError { /// Print an error if Secure Validator Mode and some mandatory errors occurred, warn otherwise. fn print_secure_mode_error_or_warning(security_status: &FullSecurityStatus) { - // 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."; - // Message to be printed only when running securely and mandatory errors occurred. - const IGNORE_SECURE_MODE_TIP: &'static str = - "\nYou can ignore this error with the `--insecure-validator-i-know-what-i-do` \ - command line argument if you understand and accept the risks of running insecurely. \ - With this flag, security features are enabled on a best-effort basis, but not mandatory. \ - \nMore information: https://wiki.polkadot.network/docs/maintain-guides-secure-validator#secure-validator-mode"; - let all_errs_allowed = security_status.all_errs_allowed(); let errs_string = security_status.errs_string(); @@ -190,16 +176,16 @@ fn print_secure_mode_error_or_warning(security_status: &FullSecurityStatus) { gum::warn!( target: LOG_TARGET, "{}{}", - SECURE_MODE_WARNING, + crate::SECURE_MODE_WARNING, errs_string, ); } else { gum::error!( target: LOG_TARGET, "{}{}{}", - SECURE_MODE_ERROR, + crate::SECURE_MODE_ERROR, errs_string, - IGNORE_SECURE_MODE_TIP + crate::IGNORE_SECURE_MODE_TIP ); } } @@ -210,29 +196,25 @@ fn print_secure_mode_error_or_warning(security_status: &FullSecurityStatus) { /// 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. async fn check_can_unshare_user_namespace_and_change_root( - #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] prepare_worker_program_path: &Path, - #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] cache_path: &Path, + cache_path: &Path, ) -> SecureModeResult { - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - let cache_dir_tempdir = tempfile::Builder::new() - .prefix("check-can-unshare-") - .tempdir_in(cache_path) - .map_err(|err| SecureModeError::CannotUnshareUserNamespaceAndChangeRoot( - format!("could not create a temporary directory in {:?}: {}", cache_path, err) - ))?; - spawn_process_for_security_check( - prepare_worker_program_path, - "--check-can-unshare-user-namespace-and-change-root", - &[cache_dir_tempdir.path()], - ).await.map_err(|err| SecureModeError::CannotUnshareUserNamespaceAndChangeRoot(err)) - } else { - Err(SecureModeError::CannotUnshareUserNamespaceAndChangeRoot( - "only available on Linux".into() + let cache_dir_tempdir = tempfile::Builder::new() + .prefix("check-can-unshare-") + .tempdir_in(cache_path) + .map_err(|err| { + SecureModeError::CannotUnshareUserNamespaceAndChangeRoot(format!( + "could not create a temporary directory in {:?}: {}", + cache_path, err )) - } - } + })?; + spawn_process_for_security_check( + prepare_worker_program_path, + "--check-can-unshare-user-namespace-and-change-root", + &[cache_dir_tempdir.path()], + ) + .await + .map_err(|err| SecureModeError::CannotUnshareUserNamespaceAndChangeRoot(err)) } /// Check if landlock is supported and return an error if not. @@ -240,25 +222,15 @@ async fn check_can_unshare_user_namespace_and_change_root( /// 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. -async fn check_landlock( - #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] - prepare_worker_program_path: &Path, -) -> SecureModeResult { - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - let abi = polkadot_node_core_pvf_common::worker::security::landlock::LANDLOCK_ABI as u8; - spawn_process_for_security_check( - prepare_worker_program_path, - "--check-can-enable-landlock", - std::iter::empty::<&str>(), - ).await.map_err(|err| SecureModeError::CannotEnableLandlock { err, abi }) - } else { - Err(SecureModeError::CannotEnableLandlock { - err: "only available on Linux".into(), - abi: 0, - }) - } - } +async fn check_landlock(prepare_worker_program_path: &Path) -> SecureModeResult { + let abi = polkadot_node_core_pvf_common::worker::security::landlock::LANDLOCK_ABI as u8; + spawn_process_for_security_check( + prepare_worker_program_path, + "--check-can-enable-landlock", + std::iter::empty::<&str>(), + ) + .await + .map_err(|err| SecureModeError::CannotEnableLandlock { err, abi }) } /// Check if seccomp is supported and return an error if not. @@ -266,39 +238,23 @@ async fn check_landlock( /// 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. -async fn check_seccomp( - #[cfg_attr(not(all(target_os = "linux", target_arch = "x86_64")), allow(unused_variables))] - prepare_worker_program_path: &Path, -) -> SecureModeResult { - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - cfg_if::cfg_if! { - if #[cfg(target_arch = "x86_64")] { - spawn_process_for_security_check( - prepare_worker_program_path, - "--check-can-enable-seccomp", - std::iter::empty::<&str>(), - ).await.map_err(|err| SecureModeError::CannotEnableSeccomp(err)) - } else { - Err(SecureModeError::CannotEnableSeccomp( - "only supported on CPUs from the x86_64 family (usually Intel or AMD)".into() - )) - } - } - } else { - 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() - )) - } - } - } - } + +#[cfg(target_arch = "x86_64")] +async fn check_seccomp(prepare_worker_program_path: &Path) -> SecureModeResult { + spawn_process_for_security_check( + prepare_worker_program_path, + "--check-can-enable-seccomp", + std::iter::empty::<&str>(), + ) + .await + .map_err(|err| SecureModeError::CannotEnableSeccomp(err)) +} + +#[cfg(not(target_arch = "x86_64"))] +async fn check_seccomp(_: &Path) -> SecureModeResult { + Err(SecureModeError::CannotEnableSeccomp( + "only supported on CPUs from the x86_64 family (usually Intel or AMD)".into(), + )) } /// Check if we can call `clone` with all sandboxing flags, and return an error if not. @@ -306,26 +262,16 @@ async fn check_seccomp( /// 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. -async fn check_can_do_secure_clone( - #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] - prepare_worker_program_path: &Path, -) -> SecureModeResult { - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - spawn_process_for_security_check( - prepare_worker_program_path, - "--check-can-do-secure-clone", - std::iter::empty::<&str>(), - ).await.map_err(|err| SecureModeError::CannotDoSecureClone(err)) - } else { - Err(SecureModeError::CannotDoSecureClone( - "only available on Linux".into() - )) - } - } +async fn check_can_do_secure_clone(prepare_worker_program_path: &Path) -> SecureModeResult { + spawn_process_for_security_check( + prepare_worker_program_path, + "--check-can-do-secure-clone", + std::iter::empty::<&str>(), + ) + .await + .map_err(|err| SecureModeError::CannotDoSecureClone(err)) } -#[cfg(target_os = "linux")] async fn spawn_process_for_security_check( prepare_worker_program_path: &Path, check_arg: &'static str, From 87cfada8175c724e5f61056dd863204c825abbfb Mon Sep 17 00:00:00 2001 From: Maksim Ryndin Date: Sun, 28 Jan 2024 15:35:38 +0000 Subject: [PATCH 2/4] pvf: fix security status for non-linux --- polkadot/node/core/pvf/common/src/lib.rs | 30 ++++++++++++++++++++++++ polkadot/node/core/pvf/src/host.rs | 1 + 2 files changed, 31 insertions(+) diff --git a/polkadot/node/core/pvf/common/src/lib.rs b/polkadot/node/core/pvf/common/src/lib.rs index d891c06bf2ad..a45eb6609fd8 100644 --- a/polkadot/node/core/pvf/common/src/lib.rs +++ b/polkadot/node/core/pvf/common/src/lib.rs @@ -86,3 +86,33 @@ pub fn framed_recv_blocking(r: &mut (impl Read + Unpin)) -> io::Result> r.read_exact(&mut buf)?; Ok(buf) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_secure_status() { + let status = SecurityStatus::default(); + assert!( + !status.secure_validator_mode, + "secure_validator_mode is false for default security status" + ); + assert!( + !status.can_enable_landlock, + "can_enable_landlock is false for default security status" + ); + assert!( + !status.can_enable_seccomp, + "can_enable_seccomp is false for default security status" + ); + assert!( + !status.can_unshare_user_namespace_and_change_root, + "can_unshare_user_namespace_and_change_root is false for default security status" + ); + assert!( + !status.can_do_secure_clone, + "can_do_secure_clone is false for default security status" + ); + } +} diff --git a/polkadot/node/core/pvf/src/host.rs b/polkadot/node/core/pvf/src/host.rs index 279313c5755a..ae9fdc7d2dea 100644 --- a/polkadot/node/core/pvf/src/host.rs +++ b/polkadot/node/core/pvf/src/host.rs @@ -249,6 +249,7 @@ pub async fn start( crate::SECURE_MODE_WARNING, crate::SECURE_LINUX_NOTE, ); + SecurityStatus::default() }; let (to_host_tx, to_host_rx) = mpsc::channel(HOST_MESSAGE_QUEUE_SIZE); From 83ce4bf25107fbb26ffc1abe7aa6eb4c6408be3d Mon Sep 17 00:00:00 2001 From: Maksim Ryndin Date: Mon, 29 Jan 2024 05:51:42 +0000 Subject: [PATCH 3/4] pvf: fix clippy error --- polkadot/node/core/pvf/common/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/node/core/pvf/common/src/lib.rs b/polkadot/node/core/pvf/common/src/lib.rs index a45eb6609fd8..15097dbd3af5 100644 --- a/polkadot/node/core/pvf/common/src/lib.rs +++ b/polkadot/node/core/pvf/common/src/lib.rs @@ -87,7 +87,7 @@ pub fn framed_recv_blocking(r: &mut (impl Read + Unpin)) -> io::Result> Ok(buf) } -#[cfg(test)] +#[cfg(all(test, not(feature = "test-utils")))] mod tests { use super::*; From 43983756bc89915a6e808625860622018065da9e Mon Sep 17 00:00:00 2001 From: Maksim Ryndin Date: Mon, 29 Jan 2024 08:54:41 +0000 Subject: [PATCH 4/4] sassafras fix markdown --- substrate/frame/sassafras/src/data/tickets-sort.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/sassafras/src/data/tickets-sort.md b/substrate/frame/sassafras/src/data/tickets-sort.md index 4d96a6825c88..64fc45e4fb00 100644 --- a/substrate/frame/sassafras/src/data/tickets-sort.md +++ b/substrate/frame/sassafras/src/data/tickets-sort.md @@ -267,7 +267,7 @@ buffer (i.e. how many tickets we retain from the last processed segment) | 3 | 14 | | 2 | 17 | | 1 | 9 | -| 0 | 13 +| 0 | 13 | # Graph of the same data