Skip to content
Open
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ possible include a PR number for easier tracking.
* ROX-34502: reload mTLS certificates on each gRPC connection attempt (#788)
* chore: add formatting and linting to integration test code (#783, #784)
* feat: add code coverage with cargo-llvm-cov and Codecov upload (#745)
* ROX-30296: adds ACL tracking via inode_set_acl LSM hook (#878)

## 0.3.0

Expand Down
6 changes: 6 additions & 0 deletions fact-ebpf/src/bpf/checks.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
42 changes: 42 additions & 0 deletions fact-ebpf/src/bpf/events.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Comment on lines +193 to +199

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files 'fact-ebpf/src/bpf/events.h' 'fact-ebpf/src/lib.rs'
wc -l fact-ebpf/src/bpf/events.h fact-ebpf/src/lib.rs
ast-grep outline fact-ebpf/src/bpf/events.h --view expanded
ast-grep outline fact-ebpf/src/lib.rs --view expanded

Repository: stackrox/fact

Length of output: 1670


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,240p' fact-ebpf/src/bpf/events.h
printf '\n---- lib.rs ----\n'
sed -n '1,220p' fact-ebpf/src/lib.rs

Repository: stackrox/fact

Length of output: 11141


Avoid copying entry.e_uid.val for every ACL tag

posix_acl_entry.e_uid is only valid for ACL_USER / ACL_GROUP; for ACL_USER_OBJ, ACL_GROUP_OBJ, ACL_MASK, and ACL_OTHER this reads uninitialized kernel heap data and emits a bogus e_id. Use ACL_UNDEFINED_ID for the other tags.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@fact-ebpf/src/bpf/events.h` around lines 193 - 199, The ACL copy loop in
events.h is always assigning entry.e_uid.val to
args->event->acl.entries[i].e_id, but posix_acl_entry.e_uid is only valid for
ACL_USER and ACL_GROUP. Update the logic in the ACL entry population path to
branch on entry.e_tag and set e_id from e_uid.val only for those tags; for
ACL_USER_OBJ, ACL_GROUP_OBJ, ACL_MASK, and ACL_OTHER, assign ACL_UNDEFINED_ID
instead. Keep the change localized to the ACL event-building loop so the rest of
the field copying in args->event->acl.entries remains unchanged.

}

// inode_set_acl does not support bpf_d_path (no struct path available)
__submit_event(args, false);
}
25 changes: 25 additions & 0 deletions fact-ebpf/src/bpf/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,31 @@ 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++;

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;
}

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();
Expand Down
19 changes: 19 additions & 0 deletions fact-ebpf/src/bpf/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
};
};

Expand Down Expand Up @@ -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;
};
3 changes: 3 additions & 0 deletions fact-ebpf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
29 changes: 23 additions & 6 deletions fact/src/bpf/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
}
}
32 changes: 22 additions & 10 deletions fact/src/bpf/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const RINGBUFFER_NAME: &str = "rb";

pub struct Bpf {
obj: Ebpf,
checks: Checks,

tx: mpsc::Sender<Event>,

Expand Down Expand Up @@ -64,6 +65,7 @@ impl Bpf {
let paths = Vec::new();
let mut bpf = Bpf {
obj,
checks,
tx,
paths,
paths_config,
Expand Down Expand Up @@ -178,28 +180,38 @@ impl Bpf {
let Some(hook) = name.strip_prefix("trace_") else {
bail!("Invalid hook name: {name}");
};

// 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:?}"),
}
};
}
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::<Result<_, _>>()?;
}
}
Ok(())
}

Expand Down
Loading
Loading