From 2a1374200405ffcbae3ae76a6847743bbb3537fa Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 17 Jun 2026 10:59:31 -0700 Subject: [PATCH 01/19] X-Smart-Branch-Parent: main From ecb4b18d03a4accb95449f97172e130c03f1cf6b Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sat, 13 Jun 2026 10:14:39 -0700 Subject: [PATCH 02/19] Added tests --- tests/test_inode_xattr.py | 199 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 tests/test_inode_xattr.py diff --git a/tests/test_inode_xattr.py b/tests/test_inode_xattr.py new file mode 100644 index 00000000..366d2125 --- /dev/null +++ b/tests/test_inode_xattr.py @@ -0,0 +1,199 @@ +from __future__ import annotations + +import os + +from event import Event, EventType, Process +from server import FileActivityService +from utils import get_metric_value + + +def get_kernel_setxattr_added(fact_config: tuple[dict, str]): + """ + Query Prometheus metrics to get the count of setxattr events + added to the ring buffer. + + Args: + fact_config: The fact configuration tuple + (config dict, config file path). + + Returns: + The current value of + kernel_inode_setxattr_events{label="Added"} metric. + """ + value = get_metric_value( + fact_config, + 'kernel_inode_setxattr_events', + {'label': 'Added'}, + ) + return int(value) if value is not None else 0 + + +def get_kernel_removexattr_added(fact_config: tuple[dict, str]): + """ + Query Prometheus metrics to get the count of removexattr events + added to the ring buffer. + + Args: + fact_config: The fact configuration tuple + (config dict, config file path). + + Returns: + The current value of + kernel_inode_removexattr_events{label="Added"} metric. + """ + value = get_metric_value( + fact_config, + 'kernel_inode_removexattr_events', + {'label': 'Added'}, + ) + return int(value) if value is not None else 0 + + +def test_setxattr( + test_file: str, + fact_config: tuple[dict, str], +): + """ + Tests that setting a user xattr on a monitored file is tracked + via kernel metrics. + + The test_file fixture creates a file before fact starts, so it is + picked up by the initial scan and its inode is already tracked. + + Args: + test_file: File monitored on the host. + fact_config: The fact configuration. + """ + initial = get_kernel_setxattr_added(fact_config) + + os.setxattr(test_file, 'user.fact_test', b'test_value') + + final = get_kernel_setxattr_added(fact_config) + delta = final - initial + assert delta == 1, f'Expected exactly 1 setxattr event added, got {delta}' + + +def test_removexattr( + test_file: str, + fact_config: tuple[dict, str], +): + """ + Tests that removing a user xattr from a monitored file is tracked + via kernel metrics. + + Args: + test_file: File monitored on the host. + fact_config: The fact configuration. + """ + os.setxattr(test_file, 'user.fact_remove', b'to_remove') + + initial = get_kernel_removexattr_added(fact_config) + + os.removexattr(test_file, 'user.fact_remove') + + final = get_kernel_removexattr_added(fact_config) + delta = final - initial + assert delta == 1, ( + f'Expected exactly 1 removexattr event added, got {delta}' + ) + + +def test_setxattr_multiple( + test_file: str, + fact_config: tuple[dict, str], +): + """ + Tests that setting multiple xattrs on a monitored file tracks + all of them. + + Args: + test_file: File monitored on the host. + fact_config: The fact configuration. + """ + initial = get_kernel_setxattr_added(fact_config) + + os.setxattr(test_file, 'user.attr1', b'value1') + os.setxattr(test_file, 'user.attr2', b'value2') + os.setxattr(test_file, 'user.attr3', b'value3') + + final = get_kernel_setxattr_added(fact_config) + delta = final - initial + assert delta == 3, f'Expected exactly 3 setxattr events added, got {delta}' + + +def test_setxattr_ignored( + test_file: str, + ignored_dir: str, + fact_config: tuple[dict, str], +): + """ + Tests that xattr changes on unmonitored files are not tracked, + while xattr changes on monitored files are. + + Args: + test_file: File monitored on the host. + ignored_dir: Temporary directory that is not monitored by fact. + fact_config: The fact configuration. + """ + ignored_file = os.path.join(ignored_dir, 'ignored.txt') + with open(ignored_file, 'w') as f: + f.write('ignored') + + initial = get_kernel_setxattr_added(fact_config) + + os.setxattr(ignored_file, 'user.ignored', b'value') + + after_ignored = get_kernel_setxattr_added(fact_config) + assert after_ignored == initial, ( + 'Setting xattr on ignored file should not increment Added metric' + ) + + os.setxattr(test_file, 'user.monitored', b'value') + + final = get_kernel_setxattr_added(fact_config) + delta = final - initial + assert delta == 1, ( + f'Expected exactly 1 setxattr event (monitored file only), got {delta}' + ) + + +def test_setxattr_new_file( + monitored_dir: str, + server: FileActivityService, + fact_config: tuple[dict, str], +): + """ + Tests that xattr tracking works for files created while fact is + running, not just files from the initial scan. + + A new file is created in the monitored directory and its creation + event is awaited to ensure the inode is tracked before setting + an xattr. + + Args: + monitored_dir: Temporary directory path that is monitored. + server: The server instance to communicate with. + fact_config: The fact configuration. + """ + process = Process.from_proc() + + test_file = os.path.join(monitored_dir, 'xattr_new.txt') + with open(test_file, 'w') as f: + f.write('new file') + + server.wait_events([ + Event( + process=process, + event_type=EventType.CREATION, + file=test_file, + host_path=test_file, + ), + ]) + + initial = get_kernel_setxattr_added(fact_config) + + os.setxattr(test_file, 'user.new_file', b'value') + + final = get_kernel_setxattr_added(fact_config) + delta = final - initial + assert delta == 1, f'Expected exactly 1 setxattr event added, got {delta}' From c8a62727594cf8269c0a55456920203aee324ea0 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sat, 13 Jun 2026 10:18:50 -0700 Subject: [PATCH 03/19] Added tracking for xattr changes --- fact-ebpf/src/bpf/events.h | 22 +++++++++ fact-ebpf/src/bpf/main.c | 74 ++++++++++++++++++++++++++++++ fact-ebpf/src/bpf/types.h | 10 ++++ fact-ebpf/src/lib.rs | 2 + fact/src/event/mod.rs | 55 +++++++++++++++++++++- fact/src/host_scanner.rs | 4 +- fact/src/metrics/kernel_metrics.rs | 18 ++++++++ 7 files changed, 182 insertions(+), 3 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index ac1d1bd8..c0ccdd3a 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -144,3 +144,25 @@ __always_inline static void submit_rmdir_event(struct submit_event_args_t* args) __submit_event(args, path_hooks_support_bpf_d_path); } + +__always_inline static void submit_setxattr_event(struct submit_event_args_t* args, + const char* xattr_name) { + if (!reserve_event(args)) { + return; + } + args->event->type = FILE_ACTIVITY_SETXATTR; + bpf_probe_read_str(args->event->xattr.name, XATTR_NAME_MAX_LEN, xattr_name); + + __submit_event(args, false); +} + +__always_inline static void submit_removexattr_event(struct submit_event_args_t* args, + const char* xattr_name) { + if (!reserve_event(args)) { + return; + } + args->event->type = FILE_ACTIVITY_REMOVEXATTR; + bpf_probe_read_str(args->event->xattr.name, XATTR_NAME_MAX_LEN, xattr_name); + + __submit_event(args, false); +} diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index eb2033e8..e4d3c66a 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -389,6 +389,80 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { return 0; } +SEC("lsm/inode_setxattr") +int BPF_PROG(trace_inode_setxattr, struct mnt_idmap* idmap, struct dentry* dentry, + const char* name, const void* value, size_t size, int flags) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + struct submit_event_args_t args = {.metrics = &m->inode_setxattr}; + + args.metrics->total++; + + args.inode = inode_to_key(dentry->d_inode); + + struct dentry* parent_dentry = BPF_CORE_READ(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 = inode_is_monitored(inode_get(&args.inode), inode_get(&args.parent_inode)); + + if (args.monitored == NOT_MONITORED) { + args.metrics->ignored++; + return 0; + } + + // Path is resolved in userspace from the inode map + struct bound_path_t* path = get_bound_path(BOUND_PATH_MAIN); + if (path == NULL) { + args.metrics->error++; + return 0; + } + path->path[0] = '\0'; + args.filename = path->path; + + submit_setxattr_event(&args, name); + return 0; +} + +SEC("lsm/inode_removexattr") +int BPF_PROG(trace_inode_removexattr, struct mnt_idmap* idmap, struct dentry* dentry, + const char* name) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + struct submit_event_args_t args = {.metrics = &m->inode_removexattr}; + + args.metrics->total++; + + args.inode = inode_to_key(dentry->d_inode); + + struct dentry* parent_dentry = BPF_CORE_READ(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 = inode_is_monitored(inode_get(&args.inode), inode_get(&args.parent_inode)); + + if (args.monitored == NOT_MONITORED) { + args.metrics->ignored++; + return 0; + } + + // Path is resolved in userspace from the inode map + struct bound_path_t* path = get_bound_path(BOUND_PATH_MAIN); + if (path == NULL) { + args.metrics->error++; + return 0; + } + path->path[0] = '\0'; + args.filename = path->path; + + submit_removexattr_event(&args, name); + return 0; +} + SEC("lsm/path_rmdir") int BPF_PROG(trace_path_rmdir, struct path* dir, struct dentry* dentry) { struct metrics_t* m = get_metrics(); diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index 2f11c0db..d7f8a21f 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -15,6 +15,9 @@ #define LINEAGE_MAX 2 +// Matches Linux kernel XATTR_NAME_MAX (255) + null terminator +#define XATTR_NAME_MAX_LEN 256 + #define LPM_SIZE_MAX 256 typedef struct lineage_t { @@ -64,6 +67,8 @@ typedef enum file_activity_type_t { FILE_ACTIVITY_RENAME, DIR_ACTIVITY_CREATION, DIR_ACTIVITY_UNLINK, + FILE_ACTIVITY_SETXATTR, + FILE_ACTIVITY_REMOVEXATTR, } file_activity_type_t; struct event_t { @@ -90,6 +95,9 @@ struct event_t { inode_key_t inode; monitored_t monitored; } rename; + struct { + char name[XATTR_NAME_MAX_LEN]; + } xattr; }; }; @@ -132,4 +140,6 @@ struct metrics_t { struct metrics_by_hook_t path_mkdir; struct metrics_by_hook_t d_instantiate; struct metrics_by_hook_t path_rmdir; + struct metrics_by_hook_t inode_setxattr; + struct metrics_by_hook_t inode_removexattr; }; diff --git a/fact-ebpf/src/lib.rs b/fact-ebpf/src/lib.rs index f38fe801..a1dd046a 100644 --- a/fact-ebpf/src/lib.rs +++ b/fact-ebpf/src/lib.rs @@ -145,6 +145,8 @@ impl metrics_t { self.path_mkdir = self.path_mkdir.accumulate(&other.path_mkdir); self.path_rmdir = self.path_rmdir.accumulate(&other.path_rmdir); self.d_instantiate = self.d_instantiate.accumulate(&other.d_instantiate); + self.inode_setxattr = self.inode_setxattr.accumulate(&other.inode_setxattr); + self.inode_removexattr = self.inode_removexattr.accumulate(&other.inode_removexattr); self } } diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 7944ada3..3a51d07e 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -9,7 +9,9 @@ use std::{ use globset::GlobSet; use serde::Serialize; -use fact_ebpf::{PATH_MAX, event_t, file_activity_type_t, inode_key_t, monitored_t}; +use fact_ebpf::{ + PATH_MAX, XATTR_NAME_MAX_LEN, event_t, file_activity_type_t, inode_key_t, monitored_t, +}; use crate::host_info; use process::Process; @@ -131,6 +133,10 @@ impl Event { matches!(self.file, FileData::Creation(_) | FileData::MkDir(_)) } + pub fn is_xattr(&self) -> bool { + matches!(self.file, FileData::SetXattr(_) | FileData::RemoveXattr(_)) + } + pub fn is_mkdir(&self) -> bool { matches!(self.file, FileData::MkDir(_)) } @@ -162,6 +168,8 @@ impl Event { FileData::Chmod(data) => &data.inner.inode, FileData::Chown(data) => &data.inner.inode, FileData::Rename(data) => &data.new.inode, + FileData::SetXattr(data) => &data.inner.inode, + FileData::RemoveXattr(data) => &data.inner.inode, } } @@ -176,6 +184,8 @@ impl Event { FileData::Chmod(data) => &data.inner.parent_inode, FileData::Chown(data) => &data.inner.parent_inode, FileData::Rename(data) => &data.new.parent_inode, + FileData::SetXattr(data) => &data.inner.parent_inode, + FileData::RemoveXattr(data) => &data.inner.parent_inode, } } @@ -199,6 +209,8 @@ impl Event { FileData::Chmod(data) => &data.inner.filename, FileData::Chown(data) => &data.inner.filename, FileData::Rename(data) => &data.new.filename, + FileData::SetXattr(data) => &data.inner.filename, + FileData::RemoveXattr(data) => &data.inner.filename, } } @@ -219,6 +231,8 @@ impl Event { FileData::Chmod(data) => &data.inner.host_file, FileData::Chown(data) => &data.inner.host_file, FileData::Rename(data) => &data.new.host_file, + FileData::SetXattr(data) => &data.inner.host_file, + FileData::RemoveXattr(data) => &data.inner.host_file, } } @@ -243,6 +257,8 @@ impl Event { FileData::Chmod(data) => data.inner.host_file = host_path, FileData::Chown(data) => data.inner.host_file = host_path, FileData::Rename(data) => data.new.host_file = host_path, + FileData::SetXattr(data) => data.inner.host_file = host_path, + FileData::RemoveXattr(data) => data.inner.host_file = host_path, } } @@ -264,6 +280,8 @@ impl Event { FileData::Chmod(data) => data.inner.monitored, FileData::Chown(data) => data.inner.monitored, FileData::Rename(data) => data.new.monitored, + FileData::SetXattr(data) => data.inner.monitored, + FileData::RemoveXattr(data) => data.inner.monitored, } } @@ -356,6 +374,8 @@ pub enum FileData { Chmod(ChmodFileData), Chown(ChownFileData), Rename(RenameFileData), + SetXattr(XattrFileData), + RemoveXattr(XattrFileData), } impl FileData { @@ -407,6 +427,18 @@ impl FileData { }; FileData::Rename(data) } + file_activity_type_t::FILE_ACTIVITY_SETXATTR => { + let xattr_name = slice_to_string( + &unsafe { extra_data.xattr }.name[..XATTR_NAME_MAX_LEN as usize], + )?; + FileData::SetXattr(XattrFileData { inner, xattr_name }) + } + file_activity_type_t::FILE_ACTIVITY_REMOVEXATTR => { + let xattr_name = slice_to_string( + &unsafe { extra_data.xattr }.name[..XATTR_NAME_MAX_LEN as usize], + )?; + FileData::RemoveXattr(XattrFileData { inner, xattr_name }) + } invalid => unreachable!("Invalid event type: {invalid:?}"), }; @@ -433,6 +465,12 @@ impl From for fact_api::file_activity::File { FileData::RmDir(_) => { unreachable!("RmDir event reached protobuf conversion"); } + FileData::SetXattr(_) => { + unreachable!("SetXattr event reached protobuf conversion"); + } + FileData::RemoveXattr(_) => { + unreachable!("RemoveXattr event reached protobuf conversion"); + } FileData::Unlink(event) => { let activity = Some(fact_api::FileActivityBase::from(event)); let f_act = fact_api::FileUnlink { activity }; @@ -465,6 +503,8 @@ impl PartialEq for FileData { (FileData::Unlink(this), FileData::Unlink(other)) => this == other, (FileData::Chmod(this), FileData::Chmod(other)) => this == other, (FileData::Rename(this), FileData::Rename(other)) => this == other, + (FileData::SetXattr(this), FileData::SetXattr(other)) => this == other, + (FileData::RemoveXattr(this), FileData::RemoveXattr(other)) => this == other, _ => false, } } @@ -595,6 +635,19 @@ impl PartialEq for RenameFileData { } } +#[derive(Debug, Clone, Serialize)] +pub struct XattrFileData { + inner: BaseFileData, + xattr_name: String, +} + +#[cfg(test)] +impl PartialEq for XattrFileData { + fn eq(&self, other: &Self) -> bool { + self.xattr_name == other.xattr_name && self.inner == other.inner + } +} + #[cfg(test)] mod test_utils { use std::os::raw::c_char; diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index 273bd5df..2b4b8585 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -452,8 +452,8 @@ You can increase this limit with: self.handle_unlink_event(&event); } - // Skip directory creation and deletion events - we track them internally but don't send to sensor - if event.is_mkdir() || event.is_rmdir() { + // Skip events that are tracked internally but not yet sent to sensor + if event.is_mkdir() || event.is_rmdir() || event.is_xattr() { continue; } diff --git a/fact/src/metrics/kernel_metrics.rs b/fact/src/metrics/kernel_metrics.rs index 15da3993..df51f432 100644 --- a/fact/src/metrics/kernel_metrics.rs +++ b/fact/src/metrics/kernel_metrics.rs @@ -16,6 +16,8 @@ pub struct KernelMetrics { path_mkdir: EventCounter, path_rmdir: EventCounter, d_instantiate: EventCounter, + inode_setxattr: EventCounter, + inode_removexattr: EventCounter, map: PerCpuArray, } @@ -61,6 +63,16 @@ impl KernelMetrics { "Events processed by the d_instantiate LSM hook", &[], // Labels are not needed since `collect` will add them all ); + let inode_setxattr = EventCounter::new( + "kernel_inode_setxattr_events", + "Events processed by the inode_setxattr LSM hook", + &[], // Labels are not needed since `collect` will add them all + ); + let inode_removexattr = EventCounter::new( + "kernel_inode_removexattr_events", + "Events processed by the inode_removexattr LSM hook", + &[], // Labels are not needed since `collect` will add them all + ); file_open.register(reg); path_unlink.register(reg); @@ -70,6 +82,8 @@ impl KernelMetrics { path_mkdir.register(reg); path_rmdir.register(reg); d_instantiate.register(reg); + inode_setxattr.register(reg); + inode_removexattr.register(reg); KernelMetrics { file_open, @@ -80,6 +94,8 @@ impl KernelMetrics { path_mkdir, path_rmdir, d_instantiate, + inode_setxattr, + inode_removexattr, map: kernel_metrics, } } @@ -132,6 +148,8 @@ impl KernelMetrics { KernelMetrics::refresh_labels(&self.path_mkdir, &metrics.path_mkdir); KernelMetrics::refresh_labels(&self.path_rmdir, &metrics.path_rmdir); KernelMetrics::refresh_labels(&self.d_instantiate, &metrics.d_instantiate); + KernelMetrics::refresh_labels(&self.inode_setxattr, &metrics.inode_setxattr); + KernelMetrics::refresh_labels(&self.inode_removexattr, &metrics.inode_removexattr); Ok(()) } From f89ab5fe2eb835ce2ff052e35aba17b45b1a5352 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sat, 13 Jun 2026 10:56:23 -0700 Subject: [PATCH 04/19] Combined calls to BPF_CORE_READ --- fact-ebpf/src/bpf/main.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index e4d3c66a..adb1d2b6 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -401,10 +401,7 @@ int BPF_PROG(trace_inode_setxattr, struct mnt_idmap* idmap, struct dentry* dentr args.metrics->total++; args.inode = inode_to_key(dentry->d_inode); - - struct dentry* parent_dentry = BPF_CORE_READ(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.parent_inode = inode_to_key(BPF_CORE_READ(dentry, d_parent, d_inode)); args.monitored = inode_is_monitored(inode_get(&args.inode), inode_get(&args.parent_inode)); @@ -438,10 +435,7 @@ int BPF_PROG(trace_inode_removexattr, struct mnt_idmap* idmap, struct dentry* de args.metrics->total++; args.inode = inode_to_key(dentry->d_inode); - - struct dentry* parent_dentry = BPF_CORE_READ(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.parent_inode = inode_to_key(BPF_CORE_READ(dentry, d_parent, d_inode)); args.monitored = inode_is_monitored(inode_get(&args.inode), inode_get(&args.parent_inode)); From 49eaa7d199c4f8283176219ff3ba8a2adf680f08 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sat, 13 Jun 2026 11:36:51 -0700 Subject: [PATCH 05/19] Clarified comment that the path is not available. Also clarified relevant code --- fact-ebpf/src/bpf/main.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index adb1d2b6..9de3a74b 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -410,14 +410,15 @@ int BPF_PROG(trace_inode_setxattr, struct mnt_idmap* idmap, struct dentry* dentr return 0; } - // Path is resolved in userspace from the inode map - struct bound_path_t* path = get_bound_path(BOUND_PATH_MAIN); - if (path == NULL) { + // inode hooks don't provide a struct path, so filename is left empty. + // __submit_event requires a valid pointer for bpf_probe_read_str. + struct bound_path_t* bound_path = get_bound_path(BOUND_PATH_MAIN); + if (bound_path == NULL) { args.metrics->error++; return 0; } - path->path[0] = '\0'; - args.filename = path->path; + bound_path->path[0] = '\0'; + args.filename = bound_path->path; submit_setxattr_event(&args, name); return 0; @@ -444,14 +445,15 @@ int BPF_PROG(trace_inode_removexattr, struct mnt_idmap* idmap, struct dentry* de return 0; } - // Path is resolved in userspace from the inode map - struct bound_path_t* path = get_bound_path(BOUND_PATH_MAIN); - if (path == NULL) { + // inode hooks don't provide a struct path, so filename is left empty. + // __submit_event requires a valid pointer for bpf_probe_read_str. + struct bound_path_t* bound_path = get_bound_path(BOUND_PATH_MAIN); + if (bound_path == NULL) { args.metrics->error++; return 0; } - path->path[0] = '\0'; - args.filename = path->path; + bound_path->path[0] = '\0'; + args.filename = bound_path->path; submit_removexattr_event(&args, name); return 0; From 02220c0b2394206e0ba84505d51c456cb248f1b3 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sun, 14 Jun 2026 10:53:53 -0700 Subject: [PATCH 06/19] Added helper to reduce dry --- fact-ebpf/src/bpf/main.c | 51 ++++++++++++++-------------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 9de3a74b..0bd1950c 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -389,14 +389,11 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { return 0; } -SEC("lsm/inode_setxattr") -int BPF_PROG(trace_inode_setxattr, struct mnt_idmap* idmap, struct dentry* dentry, - const char* name, const void* value, size_t size, int flags) { - struct metrics_t* m = get_metrics(); - if (m == NULL) { - return 0; - } - struct submit_event_args_t args = {.metrics = &m->inode_setxattr}; +__always_inline static int handle_xattr(struct metrics_by_hook_t* hook_metrics, + struct dentry* dentry, + const char* xattr_name, + file_activity_type_t event_type) { + struct submit_event_args_t args = {.metrics = hook_metrics}; args.metrics->total++; @@ -420,42 +417,28 @@ int BPF_PROG(trace_inode_setxattr, struct mnt_idmap* idmap, struct dentry* dentr bound_path->path[0] = '\0'; args.filename = bound_path->path; - submit_setxattr_event(&args, name); + submit_xattr_event(&args, event_type, xattr_name); return 0; } -SEC("lsm/inode_removexattr") -int BPF_PROG(trace_inode_removexattr, struct mnt_idmap* idmap, struct dentry* dentry, - const char* name) { +SEC("lsm/inode_setxattr") +int BPF_PROG(trace_inode_setxattr, struct mnt_idmap* idmap, struct dentry* dentry, + const char* name, const void* value, size_t size, int flags) { struct metrics_t* m = get_metrics(); if (m == NULL) { return 0; } - struct submit_event_args_t args = {.metrics = &m->inode_removexattr}; - - args.metrics->total++; - - args.inode = inode_to_key(dentry->d_inode); - args.parent_inode = inode_to_key(BPF_CORE_READ(dentry, d_parent, d_inode)); - - args.monitored = inode_is_monitored(inode_get(&args.inode), inode_get(&args.parent_inode)); - - if (args.monitored == NOT_MONITORED) { - args.metrics->ignored++; - return 0; - } + return handle_xattr(&m->inode_setxattr, dentry, name, FILE_ACTIVITY_SETXATTR); +} - // inode hooks don't provide a struct path, so filename is left empty. - // __submit_event requires a valid pointer for bpf_probe_read_str. - struct bound_path_t* bound_path = get_bound_path(BOUND_PATH_MAIN); - if (bound_path == NULL) { - args.metrics->error++; +SEC("lsm/inode_removexattr") +int BPF_PROG(trace_inode_removexattr, struct mnt_idmap* idmap, struct dentry* dentry, + const char* name) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { return 0; } - bound_path->path[0] = '\0'; - args.filename = bound_path->path; - - submit_removexattr_event(&args, name); + return handle_xattr(&m->inode_removexattr, dentry, name, FILE_ACTIVITY_REMOVEXATTR); return 0; } From 0512fae9891b3efb133858448ac9ba7d61bfa35d Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sun, 14 Jun 2026 10:55:25 -0700 Subject: [PATCH 07/19] Added events.h which had been forgotten --- fact-ebpf/src/bpf/events.h | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index c0ccdd3a..396cf0d4 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -145,23 +145,13 @@ __always_inline static void submit_rmdir_event(struct submit_event_args_t* args) __submit_event(args, path_hooks_support_bpf_d_path); } -__always_inline static void submit_setxattr_event(struct submit_event_args_t* args, - const char* xattr_name) { +__always_inline static void submit_xattr_event(struct submit_event_args_t* args, + file_activity_type_t event_type, + const char* xattr_name) { if (!reserve_event(args)) { return; } - args->event->type = FILE_ACTIVITY_SETXATTR; - bpf_probe_read_str(args->event->xattr.name, XATTR_NAME_MAX_LEN, xattr_name); - - __submit_event(args, false); -} - -__always_inline static void submit_removexattr_event(struct submit_event_args_t* args, - const char* xattr_name) { - if (!reserve_event(args)) { - return; - } - args->event->type = FILE_ACTIVITY_REMOVEXATTR; + args->event->type = event_type; bpf_probe_read_str(args->event->xattr.name, XATTR_NAME_MAX_LEN, xattr_name); __submit_event(args, false); From 9decf245611254c5c2b67dd30d6e5c67a11e6404 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sun, 14 Jun 2026 11:06:29 -0700 Subject: [PATCH 08/19] Fix format --- fact-ebpf/src/bpf/events.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 396cf0d4..1d2e158e 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -146,8 +146,8 @@ __always_inline static void submit_rmdir_event(struct submit_event_args_t* args) } __always_inline static void submit_xattr_event(struct submit_event_args_t* args, - file_activity_type_t event_type, - const char* xattr_name) { + file_activity_type_t event_type, + const char* xattr_name) { if (!reserve_event(args)) { return; } From f91f5cec3a58656b9ba1445d1711c16b5dc6bcc0 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sun, 14 Jun 2026 11:08:13 -0700 Subject: [PATCH 09/19] Removed unreachable return --- fact-ebpf/src/bpf/main.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 0bd1950c..2b8e2ed8 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -439,7 +439,6 @@ int BPF_PROG(trace_inode_removexattr, struct mnt_idmap* idmap, struct dentry* de return 0; } return handle_xattr(&m->inode_removexattr, dentry, name, FILE_ACTIVITY_REMOVEXATTR); - return 0; } SEC("lsm/path_rmdir") From 5706020b8d7c421699144cea8eee271029a26bbf Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sun, 14 Jun 2026 11:16:21 -0700 Subject: [PATCH 10/19] Fixed format again --- tests/test_inode_xattr.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/test_inode_xattr.py b/tests/test_inode_xattr.py index 366d2125..4f37ebdd 100644 --- a/tests/test_inode_xattr.py +++ b/tests/test_inode_xattr.py @@ -181,14 +181,16 @@ def test_setxattr_new_file( with open(test_file, 'w') as f: f.write('new file') - server.wait_events([ - Event( - process=process, - event_type=EventType.CREATION, - file=test_file, - host_path=test_file, - ), - ]) + server.wait_events( + [ + Event( + process=process, + event_type=EventType.CREATION, + file=test_file, + host_path=test_file, + ), + ] + ) initial = get_kernel_setxattr_added(fact_config) From 6a0e43798b3e6cbd827a288b660d1d906d7e162f Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Sun, 14 Jun 2026 12:00:03 -0700 Subject: [PATCH 11/19] Renamed test_inode_xattr.py to test_xattr.py --- tests/{test_inode_xattr.py => test_xattr.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{test_inode_xattr.py => test_xattr.py} (100%) diff --git a/tests/test_inode_xattr.py b/tests/test_xattr.py similarity index 100% rename from tests/test_inode_xattr.py rename to tests/test_xattr.py From a7e7566e477d627a83b8a167057c31083e4a029f Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Tue, 16 Jun 2026 19:55:12 -0700 Subject: [PATCH 12/19] Tests now check gRPC messages instead of metrics --- tests/event.py | 17 ++++ tests/test_xattr.py | 187 +++++++++++++++++++++++--------------------- 2 files changed, 113 insertions(+), 91 deletions(-) diff --git a/tests/event.py b/tests/event.py index 11bca769..d8639ac5 100644 --- a/tests/event.py +++ b/tests/event.py @@ -36,6 +36,7 @@ class EventType(Enum): PERMISSION = 4 OWNERSHIP = 5 RENAME = 6 + XATTR = 7 class Process: @@ -223,6 +224,7 @@ def __init__( owner_gid: int | None = None, old_file: str | Pattern[str] | None = None, old_host_path: str | Pattern[str] | None = None, + xattr_name: str | None = None, ): self._type: EventType = event_type self._process: Process = process @@ -233,6 +235,7 @@ def __init__( self._owner_gid: int | None = owner_gid self._old_file: str | Pattern[str] | None = old_file self._old_host_path: str | Pattern[str] | None = old_host_path + self._xattr_name: str | None = xattr_name @property def event_type(self) -> EventType: @@ -270,6 +273,10 @@ def old_file(self) -> str | Pattern[str] | None: def old_host_path(self) -> str | Pattern[str] | None: return self._old_host_path + @property + def xattr_name(self) -> str | None: + return self._xattr_name + @classmethod def _diff_field(cls, diff: dict, name: str, expected: Any, actual: Any): if expected != actual: @@ -378,6 +385,13 @@ def diff(self, other: FileActivity) -> dict | None: self.owner_gid, event_field.gid, ) + elif self.event_type == EventType.XATTR: + Event._diff_field( + diff, + 'xattr_name', + self.xattr_name, + event_field.xattr_name, + ) return diff if diff else None @@ -401,6 +415,9 @@ def __str__(self) -> str: f', old_host_path="{self.old_host_path}"' ) + if self.event_type == EventType.XATTR: + s += f', xattr_name="{self.xattr_name}"' + s += ')' return s diff --git a/tests/test_xattr.py b/tests/test_xattr.py index 4f37ebdd..d0b21541 100644 --- a/tests/test_xattr.py +++ b/tests/test_xattr.py @@ -4,127 +4,122 @@ from event import Event, EventType, Process from server import FileActivityService -from utils import get_metric_value - - -def get_kernel_setxattr_added(fact_config: tuple[dict, str]): - """ - Query Prometheus metrics to get the count of setxattr events - added to the ring buffer. - - Args: - fact_config: The fact configuration tuple - (config dict, config file path). - - Returns: - The current value of - kernel_inode_setxattr_events{label="Added"} metric. - """ - value = get_metric_value( - fact_config, - 'kernel_inode_setxattr_events', - {'label': 'Added'}, - ) - return int(value) if value is not None else 0 - - -def get_kernel_removexattr_added(fact_config: tuple[dict, str]): - """ - Query Prometheus metrics to get the count of removexattr events - added to the ring buffer. - - Args: - fact_config: The fact configuration tuple - (config dict, config file path). - - Returns: - The current value of - kernel_inode_removexattr_events{label="Added"} metric. - """ - value = get_metric_value( - fact_config, - 'kernel_inode_removexattr_events', - {'label': 'Added'}, - ) - return int(value) if value is not None else 0 def test_setxattr( test_file: str, - fact_config: tuple[dict, str], + server: FileActivityService, ): """ - Tests that setting a user xattr on a monitored file is tracked - via kernel metrics. + Tests that setting a user xattr on a monitored file generates + a gRPC xattr event. The test_file fixture creates a file before fact starts, so it is picked up by the initial scan and its inode is already tracked. Args: test_file: File monitored on the host. - fact_config: The fact configuration. + server: The server instance to communicate with. """ - initial = get_kernel_setxattr_added(fact_config) + process = Process.from_proc() os.setxattr(test_file, 'user.fact_test', b'test_value') - final = get_kernel_setxattr_added(fact_config) - delta = final - initial - assert delta == 1, f'Expected exactly 1 setxattr event added, got {delta}' + server.wait_events( + [ + Event( + process=process, + event_type=EventType.XATTR, + file='', + host_path=test_file, + xattr_name='user.fact_test', + ), + ], + strict=False, + ) def test_removexattr( test_file: str, - fact_config: tuple[dict, str], + server: FileActivityService, ): """ - Tests that removing a user xattr from a monitored file is tracked - via kernel metrics. + Tests that removing a user xattr from a monitored file generates + a gRPC xattr event. Args: test_file: File monitored on the host. - fact_config: The fact configuration. + server: The server instance to communicate with. """ - os.setxattr(test_file, 'user.fact_remove', b'to_remove') - - initial = get_kernel_removexattr_added(fact_config) + process = Process.from_proc() + os.setxattr(test_file, 'user.fact_remove', b'to_remove') os.removexattr(test_file, 'user.fact_remove') - final = get_kernel_removexattr_added(fact_config) - delta = final - initial - assert delta == 1, ( - f'Expected exactly 1 removexattr event added, got {delta}' + server.wait_events( + [ + Event( + process=process, + event_type=EventType.XATTR, + file='', + host_path=test_file, + xattr_name='user.fact_remove', + ), + ], + strict=False, ) def test_setxattr_multiple( test_file: str, - fact_config: tuple[dict, str], + server: FileActivityService, ): """ - Tests that setting multiple xattrs on a monitored file tracks - all of them. + Tests that setting multiple xattrs on a monitored file generates + a gRPC event for each. Args: test_file: File monitored on the host. - fact_config: The fact configuration. + server: The server instance to communicate with. """ - initial = get_kernel_setxattr_added(fact_config) + process = Process.from_proc() os.setxattr(test_file, 'user.attr1', b'value1') os.setxattr(test_file, 'user.attr2', b'value2') os.setxattr(test_file, 'user.attr3', b'value3') - final = get_kernel_setxattr_added(fact_config) - delta = final - initial - assert delta == 3, f'Expected exactly 3 setxattr events added, got {delta}' + server.wait_events( + [ + Event( + process=process, + event_type=EventType.XATTR, + file='', + host_path=test_file, + xattr_name='user.attr1', + ), + Event( + process=process, + event_type=EventType.XATTR, + file='', + host_path=test_file, + xattr_name='user.attr2', + ), + Event( + process=process, + event_type=EventType.XATTR, + file='', + host_path=test_file, + xattr_name='user.attr3', + ), + ], + strict=False, + ) def test_setxattr_ignored( test_file: str, ignored_dir: str, - fact_config: tuple[dict, str], + server: FileActivityService, ): """ Tests that xattr changes on unmonitored files are not tracked, @@ -133,34 +128,38 @@ def test_setxattr_ignored( Args: test_file: File monitored on the host. ignored_dir: Temporary directory that is not monitored by fact. - fact_config: The fact configuration. + server: The server instance to communicate with. """ + process = Process.from_proc() + ignored_file = os.path.join(ignored_dir, 'ignored.txt') with open(ignored_file, 'w') as f: f.write('ignored') - initial = get_kernel_setxattr_added(fact_config) - + # Set xattr on ignored file - should NOT generate an event os.setxattr(ignored_file, 'user.ignored', b'value') - after_ignored = get_kernel_setxattr_added(fact_config) - assert after_ignored == initial, ( - 'Setting xattr on ignored file should not increment Added metric' - ) - + # Set xattr on monitored file - should generate an event os.setxattr(test_file, 'user.monitored', b'value') - final = get_kernel_setxattr_added(fact_config) - delta = final - initial - assert delta == 1, ( - f'Expected exactly 1 setxattr event (monitored file only), got {delta}' + # Only the monitored file's xattr event should arrive + server.wait_events( + [ + Event( + process=process, + event_type=EventType.XATTR, + file='', + host_path=test_file, + xattr_name='user.monitored', + ), + ], + strict=False, ) def test_setxattr_new_file( monitored_dir: str, server: FileActivityService, - fact_config: tuple[dict, str], ): """ Tests that xattr tracking works for files created while fact is @@ -173,7 +172,6 @@ def test_setxattr_new_file( Args: monitored_dir: Temporary directory path that is monitored. server: The server instance to communicate with. - fact_config: The fact configuration. """ process = Process.from_proc() @@ -189,13 +187,20 @@ def test_setxattr_new_file( file=test_file, host_path=test_file, ), - ] + ], ) - initial = get_kernel_setxattr_added(fact_config) - os.setxattr(test_file, 'user.new_file', b'value') - final = get_kernel_setxattr_added(fact_config) - delta = final - initial - assert delta == 1, f'Expected exactly 1 setxattr event added, got {delta}' + server.wait_events( + [ + Event( + process=process, + event_type=EventType.XATTR, + file='', + host_path=test_file, + xattr_name='user.new_file', + ), + ], + strict=False, + ) From 5dc6136be075568461bf1fc283e125c1618861ef Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Tue, 16 Jun 2026 19:56:46 -0700 Subject: [PATCH 13/19] Fact now sends xattr events as gRPC messages --- fact/src/event/mod.rs | 44 ++++++++++++++++++++++++++++++++++------ fact/src/host_scanner.rs | 4 ++-- third_party/stackrox | 2 +- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 3a51d07e..f09ebb4a 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -431,13 +431,21 @@ impl FileData { let xattr_name = slice_to_string( &unsafe { extra_data.xattr }.name[..XATTR_NAME_MAX_LEN as usize], )?; - FileData::SetXattr(XattrFileData { inner, xattr_name }) + FileData::SetXattr(XattrFileData { + inner, + xattr_name, + operation: XattrOperation::Set, + }) } file_activity_type_t::FILE_ACTIVITY_REMOVEXATTR => { let xattr_name = slice_to_string( &unsafe { extra_data.xattr }.name[..XATTR_NAME_MAX_LEN as usize], )?; - FileData::RemoveXattr(XattrFileData { inner, xattr_name }) + FileData::RemoveXattr(XattrFileData { + inner, + xattr_name, + operation: XattrOperation::Remove, + }) } invalid => unreachable!("Invalid event type: {invalid:?}"), }; @@ -465,11 +473,13 @@ impl From for fact_api::file_activity::File { FileData::RmDir(_) => { unreachable!("RmDir event reached protobuf conversion"); } - FileData::SetXattr(_) => { - unreachable!("SetXattr event reached protobuf conversion"); + FileData::SetXattr(event) => { + let f_act = fact_api::FileXattrChange::from(event); + fact_api::file_activity::File::Xattr(f_act) } - FileData::RemoveXattr(_) => { - unreachable!("RemoveXattr event reached protobuf conversion"); + FileData::RemoveXattr(event) => { + let f_act = fact_api::FileXattrChange::from(event); + fact_api::file_activity::File::Xattr(f_act) } FileData::Unlink(event) => { let activity = Some(fact_api::FileActivityBase::from(event)); @@ -635,10 +645,32 @@ impl PartialEq for RenameFileData { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub enum XattrOperation { + Set, + Remove, +} + #[derive(Debug, Clone, Serialize)] pub struct XattrFileData { inner: BaseFileData, xattr_name: String, + operation: XattrOperation, +} + +impl From for fact_api::FileXattrChange { + fn from(value: XattrFileData) -> Self { + let activity = fact_api::FileActivityBase::from(value.inner); + let operation = match value.operation { + XattrOperation::Set => fact_api::file_xattr_change::Operation::Set, + XattrOperation::Remove => fact_api::file_xattr_change::Operation::Remove, + }; + fact_api::FileXattrChange { + activity: Some(activity), + xattr_name: value.xattr_name, + operation: operation.into(), + } + } } #[cfg(test)] diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index 2b4b8585..273bd5df 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -452,8 +452,8 @@ You can increase this limit with: self.handle_unlink_event(&event); } - // Skip events that are tracked internally but not yet sent to sensor - if event.is_mkdir() || event.is_rmdir() || event.is_xattr() { + // Skip directory creation and deletion events - we track them internally but don't send to sensor + if event.is_mkdir() || event.is_rmdir() { continue; } diff --git a/third_party/stackrox b/third_party/stackrox index e6bcbecf..d8239afc 160000 --- a/third_party/stackrox +++ b/third_party/stackrox @@ -1 +1 @@ -Subproject commit e6bcbecfe70a809fbcc809b8be496cd4c70a0691 +Subproject commit d8239afc33c2041c92fba2cb485738cf63a634ef From c3ea63b4ea00b42da7951be4bf751679ca40fccc Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Tue, 16 Jun 2026 20:12:32 -0700 Subject: [PATCH 14/19] Added parameterized UTF-8 tests --- tests/test_xattr.py | 66 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/test_xattr.py b/tests/test_xattr.py index d0b21541..d9f01168 100644 --- a/tests/test_xattr.py +++ b/tests/test_xattr.py @@ -2,8 +2,11 @@ import os +import pytest + from event import Event, EventType, Process from server import FileActivityService +from utils import join_path_with_filename, path_to_string def test_setxattr( @@ -204,3 +207,66 @@ def test_setxattr_new_file( ], strict=False, ) + + +@pytest.mark.parametrize( + 'filename', + [ + pytest.param('xattr.txt', id='ASCII'), + pytest.param('café.txt', id='French'), + pytest.param('файл.txt', id='Cyrillic'), + pytest.param('测试.txt', id='Chinese'), + pytest.param('🔒secure.txt', id='Emoji'), + pytest.param(b'xattr\xff\xfe.txt', id='InvalidUTF8'), + ], +) +def test_setxattr_utf8_filenames( + monitored_dir: str, + server: FileActivityService, + filename: str | bytes, +): + """ + Tests that xattr events are correctly tracked on files with + various UTF-8 and non-UTF-8 filenames. + + Args: + monitored_dir: Temporary directory path for creating the test file. + server: The server instance to communicate with. + filename: Name of the file to create (includes UTF-8 test cases). + """ + fut = join_path_with_filename(monitored_dir, filename) + + with open(fut, 'w') as f: + f.write('test') + + # gRPC events use lossy UTF-8 conversion, but os.setxattr + # needs the original path to find the file on disk. + fut_str = path_to_string(fut) + + process = Process.from_proc() + + server.wait_events( + [ + Event( + process=process, + event_type=EventType.CREATION, + file=fut_str, + host_path=fut_str, + ), + ], + ) + + os.setxattr(fut, 'user.utf8_test', b'value') + + server.wait_events( + [ + Event( + process=process, + event_type=EventType.XATTR, + file='', + host_path=fut_str, + xattr_name='user.utf8_test', + ), + ], + strict=False, + ) From 333df8c0ff8dfdc8759e05dfb594987a69767d44 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 17 Jun 2026 10:00:22 -0700 Subject: [PATCH 15/19] Test are more forgiving of extra node events --- tests/server.py | 8 +++++++- tests/test_xattr.py | 6 ------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/server.py b/tests/server.py index 048b4b26..bb834a49 100644 --- a/tests/server.py +++ b/tests/server.py @@ -109,7 +109,13 @@ def _wait_events( if len(events) == 0: return elif strict: - raise ValueError(json.dumps(diff, indent=4)) + # Container events are fully controlled by the test, + # so any mismatch is a real failure. Node events may + # include system noise (e.g. SELinux xattr changes) + # that the test cannot predict, so they are skipped. + is_container_event = bool(msg.process.container_id) + if is_container_event: + raise ValueError(json.dumps(diff, indent=4)) def wait_events(self, events: list[Event], strict: bool = True): """ diff --git a/tests/test_xattr.py b/tests/test_xattr.py index d9f01168..61c89d68 100644 --- a/tests/test_xattr.py +++ b/tests/test_xattr.py @@ -38,7 +38,6 @@ def test_setxattr( xattr_name='user.fact_test', ), ], - strict=False, ) @@ -69,7 +68,6 @@ def test_removexattr( xattr_name='user.fact_remove', ), ], - strict=False, ) @@ -115,7 +113,6 @@ def test_setxattr_multiple( xattr_name='user.attr3', ), ], - strict=False, ) @@ -156,7 +153,6 @@ def test_setxattr_ignored( xattr_name='user.monitored', ), ], - strict=False, ) @@ -205,7 +201,6 @@ def test_setxattr_new_file( xattr_name='user.new_file', ), ], - strict=False, ) @@ -268,5 +263,4 @@ def test_setxattr_utf8_filenames( xattr_name='user.utf8_test', ), ], - strict=False, ) From 5fc913d7506ba64e95d2ededbd09873bf5295650 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 17 Jun 2026 10:20:54 -0700 Subject: [PATCH 16/19] Validating the path in __submit_event instead of setting it in xattr LSM hooks --- fact-ebpf/src/bpf/events.h | 6 +++++- fact-ebpf/src/bpf/main.c | 10 ---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 1d2e158e..efedf86e 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -37,7 +37,11 @@ __always_inline static void __submit_event(struct submit_event_args_t* args, event->monitored = args->monitored; inode_copy(&event->inode, &args->inode); inode_copy(&event->parent_inode, &args->parent_inode); - bpf_probe_read_str(event->filename, PATH_MAX, args->filename); + if (args->filename != NULL) { + bpf_probe_read_str(event->filename, PATH_MAX, args->filename); + } else { + event->filename[0] = '\0'; + } struct helper_t* helper = get_helper(); if (helper == NULL) { diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 2b8e2ed8..4eacdd79 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -407,16 +407,6 @@ __always_inline static int handle_xattr(struct metrics_by_hook_t* hook_metrics, return 0; } - // inode hooks don't provide a struct path, so filename is left empty. - // __submit_event requires a valid pointer for bpf_probe_read_str. - struct bound_path_t* bound_path = get_bound_path(BOUND_PATH_MAIN); - if (bound_path == NULL) { - args.metrics->error++; - return 0; - } - bound_path->path[0] = '\0'; - args.filename = bound_path->path; - submit_xattr_event(&args, event_type, xattr_name); return 0; } From b7af95d825e27887b1986c06a5b77e3ccea2819f Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 17 Jun 2026 10:51:39 -0700 Subject: [PATCH 17/19] Added link to XATTR_NAME_MAX --- fact-ebpf/src/bpf/types.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index d7f8a21f..3d3ec49e 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -15,7 +15,8 @@ #define LINEAGE_MAX 2 -// Matches Linux kernel XATTR_NAME_MAX (255) + null terminator +// Matches Linux kernel XATTR_NAME_MAX (255) + null terminator. +// https://github.com/torvalds/linux/blob/66affa37cfac0aec061cc4bcf4a065b0c52f7e19/include/uapi/linux/limits.h#L15 #define XATTR_NAME_MAX_LEN 256 #define LPM_SIZE_MAX 256 From dd9da9bb9005ddad9675664d0429f794f3f0c163 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 17 Jun 2026 11:39:24 -0700 Subject: [PATCH 18/19] Tests ignore events of unexpected type --- tests/server.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/server.py b/tests/server.py index bb834a49..2056bdad 100644 --- a/tests/server.py +++ b/tests/server.py @@ -109,12 +109,11 @@ def _wait_events( if len(events) == 0: return elif strict: - # Container events are fully controlled by the test, - # so any mismatch is a real failure. Node events may - # include system noise (e.g. SELinux xattr changes) - # that the test cannot predict, so they are skipped. - is_container_event = bool(msg.process.container_id) - if is_container_event: + # In strict mode, fail when the event type matches but + # the content differs. Events of a different type are + # skipped since they may be system noise that the test + # cannot predict (e.g. SELinux xattr changes). + if 'event_type' not in diff: raise ValueError(json.dumps(diff, indent=4)) def wait_events(self, events: list[Event], strict: bool = True): From ba5673510bd5cbabf1d5a80fad8e0a873013568b Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 18 Jun 2026 15:23:13 -0700 Subject: [PATCH 19/19] Updated protobuf definitions --- fact/src/event/mod.rs | 18 ++---------------- tests/event.py | 7 ++++--- tests/test_xattr.py | 16 ++++++++-------- third_party/stackrox | 2 +- 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index f09ebb4a..1c8a591d 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -434,7 +434,6 @@ impl FileData { FileData::SetXattr(XattrFileData { inner, xattr_name, - operation: XattrOperation::Set, }) } file_activity_type_t::FILE_ACTIVITY_REMOVEXATTR => { @@ -444,7 +443,6 @@ impl FileData { FileData::RemoveXattr(XattrFileData { inner, xattr_name, - operation: XattrOperation::Remove, }) } invalid => unreachable!("Invalid event type: {invalid:?}"), @@ -475,11 +473,11 @@ impl From for fact_api::file_activity::File { } FileData::SetXattr(event) => { let f_act = fact_api::FileXattrChange::from(event); - fact_api::file_activity::File::Xattr(f_act) + fact_api::file_activity::File::XattrSet(f_act) } FileData::RemoveXattr(event) => { let f_act = fact_api::FileXattrChange::from(event); - fact_api::file_activity::File::Xattr(f_act) + fact_api::file_activity::File::XattrRemove(f_act) } FileData::Unlink(event) => { let activity = Some(fact_api::FileActivityBase::from(event)); @@ -645,30 +643,18 @@ impl PartialEq for RenameFileData { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] -pub enum XattrOperation { - Set, - Remove, -} - #[derive(Debug, Clone, Serialize)] pub struct XattrFileData { inner: BaseFileData, xattr_name: String, - operation: XattrOperation, } impl From for fact_api::FileXattrChange { fn from(value: XattrFileData) -> Self { let activity = fact_api::FileActivityBase::from(value.inner); - let operation = match value.operation { - XattrOperation::Set => fact_api::file_xattr_change::Operation::Set, - XattrOperation::Remove => fact_api::file_xattr_change::Operation::Remove, - }; fact_api::FileXattrChange { activity: Some(activity), xattr_name: value.xattr_name, - operation: operation.into(), } } } diff --git a/tests/event.py b/tests/event.py index d8639ac5..07bc88cf 100644 --- a/tests/event.py +++ b/tests/event.py @@ -36,7 +36,8 @@ class EventType(Enum): PERMISSION = 4 OWNERSHIP = 5 RENAME = 6 - XATTR = 7 + XATTR_SET = 7 + XATTR_REMOVE = 8 class Process: @@ -385,7 +386,7 @@ def diff(self, other: FileActivity) -> dict | None: self.owner_gid, event_field.gid, ) - elif self.event_type == EventType.XATTR: + elif self.event_type in (EventType.XATTR_SET, EventType.XATTR_REMOVE): Event._diff_field( diff, 'xattr_name', @@ -415,7 +416,7 @@ def __str__(self) -> str: f', old_host_path="{self.old_host_path}"' ) - if self.event_type == EventType.XATTR: + if self.event_type in (EventType.XATTR_SET, EventType.XATTR_REMOVE): s += f', xattr_name="{self.xattr_name}"' s += ')' diff --git a/tests/test_xattr.py b/tests/test_xattr.py index 61c89d68..0cb7ddbf 100644 --- a/tests/test_xattr.py +++ b/tests/test_xattr.py @@ -32,7 +32,7 @@ def test_setxattr( [ Event( process=process, - event_type=EventType.XATTR, + event_type=EventType.XATTR_SET, file='', host_path=test_file, xattr_name='user.fact_test', @@ -62,7 +62,7 @@ def test_removexattr( [ Event( process=process, - event_type=EventType.XATTR, + event_type=EventType.XATTR_REMOVE, file='', host_path=test_file, xattr_name='user.fact_remove', @@ -93,21 +93,21 @@ def test_setxattr_multiple( [ Event( process=process, - event_type=EventType.XATTR, + event_type=EventType.XATTR_SET, file='', host_path=test_file, xattr_name='user.attr1', ), Event( process=process, - event_type=EventType.XATTR, + event_type=EventType.XATTR_SET, file='', host_path=test_file, xattr_name='user.attr2', ), Event( process=process, - event_type=EventType.XATTR, + event_type=EventType.XATTR_SET, file='', host_path=test_file, xattr_name='user.attr3', @@ -147,7 +147,7 @@ def test_setxattr_ignored( [ Event( process=process, - event_type=EventType.XATTR, + event_type=EventType.XATTR_SET, file='', host_path=test_file, xattr_name='user.monitored', @@ -195,7 +195,7 @@ def test_setxattr_new_file( [ Event( process=process, - event_type=EventType.XATTR, + event_type=EventType.XATTR_SET, file='', host_path=test_file, xattr_name='user.new_file', @@ -257,7 +257,7 @@ def test_setxattr_utf8_filenames( [ Event( process=process, - event_type=EventType.XATTR, + event_type=EventType.XATTR_SET, file='', host_path=fut_str, xattr_name='user.utf8_test', diff --git a/third_party/stackrox b/third_party/stackrox index d8239afc..2bf3c595 160000 --- a/third_party/stackrox +++ b/third_party/stackrox @@ -1 +1 @@ -Subproject commit d8239afc33c2041c92fba2cb485738cf63a634ef +Subproject commit 2bf3c595613cade50564c66228414053fcfa3a3f