diff --git a/CHANGELOG.md b/CHANGELOG.md index 9807023f..4fc8737b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ possible include a PR number for easier tracking. ## Next +* ROX-33036: add mount-related operations (#1059) * feat(endpoints): add inodes introspection endpoint (#1273) * feat: add --replay mode for JSONL event replay without eBPF (#1010) * feat(config): configurable loaded BPF programs (#1086) diff --git a/fact-ebpf/src/bpf/bound_path.h b/fact-ebpf/src/bpf/bound_path.h index 7a2091f9..cf249b68 100644 --- a/fact-ebpf/src/bpf/bound_path.h +++ b/fact-ebpf/src/bpf/bound_path.h @@ -38,8 +38,12 @@ __always_inline static struct bound_path_t* _path_read(struct path* path, bound_ return bound_path; } -__always_inline static struct bound_path_t* path_read_unchecked(struct path* path) { - return _path_read(path, BOUND_PATH_MAIN, true); +__always_inline static struct bound_path_t* path_read_unchecked(struct path* path, bool use_bpf_d_path) { + return _path_read(path, BOUND_PATH_MAIN, use_bpf_d_path); +} + +__always_inline static struct bound_path_t* path_read_alt_unchecked(struct path* path, bool use_bpf_d_path) { + return _path_read(path, BOUND_PATH_ALTERNATE, use_bpf_d_path); } __always_inline static struct bound_path_t* path_read(struct path* path) { diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 00d91027..249daf53 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -123,9 +123,9 @@ __always_inline static void submit_rename_event(struct submit_event_args_t* args } args->event->type = FILE_ACTIVITY_RENAME; - bpf_probe_read_str(args->event->rename.filename, PATH_MAX, old_filename); - inode_copy(&args->event->rename.inode, old_inode); - args->event->rename.monitored = old_monitored; + bpf_probe_read_str(args->event->from.filename, PATH_MAX, old_filename); + inode_copy(&args->event->from.inode, old_inode); + args->event->from.monitored = old_monitored; __submit_event(args, path_hooks_support_bpf_d_path); } @@ -212,3 +212,36 @@ __always_inline static void submit_acl_event(struct submit_event_args_t* args, // inode_set_acl does not support bpf_d_path (no struct path available) __submit_event(args, false); } + +__always_inline static void submit_mount_event(struct submit_event_args_t* args) { + if (!reserve_event(args)) { + return; + } + args->event->type = FILE_ACTIVITY_MOUNT; + + __submit_event(args, true); +} + +__always_inline static void submit_umount_event(struct submit_event_args_t* args) { + if (!reserve_event(args)) { + return; + } + args->event->type = FILE_ACTIVITY_UMOUNT; + + __submit_event(args, true); +} + +__always_inline static void submit_move_mount_event(struct submit_event_args_t* args, + const char from_filename[PATH_MAX], + inode_key_t* from_inode, + monitored_t from_monitored) { + if (!reserve_event(args)) { + return; + } + args->event->type = FILE_ACTIVITY_MOVE_MOUNT; + bpf_probe_read_str(args->event->from.filename, PATH_MAX, from_filename); + inode_copy(&args->event->from.inode, from_inode); + args->event->from.monitored = from_monitored; + + __submit_event(args, false); +} diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index b880a573..12d5e86b 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -54,7 +54,7 @@ int BPF_PROG(trace_file_open, struct file* file) { } } - struct bound_path_t* path = path_read_unchecked(&file->f_path); + struct bound_path_t* path = path_read_unchecked(&file->f_path, true); if (path == NULL) { bpf_printk("Failed to read path"); m->file_open.error++; @@ -484,3 +484,120 @@ int BPF_PROG(trace_path_rmdir, struct path* dir, struct dentry* dentry) { submit_rmdir_event(&args); return 0; } + +SEC("lsm/sb_mount") +int BPF_PROG(trace_sb_mount, const char* dev_name, struct path* path, const char* type, unsigned long flags, void* data) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + struct submit_event_args_t args = {.metrics = &m->sb_mount}; + args.metrics->total++; + + struct bound_path_t* bound_path = path_read_unchecked(path, true); + if (bound_path == NULL) { + bpf_printk("Failed to read mount directory"); + args.metrics->error++; + return 0; + } + args.filename = bound_path->path; + + args.inode = inode_to_key(path->dentry->d_inode); + + struct dentry* parent_dentry = BPF_CORE_READ(path, dentry, d_parent); + struct inode* parent_inode_ptr = parent_dentry ? BPF_CORE_READ(parent_dentry, d_inode) : NULL; + args.parent_inode = inode_to_key(parent_inode_ptr); + + args.monitored = is_monitored(&args.inode, bound_path, &args.parent_inode); + if (args.monitored == NOT_MONITORED) { + args.metrics->ignored++; + return 0; + } + + submit_mount_event(&args); + + return 0; +} + +SEC("lsm/sb_umount") +int BPF_PROG(trace_sb_umount, struct vfsmount* mnt, int flags) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + struct submit_event_args_t args = {.metrics = &m->sb_umount}; + args.metrics->total++; + + struct path p = {.dentry = BPF_CORE_READ(mnt, mnt_root), .mnt = mnt}; + struct bound_path_t* bound_path = _path_read(&p, BOUND_PATH_MAIN, false); + if (bound_path == NULL) { + bpf_printk("Failed to read umount directory"); + args.metrics->error++; + return 0; + } + args.filename = bound_path->path; + + args.inode = inode_to_key(mnt->mnt_root->d_inode); + + struct dentry* parent_dentry = BPF_CORE_READ(mnt, mnt_root, d_parent); + struct inode* parent_inode_ptr = parent_dentry ? BPF_CORE_READ(parent_dentry, d_inode) : NULL; + args.parent_inode = inode_to_key(parent_inode_ptr); + + args.monitored = is_monitored(&args.inode, bound_path, &args.parent_inode); + if (args.monitored == NOT_MONITORED) { + args.metrics->ignored++; + return 0; + } + + submit_umount_event(&args); + + return 0; +} + +SEC("lsm/move_mount") +int BPF_PROG(trace_move_mount, struct path* from, struct path* to) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + struct submit_event_args_t args = {.metrics = &m->move_mount}; + + args.metrics->total++; + + struct bound_path_t* to_path = path_read_unchecked(to, false); + if (to_path == NULL) { + bpf_printk("Failed to read to_path"); + goto error; + } + args.filename = to_path->path; + + struct bound_path_t* from_path = path_read_alt_unchecked(from, false); + if (from_path == NULL) { + bpf_printk("Failed to read from_path"); + goto error; + } + + args.inode = inode_to_key(to->dentry->d_inode); + args.parent_inode = inode_to_key(to->dentry->d_inode); + args.monitored = is_monitored(&args.inode, to_path, &args.parent_inode); + + inode_key_t from_inode = inode_to_key(from->dentry->d_inode); + monitored_t from_monitored = is_monitored(&from_inode, from_path, NULL); + + if (args.monitored != MONITORED_BY_INODE) { + args.metrics->ignored++; + return 0; + } + + // Ensure the new mount is tracked. + if (from_monitored != MONITORED_BY_INODE) { + inode_add(&from_inode); + } + + submit_move_mount_event(&args, from_path->path, &from_inode, from_monitored); + return 0; + +error: + args.metrics->error++; + return 0; +} diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index ce496672..80b94d22 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -115,6 +115,9 @@ typedef enum file_activity_type_t { FILE_ACTIVITY_SETXATTR, FILE_ACTIVITY_REMOVEXATTR, FILE_ACTIVITY_ACL_SET, + FILE_ACTIVITY_MOUNT, + FILE_ACTIVITY_UMOUNT, + FILE_ACTIVITY_MOVE_MOUNT, } file_activity_type_t; struct event_t { @@ -140,7 +143,7 @@ struct event_t { char filename[PATH_MAX]; inode_key_t inode; monitored_t monitored; - } rename; + } from; // Used by events that have two paths (like rename or move_mount). struct { char name[XATTR_NAME_MAX_LEN]; } xattr; @@ -194,4 +197,7 @@ struct metrics_t { struct metrics_by_hook_t inode_setxattr; struct metrics_by_hook_t inode_removexattr; struct metrics_by_hook_t inode_set_acl; + struct metrics_by_hook_t sb_mount; + struct metrics_by_hook_t sb_umount; + struct metrics_by_hook_t move_mount; }; diff --git a/fact-ebpf/src/lib.rs b/fact-ebpf/src/lib.rs index c1390cac..401cb5a3 100644 --- a/fact-ebpf/src/lib.rs +++ b/fact-ebpf/src/lib.rs @@ -226,6 +226,9 @@ impl_metrics_t!( inode_setxattr, inode_removexattr, inode_set_acl, + sb_mount, + sb_umount, + move_mount, ); unsafe impl Pod for metrics_t {} diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 01ce7c2c..3d7b629c 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -155,6 +155,13 @@ impl Event { matches!(self.file, FileData::Rename { .. }) } + pub fn is_mount_related(&self) -> bool { + matches!( + self.file, + FileData::Mount(_) | FileData::Umount(_) | FileData::MoveMount { .. } + ) + } + /// Unwrap the inner FileData and return the inode that triggered /// the event. /// @@ -170,6 +177,9 @@ impl Event { | FileData::Chmod(ChmodFileData { inner, .. }) | FileData::Chown(ChownFileData { inner, .. }) | FileData::Rename { new: inner, .. } + | FileData::MoveMount { to: inner, .. } + | FileData::Mount(inner) + | FileData::Umount(inner) | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.inode, @@ -187,6 +197,9 @@ impl Event { | FileData::Chmod(ChmodFileData { inner, .. }) | FileData::Chown(ChownFileData { inner, .. }) | FileData::Rename { new: inner, .. } + | FileData::MoveMount { to: inner, .. } + | FileData::Mount(inner) + | FileData::Umount(inner) | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.parent_inode, @@ -198,7 +211,9 @@ impl Event { /// will be returned. pub fn get_old_inode(&self) -> Option<&inode_key_t> { match &self.file { - FileData::Rename { old, .. } => Some(&old.inode), + FileData::Rename { old: from, .. } | FileData::MoveMount { from, .. } => { + Some(&from.inode) + } _ => None, } } @@ -213,6 +228,9 @@ impl Event { | FileData::Chmod(ChmodFileData { inner, .. }) | FileData::Chown(ChownFileData { inner, .. }) | FileData::Rename { new: inner, .. } + | FileData::MoveMount { to: inner, .. } + | FileData::Mount(inner) + | FileData::Umount(inner) | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.filename, @@ -221,7 +239,9 @@ impl Event { pub fn get_old_filename(&self) -> Option<&PathBuf> { match &self.file { - FileData::Rename { old, .. } => Some(&old.filename), + FileData::Rename { old: from, .. } | FileData::MoveMount { from, .. } => { + Some(&from.filename) + } _ => None, } } @@ -236,6 +256,9 @@ impl Event { | FileData::Chmod(ChmodFileData { inner, .. }) | FileData::Chown(ChownFileData { inner, .. }) | FileData::Rename { new: inner, .. } + | FileData::MoveMount { to: inner, .. } + | FileData::Mount(inner) + | FileData::Umount(inner) | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.host_file, @@ -244,7 +267,9 @@ impl Event { pub fn get_old_host_path(&self) -> Option<&PathBuf> { match &self.file { - FileData::Rename { old, .. } => Some(&old.host_file), + FileData::Rename { old: from, .. } | FileData::MoveMount { from, .. } => { + Some(&from.host_file) + } _ => None, } } @@ -263,6 +288,9 @@ impl Event { | FileData::Chmod(ChmodFileData { inner, .. }) | FileData::Chown(ChownFileData { inner, .. }) | FileData::Rename { new: inner, .. } + | FileData::MoveMount { to: inner, .. } + | FileData::Mount(inner) + | FileData::Umount(inner) | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => inner.host_file = host_path, @@ -272,8 +300,11 @@ impl Event { /// Same as `set_host_path` but setting the 'old' host_file for /// operations that have one, like rename. pub fn set_old_host_path(&mut self, host_path: PathBuf) { - if let FileData::Rename { old, .. } = &mut self.file { - old.host_file = host_path + match &mut self.file { + FileData::Rename { old: from, .. } | FileData::MoveMount { from, .. } => { + from.host_file = host_path + } + _ => unreachable!("Called set_old_host_path on invalid type"), } } @@ -287,6 +318,9 @@ impl Event { | FileData::Chmod(ChmodFileData { inner, .. }) | FileData::Chown(ChownFileData { inner, .. }) | FileData::Rename { new: inner, .. } + | FileData::MoveMount { to: inner, .. } + | FileData::Mount(inner) + | FileData::Umount(inner) | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => inner.monitored, @@ -295,7 +329,9 @@ impl Event { pub fn get_old_monitored(&self) -> Option { match &self.file { - FileData::Rename { old, .. } => Some(old.monitored), + FileData::Rename { old: from, .. } | FileData::MoveMount { from, .. } => { + Some(from.monitored) + } _ => None, } } @@ -405,6 +441,12 @@ pub enum FileData { SetXattr(XattrFileData), RemoveXattr(XattrFileData), AclSet(AclSetFileData), + Mount(BaseFileData), + MoveMount { + to: BaseFileData, + from: BaseFileData, + }, + Umount(BaseFileData), } impl FileData { @@ -416,7 +458,15 @@ impl FileData { monitored: monitored_t, extra_data: fact_ebpf::event_t__bindgen_ty_1, ) -> anyhow::Result { - let inner = BaseFileData::new(filename, inode, parent_inode, monitored)?; + fn read_from_data(extra_data: fact_ebpf::event_t__bindgen_ty_1) -> BaseFileData { + let filename = unsafe { extra_data.from.filename }; + let inode = unsafe { extra_data.from.inode }; + let monitored = unsafe { extra_data.from.monitored }; + + BaseFileData::new(filename, inode, Default::default(), monitored) + } + + let inner = BaseFileData::new(filename, inode, parent_inode, monitored); let file = match event_type { file_activity_type_t::FILE_ACTIVITY_OPEN => FileData::Open(inner), file_activity_type_t::FILE_ACTIVITY_CREATION => FileData::Creation(inner), @@ -442,18 +492,8 @@ impl FileData { FileData::Chown(data) } file_activity_type_t::FILE_ACTIVITY_RENAME => { - let old_filename = unsafe { extra_data.rename.filename }; - let old_inode = unsafe { extra_data.rename.inode }; - let old_monitored = unsafe { extra_data.rename.monitored }; - FileData::Rename { - new: inner, - old: BaseFileData::new( - old_filename, - old_inode, - Default::default(), - old_monitored, - )?, - } + let old = read_from_data(extra_data); + FileData::Rename { new: inner, old } } file_activity_type_t::FILE_ACTIVITY_SETXATTR => { let xattr_name = slice_to_string( @@ -482,6 +522,12 @@ impl FileData { entries, }) } + file_activity_type_t::FILE_ACTIVITY_MOUNT => FileData::Mount(inner), + file_activity_type_t::FILE_ACTIVITY_UMOUNT => FileData::Umount(inner), + file_activity_type_t::FILE_ACTIVITY_MOVE_MOUNT => { + let from = read_from_data(extra_data); + FileData::MoveMount { to: inner, from } + } invalid => unreachable!("Invalid event type: {invalid:?}"), }; @@ -502,6 +548,9 @@ impl FileData { FileData::SetXattr(_) => "xattr_set", FileData::RemoveXattr(_) => "xattr_remove", FileData::AclSet(_) => "acl", + FileData::Mount(_) => "mount", + FileData::MoveMount { .. } => "move_mount", + FileData::Umount(_) => "umount", } } } @@ -557,6 +606,15 @@ impl From for fact_api::file_activity::File { let f_act = fact_api::FileAclChange::from(event); fact_api::file_activity::File::Acl(f_act) } + FileData::Mount(_) => { + unreachable!("Mount event reached protobuf conversion"); + } + FileData::MoveMount { .. } => { + unreachable!("MoveMount event reached protobuf conversion"); + } + FileData::Umount(_) => { + unreachable!("Umount event reached protobuf conversion"); + } } } } @@ -570,14 +628,16 @@ impl From for opentelemetry::logs::AnyValue { | FileData::Creation(data) | FileData::MkDir(data) | FileData::RmDir(data) + | FileData::Mount(data) + | FileData::Umount(data) | FileData::Unlink(data) => AnyValue::from(data), FileData::Chmod(data) => AnyValue::from(data), FileData::Chown(data) => AnyValue::from(data), - FileData::Rename { new, old } => { - let AnyValue::Map(mut map) = AnyValue::from(new) else { - unreachable!("new value did not serialize to map"); + FileData::MoveMount { to, from } | FileData::Rename { new: to, old: from } => { + let AnyValue::Map(mut map) = AnyValue::from(to) else { + unreachable!("to value did not serialize to map"); }; - map.insert("old".into(), AnyValue::from(old)); + map.insert("old".into(), AnyValue::from(from)); AnyValue::Map(map) } @@ -641,14 +701,14 @@ impl BaseFileData { inode: inode_key_t, parent_inode: inode_key_t, monitored: monitored_t, - ) -> anyhow::Result { - Ok(BaseFileData { + ) -> Self { + BaseFileData { filename: sanitize_d_path(&filename), host_file: PathBuf::new(), // this field is set by HostScanner inode, parent_inode, monitored, - }) + } } } diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index 04dff52a..8bb5604d 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -426,6 +426,17 @@ You can increase this limit with: } } + /// Handle a mount being modified in a monitored directory. + /// + /// This should really do a partial scan of the directory where the + /// mount is being changed, but we don't have an easy way to do that + /// at the moment, so we trigger a full scan instead. + fn handle_mount_event(&self) { + if let Err(e) = self.scan() { + warn!("Host scan failed: {e:?}"); + } + } + /// Periodically notify the host scanner main task that a scan needs /// to happen. /// @@ -479,6 +490,12 @@ You can increase this limit with: warn!("Failed to handle creation event: {e}"); } + // Handle mount events and move on. + if event.is_mount_related() { + self.handle_mount_event(); + continue; + } + if let Some(host_path) = self.get_host_path(Some(event.get_inode())) { self.metrics.scan_inc(ScanLabels::InodeHit); event.set_host_path(host_path); diff --git a/fact/src/metrics/kernel_metrics.rs b/fact/src/metrics/kernel_metrics.rs index 2039d7d7..50f845a0 100644 --- a/fact/src/metrics/kernel_metrics.rs +++ b/fact/src/metrics/kernel_metrics.rs @@ -76,4 +76,7 @@ define_kernel_metrics!( inode_setxattr, inode_removexattr, inode_set_acl, + sb_mount, + sb_umount, + move_mount, ); diff --git a/tests/conftest.py b/tests/conftest.py index b66581c8..d148f01d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -244,6 +244,7 @@ def fact( environment={ 'FACT_LOGLEVEL': 'debug', 'FACT_HOST_MOUNT': '/host', + 'FACT_ENDPOINT_INTROSPECTION': 'true', 'OTEL_BLRP_SCHEDULE_DELAY': '100', 'OTEL_BLRP_MAX_EXPORT_BATCH_SIZE': '1', 'RUST_BACKTRACE': '1', @@ -254,7 +255,7 @@ def fact( volumes={ '/': { 'bind': '/host', - 'mode': 'ro', + 'mode': 'ro,rslave', }, config_file: { 'bind': '/etc/stackrox/fact.yml', diff --git a/tests/test_config_hotreload.py b/tests/test_config_hotreload.py index 7ef6a8f5..cd1bbe94 100644 --- a/tests/test_config_hotreload.py +++ b/tests/test_config_hotreload.py @@ -71,8 +71,8 @@ def test_endpoint_disable_all( } reload_config(fact, config, config_file) - with pytest.raises(requests.ConnectionError): - requests.get(f'{DEFAULT_URL}/metrics') + resp = requests.get(f'{DEFAULT_URL}/metrics') + assert resp.status_code == 503 def test_endpoint_address_change( diff --git a/tests/test_mount.py b/tests/test_mount.py new file mode 100644 index 00000000..e010a48b --- /dev/null +++ b/tests/test_mount.py @@ -0,0 +1,57 @@ +import os +import subprocess +from time import sleep + +import pytest +import requests + +pytestmark = pytest.mark.skipif( + os.geteuid() != 0, + reason='mount operations require root access', +) + + +def get_inodes() -> dict[str, str]: + FACT_INTROSPECTION_INODES = 'http://127.0.0.1:9000/inodes' + res = requests.get(FACT_INTROSPECTION_INODES, timeout=5) + res.raise_for_status() + return res.json() + + +def assert_tracked_path(monitored_dir: str): + stat = os.stat(monitored_dir) + inode = f'{stat.st_dev}:{stat.st_ino}' + + for _ in range(3): + try: + res = get_inodes() + assert res[inode] == monitored_dir + return + except KeyError as e: + print(f'Failed to find inode: {e}') + sleep(1) + + raise TimeoutError + + +@pytest.fixture +def cleanup_mount(monitored_dir: str): + yield + if os.path.ismount(monitored_dir): + subprocess.run(['umount', monitored_dir], check=True) + + +def test_mount( + monitored_dir: str, + cleanup_mount: None, +): + assert_tracked_path(monitored_dir) + + subprocess.run( + ['mount', '-t', 'tmpfs', '-o', 'size=10M', 'tmpfs', monitored_dir], + check=True, + ) + assert_tracked_path(monitored_dir) + + subprocess.run(['umount', monitored_dir], check=True) + assert_tracked_path(monitored_dir)