Skip to content

Commit 132ba0c

Browse files
authored
PVF worker: bump landlock, update ABI docs (#1850)
1 parent cfb2925 commit 132ba0c

File tree

3 files changed

+50
-8
lines changed

3 files changed

+50
-8
lines changed

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

polkadot/node/core/pvf/common/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ sp-io = { path = "../../../../../substrate/primitives/io" }
2929
sp-tracing = { path = "../../../../../substrate/primitives/tracing" }
3030

3131
[target.'cfg(target_os = "linux")'.dependencies]
32-
landlock = "0.2.0"
32+
landlock = "0.3.0"
3333

3434
[dev-dependencies]
3535
assert_matches = "1.4.0"

polkadot/node/core/pvf/common/src/worker/security.rs

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -223,13 +223,22 @@ pub mod landlock {
223223
/// Landlock ABI version. We use ABI V1 because:
224224
///
225225
/// 1. It is supported by our reference kernel version.
226-
/// 2. Later versions do not (yet) provide additional security.
226+
/// 2. Later versions do not (yet) provide additional security that would benefit us.
227227
///
228-
/// # Versions (as of June 2023)
228+
/// # Versions (as of October 2023)
229229
///
230230
/// - Polkadot reference kernel version: 5.16+
231-
/// - ABI V1: 5.13 - introduces landlock, including full restrictions on file reads
232-
/// - ABI V2: 5.19 - adds ability to configure file renaming (not used by us)
231+
///
232+
/// - ABI V1: kernel 5.13 - Introduces landlock, including full restrictions on file reads.
233+
///
234+
/// - ABI V2: kernel 5.19 - Adds ability to prevent file renaming. Does not help us. During
235+
/// execution an attacker can only affect the name of a symlinked artifact and not the
236+
/// original one.
237+
///
238+
/// - ABI V3: kernel 6.2 - Adds ability to prevent file truncation. During execution, can
239+
/// prevent attackers from affecting a symlinked artifact. We don't strictly need this as we
240+
/// plan to check for file integrity anyway; see
241+
/// <https://github.com/paritytech/polkadot-sdk/issues/677>.
233242
///
234243
/// # Determinism
235244
///
@@ -335,7 +344,7 @@ pub mod landlock {
335344
A: Into<BitFlags<AccessFs>>,
336345
{
337346
let mut ruleset =
338-
Ruleset::new().handle_access(AccessFs::from_all(LANDLOCK_ABI))?.create()?;
347+
Ruleset::default().handle_access(AccessFs::from_all(LANDLOCK_ABI))?.create()?;
339348
for (fs_path, access_bits) in fs_exceptions {
340349
let paths = &[fs_path.as_ref().to_owned()];
341350
let mut rules = path_beneath_rules(paths, access_bits).peekable();
@@ -466,5 +475,38 @@ pub mod landlock {
466475

467476
assert!(handle.join().is_ok());
468477
}
478+
479+
// Test that checks whether landlock under our ABI version is able to truncate files.
480+
#[test]
481+
fn restricted_thread_can_truncate_file() {
482+
// TODO: This would be nice: <https://github.com/rust-lang/rust/issues/68007>.
483+
if !check_is_fully_enabled() {
484+
return
485+
}
486+
487+
// Restricted thread can truncate file.
488+
let handle =
489+
thread::spawn(|| {
490+
// Create and write a file. This should succeed before any landlock
491+
// restrictions are applied.
492+
const TEXT: &str = "foo";
493+
let tmpfile = tempfile::NamedTempFile::new().unwrap();
494+
let path = tmpfile.path();
495+
496+
fs::write(path, TEXT).unwrap();
497+
498+
// Apply Landlock with all exceptions under the current ABI.
499+
let status = try_restrict(vec![(path, AccessFs::from_all(LANDLOCK_ABI))]);
500+
if !matches!(status, Ok(RulesetStatus::FullyEnforced)) {
501+
panic!("Ruleset should be enforced since we checked if landlock is enabled: {:?}", status);
502+
}
503+
504+
// Try to truncate the file.
505+
let result = tmpfile.as_file().set_len(0);
506+
assert!(result.is_ok());
507+
});
508+
509+
assert!(handle.join().is_ok());
510+
}
469511
}
470512
}

0 commit comments

Comments
 (0)