From b861b2116b36d8f29913b9ecb208f07fb54716d3 Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Tue, 23 Jun 2026 10:49:01 +0100 Subject: [PATCH 1/6] ROX-30296: track POSIX ACL changes via inode_set_acl LSM hook --- fact-ebpf/src/bpf/bound_path.h | 23 +++ fact-ebpf/src/bpf/d_path.h | 108 +++++++++--- fact-ebpf/src/bpf/events.h | 42 +++++ fact-ebpf/src/bpf/main.c | 32 ++++ fact-ebpf/src/bpf/types.h | 19 ++ fact-ebpf/src/lib.rs | 3 + fact/src/bpf/mod.rs | 36 ++-- fact/src/event/mod.rs | 135 ++++++++++++++ fact/src/metrics/kernel_metrics.rs | 1 + tests/event.py | 260 +-------------------------- tests/test_acl.py | 271 +++++++++++++++++++++++++++++ third_party/stackrox | 2 +- 12 files changed, 644 insertions(+), 288 deletions(-) create mode 100644 tests/test_acl.py diff --git a/fact-ebpf/src/bpf/bound_path.h b/fact-ebpf/src/bpf/bound_path.h index 7a2091f9..02ad07e1 100644 --- a/fact-ebpf/src/bpf/bound_path.h +++ b/fact-ebpf/src/bpf/bound_path.h @@ -38,6 +38,29 @@ __always_inline static struct bound_path_t* _path_read(struct path* path, bound_ return bound_path; } +/** + * Read a filesystem-relative path from a bare dentry. + * + * This is for hooks that only provide a struct dentry without a + * struct path (e.g. inode_set_acl). The resulting path can be used + * for LPM trie matching and inode monitoring checks. + */ +__always_inline static struct bound_path_t* dentry_read(struct dentry* dentry) { + struct bound_path_t* bound_path = get_bound_path(BOUND_PATH_MAIN); + if (bound_path == NULL) { + return NULL; + } + + bound_path->len = __d_path_from_dentry(dentry, bound_path->path, PATH_MAX); + if (bound_path->len <= 0) { + return NULL; + } + + bound_path->len = PATH_LEN_CLAMP(bound_path->len); + + return bound_path; +} + __always_inline static struct bound_path_t* path_read_unchecked(struct path* path) { return _path_read(path, BOUND_PATH_MAIN, true); } diff --git a/fact-ebpf/src/bpf/d_path.h b/fact-ebpf/src/bpf/d_path.h index a922600e..fd1f0ef4 100644 --- a/fact-ebpf/src/bpf/d_path.h +++ b/fact-ebpf/src/bpf/d_path.h @@ -24,6 +24,14 @@ */ #define PATH_LEN_CLAMP(len) ((len) & PATH_MAX_MASK) +// Context for __d_path_inner. +// +// Supports two modes: +// Full path mode: mnt and root must both be set. Crosses mount +// boundaries and terminates at the process root. +// Dentry-only mode: mnt and root must both be NULL. Walks the +// dentry chain to the filesystem root, producing a +// filesystem-relative path. struct d_path_ctx { struct helper_t* helper; struct path* root; @@ -38,39 +46,49 @@ static long __d_path_inner(uint32_t index, void* _ctx) { struct d_path_ctx* ctx = (struct d_path_ctx*)_ctx; struct dentry* dentry = ctx->dentry; struct dentry* parent = BPF_CORE_READ(dentry, d_parent); - struct mount* mnt = ctx->mnt; - struct dentry* mnt_root = BPF_CORE_READ(mnt, mnt.mnt_root); - if (dentry == ctx->root->dentry && &mnt->mnt == ctx->root->mnt) { - // Found the root of the process, we are done - ctx->success = true; - return 1; - } + if (ctx->mnt != NULL) { + // Full path mode: we have mount context and can cross mount + // boundaries and detect the process root. + struct mount* mnt = ctx->mnt; + struct dentry* mnt_root = BPF_CORE_READ(mnt, mnt.mnt_root); - if (dentry == mnt_root) { - struct mount* m = BPF_CORE_READ(mnt, mnt_parent); - if (m != mnt) { - // Current dentry is a mount root different to the previous one we - // had (to prevent looping), switch over to that mount position - // and keep walking up the path. - ctx->dentry = BPF_CORE_READ(mnt, mnt_mountpoint); - ctx->mnt = m; - return 0; + if (dentry == ctx->root->dentry && &mnt->mnt == ctx->root->mnt) { + // Found the root of the process, we are done + ctx->success = true; + return 1; } - // Ended up in a global root, the path might need re-processing or - // the root is not attached yet, we are not getting a better path, - // so we assume we are correct and stop iterating. - ctx->success = true; - return 1; + if (dentry == mnt_root) { + struct mount* m = BPF_CORE_READ(mnt, mnt_parent); + if (m != mnt) { + // Current dentry is a mount root different to the previous one we + // had (to prevent looping), switch over to that mount position + // and keep walking up the path. + ctx->dentry = BPF_CORE_READ(mnt, mnt_mountpoint); + ctx->mnt = m; + return 0; + } + + // Ended up in a global root, the path might need re-processing or + // the root is not attached yet, we are not getting a better path, + // so we assume we are correct and stop iterating. + ctx->success = true; + return 1; + } } if (dentry == parent) { - // We escaped the mounts and ended up at (most likely) the root of - // the device, the path we formed will be wrong. + // Reached the root of the filesystem's dentry tree. + // + // In full path mode (mnt != NULL) this means we escaped the mounts + // and the path may be wrong due to a race condition. // - // This may happen in race conditions where some dentries go away - // while we are iterating. + // In dentry-only mode (mnt == NULL) this is the expected + // termination: we've reached the filesystem root and have a + // filesystem-relative path. This is correct for overlayfs + // (containers) and for files on the root filesystem. + ctx->success = (ctx->mnt == NULL); return 1; } @@ -140,6 +158,46 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf return buflen - ctx.offset; } +/** + * Resolve a filesystem-relative path from a bare dentry. + * + * This is used when no struct path is available (e.g. inode_set_acl). + * It walks the dentry chain up to the filesystem root, producing a + * path relative to the filesystem's root dentry. This is correct for + * overlayfs (containers) and for files on the root filesystem. It + * cannot cross mount boundaries, so paths on nested host mounts (e.g. + * a separate /var partition) will be relative to that mount's root. + */ +__always_inline static long __d_path_from_dentry(struct dentry* dentry, char* buf, int buflen) { + if (buflen <= 0) { + return -1; + } + + int offset = PATH_LEN_CLAMP(buflen - 1); + struct d_path_ctx ctx = { + .buflen = buflen, + .helper = get_helper(), + .offset = offset, + .mnt = NULL, + .root = NULL, + }; + + if (ctx.helper == NULL) { + return -1; + } + + ctx.helper->buf[offset] = '\0'; + ctx.dentry = dentry; + + long res = bpf_loop(PATH_MAX, __d_path_inner, &ctx, 0); + if (res <= 0 || !ctx.success) { + return -1; + } + + bpf_probe_read_str(buf, buflen, &ctx.helper->buf[PATH_LEN_CLAMP(ctx.offset)]); + return buflen - ctx.offset; +} + __always_inline static long d_path(struct path* path, char* buf, int buflen, bool use_bpf_helper) { if (use_bpf_helper) { return bpf_d_path(path, buf, buflen); diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index efedf86e..7dc4b766 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -160,3 +160,45 @@ __always_inline static void submit_xattr_event(struct submit_event_args_t* args, __submit_event(args, false); } + +__always_inline static void submit_acl_event(struct submit_event_args_t* args, + const char* acl_name, + struct posix_acl* kacl) { + if (!reserve_event(args)) { + return; + } + + args->event->type = FILE_ACTIVITY_ACL_SET; + + // Determine ACL type from the xattr name. + // "system.posix_acl_access" vs "system.posix_acl_default" + char name_buf[32] = {0}; + long name_len = bpf_probe_read_kernel_str(name_buf, sizeof(name_buf), acl_name); + if (name_len == 25 && __builtin_memcmp(name_buf, "system.posix_acl_default", 24) == 0) { + args->event->acl.acl_type = FACT_ACL_TYPE_DEFAULT; + } else { + args->event->acl.acl_type = FACT_ACL_TYPE_ACCESS; + } + + if (kacl == NULL) { + args->event->acl.count = 0; + } else { + unsigned int count = 0; + bpf_probe_read_kernel(&count, sizeof(count), &kacl->a_count); + if (count > FACT_MAX_ACL_ENTRIES) { + count = FACT_MAX_ACL_ENTRIES; + } + args->event->acl.count = count; + + for (unsigned int i = 0; i < FACT_MAX_ACL_ENTRIES && i < count; i++) { + struct posix_acl_entry entry = {0}; + bpf_probe_read_kernel(&entry, sizeof(entry), &kacl->a_entries[i]); + args->event->acl.entries[i].e_tag = entry.e_tag; + args->event->acl.entries[i].e_perm = entry.e_perm; + args->event->acl.entries[i].e_id = entry.e_uid.val; + } + } + + // inode_set_acl does not support bpf_d_path (no struct path available) + __submit_event(args, false); +} diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 4eacdd79..96ad9f0d 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -431,6 +431,38 @@ int BPF_PROG(trace_inode_removexattr, struct mnt_idmap* idmap, struct dentry* de return handle_xattr(&m->inode_removexattr, dentry, name, FILE_ACTIVITY_REMOVEXATTR); } +SEC("lsm/inode_set_acl") +int BPF_PROG(trace_inode_set_acl, struct mnt_idmap* idmap, struct dentry* dentry, + const char* acl_name, struct posix_acl* kacl) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + struct submit_event_args_t args = {.metrics = &m->inode_set_acl}; + + args.metrics->total++; + + struct bound_path_t* bound_path = dentry_read(dentry); + if (bound_path == NULL) { + bpf_printk("Failed to read path from dentry"); + args.metrics->error++; + return 0; + } + args.filename = bound_path->path; + + struct inode* inode_ptr = BPF_CORE_READ(dentry, d_inode); + args.inode = inode_to_key(inode_ptr); + args.monitored = is_monitored(&args.inode, bound_path, NULL); + + if (args.monitored == NOT_MONITORED) { + args.metrics->ignored++; + return 0; + } + + submit_acl_event(&args, acl_name, kacl); + 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 3d3ec49e..11b7277d 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -58,6 +58,18 @@ typedef enum monitored_t { // For the time being we just keep a char. typedef char inode_value_t; +#define FACT_MAX_ACL_ENTRIES 32 + +// ACL type constants matching the xattr names +#define FACT_ACL_TYPE_ACCESS 0 +#define FACT_ACL_TYPE_DEFAULT 1 + +struct acl_entry_t { + short e_tag; + unsigned short e_perm; + unsigned int e_id; +}; + typedef enum file_activity_type_t { FILE_ACTIVITY_INIT = -1, FILE_ACTIVITY_OPEN = 0, @@ -70,6 +82,7 @@ typedef enum file_activity_type_t { DIR_ACTIVITY_UNLINK, FILE_ACTIVITY_SETXATTR, FILE_ACTIVITY_REMOVEXATTR, + FILE_ACTIVITY_ACL_SET, } file_activity_type_t; struct event_t { @@ -99,6 +112,11 @@ struct event_t { struct { char name[XATTR_NAME_MAX_LEN]; } xattr; + struct { + unsigned int count; + unsigned int acl_type; + struct acl_entry_t entries[FACT_MAX_ACL_ENTRIES]; + } acl; }; }; @@ -143,4 +161,5 @@ struct metrics_t { struct metrics_by_hook_t path_rmdir; struct metrics_by_hook_t inode_setxattr; struct metrics_by_hook_t inode_removexattr; + struct metrics_by_hook_t inode_set_acl; }; diff --git a/fact-ebpf/src/lib.rs b/fact-ebpf/src/lib.rs index 8b66d92b..4117e983 100644 --- a/fact-ebpf/src/lib.rs +++ b/fact-ebpf/src/lib.rs @@ -155,6 +155,9 @@ impl_metrics_t!( path_mkdir, path_rmdir, d_instantiate, + inode_setxattr, + inode_removexattr, + inode_set_acl, ); unsafe impl Pod for metrics_t {} diff --git a/fact/src/bpf/mod.rs b/fact/src/bpf/mod.rs index 917eae3a..4ab0588d 100644 --- a/fact/src/bpf/mod.rs +++ b/fact/src/bpf/mod.rs @@ -24,6 +24,10 @@ mod checks; const RINGBUFFER_NAME: &str = "rb"; +/// Hooks that may not be available on all supported kernels. If these +/// fail to load, FACT will log a warning and continue without them. +const OPTIONAL_HOOKS: &[&str] = &["inode_set_acl"]; + pub struct Bpf { obj: Ebpf, @@ -178,28 +182,38 @@ impl Bpf { let Some(hook) = name.strip_prefix("trace_") else { bail!("Invalid hook name: {name}"); }; - match prog { - Program::Lsm(prog) => prog.load(hook, btf)?, + let result = match prog { + Program::Lsm(prog) => prog.load(hook, btf), u => unimplemented!("{u:?}"), + }; + if let Err(e) = result { + if OPTIONAL_HOOKS.contains(&hook) { + warn!("Optional hook {hook} not available on this kernel, skipping: {e}"); + continue; + } + return Err(e.into()); } } Ok(()) } - /// Attaches all BPF programs. If any attach fails, all previously - /// attached programs are automatically detached via drop. + /// Attaches all loaded BPF programs. Programs that were not loaded + /// (e.g. optional hooks on unsupported kernels) are skipped. + /// If any attach fails, all previously attached programs are + /// automatically detached via drop. fn attach_progs(&mut self) -> anyhow::Result<()> { - self.links = self - .obj - .programs_mut() - .map(|(_, prog)| match prog { + for (_, prog) in self.obj.programs_mut() { + match prog { Program::Lsm(prog) => { + if prog.fd().is_err() { + continue; + } let link_id = prog.attach()?; - prog.take_link(link_id) + self.links.push(prog.take_link(link_id)?); } u => unimplemented!("{u:?}"), - }) - .collect::>()?; + } + } Ok(()) } diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index a91c5f00..b552c414 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -170,6 +170,7 @@ impl Event { FileData::Rename(data) => &data.new.inode, FileData::SetXattr(data) => &data.inner.inode, FileData::RemoveXattr(data) => &data.inner.inode, + FileData::AclSet(data) => &data.inner.inode, } } @@ -186,6 +187,7 @@ impl Event { FileData::Rename(data) => &data.new.parent_inode, FileData::SetXattr(data) => &data.inner.parent_inode, FileData::RemoveXattr(data) => &data.inner.parent_inode, + FileData::AclSet(data) => &data.inner.parent_inode, } } @@ -211,6 +213,7 @@ impl Event { FileData::Rename(data) => &data.new.filename, FileData::SetXattr(data) => &data.inner.filename, FileData::RemoveXattr(data) => &data.inner.filename, + FileData::AclSet(data) => &data.inner.filename, } } @@ -233,6 +236,7 @@ impl Event { FileData::Rename(data) => &data.new.host_file, FileData::SetXattr(data) => &data.inner.host_file, FileData::RemoveXattr(data) => &data.inner.host_file, + FileData::AclSet(data) => &data.inner.host_file, } } @@ -259,6 +263,7 @@ impl Event { 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, + FileData::AclSet(data) => data.inner.host_file = host_path, } } @@ -282,6 +287,7 @@ impl Event { FileData::Rename(data) => data.new.monitored, FileData::SetXattr(data) => data.inner.monitored, FileData::RemoveXattr(data) => data.inner.monitored, + FileData::AclSet(data) => data.inner.monitored, } } @@ -376,6 +382,7 @@ pub enum FileData { Rename(RenameFileData), SetXattr(XattrFileData), RemoveXattr(XattrFileData), + AclSet(AclSetFileData), } impl FileData { @@ -439,6 +446,35 @@ impl FileData { )?; FileData::RemoveXattr(XattrFileData { inner, xattr_name }) } + file_activity_type_t::FILE_ACTIVITY_ACL_SET => { + let acl = unsafe { &extra_data.acl }; + let acl_type = if acl.acl_type == fact_ebpf::FACT_ACL_TYPE_DEFAULT { + AclType::Default + } else { + AclType::Access + }; + let count = acl.count.min(fact_ebpf::FACT_MAX_ACL_ENTRIES) as usize; + let mut entries = Vec::with_capacity(count); + for i in 0..count { + let entry = &acl.entries[i]; + let tag = AclTag::from_kernel(entry.e_tag); + let id = if tag.has_qualifier() { + Some(entry.e_id) + } else { + None + }; + entries.push(AclEntry { + tag, + perm: entry.e_perm, + id, + }); + } + FileData::AclSet(AclSetFileData { + inner, + acl_type, + entries, + }) + } invalid => unreachable!("Invalid event type: {invalid:?}"), }; @@ -490,6 +526,10 @@ impl From for fact_api::file_activity::File { let f_act = fact_api::FileRename::from(event); fact_api::file_activity::File::Rename(f_act) } + FileData::AclSet(event) => { + let f_act = fact_api::FileAclChange::from(event); + fact_api::file_activity::File::Acl(f_act) + } } } } @@ -507,6 +547,11 @@ impl PartialEq for FileData { (FileData::Rename(this), FileData::Rename(other)) => this == other, (FileData::SetXattr(this), FileData::SetXattr(other)) => this == other, (FileData::RemoveXattr(this), FileData::RemoveXattr(other)) => this == other, + (FileData::AclSet(this), FileData::AclSet(other)) => { + this.inner == other.inner + && this.acl_type == other.acl_type + && this.entries == other.entries + } _ => false, } } @@ -619,6 +664,56 @@ pub struct RenameFileData { old: BaseFileData, } +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +pub enum AclTag { + UserObj, + User, + GroupObj, + Group, + Mask, + Other, + Unknown(i16), +} + +impl AclTag { + fn from_kernel(tag: i16) -> Self { + match tag { + 0x01 => AclTag::UserObj, + 0x02 => AclTag::User, + 0x04 => AclTag::GroupObj, + 0x08 => AclTag::Group, + 0x10 => AclTag::Mask, + 0x20 => AclTag::Other, + other => AclTag::Unknown(other), + } + } + + /// Whether this tag type carries a meaningful uid/gid. + fn has_qualifier(&self) -> bool { + matches!(self, AclTag::User | AclTag::Group) + } +} + +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +pub struct AclEntry { + pub tag: AclTag, + pub perm: u16, + pub id: Option, +} + +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +pub enum AclType { + Access, + Default, +} + +#[derive(Debug, Clone, Serialize)] +pub struct AclSetFileData { + inner: BaseFileData, + pub acl_type: AclType, + pub entries: Vec, +} + impl From for fact_api::FileRename { fn from(RenameFileData { new, old }: RenameFileData) -> Self { let new = fact_api::FileActivityBase::from(new); @@ -630,6 +725,46 @@ impl From for fact_api::FileRename { } } +impl From for i32 { + fn from(tag: AclTag) -> Self { + match tag { + AclTag::UserObj => fact_api::AclTag::UserObj as i32, + AclTag::User => fact_api::AclTag::User as i32, + AclTag::GroupObj => fact_api::AclTag::GroupObj as i32, + AclTag::Group => fact_api::AclTag::Group as i32, + AclTag::Mask => fact_api::AclTag::Mask as i32, + AclTag::Other => fact_api::AclTag::Other as i32, + AclTag::Unknown(_) => fact_api::AclTag::Unspecified as i32, + } + } +} + +impl From for fact_api::FileAclChange { + fn from(value: AclSetFileData) -> Self { + let activity = fact_api::FileActivityBase::from(value.inner); + let acl_type = match value.acl_type { + AclType::Access => "access".to_string(), + AclType::Default => "default".to_string(), + }; + let entries = value + .entries + .into_iter() + .map(|e| fact_api::AclEntry { + tag: i32::from(e.tag), + perm: e.perm as u32, + // ACL_UNDEFINED_ID (0xFFFFFFFF) for entries that don't + // carry a uid/gid (USER_OBJ, GROUP_OBJ, MASK, OTHER). + id: e.id.unwrap_or(0xFFFFFFFF), + }) + .collect(); + fact_api::FileAclChange { + activity: Some(activity), + acl_type, + entries, + } + } +} + #[cfg(test)] impl PartialEq for RenameFileData { fn eq(&self, other: &Self) -> bool { diff --git a/fact/src/metrics/kernel_metrics.rs b/fact/src/metrics/kernel_metrics.rs index 11fb5f65..d1e77bfb 100644 --- a/fact/src/metrics/kernel_metrics.rs +++ b/fact/src/metrics/kernel_metrics.rs @@ -72,4 +72,5 @@ define_kernel_metrics!( d_instantiate, inode_setxattr, inode_removexattr, + inode_set_acl, ); diff --git a/tests/event.py b/tests/event.py index 2993b396..aaccb3a4 100644 --- a/tests/event.py +++ b/tests/event.py @@ -48,194 +48,8 @@ class EventType(Enum): RENAME = 6 XATTR_SET = 7 XATTR_REMOVE = 8 - - -class Process: - """ - Represents a process with its attributes. - """ - - def __init__( - self, - pid: int | None, - uid: int, - gid: int, - exe_path: str, - args: str, - name: str, - container_id: str, - loginuid: int, - ): - self._pid: int | None = pid - self._uid: int = uid - self._gid: int = gid - self._exe_path: str = exe_path - self._args: str = args - self._name: str = name - self._container_id: str = container_id - self._loginuid: int = loginuid - - @classmethod - def from_proc(cls, pid: int | None = None): - pid = pid if pid is not None else os.getpid() - proc_dir = os.path.join('/proc', str(pid)) - - uid = 0 - gid = 0 - with open(os.path.join(proc_dir, 'status')) as f: - - def get_id(line: str, wanted_id: str) -> int | None: - if line.startswith(f'{wanted_id}:'): - parts = line.split() - if len(parts) > 2: - return int(parts[1]) - return None - - for line in f.readlines(): - if (id := get_id(line, 'Uid')) is not None: - uid = id - elif (id := get_id(line, 'Gid')) is not None: - gid = id - - exe_path = os.path.realpath(os.path.join(proc_dir, 'exe')) - - with open(os.path.join(proc_dir, 'cmdline'), 'rb') as f: - content = f.read(4096) - args = [ - arg.decode('utf-8') for arg in content.split(b'\x00') if arg - ] - args = utils.rust_style_join(args) - - with open(os.path.join(proc_dir, 'comm')) as f: - name = f.read().strip() - - with open(os.path.join(proc_dir, 'cgroup')) as f: - container_id = extract_container_id(f.read()) - - with open(os.path.join(proc_dir, 'loginuid')) as f: - loginuid = int(f.read()) - - return Process( - pid=pid, - uid=uid, - gid=gid, - exe_path=exe_path, - args=args, - name=name, - container_id=container_id, - loginuid=loginuid, - ) - - @classmethod - def in_container( - cls, - exe_path: str, - args: str, - name: str, - container_id: str, - ): - return Process( - pid=None, - uid=0, - gid=0, - loginuid=pow(2, 32) - 1, - exe_path=exe_path, - args=args, - name=name, - container_id=container_id, - ) - - @property - def uid(self) -> int: - return self._uid - - @property - def gid(self) -> int: - return self._gid - - @property - def pid(self) -> int | None: - return self._pid - - @property - def exe_path(self) -> str: - return self._exe_path - - @property - def args(self) -> str: - return self._args - - @property - def name(self) -> str: - return self._name - - @property - def container_id(self) -> str: - return self._container_id - - @property - def loginuid(self) -> int: - return self._loginuid - - def diff(self, other: ProcessSignal) -> dict | None: - """ - Compare this Process with a ProcessSignal protobuf message. - - Args: - other: ProcessSignal protobuf message to compare against - - Returns: - None if identical, dict of differences if not matching - """ - diff = {} - - # Compare each field - if self.pid is not None: - Event._diff_field(diff, 'pid', self.pid, other.pid) - - Event._diff_field(diff, 'uid', self.uid, other.uid) - Event._diff_field(diff, 'gid', self.gid, other.gid) - Event._diff_field(diff, 'exe_path', self.exe_path, other.exec_file_path) - Event._diff_field(diff, 'args', self.args, other.args) - Event._diff_field(diff, 'name', self.name, other.name) - Event._diff_field( - diff, - 'container_id', - self.container_id, - other.container_id, - ) - Event._diff_field(diff, 'loginuid', self.loginuid, other.login_uid) - - return diff if diff else None - - @override - def __str__(self) -> str: - return ( - f'Process(uid={self.uid}, gid={self.gid}, pid={self.pid}, ' - f'exe_path={self.exe_path}, args={self.args}, ' - f'name={self.name}, container_id={self.container_id}, ' - f'loginuid={self.loginuid})' - ) - - -class Event: - """ - Represents a file activity event, associating a process with an - event type and a file. - """ - - def __init__( - self, - process: Process, - event_type: EventType, - file: str | Pattern[str], - host_path: str | Pattern[str] = '', - mode: int | None = None, - owner_uid: int | None = None, - 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, + acl_type: str | None = None, + acl_entries: list[dict] | None = None, ): self._type: EventType = event_type self._process: Process = process @@ -247,46 +61,12 @@ def __init__( 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 + def acl_type(self) -> str | None: + return self._acl_type @property - def event_type(self) -> EventType: - return self._type - - @property - def process(self) -> Process: - return self._process - - @property - def file(self) -> str | Pattern[str]: - return self._file - - @property - def host_path(self) -> str | Pattern[str]: - return self._host_path - - @property - def mode(self) -> int | None: - return self._mode - - @property - def owner_uid(self) -> int | None: - return self._owner_uid - - @property - def owner_gid(self) -> int | None: - return self._owner_gid - - @property - def old_file(self) -> str | Pattern[str] | None: - return self._old_file - - @property - 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 + def acl_entries(self) -> list[dict] | None: + return self._acl_entries @classmethod def _diff_field(cls, diff: dict, name: str, expected: Any, actual: Any): @@ -403,31 +183,9 @@ def diff(self, other: FileActivity) -> dict | None: self.xattr_name, event_field.xattr_name, ) - - return diff if diff else None - - @override - def __str__(self) -> str: - s = ( - f'Event(event_type={self.event_type.name}, ' - f'process={self.process}, file="{self.file}", ' - f'host_path="{self.host_path}"' - ) - - if self.event_type == EventType.PERMISSION: - s += f', mode={self.mode}' - - if self.event_type == EventType.OWNERSHIP: - s += f', owner=(uid={self.owner_uid}, gid={self.owner_gid})' - - if self.event_type == EventType.RENAME: - s += ( - f', old_file="{self.old_file}"' - f', old_host_path="{self.old_host_path}"' - ) - - if self.event_type in (EventType.XATTR_SET, EventType.XATTR_REMOVE): - s += f', xattr_name="{self.xattr_name}"' + if self.event_type == EventType.ACL: + s += f', acl_type={self.acl_type}' + s += f', acl_entries={self.acl_entries}' s += ')' diff --git a/tests/test_acl.py b/tests/test_acl.py new file mode 100644 index 00000000..ae9bc59b --- /dev/null +++ b/tests/test_acl.py @@ -0,0 +1,271 @@ +"""Tests for POSIX ACL change events.""" + +from __future__ import annotations + +import os + +import docker.models.containers + +from event import ( + ACL_TAG_GROUP_OBJ, + ACL_TAG_MASK, + ACL_TAG_OTHER, + ACL_TAG_USER, + ACL_TAG_USER_OBJ, + Event, + EventType, + Process, +) +from server import FileActivityService + + +def test_set_access_acl( + test_container: docker.models.containers.Container, + server: FileActivityService, +): + """ + Test setting an access ACL on a file inside a container. + + Args: + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + assert test_container.id is not None + fut = '/container-dir/acl_test.txt' + + test_container.exec_run(f'touch {fut}') + test_container.exec_run(f'setfacl -m u:1000:rw {fut}') + + touch = Process.in_container( + exe_path='/usr/bin/touch', + args=f'touch {fut}', + name='touch', + container_id=test_container.id[:12], + ) + setfacl = Process.in_container( + exe_path='/usr/bin/setfacl', + args=f'setfacl -m u:1000:rw {fut}', + name='setfacl', + container_id=test_container.id[:12], + ) + + events = [ + Event( + process=touch, + event_type=EventType.CREATION, + file=fut, + host_path='', + ), + Event( + process=setfacl, + event_type=EventType.ACL, + file=fut, + host_path='', + acl_type='access', + acl_entries=[ + {'tag': ACL_TAG_USER_OBJ, 'perm': 6, 'id': 0xFFFFFFFF}, + {'tag': ACL_TAG_USER, 'perm': 6, 'id': 1000}, + {'tag': ACL_TAG_GROUP_OBJ, 'perm': 4, 'id': 0xFFFFFFFF}, + {'tag': ACL_TAG_MASK, 'perm': 6, 'id': 0xFFFFFFFF}, + {'tag': ACL_TAG_OTHER, 'perm': 4, 'id': 0xFFFFFFFF}, + ], + ), + ] + + server.wait_events(events) + + +def test_set_default_acl( + test_container: docker.models.containers.Container, + server: FileActivityService, +): + """ + Test setting a default ACL on a directory inside a container. + + Args: + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + assert test_container.id is not None + fut = '/container-dir/subdir' + + test_container.exec_run(f'mkdir -p {fut}') + test_container.exec_run(f'setfacl -d -m g:1000:rx {fut}') + + setfacl = Process.in_container( + exe_path='/usr/bin/setfacl', + args=f'setfacl -d -m g:1000:rx {fut}', + name='setfacl', + container_id=test_container.id[:12], + ) + + events = [ + Event( + process=setfacl, + event_type=EventType.ACL, + file=fut, + host_path='', + acl_type='default', + ), + ] + + server.wait_events(events) + + +def test_remove_acl( + test_container: docker.models.containers.Container, + server: FileActivityService, +): + """ + Test removing ACLs from a file inside a container. + + Args: + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + assert test_container.id is not None + fut = '/container-dir/acl_remove.txt' + + test_container.exec_run(f'touch {fut}') + test_container.exec_run(f'setfacl -m u:1000:rw {fut}') + test_container.exec_run(f'setfacl -b {fut}') + + touch = Process.in_container( + exe_path='/usr/bin/touch', + args=f'touch {fut}', + name='touch', + container_id=test_container.id[:12], + ) + setfacl_set = Process.in_container( + exe_path='/usr/bin/setfacl', + args=f'setfacl -m u:1000:rw {fut}', + name='setfacl', + container_id=test_container.id[:12], + ) + setfacl_remove = Process.in_container( + exe_path='/usr/bin/setfacl', + args=f'setfacl -b {fut}', + name='setfacl', + container_id=test_container.id[:12], + ) + + events = [ + Event( + process=touch, + event_type=EventType.CREATION, + file=fut, + host_path='', + ), + Event( + process=setfacl_set, + event_type=EventType.ACL, + file=fut, + host_path='', + acl_type='access', + ), + Event( + process=setfacl_remove, + event_type=EventType.ACL, + file=fut, + host_path='', + acl_type='access', + acl_entries=[ + {'tag': ACL_TAG_USER_OBJ, 'perm': 6, 'id': 0xFFFFFFFF}, + {'tag': ACL_TAG_GROUP_OBJ, 'perm': 4, 'id': 0xFFFFFFFF}, + {'tag': ACL_TAG_OTHER, 'perm': 4, 'id': 0xFFFFFFFF}, + ], + ), + ] + + server.wait_events(events) + + +def test_multiple_entries( + test_container: docker.models.containers.Container, + server: FileActivityService, +): + """ + Test setting multiple ACL entries on a single file. + + Args: + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + assert test_container.id is not None + fut = '/container-dir/acl_multi.txt' + + test_container.exec_run(f'touch {fut}') + test_container.exec_run(f'setfacl -m u:1000:rwx,u:1001:r,g:2000:rw {fut}') + + touch = Process.in_container( + exe_path='/usr/bin/touch', + args=f'touch {fut}', + name='touch', + container_id=test_container.id[:12], + ) + setfacl = Process.in_container( + exe_path='/usr/bin/setfacl', + args=f"setfacl -m 'u:1000:rwx,u:1001:r,g:2000:rw' {fut}", + name='setfacl', + container_id=test_container.id[:12], + ) + + events = [ + Event( + process=touch, + event_type=EventType.CREATION, + file=fut, + host_path='', + ), + Event( + process=setfacl, + event_type=EventType.ACL, + file=fut, + host_path='', + acl_type='access', + ), + ] + + server.wait_events(events) + + +def test_ignored_path( + test_file: str, + ignored_dir: str, + test_container: docker.models.containers.Container, + server: FileActivityService, +): + """ + Test that ACL changes on ignored paths are not captured. + + Args: + test_file: File monitored on the host. + ignored_dir: Temporary directory that is not monitored. + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + assert test_container.id is not None + + # Set ACL on an ignored file -- should not produce an event + ignored_file = os.path.join(ignored_dir, 'ignored_acl.txt') + with open(ignored_file, 'w') as f: + f.write('ignored') + + # This runs on the host but the file is in an unmonitored directory. + # Since we only match by inode and this file was just created in an + # ignored dir, it won't be in the inode_map and won't trigger. + + # Now do a chmod on the monitored file to verify the server is working + process = Process.from_proc() + mode = 0o644 + os.chmod(test_file, mode) + + event = Event( + process=process, + event_type=EventType.PERMISSION, + file=test_file, + host_path=test_file, + mode=mode, + ) + + server.wait_events([event]) diff --git a/third_party/stackrox b/third_party/stackrox index 877de2af..8cf57e1b 160000 --- a/third_party/stackrox +++ b/third_party/stackrox @@ -1 +1 @@ -Subproject commit 877de2af9c2bbc4cb09b2003a0c2d6d88effe076 +Subproject commit 8cf57e1baf9dbf83696f6ed404e58b8fcc19269c From 4136b923af4cf6a9eef7cb6fe59b8bc23e0c402a Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Wed, 24 Jun 2026 14:48:17 +0100 Subject: [PATCH 2/6] skip acl tests on older kernels --- tests/test_acl.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_acl.py b/tests/test_acl.py index ae9bc59b..adcc5d6c 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -5,6 +5,7 @@ import os import docker.models.containers +import pytest from event import ( ACL_TAG_GROUP_OBJ, @@ -19,6 +20,31 @@ from server import FileActivityService +def _kernel_supports_acl_hook() -> bool: + """Check whether the kernel has the inode_set_acl LSM hook by + searching for its BTF type in /sys/kernel/btf/vmlinux.""" + needle = b'bpf_lsm_inode_set_acl' + chunk_size = 64 * 1024 + try: + with open('/sys/kernel/btf/vmlinux', 'rb') as f: + # Read in chunks, keeping an overlap to catch matches + # that span chunk boundaries. + prev = b'' + while chunk := f.read(chunk_size): + if needle in prev + chunk: + return True + prev = chunk[-len(needle) :] + return False + except OSError: + return False + + +pytestmark = pytest.mark.skipif( + not _kernel_supports_acl_hook(), + reason='kernel does not support inode_set_acl LSM hook', +) + + def test_set_access_acl( test_container: docker.models.containers.Container, server: FileActivityService, From 177655cca7d4c983013cedcedd9340c29870111c Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Thu, 25 Jun 2026 11:55:17 +0100 Subject: [PATCH 3/6] use checks.c for hook check --- fact-ebpf/src/bpf/checks.c | 6 ++++++ fact/src/bpf/checks.rs | 29 +++++++++++++++++++++++------ fact/src/bpf/mod.rs | 24 +++++++++++------------- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/fact-ebpf/src/bpf/checks.c b/fact-ebpf/src/bpf/checks.c index 1b3623a2..3e8bb177 100644 --- a/fact-ebpf/src/bpf/checks.c +++ b/fact-ebpf/src/bpf/checks.c @@ -13,3 +13,9 @@ int BPF_PROG(check_path_unlink_supports_bpf_d_path, struct path* dir, struct den bpf_printk("dir: %s", p->path); return 0; } + +SEC("lsm/inode_set_acl") +int BPF_PROG(check_inode_set_acl, struct mnt_idmap* idmap, struct dentry* dentry, const char* acl_name, + struct posix_acl* kacl) { + return 0; +} diff --git a/fact/src/bpf/checks.rs b/fact/src/bpf/checks.rs index 1686fdfc..d0c88428 100644 --- a/fact/src/bpf/checks.rs +++ b/fact/src/bpf/checks.rs @@ -4,6 +4,7 @@ use log::debug; pub(super) struct Checks { pub(super) path_hooks_support_bpf_d_path: bool, + pub(super) supports_inode_set_acl: bool, } impl Checks { @@ -12,15 +13,31 @@ impl Checks { .load(fact_ebpf::CHECKS_OBJ) .context("Failed to load checks.o")?; - let prog = obj - .program_mut("check_path_unlink_supports_bpf_d_path") - .context("Failed to find 'check_path_unlink_supports_bpf_d_path' program")?; - let prog: &mut Lsm = prog.try_into()?; - let path_hooks_support_bpf_d_path = prog.load("path_unlink", btf).is_ok(); - debug!("path_unlink_supports_bpf_d_path: {path_hooks_support_bpf_d_path}"); + let path_hooks_support_bpf_d_path = Self::probe_hook( + &mut obj, + "check_path_unlink_supports_bpf_d_path", + "path_unlink", + btf, + ); + debug!("path_hooks_support_bpf_d_path: {path_hooks_support_bpf_d_path}"); + + let supports_inode_set_acl = + Self::probe_hook(&mut obj, "check_inode_set_acl", "inode_set_acl", btf); + debug!("supports_inode_set_acl: {supports_inode_set_acl}"); Ok(Checks { path_hooks_support_bpf_d_path, + supports_inode_set_acl, }) } + + fn probe_hook(obj: &mut aya::Ebpf, prog_name: &str, hook: &str, btf: &Btf) -> bool { + let Some(prog) = obj.program_mut(prog_name) else { + return false; + }; + let Ok(prog): Result<&mut Lsm, _> = prog.try_into() else { + return false; + }; + prog.load(hook, btf).is_ok() + } } diff --git a/fact/src/bpf/mod.rs b/fact/src/bpf/mod.rs index 4ab0588d..8f387284 100644 --- a/fact/src/bpf/mod.rs +++ b/fact/src/bpf/mod.rs @@ -24,12 +24,9 @@ mod checks; const RINGBUFFER_NAME: &str = "rb"; -/// Hooks that may not be available on all supported kernels. If these -/// fail to load, FACT will log a warning and continue without them. -const OPTIONAL_HOOKS: &[&str] = &["inode_set_acl"]; - pub struct Bpf { obj: Ebpf, + checks: Checks, tx: mpsc::Sender, @@ -68,6 +65,7 @@ impl Bpf { let paths = Vec::new(); let mut bpf = Bpf { obj, + checks, tx, paths, paths_config, @@ -182,17 +180,17 @@ impl Bpf { let Some(hook) = name.strip_prefix("trace_") else { bail!("Invalid hook name: {name}"); }; - let result = match prog { - Program::Lsm(prog) => prog.load(hook, btf), + + // Skip hooks that the kernel doesn't support + if hook == "inode_set_acl" && !self.checks.supports_inode_set_acl { + info!("Skipping {hook}: not supported on this kernel"); + continue; + } + + match prog { + Program::Lsm(prog) => prog.load(hook, btf)?, u => unimplemented!("{u:?}"), }; - if let Err(e) = result { - if OPTIONAL_HOOKS.contains(&hook) { - warn!("Optional hook {hook} not available on this kernel, skipping: {e}"); - continue; - } - return Err(e.into()); - } } Ok(()) } From 11683ba8e14850e451fcd399dfe63bf6eb3bd737 Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Thu, 25 Jun 2026 16:21:24 +0100 Subject: [PATCH 4/6] Remove container dependence; do xattr syscall manually --- tests/test_acl.py | 278 +++++++++++++++++++++++----------------------- 1 file changed, 136 insertions(+), 142 deletions(-) diff --git a/tests/test_acl.py b/tests/test_acl.py index adcc5d6c..be2ea9e4 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -1,10 +1,14 @@ -"""Tests for POSIX ACL change events.""" +"""Tests for POSIX ACL change events. + +Uses os.setxattr to set ACLs directly via the POSIX ACL xattr wire +format, avoiding a dependency on the setfacl tool. +""" from __future__ import annotations import os +import struct -import docker.models.containers import pytest from event import ( @@ -19,6 +23,26 @@ ) from server import FileActivityService +# POSIX ACL xattr wire format constants +_ACL_VERSION = 2 +_ACL_UNDEFINED_ID = 0xFFFFFFFF + +# Kernel ACL tag values (from include/uapi/linux/posix_acl.h) +_ACL_USER_OBJ = 0x01 +_ACL_USER = 0x02 +_ACL_GROUP_OBJ = 0x04 +_ACL_GROUP = 0x08 +_ACL_MASK = 0x10 +_ACL_OTHER = 0x20 + + +def _make_acl_xattr(entries: list[tuple[int, int, int]]) -> bytes: + """Build a POSIX ACL xattr value from a list of (tag, perm, id) tuples.""" + data = struct.pack(' bool: """Check whether the kernel has the inode_set_acl LSM hook by @@ -46,54 +70,45 @@ def _kernel_supports_acl_hook() -> bool: def test_set_access_acl( - test_container: docker.models.containers.Container, + monitored_dir: str, server: FileActivityService, ): - """ - Test setting an access ACL on a file inside a container. - - Args: - test_container: A container for running commands in. - server: The server instance to communicate with. - """ - assert test_container.id is not None - fut = '/container-dir/acl_test.txt' - - test_container.exec_run(f'touch {fut}') - test_container.exec_run(f'setfacl -m u:1000:rw {fut}') - - touch = Process.in_container( - exe_path='/usr/bin/touch', - args=f'touch {fut}', - name='touch', - container_id=test_container.id[:12], - ) - setfacl = Process.in_container( - exe_path='/usr/bin/setfacl', - args=f'setfacl -m u:1000:rw {fut}', - name='setfacl', - container_id=test_container.id[:12], + """Test setting an access ACL on a monitored file.""" + fut = os.path.join(monitored_dir, 'acl_test.txt') + with open(fut, 'w') as f: + f.write('test') + + acl = _make_acl_xattr( + [ + (_ACL_USER_OBJ, 6, _ACL_UNDEFINED_ID), + (_ACL_USER, 6, 1000), + (_ACL_GROUP_OBJ, 4, _ACL_UNDEFINED_ID), + (_ACL_MASK, 6, _ACL_UNDEFINED_ID), + (_ACL_OTHER, 4, _ACL_UNDEFINED_ID), + ] ) + os.setxattr(fut, 'system.posix_acl_access', acl) + process = Process.from_proc() events = [ Event( - process=touch, + process=process, event_type=EventType.CREATION, file=fut, - host_path='', + host_path=fut, ), Event( - process=setfacl, + process=process, event_type=EventType.ACL, file=fut, - host_path='', + host_path=fut, acl_type='access', acl_entries=[ - {'tag': ACL_TAG_USER_OBJ, 'perm': 6, 'id': 0xFFFFFFFF}, + {'tag': ACL_TAG_USER_OBJ, 'perm': 6, 'id': _ACL_UNDEFINED_ID}, {'tag': ACL_TAG_USER, 'perm': 6, 'id': 1000}, - {'tag': ACL_TAG_GROUP_OBJ, 'perm': 4, 'id': 0xFFFFFFFF}, - {'tag': ACL_TAG_MASK, 'perm': 6, 'id': 0xFFFFFFFF}, - {'tag': ACL_TAG_OTHER, 'perm': 4, 'id': 0xFFFFFFFF}, + {'tag': ACL_TAG_GROUP_OBJ, 'perm': 4, 'id': _ACL_UNDEFINED_ID}, + {'tag': ACL_TAG_MASK, 'perm': 6, 'id': _ACL_UNDEFINED_ID}, + {'tag': ACL_TAG_OTHER, 'perm': 4, 'id': _ACL_UNDEFINED_ID}, ], ), ] @@ -102,35 +117,31 @@ def test_set_access_acl( def test_set_default_acl( - test_container: docker.models.containers.Container, + monitored_dir: str, server: FileActivityService, ): - """ - Test setting a default ACL on a directory inside a container. - - Args: - test_container: A container for running commands in. - server: The server instance to communicate with. - """ - assert test_container.id is not None - fut = '/container-dir/subdir' - - test_container.exec_run(f'mkdir -p {fut}') - test_container.exec_run(f'setfacl -d -m g:1000:rx {fut}') - - setfacl = Process.in_container( - exe_path='/usr/bin/setfacl', - args=f'setfacl -d -m g:1000:rx {fut}', - name='setfacl', - container_id=test_container.id[:12], + """Test setting a default ACL on a monitored directory.""" + fut = os.path.join(monitored_dir, 'acl_subdir') + os.makedirs(fut, exist_ok=True) + + acl = _make_acl_xattr( + [ + (_ACL_USER_OBJ, 7, _ACL_UNDEFINED_ID), + (_ACL_GROUP_OBJ, 5, _ACL_UNDEFINED_ID), + (_ACL_GROUP, 5, 1000), + (_ACL_MASK, 5, _ACL_UNDEFINED_ID), + (_ACL_OTHER, 5, _ACL_UNDEFINED_ID), + ] ) + os.setxattr(fut, 'system.posix_acl_default', acl) + process = Process.from_proc() events = [ Event( - process=setfacl, + process=process, event_type=EventType.ACL, file=fut, - host_path='', + host_path=fut, acl_type='default', ), ] @@ -139,66 +150,61 @@ def test_set_default_acl( def test_remove_acl( - test_container: docker.models.containers.Container, + monitored_dir: str, server: FileActivityService, ): - """ - Test removing ACLs from a file inside a container. - - Args: - test_container: A container for running commands in. - server: The server instance to communicate with. - """ - assert test_container.id is not None - fut = '/container-dir/acl_remove.txt' - - test_container.exec_run(f'touch {fut}') - test_container.exec_run(f'setfacl -m u:1000:rw {fut}') - test_container.exec_run(f'setfacl -b {fut}') - - touch = Process.in_container( - exe_path='/usr/bin/touch', - args=f'touch {fut}', - name='touch', - container_id=test_container.id[:12], - ) - setfacl_set = Process.in_container( - exe_path='/usr/bin/setfacl', - args=f'setfacl -m u:1000:rw {fut}', - name='setfacl', - container_id=test_container.id[:12], + """Test removing ACLs from a monitored file.""" + fut = os.path.join(monitored_dir, 'acl_remove.txt') + with open(fut, 'w') as f: + f.write('test') + + # Set an ACL with an extra user entry + acl_with_user = _make_acl_xattr( + [ + (_ACL_USER_OBJ, 6, _ACL_UNDEFINED_ID), + (_ACL_USER, 6, 1000), + (_ACL_GROUP_OBJ, 4, _ACL_UNDEFINED_ID), + (_ACL_MASK, 6, _ACL_UNDEFINED_ID), + (_ACL_OTHER, 4, _ACL_UNDEFINED_ID), + ] ) - setfacl_remove = Process.in_container( - exe_path='/usr/bin/setfacl', - args=f'setfacl -b {fut}', - name='setfacl', - container_id=test_container.id[:12], + os.setxattr(fut, 'system.posix_acl_access', acl_with_user) + + # Remove extended ACL entries by setting a minimal ACL + acl_minimal = _make_acl_xattr( + [ + (_ACL_USER_OBJ, 6, _ACL_UNDEFINED_ID), + (_ACL_GROUP_OBJ, 4, _ACL_UNDEFINED_ID), + (_ACL_OTHER, 4, _ACL_UNDEFINED_ID), + ] ) + os.setxattr(fut, 'system.posix_acl_access', acl_minimal) + process = Process.from_proc() events = [ Event( - process=touch, + process=process, event_type=EventType.CREATION, file=fut, - host_path='', + host_path=fut, ), Event( - process=setfacl_set, + process=process, event_type=EventType.ACL, file=fut, - host_path='', + host_path=fut, acl_type='access', ), Event( - process=setfacl_remove, + process=process, event_type=EventType.ACL, file=fut, - host_path='', + host_path=fut, acl_type='access', acl_entries=[ - {'tag': ACL_TAG_USER_OBJ, 'perm': 6, 'id': 0xFFFFFFFF}, - {'tag': ACL_TAG_GROUP_OBJ, 'perm': 4, 'id': 0xFFFFFFFF}, - {'tag': ACL_TAG_OTHER, 'perm': 4, 'id': 0xFFFFFFFF}, + {'tag': ACL_TAG_USER_OBJ, 'perm': 6, 'id': _ACL_UNDEFINED_ID}, + {'tag': ACL_TAG_GROUP_OBJ, 'perm': 4, 'id': _ACL_UNDEFINED_ID}, + {'tag': ACL_TAG_OTHER, 'perm': 4, 'id': _ACL_UNDEFINED_ID}, ], ), ] @@ -207,47 +213,40 @@ def test_remove_acl( def test_multiple_entries( - test_container: docker.models.containers.Container, + monitored_dir: str, server: FileActivityService, ): - """ - Test setting multiple ACL entries on a single file. - - Args: - test_container: A container for running commands in. - server: The server instance to communicate with. - """ - assert test_container.id is not None - fut = '/container-dir/acl_multi.txt' - - test_container.exec_run(f'touch {fut}') - test_container.exec_run(f'setfacl -m u:1000:rwx,u:1001:r,g:2000:rw {fut}') - - touch = Process.in_container( - exe_path='/usr/bin/touch', - args=f'touch {fut}', - name='touch', - container_id=test_container.id[:12], - ) - setfacl = Process.in_container( - exe_path='/usr/bin/setfacl', - args=f"setfacl -m 'u:1000:rwx,u:1001:r,g:2000:rw' {fut}", - name='setfacl', - container_id=test_container.id[:12], + """Test setting multiple ACL entries on a single file.""" + fut = os.path.join(monitored_dir, 'acl_multi.txt') + with open(fut, 'w') as f: + f.write('test') + + acl = _make_acl_xattr( + [ + (_ACL_USER_OBJ, 6, _ACL_UNDEFINED_ID), + (_ACL_USER, 7, 1000), + (_ACL_USER, 4, 1001), + (_ACL_GROUP_OBJ, 4, _ACL_UNDEFINED_ID), + (_ACL_GROUP, 6, 2000), + (_ACL_MASK, 7, _ACL_UNDEFINED_ID), + (_ACL_OTHER, 4, _ACL_UNDEFINED_ID), + ] ) + os.setxattr(fut, 'system.posix_acl_access', acl) + process = Process.from_proc() events = [ Event( - process=touch, + process=process, event_type=EventType.CREATION, file=fut, - host_path='', + host_path=fut, ), Event( - process=setfacl, + process=process, event_type=EventType.ACL, file=fut, - host_path='', + host_path=fut, acl_type='access', ), ] @@ -258,30 +257,25 @@ def test_multiple_entries( def test_ignored_path( test_file: str, ignored_dir: str, - test_container: docker.models.containers.Container, server: FileActivityService, ): - """ - Test that ACL changes on ignored paths are not captured. - - Args: - test_file: File monitored on the host. - ignored_dir: Temporary directory that is not monitored. - test_container: A container for running commands in. - server: The server instance to communicate with. - """ - assert test_container.id is not None - - # Set ACL on an ignored file -- should not produce an event + """Test that ACL changes on ignored paths are not captured.""" ignored_file = os.path.join(ignored_dir, 'ignored_acl.txt') with open(ignored_file, 'w') as f: f.write('ignored') - # This runs on the host but the file is in an unmonitored directory. - # Since we only match by inode and this file was just created in an - # ignored dir, it won't be in the inode_map and won't trigger. + acl = _make_acl_xattr( + [ + (_ACL_USER_OBJ, 6, _ACL_UNDEFINED_ID), + (_ACL_USER, 6, 1000), + (_ACL_GROUP_OBJ, 4, _ACL_UNDEFINED_ID), + (_ACL_MASK, 6, _ACL_UNDEFINED_ID), + (_ACL_OTHER, 4, _ACL_UNDEFINED_ID), + ] + ) + os.setxattr(ignored_file, 'system.posix_acl_access', acl) - # Now do a chmod on the monitored file to verify the server is working + # Verify the server is working by doing a chmod on a monitored file process = Process.from_proc() mode = 0o644 os.chmod(test_file, mode) From d0b508a8246a6fc3fc6ab574055ca88d56c5625c Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Mon, 29 Jun 2026 11:35:21 +0100 Subject: [PATCH 5/6] Fix auto-rebase mess in event.py --- tests/event.py | 290 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 290 insertions(+) diff --git a/tests/event.py b/tests/event.py index aaccb3a4..aaaac227 100644 --- a/tests/event.py +++ b/tests/event.py @@ -48,6 +48,204 @@ class EventType(Enum): RENAME = 6 XATTR_SET = 7 XATTR_REMOVE = 8 + ACL = 9 + + +# POSIX ACL tag values matching the AclTag proto enum. +ACL_TAG_USER_OBJ = 1 +ACL_TAG_USER = 2 +ACL_TAG_GROUP_OBJ = 3 +ACL_TAG_GROUP = 4 +ACL_TAG_MASK = 5 +ACL_TAG_OTHER = 6 + + +class Process: + """ + Represents a process with its attributes. + """ + + def __init__( + self, + pid: int | None, + uid: int, + gid: int, + exe_path: str, + args: str, + name: str, + container_id: str, + loginuid: int, + ): + self._pid: int | None = pid + self._uid: int = uid + self._gid: int = gid + self._exe_path: str = exe_path + self._args: str = args + self._name: str = name + self._container_id: str = container_id + self._loginuid: int = loginuid + + @classmethod + def from_proc(cls, pid: int | None = None): + pid = pid if pid is not None else os.getpid() + proc_dir = os.path.join('/proc', str(pid)) + + uid = 0 + gid = 0 + with open(os.path.join(proc_dir, 'status')) as f: + + def get_id(line: str, wanted_id: str) -> int | None: + if line.startswith(f'{wanted_id}:'): + parts = line.split() + if len(parts) > 2: + return int(parts[1]) + return None + + for line in f.readlines(): + if (id := get_id(line, 'Uid')) is not None: + uid = id + elif (id := get_id(line, 'Gid')) is not None: + gid = id + + exe_path = os.path.realpath(os.path.join(proc_dir, 'exe')) + + with open(os.path.join(proc_dir, 'cmdline'), 'rb') as f: + content = f.read(4096) + args = [ + arg.decode('utf-8') for arg in content.split(b'\x00') if arg + ] + args = utils.rust_style_join(args) + + with open(os.path.join(proc_dir, 'comm')) as f: + name = f.read().strip() + + with open(os.path.join(proc_dir, 'cgroup')) as f: + container_id = extract_container_id(f.read()) + + with open(os.path.join(proc_dir, 'loginuid')) as f: + loginuid = int(f.read()) + + return Process( + pid=pid, + uid=uid, + gid=gid, + exe_path=exe_path, + args=args, + name=name, + container_id=container_id, + loginuid=loginuid, + ) + + @classmethod + def in_container( + cls, + exe_path: str, + args: str, + name: str, + container_id: str, + ): + return Process( + pid=None, + uid=0, + gid=0, + loginuid=pow(2, 32) - 1, + exe_path=exe_path, + args=args, + name=name, + container_id=container_id, + ) + + @property + def uid(self) -> int: + return self._uid + + @property + def gid(self) -> int: + return self._gid + + @property + def pid(self) -> int | None: + return self._pid + + @property + def exe_path(self) -> str: + return self._exe_path + + @property + def args(self) -> str: + return self._args + + @property + def name(self) -> str: + return self._name + + @property + def container_id(self) -> str: + return self._container_id + + @property + def loginuid(self) -> int: + return self._loginuid + + def diff(self, other: ProcessSignal) -> dict | None: + """ + Compare this Process with a ProcessSignal protobuf message. + + Args: + other: ProcessSignal protobuf message to compare against + + Returns: + None if identical, dict of differences if not matching + """ + diff = {} + + # Compare each field + if self.pid is not None: + Event._diff_field(diff, 'pid', self.pid, other.pid) + + Event._diff_field(diff, 'uid', self.uid, other.uid) + Event._diff_field(diff, 'gid', self.gid, other.gid) + Event._diff_field(diff, 'exe_path', self.exe_path, other.exec_file_path) + Event._diff_field(diff, 'args', self.args, other.args) + Event._diff_field(diff, 'name', self.name, other.name) + Event._diff_field( + diff, + 'container_id', + self.container_id, + other.container_id, + ) + Event._diff_field(diff, 'loginuid', self.loginuid, other.login_uid) + + return diff if diff else None + + @override + def __str__(self) -> str: + return ( + f'Process(uid={self.uid}, gid={self.gid}, pid={self.pid}, ' + f'exe_path={self.exe_path}, args={self.args}, ' + f'name={self.name}, container_id={self.container_id}, ' + f'loginuid={self.loginuid})' + ) + + +class Event: + """ + Represents a file activity event, associating a process with an + event type and a file. + """ + + def __init__( + self, + process: Process, + event_type: EventType, + file: str | Pattern[str], + host_path: str | Pattern[str] = '', + mode: int | None = None, + owner_uid: int | None = None, + 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, acl_type: str | None = None, acl_entries: list[dict] | None = None, ): @@ -61,6 +259,50 @@ class EventType(Enum): 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 + self._acl_type: str | None = acl_type + self._acl_entries: list[dict] | None = acl_entries + + @property + def event_type(self) -> EventType: + return self._type + + @property + def process(self) -> Process: + return self._process + + @property + def file(self) -> str | Pattern[str]: + return self._file + + @property + def host_path(self) -> str | Pattern[str]: + return self._host_path + + @property + def mode(self) -> int | None: + return self._mode + + @property + def owner_uid(self) -> int | None: + return self._owner_uid + + @property + def owner_gid(self) -> int | None: + return self._owner_gid + + @property + def old_file(self) -> str | Pattern[str] | None: + return self._old_file + + @property + 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 + + @property def acl_type(self) -> str | None: return self._acl_type @@ -183,6 +425,54 @@ def diff(self, other: FileActivity) -> dict | None: self.xattr_name, event_field.xattr_name, ) + elif self.event_type == EventType.ACL: + Event._diff_field( + diff, + 'acl_type', + self.acl_type, + event_field.acl_type, + ) + if self.acl_entries is not None: + actual_entries = [ + { + 'tag': e.tag, + 'perm': e.perm, + 'id': e.id, + } + for e in event_field.entries + ] + Event._diff_field( + diff, + 'acl_entries', + self.acl_entries, + actual_entries, + ) + + return diff if diff else None + + @override + def __str__(self) -> str: + s = ( + f'Event(event_type={self.event_type.name}, ' + f'process={self.process}, file="{self.file}", ' + f'host_path="{self.host_path}"' + ) + + if self.event_type == EventType.PERMISSION: + s += f', mode={self.mode}' + + if self.event_type == EventType.OWNERSHIP: + s += f', owner=(uid={self.owner_uid}, gid={self.owner_gid})' + + if self.event_type == EventType.RENAME: + s += ( + f', old_file="{self.old_file}"' + f', old_host_path="{self.old_host_path}"' + ) + + if self.event_type in (EventType.XATTR_SET, EventType.XATTR_REMOVE): + s += f', xattr_name="{self.xattr_name}"' + if self.event_type == EventType.ACL: s += f', acl_type={self.acl_type}' s += f', acl_entries={self.acl_entries}' From 71c150738bab223b78c265636e727618d535026b Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Mon, 29 Jun 2026 16:05:53 +0100 Subject: [PATCH 6/6] Add ACL skip, make generic --- tests/server.py | 25 ++++++++++++++++++------- tests/test_acl.py | 8 ++++---- tests/test_xattr.py | 14 +++++++------- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/tests/server.py b/tests/server.py index 89835cd6..c2a69330 100644 --- a/tests/server.py +++ b/tests/server.py @@ -15,6 +15,15 @@ from event import Event +# Mapping from friendly skip names to protobuf oneof field names. +SKIP_EVENT_TYPES: dict[str, tuple[str, ...]] = { + 'xattr': ('xattr_set', 'xattr_remove'), + 'acl': ('acl',), +} + +DEFAULT_SKIP = ('xattr', 'acl') + + class FileActivityService(sfa_iservice_pb2_grpc.FileActivityServiceServicer): """ GRPC server for the File Activity Service. @@ -92,7 +101,7 @@ def _wait_events( self, events: list[Event], strict: bool, - skip_xattr: bool, + skip_oneof_names: frozenset[str], cancel: ThreadingEvent, ): while self.is_running() and not cancel.is_set(): @@ -103,10 +112,7 @@ def _wait_events( print(f'Got event: {msg}') - if skip_xattr and msg.WhichOneof('file') in ( - 'xattr_set', - 'xattr_remove', - ): + if msg.WhichOneof('file') in skip_oneof_names: continue # Check if msg matches the next expected event @@ -122,7 +128,7 @@ def wait_events( self, events: list[Event], strict: bool = True, - skip_xattr: bool = True, + skip: tuple[str, ...] = DEFAULT_SKIP, ): """ Continuously checks the server for incoming events until the @@ -131,17 +137,22 @@ def wait_events( Args: events (list['Event']): The events to search for. strict (bool): Fail if an unexpected event is detected. + skip: Event categories to silently ignore (e.g. 'xattr', + 'acl'). Pass an empty tuple to receive all events. Raises: TimeoutError: If the required events are not found in 5 seconds. """ + skip_oneof_names = frozenset( + name for key in skip for name in SKIP_EVENT_TYPES.get(key, (key,)) + ) print('Waiting for events:', *events, sep='\n') cancel = ThreadingEvent() fs = self.executor.submit( self._wait_events, events, strict, - skip_xattr, + skip_oneof_names, cancel, ) try: diff --git a/tests/test_acl.py b/tests/test_acl.py index be2ea9e4..72309d67 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -113,7 +113,7 @@ def test_set_access_acl( ), ] - server.wait_events(events) + server.wait_events(events, skip=()) def test_set_default_acl( @@ -146,7 +146,7 @@ def test_set_default_acl( ), ] - server.wait_events(events) + server.wait_events(events, skip=()) def test_remove_acl( @@ -209,7 +209,7 @@ def test_remove_acl( ), ] - server.wait_events(events) + server.wait_events(events, skip=()) def test_multiple_entries( @@ -251,7 +251,7 @@ def test_multiple_entries( ), ] - server.wait_events(events) + server.wait_events(events, skip=()) def test_ignored_path( diff --git a/tests/test_xattr.py b/tests/test_xattr.py index eb2b1971..a3dd1b79 100644 --- a/tests/test_xattr.py +++ b/tests/test_xattr.py @@ -56,7 +56,7 @@ def test_setxattr( os.setxattr(test_file, 'user.fact_test', b'test_value') server.wait_events( - skip_xattr=False, + skip=(), events=[ Event( process=process, @@ -87,7 +87,7 @@ def test_xattr_set_and_remove( os.removexattr(test_file, 'user.fact_remove') server.wait_events( - skip_xattr=False, + skip=(), events=[ Event( process=process, @@ -129,7 +129,7 @@ def test_xattr_multiple( os.removexattr(test_file, 'user.attr3') server.wait_events( - skip_xattr=False, + skip=(), events=[ Event( process=process, @@ -211,7 +211,7 @@ def test_xattr_ignored( # Only the monitored file's xattr events should arrive server.wait_events( - skip_xattr=False, + skip=(), events=[ Event( process=process, @@ -268,7 +268,7 @@ def test_xattr_new_file( os.removexattr(test_file, 'user.new_file') server.wait_events( - skip_xattr=False, + skip=(), events=[ Event( process=process, @@ -339,7 +339,7 @@ def test_xattr_utf8_filenames( os.removexattr(fut, 'user.utf8_test') server.wait_events( - skip_xattr=False, + skip=(), events=[ Event( process=process, @@ -389,7 +389,7 @@ def test_xattr_utf8_names( os.removexattr(test_file, xattr_name) server.wait_events( - skip_xattr=False, + skip=(), events=[ Event( process=process,