Skip to content

Commit 1c7c9d7

Browse files
authored
fix(fspy): improve seccomp_unotify ipc (#255)
# Fix seccomp filter IPC and add static executable support This PR improves the seccomp filter IPC mechanism by using path-based Unix domain sockets instead of directly passing file descriptors. This approach is consistent with #234 and eliminates the need for pre-exec handling. Key changes: - Replace direct fd passing with Unix domain socket communication - Fix handling of path overflow in CStrPtr by returning a boolean success indicator - Add support for tracking static executables with tests - Add a test binary crate that can be included as a static binary for testing - Implement proper handling of the `open` syscall on x86_64 architecture - Enable typeaware linting test on Linux The PR also includes various code improvements and cleanup, such as better error handling and more robust syscall tracking.
1 parent de47ed9 commit 1c7c9d7

21 files changed

Lines changed: 259 additions & 110 deletions

File tree

.devcontainer/devcontainer.json

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,13 @@
55
// Or use a Dockerfile or Docker Compose file. More info: https://containers.dev/guide/dockerfile
66
"image": "mcr.microsoft.com/vscode/devcontainers/base:ubuntu-22.04",
77
"updateContentCommand": {
8-
"systemPackages": "sudo apt-get update -y && sudo apt-get install -y pkg-config libssl-dev",
9-
"rustToolchain": "rustup show",
10-
"mise": "mise trust && mise install && mkdir -p ~/.config/fish && echo 'mise activate fish --shims | source' >> ~/.config/fish/config.fish"
8+
"rustToolchain": "rustup show"
119
},
1210
"containerEnv": {
1311
"CARGO_TARGET_DIR": "/tmp/target"
1412
},
1513
"features": {
1614
"ghcr.io/devcontainers/features/rust:1": {},
17-
"ghcr.io/devcontainers-extra/features/mise:1": {},
1815
"ghcr.io/devcontainers-extra/features/fish-apt-get:1": {}
1916
},
2017
"customizations": {

.github/workflows/ci.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ jobs:
4949
save-cache: ${{ github.ref_name == 'main' }}
5050
cache-key: test
5151

52+
- run: rustup target add x86_64-unknown-linux-musl
53+
if: ${{ matrix.os == 'ubuntu-latest' }}
54+
5255
- run: cargo check --all-targets --all-features
5356
env:
5457
RUSTFLAGS: '-D warnings --cfg tokio_unstable' # also update .cargo/config.toml

Cargo.lock

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

crates/fspy/Cargo.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ csv-async = { workspace = true }
5555
ctor = { workspace = true }
5656
tokio = { workspace = true, features = ["rt-multi-thread", "macros", "fs", "io-std"] }
5757

58+
[target.'cfg(all(target_os = "linux", target_arch = "aarch64"))'.dev-dependencies]
59+
fspy_test_bin = { path = "../fspy_test_bin", artifact = "bin", target = "aarch64-unknown-linux-musl" }
60+
61+
[target.'cfg(all(target_os = "linux", target_arch = "x86_64"))'.dev-dependencies]
62+
fspy_test_bin = { path = "../fspy_test_bin", artifact = "bin", target = "x86_64-unknown-linux-musl" }
63+
5864
[build-dependencies]
5965
anyhow = { workspace = true }
6066
attohttpc = { workspace = true }

crates/fspy/src/unix/mod.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,6 @@ pub(crate) async fn spawn_impl(mut command: Command) -> io::Result<TrackedChild>
8585
#[cfg(target_os = "linux")]
8686
let supervisor = supervise::<SyscallHandler>()?;
8787

88-
#[cfg(target_os = "linux")]
89-
let supervisor_pre_exec = supervisor.pre_exec;
90-
9188
let (ipc_channel_conf, ipc_receiver) = channel(SHM_CAPACITY)?;
9289

9390
let payload = Payload {
@@ -99,7 +96,7 @@ pub(crate) async fn spawn_impl(mut command: Command) -> io::Result<TrackedChild>
9996
preload_path: command.spy_inner.preload_path.clone(),
10097

10198
#[cfg(target_os = "linux")]
102-
seccomp_payload: supervisor.payload,
99+
seccomp_payload: supervisor.payload().clone(),
103100
};
104101

105102
let encoded_payload = encode_payload(payload);
@@ -120,8 +117,6 @@ pub(crate) async fn spawn_impl(mut command: Command) -> io::Result<TrackedChild>
120117

121118
unsafe {
122119
tokio_command.pre_exec(move || {
123-
#[cfg(target_os = "linux")]
124-
supervisor_pre_exec.run()?;
125120
if let Some(pre_exec) = pre_exec.as_ref() {
126121
pre_exec.run()?;
127122
}
@@ -137,7 +132,7 @@ pub(crate) async fn spawn_impl(mut command: Command) -> io::Result<TrackedChild>
137132
let arenas = std::iter::once(exec_resolve_accesses);
138133
#[cfg(target_os = "linux")]
139134
let arenas =
140-
arenas.chain(supervisor.handling_loop.await?.into_iter().map(|handler| handler.arena));
135+
arenas.chain(supervisor.stop().await?.into_iter().map(|handler| handler.arena));
141136
io::Result::Ok(arenas.collect::<Vec<_>>())
142137
};
143138

crates/fspy/src/unix/syscall_handler/mod.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,28 @@ pub struct SyscallHandler {
1616
}
1717

1818
impl SyscallHandler {
19-
fn openat(&mut self, (_, path): (Ignored, CStrPtr)) -> io::Result<()> {
19+
fn handle_open(&mut self, path: CStrPtr) -> io::Result<()> {
2020
path.read_with_buf::<PATH_MAX, _, _>(|path| {
21+
let Some(path) = path else {
22+
// Ignore paths that are too long to fit in PATH_MAX
23+
return Ok(());
24+
};
2125
self.arena
2226
.add(PathAccess { mode: AccessMode::Read, path: NativeStr::from_bytes(path) });
2327
Ok(())
2428
})?;
2529
Ok(())
2630
}
2731

32+
#[cfg(target_arch = "x86_64")]
33+
fn open(&mut self, (path,): (CStrPtr,)) -> io::Result<()> {
34+
self.handle_open(path)
35+
}
36+
37+
fn openat(&mut self, (_, path): (Ignored, CStrPtr)) -> io::Result<()> {
38+
self.handle_open(path)
39+
}
40+
2841
fn getdents64(&mut self, (fd,): (Fd,)) -> io::Result<()> {
2942
let path = fd.get_path()?;
3043
self.arena.add(PathAccess {
@@ -36,7 +49,8 @@ impl SyscallHandler {
3649
}
3750

3851
impl_handler!(
39-
SyscallHandler,
40-
openat
41-
getdents64
52+
SyscallHandler:
53+
#[cfg(target_arch = "x86_64")] open,
54+
openat,
55+
getdents64,
4256
);
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
#![cfg(target_os = "linux")]
2+
3+
use std::{
4+
fs::{self, Permissions},
5+
os::unix::fs::PermissionsExt as _,
6+
path::{Path, PathBuf},
7+
sync::LazyLock,
8+
};
9+
10+
use fspy::PathAccessIterable;
11+
use fspy_shared_unix::is_dynamically_linked_to_libc;
12+
13+
use crate::test_utils::assert_contains;
14+
15+
mod test_utils;
16+
17+
const TEST_BIN_CONTENT: &[u8] = include_bytes!(env!("CARGO_BIN_FILE_FSPY_TEST_BIN"));
18+
19+
fn test_bin_path() -> &'static Path {
20+
static TEST_BIN_PATH: LazyLock<PathBuf> = LazyLock::new(|| {
21+
assert_eq!(
22+
is_dynamically_linked_to_libc(&TEST_BIN_CONTENT),
23+
Ok(false),
24+
"Test binary is not a static executable"
25+
);
26+
27+
let tmp_dir = env!("CARGO_TARGET_TMPDIR");
28+
let test_bin_path = PathBuf::from(tmp_dir).join("fspy-test-bin");
29+
fs::write(&test_bin_path, TEST_BIN_CONTENT).expect("failed to write test binary");
30+
fs::set_permissions(&test_bin_path, Permissions::from_mode(0o755))
31+
.expect("failed to set permissions on test binary");
32+
33+
test_bin_path
34+
});
35+
TEST_BIN_PATH.as_path()
36+
}
37+
38+
async fn track_test_bin(args: &[&str]) -> PathAccessIterable {
39+
let mut cmd = fspy::Spy::global().unwrap().new_command(test_bin_path());
40+
cmd.args(args);
41+
let mut tracked_child = cmd.spawn().await.unwrap();
42+
43+
let output = tracked_child.tokio_child.wait().await.unwrap();
44+
assert!(output.success());
45+
46+
tracked_child.accesses_future.await.unwrap()
47+
}
48+
49+
#[tokio::test]
50+
async fn open_read() {
51+
let accesses = track_test_bin(&["open_read", "/hello"]).await;
52+
assert_contains(&accesses, Path::new("/hello"), fspy::AccessMode::Read);
53+
}

crates/fspy/tests/test_utils.rs

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,25 @@ pub fn assert_contains(
1111
expected_path: &Path,
1212
expected_mode: AccessMode,
1313
) {
14-
accesses
15-
.iter()
16-
.find(|access| {
17-
let Ok(stripped) =
18-
access.path.strip_path_prefix::<_, Result<PathBuf, StripPrefixError>, _>(
19-
expected_path,
20-
|strip_result| strip_result.map(Path::to_path_buf),
21-
)
22-
else {
23-
return false;
24-
};
25-
stripped.as_os_str().is_empty() && access.mode == expected_mode
26-
})
27-
.unwrap();
14+
let found = accesses.iter().any(|access| {
15+
let Ok(stripped) =
16+
access.path.strip_path_prefix::<_, Result<PathBuf, StripPrefixError>, _>(
17+
expected_path,
18+
|strip_result| strip_result.map(Path::to_path_buf),
19+
)
20+
else {
21+
return false;
22+
};
23+
stripped.as_os_str().is_empty() && access.mode == expected_mode
24+
});
25+
if !found {
26+
panic!(
27+
"Expected to find access to path {:?} with mode {:?}, but it was not found in: {:?}",
28+
expected_path,
29+
expected_mode,
30+
accesses.iter().collect::<Vec<_>>()
31+
);
32+
}
2833
}
2934

3035
#[macro_export]

crates/fspy_seccomp_unotify/Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ nix = { workspace = true, features = ["process", "fs", "poll", "socket", "uio"]
1313
passfd = { workspace = true, default-features = false, optional = true }
1414
seccompiler = { workspace = true }
1515
syscalls = { workspace = true, features = ["std"] }
16-
tokio = { workspace = true, features = ["net", "process", "io-util", "rt"] }
16+
tokio = { workspace = true, features = ["net", "process", "io-util", "rt", "sync"] }
1717
tracing = { workspace = true }
18+
tempfile = { workspace = true }
19+
futures-util = { workspace = true }
1820

1921
[target.'cfg(target_os = "linux")'.dev-dependencies]
2022
assertables = { workspace = true }
Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
use std::os::fd::RawFd;
21
mod filter;
32
use bincode::{Decode, Encode};
43
pub use filter::Filter;
54

65
#[derive(Debug, Encode, Decode, Clone)]
76
pub struct SeccompPayload {
8-
pub(crate) ipc_fd: RawFd,
7+
pub(crate) ipc_path: Vec<u8>,
98
pub(crate) filter: Filter,
109
}

0 commit comments

Comments
 (0)