From 132ba0c89fc1d48d770f28a5d5448c9dd1bb164a Mon Sep 17 00:00:00 2001
From: Marcin S <marcin@realemail.net>
Date: Wed, 11 Oct 2023 16:13:07 +0200
Subject: [PATCH] PVF worker: bump landlock, update ABI docs (#1850)
---
Cargo.lock | 4 +-
polkadot/node/core/pvf/common/Cargo.toml | 2 +-
.../core/pvf/common/src/worker/security.rs | 52 +++++++++++++++++--
3 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index d8e7d055d77..58bacc9db73 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 0f7308396d8..4bdacca72f4 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 b7abf028f94..1b761417744 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
+ /// <https://github.com/paritytech/polkadot-sdk/issues/677>.
///
/// # Determinism
///
@@ -335,7 +344,7 @@ pub mod landlock {
A: Into<BitFlags<AccessFs>>,
{
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: <https://github.com/rust-lang/rust/issues/68007>.
+ 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());
+ }
}
}
--
GitLab