diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 205b6339..0fdbe97f 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -1,6 +1,10 @@ #[cfg(test)] use std::time::{SystemTime, UNIX_EPOCH}; -use std::{ffi::CStr, os::raw::c_char, path::PathBuf}; +use std::{ + ffi::{CStr, OsStr}, + os::{raw::c_char, unix::ffi::OsStrExt}, + path::{Path, PathBuf}, +}; use serde::Serialize; @@ -15,6 +19,31 @@ fn slice_to_string(s: &[c_char]) -> anyhow::Result { Ok(unsafe { CStr::from_ptr(s.as_ptr()) }.to_str()?.to_owned()) } +/// Parse a buffer obtained from calling d_path kernel side. +/// +/// Parsing this type of buffer is a special case, because the kernel +/// may append " (deleted)" to a path when the file has been removed and +/// can mess with the event we report. This method will take a slice of +/// c_char and return a PathBuf with the " (deleted)" portion removed. +/// +/// With the current implementation, non UTF-8 characters in the file +/// name will be replaced with the U+FFFD character. +fn parse_d_path(s: &[c_char]) -> PathBuf { + let s = unsafe { CStr::from_ptr(s.as_ptr()) }; + let p = Path::new(OsStr::from_bytes(s.to_bytes())); + + // Take the file name of the path and remove the " (deleted)" suffix + // if present. + if let Some(file_name) = p.file_name() { + if let Some(file_name) = file_name.to_string_lossy().strip_suffix(" (deleted)") { + // The file name needed to be sanitized + return p.parent().map(|p| p.join(file_name)).unwrap_or_default(); + } + } + + p.to_path_buf() +} + fn timestamp_to_proto(ts: u64) -> prost_types::Timestamp { let seconds = (ts / 1_000_000_000) as i64; let nanos = (ts % 1_000_000_000) as i32; @@ -186,10 +215,8 @@ pub struct BaseFileData { impl BaseFileData { pub fn new(filename: [c_char; PATH_MAX as usize], inode: inode_key_t) -> anyhow::Result { - let filename = slice_to_string(&filename)?.into(); - Ok(BaseFileData { - filename, + filename: parse_d_path(&filename), host_file: PathBuf::new(), // this field is set by HostScanner inode, }) diff --git a/fact/src/event/process.rs b/fact/src/event/process.rs index aeb8ea5c..f3932675 100644 --- a/fact/src/event/process.rs +++ b/fact/src/event/process.rs @@ -1,4 +1,4 @@ -use std::ffi::CStr; +use std::{ffi::CStr, path::PathBuf}; use fact_ebpf::{lineage_t, process_t}; use serde::Serialize; @@ -6,21 +6,12 @@ use uuid::Uuid; use crate::host_info; -use super::slice_to_string; +use super::{parse_d_path, slice_to_string}; #[derive(Debug, Clone, Default, Serialize)] pub struct Lineage { uid: u32, - exe_path: String, -} - -impl Lineage { - fn new(uid: u32, exe_path: &str) -> Self { - Lineage { - uid, - exe_path: exe_path.to_owned(), - } - } + exe_path: PathBuf, } impl TryFrom<&lineage_t> for Lineage { @@ -28,9 +19,12 @@ impl TryFrom<&lineage_t> for Lineage { fn try_from(value: &lineage_t) -> Result { let lineage_t { uid, exe_path } = value; - let exe_path = unsafe { CStr::from_ptr(exe_path.as_ptr()) }.to_str()?; + let exe_path = parse_d_path(exe_path); - Ok(Lineage::new(*uid, exe_path)) + Ok(Lineage { + uid: *uid, + exe_path, + }) } } @@ -39,7 +33,7 @@ impl From for fact_api::process_signal::LineageInfo { let Lineage { uid, exe_path } = value; Self { parent_uid: uid, - parent_exec_file_path: exe_path, + parent_exec_file_path: exe_path.to_string_lossy().to_string(), } } } @@ -48,7 +42,7 @@ impl From for fact_api::process_signal::LineageInfo { pub struct Process { comm: String, args: Vec, - exe_path: String, + exe_path: PathBuf, container_id: Option, uid: u32, username: &'static str, @@ -66,11 +60,7 @@ impl Process { pub fn current() -> Self { use crate::host_info::{get_host_mount_ns, get_mount_ns}; - let exe_path = std::env::current_exe() - .expect("Failed to get current exe") - .into_os_string() - .into_string() - .unwrap(); + let exe_path = std::env::current_exe().expect("Failed to get current exe"); let args = std::env::args().collect::>(); let cgroup = std::fs::read_to_string("/proc/self/cgroup").expect("Failed to read cgroup"); let container_id = Process::extract_container_id(&cgroup); @@ -142,7 +132,7 @@ impl TryFrom for Process { fn try_from(value: process_t) -> Result { let comm = slice_to_string(value.comm.as_slice())?; - let exe_path = slice_to_string(value.exe_path.as_slice())?; + let exe_path = parse_d_path(value.exe_path.as_slice()); let memory_cgroup = unsafe { CStr::from_ptr(value.memory_cgroup.as_ptr()) }.to_str()?; let container_id = Process::extract_container_id(memory_cgroup); let in_root_mount_ns = value.in_root_mount_ns != 0; @@ -213,7 +203,7 @@ impl From for fact_api::ProcessSignal { creation_time: None, name: comm, args, - exec_file_path: exe_path, + exec_file_path: exe_path.to_string_lossy().to_string(), pid, uid, gid, diff --git a/tests/containers/self-deleter/Cargo.lock b/tests/containers/self-deleter/Cargo.lock new file mode 100644 index 00000000..bf154988 --- /dev/null +++ b/tests/containers/self-deleter/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "self-deleter" +version = "0.1.0" diff --git a/tests/containers/self-deleter/Cargo.toml b/tests/containers/self-deleter/Cargo.toml new file mode 100644 index 00000000..e238ba72 --- /dev/null +++ b/tests/containers/self-deleter/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "self-deleter" +version = "0.1.0" +edition = "2021" + +[dependencies] + +[workspace] diff --git a/tests/containers/self-deleter/Containerfile b/tests/containers/self-deleter/Containerfile new file mode 100644 index 00000000..5f9a0fa1 --- /dev/null +++ b/tests/containers/self-deleter/Containerfile @@ -0,0 +1,12 @@ +FROM rust:1.84-alpine AS builder + +WORKDIR /app + +COPY . . +RUN cargo build --release + +FROM alpine:3.23 + +COPY --from=builder /app/target/release/self-deleter /usr/local/bin + +ENTRYPOINT ["self-deleter"] diff --git a/tests/containers/self-deleter/src/main.rs b/tests/containers/self-deleter/src/main.rs new file mode 100644 index 00000000..657e6814 --- /dev/null +++ b/tests/containers/self-deleter/src/main.rs @@ -0,0 +1,18 @@ +use std::{ + fs::{remove_file, File}, + io::Write, +}; + +fn main() { + let exe_path = std::env::current_exe().expect("Failed to get executable path"); + println!("Removing executable: {}", exe_path.display()); + remove_file(exe_path).expect("Failed to remove executable"); + + let mut args = std::env::args(); + let path = args.nth(1).expect("File to modify not provided"); + + println!("Opening file: {path}"); + let mut f = File::create(path).expect("Failed to create test file"); + f.write_all(b"This is a test") + .expect("Failed to write to test file"); +} diff --git a/tests/test_misc.py b/tests/test_misc.py new file mode 100644 index 00000000..71c662dd --- /dev/null +++ b/tests/test_misc.py @@ -0,0 +1,67 @@ +import os + +from conftest import dump_logs +from event import Event, EventType, Process + +import pytest + + +@pytest.fixture +def build_self_deleter(docker_client): + image, _ = docker_client.images.build( + path='containers/self-deleter', + tag='self-deleter:latest', + dockerfile='Containerfile' + ) + return image + + +@pytest.fixture +def run_self_deleter(fact, monitored_dir, logs_dir, docker_client, build_self_deleter): + image = build_self_deleter.tags[0] + container = docker_client.containers.run( + image, + '/mounted/test.txt', + detach=True, + volumes={ + monitored_dir: { + 'bind': '/mounted', + 'mode': 'z', + }, + }, + name='self-deleter', + ) + + yield container + + container_log = os.path.join(logs_dir, 'self-deleter.log') + container.stop(timeout=1) + dump_logs(container, container_log) + container.remove() + + +def test_d_path_sanitization(fact, monitored_dir, server, run_self_deleter, docker_client): + """ + Ensure the sanitization of paths obtained by calling the bpf_d_path + helper don't include the " (deleted)" suffix when the file is + removed. + """ + # File Under Test + fut = '/mounted/test.txt' + host_path = os.path.join(monitored_dir, 'test.txt') + + container = run_self_deleter + + process = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/local/bin/self-deleter', + args=f'self-deleter {fut}', + name='self-deleter', + container_id=container.id[:12], + loginuid=pow(2, 32)-1) + event = Event(process=process, event_type=EventType.OPEN, + file=fut, host_path=host_path) + print(f'Waiting for event: {event}') + + server.wait_events([event])