Skip to content

Commit ec94072

Browse files
committed
Harden filesystem analysis boundaries
1 parent 7416b98 commit ec94072

5 files changed

Lines changed: 39 additions & 21 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ behavior when the change improves security or project direction.
8989
- Fail malformed `Accept-Encoding` negotiation closed, honor explicit coding
9090
rejection over wildcard acceptance, and treat qualified `private` cache
9191
directives as compression-blocking security policy.
92+
- Move config trust traversal to no-follow `statat` inspection and remove the
93+
environment-derived marker-file write from storage-lease subprocess tests.
9294

9395
### Fixed
9496

crates/fluxheim-config/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ fluxheim-protocol = { path = "../fluxheim-protocol" }
3434
fluxheim-wasm = { path = "../fluxheim-wasm", optional = true }
3535
log = "0.4.33"
3636
regex = "1.13.0"
37-
rustix = { version = "1.1.4", features = ["process"] }
37+
rustix = { version = "1.1.4", features = ["fs", "process"] }
3838
serde = { version = "1.0.228", features = ["derive"] }
3939
toml = "1.1.2"
4040

crates/fluxheim-config/src/fs_trust.rs

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ fn existing_path_has_insecure_write_permissions(current: &Path) -> std::io::Resu
3737
let mut inspected_depth = 0usize;
3838
loop {
3939
check_inspection_depth(&mut inspected_depth)?;
40-
match std::fs::symlink_metadata(&current) {
40+
match path_stat_no_follow(&current) {
4141
Ok(_) => break,
4242
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
4343
if !current.pop() {
@@ -51,11 +51,13 @@ fn existing_path_has_insecure_write_permissions(current: &Path) -> std::io::Resu
5151
}
5252
}
5353

54+
let process_uid = rustix::process::geteuid().as_raw();
55+
let root_uid = path_stat_no_follow(Path::new("/"))?.st_uid;
5456
loop {
5557
check_inspection_depth(&mut inspected_depth)?;
56-
let metadata = std::fs::symlink_metadata(&current)?;
57-
if metadata.file_type().is_symlink()
58-
|| metadata_has_insecure_owner_or_write_permissions(&metadata)?
58+
let stat = path_stat_no_follow(&current)?;
59+
if rustix::fs::FileType::from_raw_mode(stat.st_mode).is_symlink()
60+
|| stat_has_insecure_owner_or_write_permissions(&stat, process_uid, root_uid)
5961
{
6062
return Ok(true);
6163
}
@@ -65,14 +67,30 @@ fn existing_path_has_insecure_write_permissions(current: &Path) -> std::io::Resu
6567
}
6668
}
6769

70+
#[cfg(unix)]
71+
fn path_stat_no_follow(path: &Path) -> std::io::Result<rustix::fs::Stat> {
72+
rustix::fs::statat(rustix::fs::CWD, path, rustix::fs::AtFlags::SYMLINK_NOFOLLOW)
73+
.map_err(std::io::Error::from)
74+
}
75+
76+
#[cfg(unix)]
77+
fn stat_has_insecure_owner_or_write_permissions(
78+
stat: &rustix::fs::Stat,
79+
process_uid: u32,
80+
root_uid: u32,
81+
) -> bool {
82+
(stat.st_uid != 0 && stat.st_uid != process_uid && stat.st_uid != root_uid)
83+
|| stat.st_mode & 0o022 != 0
84+
}
85+
6886
#[cfg(unix)]
6987
pub(crate) fn metadata_has_insecure_owner_or_write_permissions(
7088
metadata: &std::fs::Metadata,
7189
) -> std::io::Result<bool> {
7290
use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _};
7391

7492
let process_uid = rustix::process::geteuid().as_raw();
75-
let root_uid = std::fs::symlink_metadata(Path::new("/"))?.uid();
93+
let root_uid = path_stat_no_follow(Path::new("/"))?.st_uid;
7694
Ok(
7795
(metadata.uid() != 0 && metadata.uid() != process_uid && metadata.uid() != root_uid)
7896
|| metadata.permissions().mode() & 0o022 != 0,

crates/fluxheim-server/src/native_http1_cache_tests.rs

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -322,50 +322,46 @@ fn live_cache_inspection_uses_registered_allocator_during_inserts() {
322322
}
323323

324324
const STORAGE_BIN_LEASE_CHILD_ROOT: &str = "FLUXHEIM_STORAGE_BIN_LEASE_CHILD_ROOT";
325-
const STORAGE_BIN_LEASE_CHILD_MARKER: &str = "FLUXHEIM_STORAGE_BIN_LEASE_CHILD_MARKER";
326325
const STORAGE_BIN_LEASE_CHILD_MAX_BYTES: &str = "FLUXHEIM_STORAGE_BIN_LEASE_CHILD_MAX_BYTES";
326+
const STORAGE_BIN_LEASE_CHILD_CONFIRMED: &str = "fluxheim-storage-bin-lease-child-confirmed";
327327

328328
#[test]
329329
fn storage_bin_lease_child_process() {
330330
let Some(root) = std::env::var_os(STORAGE_BIN_LEASE_CHILD_ROOT) else {
331331
return;
332332
};
333-
let marker = std::env::var_os(STORAGE_BIN_LEASE_CHILD_MARKER).unwrap();
334333
let mut config = storage_bin_config(std::path::Path::new(&root));
335334
if let Ok(max_bytes) = std::env::var(STORAGE_BIN_LEASE_CHILD_MAX_BYTES) {
336335
config.disk.max_size_bytes = ByteSize::from_bytes(max_bytes.parse().unwrap());
337336
}
338337

339338
let error = NativeDiskCacheBackend::from_config(&config).unwrap_err();
340339
assert_eq!(error.kind(), std::io::ErrorKind::AlreadyExists);
341-
std::fs::write(marker, b"locked").unwrap();
340+
println!("{STORAGE_BIN_LEASE_CHILD_CONFIRMED}");
342341
}
343342

344343
#[test]
345344
fn storage_bin_lease_rejects_second_process() {
346345
let directory = tempfile::tempdir().unwrap();
347346
let root = directory.path().join("cache");
348-
let marker = directory.path().join("child-confirmed");
349347
let config = storage_bin_config(&root);
350348
let _cache = NativeDiskCache::from_config(&config).unwrap();
351-
let status = std::process::Command::new(std::env::current_exe().unwrap())
349+
let output = std::process::Command::new(std::env::current_exe().unwrap())
352350
.arg("--exact")
353351
.arg("native_http1_cache::tests::storage_bin_lease_child_process")
354352
.arg("--nocapture")
355353
.env(STORAGE_BIN_LEASE_CHILD_ROOT, &root)
356-
.env(STORAGE_BIN_LEASE_CHILD_MARKER, &marker)
357-
.status()
354+
.output()
358355
.unwrap();
359356

360-
assert!(status.success());
361-
assert_eq!(std::fs::read(marker).unwrap(), b"locked");
357+
assert!(output.status.success());
358+
assert!(String::from_utf8_lossy(&output.stdout).contains(STORAGE_BIN_LEASE_CHILD_CONFIRMED));
362359
}
363360

364361
#[test]
365362
fn storage_bin_loser_cannot_mutate_winner_layout() {
366363
let directory = tempfile::tempdir().unwrap();
367364
let configured_root = directory.path().join("cache");
368-
let marker = directory.path().join("child-confirmed");
369365
let winner_config = storage_bin_config(&configured_root);
370366
let root = super::prepare_native_storage_bin_root_for_lease(&winner_config).unwrap();
371367
let _lease = acquire_native_storage_bin_lease(&root).unwrap();
@@ -376,20 +372,19 @@ fn storage_bin_loser_cannot_mutate_winner_layout() {
376372
let manifest_path = root.join(".fluxheim-storage-bin-v1");
377373
let winning_manifest = std::fs::read(&manifest_path).unwrap();
378374

379-
let status = std::process::Command::new(std::env::current_exe().unwrap())
375+
let output = std::process::Command::new(std::env::current_exe().unwrap())
380376
.arg("--exact")
381377
.arg("native_http1_cache::tests::storage_bin_lease_child_process")
382378
.arg("--nocapture")
383379
.env(STORAGE_BIN_LEASE_CHILD_ROOT, &root)
384-
.env(STORAGE_BIN_LEASE_CHILD_MARKER, &marker)
385380
.env(
386381
STORAGE_BIN_LEASE_CHILD_MAX_BYTES,
387382
(2 * 1024 * 1024).to_string(),
388383
)
389-
.status()
384+
.output()
390385
.unwrap();
391386

392-
assert!(status.success());
393-
assert_eq!(std::fs::read(marker).unwrap(), b"locked");
387+
assert!(output.status.success());
388+
assert!(String::from_utf8_lossy(&output.stdout).contains(STORAGE_BIN_LEASE_CHILD_CONFIRMED));
394389
assert_eq!(std::fs::read(manifest_path).unwrap(), winning_manifest);
395390
}

release-notes/RELEASE_NOTES_1.7.7.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ deterministic unsupported-call rejection.
100100
- Fail response compression closed for malformed `Accept-Encoding` fields,
101101
honor explicit coding rejection over wildcard acceptance, and suppress
102102
compression for qualified `Cache-Control: private="..."` responses.
103+
- Perform config ownership, permission, and symlink traversal checks through
104+
no-follow `statat` metadata inspection, and remove environment-derived
105+
filesystem writes from storage-lease subprocess coverage.
103106

104107
## Changed
105108

0 commit comments

Comments
 (0)