diff --git a/Cargo.lock b/Cargo.lock index d8e7d055d770..58bacc9db739 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6874,9 +6874,9 @@ dependencies = [ [[package]] name = "landlock" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "520baa32708c4e957d2fc3a186bc5bd8d26637c33137f399ddfc202adb240068" +checksum = "1530c5b973eeed4ac216af7e24baf5737645a6272e361f1fb95710678b67d9cc" dependencies = [ "enumflags2", "libc", diff --git a/polkadot/node/core/pvf/common/Cargo.toml b/polkadot/node/core/pvf/common/Cargo.toml index 0f7308396d80..4bdacca72f42 100644 --- a/polkadot/node/core/pvf/common/Cargo.toml +++ b/polkadot/node/core/pvf/common/Cargo.toml @@ -29,7 +29,7 @@ sp-io = { path = "../../../../../substrate/primitives/io" } sp-tracing = { path = "../../../../../substrate/primitives/tracing" } [target.'cfg(target_os = "linux")'.dependencies] -landlock = "0.2.0" +landlock = "0.3.0" [dev-dependencies] assert_matches = "1.4.0" diff --git a/polkadot/node/core/pvf/common/src/worker/security.rs b/polkadot/node/core/pvf/common/src/worker/security.rs index b7abf028f941..1b7614177448 100644 --- a/polkadot/node/core/pvf/common/src/worker/security.rs +++ b/polkadot/node/core/pvf/common/src/worker/security.rs @@ -223,13 +223,22 @@ pub mod landlock { /// Landlock ABI version. We use ABI V1 because: /// /// 1. It is supported by our reference kernel version. - /// 2. Later versions do not (yet) provide additional security. + /// 2. Later versions do not (yet) provide additional security that would benefit us. /// - /// # Versions (as of June 2023) + /// # Versions (as of October 2023) /// /// - Polkadot reference kernel version: 5.16+ - /// - ABI V1: 5.13 - introduces landlock, including full restrictions on file reads - /// - ABI V2: 5.19 - adds ability to configure file renaming (not used by us) + /// + /// - ABI V1: kernel 5.13 - Introduces landlock, including full restrictions on file reads. + /// + /// - ABI V2: kernel 5.19 - Adds ability to prevent file renaming. Does not help us. During + /// execution an attacker can only affect the name of a symlinked artifact and not the + /// original one. + /// + /// - ABI V3: kernel 6.2 - Adds ability to prevent file truncation. During execution, can + /// prevent attackers from affecting a symlinked artifact. We don't strictly need this as we + /// plan to check for file integrity anyway; see + /// . /// /// # Determinism /// @@ -335,7 +344,7 @@ pub mod landlock { A: Into>, { let mut ruleset = - Ruleset::new().handle_access(AccessFs::from_all(LANDLOCK_ABI))?.create()?; + Ruleset::default().handle_access(AccessFs::from_all(LANDLOCK_ABI))?.create()?; for (fs_path, access_bits) in fs_exceptions { let paths = &[fs_path.as_ref().to_owned()]; let mut rules = path_beneath_rules(paths, access_bits).peekable(); @@ -466,5 +475,38 @@ pub mod landlock { assert!(handle.join().is_ok()); } + + // Test that checks whether landlock under our ABI version is able to truncate files. + #[test] + fn restricted_thread_can_truncate_file() { + // TODO: This would be nice: . + if !check_is_fully_enabled() { + return + } + + // Restricted thread can truncate file. + let handle = + thread::spawn(|| { + // Create and write a file. This should succeed before any landlock + // restrictions are applied. + const TEXT: &str = "foo"; + let tmpfile = tempfile::NamedTempFile::new().unwrap(); + let path = tmpfile.path(); + + fs::write(path, TEXT).unwrap(); + + // Apply Landlock with all exceptions under the current ABI. + let status = try_restrict(vec![(path, AccessFs::from_all(LANDLOCK_ABI))]); + if !matches!(status, Ok(RulesetStatus::FullyEnforced)) { + panic!("Ruleset should be enforced since we checked if landlock is enabled: {:?}", status); + } + + // Try to truncate the file. + let result = tmpfile.as_file().set_len(0); + assert!(result.is_ok()); + }); + + assert!(handle.join().is_ok()); + } } }