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;
}
52 changes: 52 additions & 0 deletions fact-ebpf/src/bpf/events.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,55 @@ __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". name_buf
// only needs to hold the longer of the two names, 25 bytes including
// the null terminator, rounded up to the next power of two for
// alignment.
char name_buf[32] = {0};
Comment thread
Molter73 marked this conversation as resolved.
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) {
Comment thread
Molter73 marked this conversation as resolved.
args->event->acl.acl_type = FACT_ACL_TYPE_DEFAULT;
} else {
args->event->acl.acl_type = FACT_ACL_TYPE_ACCESS;
}

if (kacl != NULL) {
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 = (acl_tag_t)entry.e_tag;
args->event->acl.entries[i].e_perm = entry.e_perm;
// e_uid is only meaningful for USER/GROUP entries; the kernel
// leaves it unset for USER_OBJ/GROUP_OBJ/MASK/OTHER, so we must
// not read it for those tags.
if (entry.e_tag == FACT_ACL_TAG_USER || entry.e_tag == FACT_ACL_TAG_GROUP) {
args->event->acl.entries[i].e_id = entry.e_uid.val;
} else {
args->event->acl.entries[i].e_id = ACL_UNDEFINED_ID;
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else {
args->event->acl.count = 0;
}

// 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
51 changes: 51 additions & 0 deletions fact-ebpf/src/bpf/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,50 @@ typedef enum monitored_t {
// For the time being we just keep a char.
typedef char inode_value_t;

// Generic xattr values are capped at XATTR_SIZE_MAX (64KiB), which bounds
// a POSIX ACL xattr (4 byte header + 8 bytes per entry) to ~8191 entries
// in theory:
// https://github.com/torvalds/linux/blob/d2c9a99135da931377240942d44f3dea104cedb8/include/uapi/linux/limits.h#L16
// In practice, individual filesystems impose much lower limits tied to
// their own xattr/block storage (e.g. ext4 caps out in the low hundreds
// for a typical 4K xattr block). 32 comfortably covers real-world ACLs
// (a handful of named users/groups plus the standard entries); raise it
// if we ever see it hit in practice.
#define FACT_MAX_ACL_ENTRIES 32
Comment thread
Molter73 marked this conversation as resolved.

// Sentinel used by the kernel for ACL entries that don't carry a uid/gid
// (USER_OBJ, GROUP_OBJ, MASK, OTHER). Kernel defines this as (-1), which
// is 0xFFFFFFFF once read as the unsigned e_id we store it in.
// https://github.com/torvalds/linux/blob/d2c9a99135da931377240942d44f3dea104cedb8/include/uapi/linux/posix_acl.h#L21
#define ACL_UNDEFINED_ID 0xFFFFFFFF

// ACL type, matching the xattr name the change came in on:
// "system.posix_acl_access" vs "system.posix_acl_default". These are the
// only two POSIX ACL xattrs the kernel supports, so a small enum is
// sufficient here rather than keeping the full xattr name around.
typedef enum acl_type_t {
FACT_ACL_TYPE_ACCESS = 0,
FACT_ACL_TYPE_DEFAULT = 1,
} acl_type_t;

// Mirrors the kernel's ACL tag bit values so we can encode/compare
// against them without magic numbers, both here and on the Rust side.
// https://github.com/torvalds/linux/blob/d2c9a99135da931377240942d44f3dea104cedb8/include/uapi/linux/posix_acl.h#L28-L33
typedef enum acl_tag_t {
FACT_ACL_TAG_USER_OBJ = 0x01,
FACT_ACL_TAG_USER = 0x02,
FACT_ACL_TAG_GROUP_OBJ = 0x04,
FACT_ACL_TAG_GROUP = 0x08,
FACT_ACL_TAG_MASK = 0x10,
FACT_ACL_TAG_OTHER = 0x20,
} acl_tag_t;

struct acl_entry_t {
acl_tag_t 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 +114,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 +144,11 @@ struct event_t {
struct {
char name[XATTR_NAME_MAX_LEN];
} xattr;
struct {
unsigned int count;
acl_type_t acl_type;
struct acl_entry_t entries[FACT_MAX_ACL_ENTRIES];
} acl;
};
};

Expand Down Expand Up @@ -143,4 +193,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
35 changes: 29 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,37 @@ 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;
};
match prog.load(hook, btf) {
Ok(()) => true,
Err(e) => {
debug!("Probe {prog_name} failed to load for hook {hook}: {e:#}");
false
}
}
}
}
33 changes: 23 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,39 @@ 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 {
self.links.clear();
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<_, _>>()?;
}
}
Comment on lines +204 to +215

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The new approach is fine, but if you wanted to keep the functional one you could do so using filter_map like this:

        self.links = self
            .obj
            .programs_mut()
            .filter_map(|(_, prog)| match prog {
                Program::Lsm(prog) => {
                    if prog.fd().is_err() {
                        return None;
                    }
                    let link_id = match prog.attach() {
                        Ok(link_id) => link_id,
                        Err(e) => return Some(Err(e)),
                    };
                    Some(prog.take_link(link_id))
                }
                u => unimplemented!("{u:?}"),
            })
            .collect::<Result<_, _>>()?;

There is also one small change that I think won't matter, but the new approach does not drop any existing links from the vector before pushing the new ones, we might want to clear the vector just in case if we want to keep this new approach.

Ok(())
}

Expand Down
Loading
Loading