ROX-30296: track POSIX ACL changes via inode_set_acl LSM hook (#878)

Author: Stringy

CHANGELOG.md

@@ -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
 

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

fact-ebpf/src/bpf/events.h

@@ -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};
+  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) {
+    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;
+      }
+    }
+  } else {
+    args->event->acl.count = 0;
+  }
+
+  // inode_set_acl does not support bpf_d_path (no struct path available)
+  __submit_event(args, false);
+}

fact-ebpf/src/bpf/main.c

@@ -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();

fact-ebpf/src/bpf/types.h

@@ -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
+
+// 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,
@@ -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 {
@@ -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;
   };
 };
 
@@ -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;
 };

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

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()
+    }
 }

fact/src/bpf/mod.rs

@@ -26,6 +26,7 @@ const RINGBUFFER_NAME: &str = "rb";
 
 pub struct Bpf {
     obj: Ebpf,
+    checks: Checks,
 
     tx: mpsc::Sender<Event>,
 
@@ -64,6 +65,7 @@ impl Bpf {
         let paths = Vec::new();
         let mut bpf = Bpf {
             obj,
+            checks,
             tx,
             paths,
             paths_config,
@@ -178,28 +180,40 @@ 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, programs that were already attached during
+    /// this call remain attached (they are not rolled back); callers
+    /// should treat an `Err` here as fatal.
     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<_, _>>()?;
+            }
+        }
         Ok(())
     }