ROX-33034: Track xattr changes (#790)

Author: JoukoVirtanen

fact-ebpf/src/bpf/events.h

@@ -37,7 +37,11 @@ __always_inline static void __submit_event(struct submit_event_args_t* args,
   event->monitored = args->monitored;
   inode_copy(&event->inode, &args->inode);
   inode_copy(&event->parent_inode, &args->parent_inode);
-  bpf_probe_read_str(event->filename, PATH_MAX, args->filename);
+  if (args->filename != NULL) {
+    bpf_probe_read_str(event->filename, PATH_MAX, args->filename);
+  } else {
+    event->filename[0] = '\0';
+  }
 
   struct helper_t* helper = get_helper();
   if (helper == NULL) {
@@ -144,3 +148,15 @@ __always_inline static void submit_rmdir_event(struct submit_event_args_t* args)
 
   __submit_event(args, path_hooks_support_bpf_d_path);
 }
+
+__always_inline static void submit_xattr_event(struct submit_event_args_t* args,
+                                               file_activity_type_t event_type,
+                                               const char* xattr_name) {
+  if (!reserve_event(args)) {
+    return;
+  }
+  args->event->type = event_type;
+  bpf_probe_read_str(args->event->xattr.name, XATTR_NAME_MAX_LEN, xattr_name);
+
+  __submit_event(args, false);
+}

fact-ebpf/src/bpf/main.c

@@ -389,6 +389,48 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) {
   return 0;
 }
 
+__always_inline static int handle_xattr(struct metrics_by_hook_t* hook_metrics,
+                                        struct dentry* dentry,
+                                        const char* xattr_name,
+                                        file_activity_type_t event_type) {
+  struct submit_event_args_t args = {.metrics = hook_metrics};
+
+  args.metrics->total++;
+
+  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_xattr_event(&args, event_type, xattr_name);
+  return 0;
+}
+
+SEC("lsm/inode_setxattr")
+int BPF_PROG(trace_inode_setxattr, struct mnt_idmap* idmap, struct dentry* dentry,
+             const char* name, const void* value, size_t size, int flags) {
+  struct metrics_t* m = get_metrics();
+  if (m == NULL) {
+    return 0;
+  }
+  return handle_xattr(&m->inode_setxattr, dentry, name, FILE_ACTIVITY_SETXATTR);
+}
+
+SEC("lsm/inode_removexattr")
+int BPF_PROG(trace_inode_removexattr, struct mnt_idmap* idmap, struct dentry* dentry,
+             const char* name) {
+  struct metrics_t* m = get_metrics();
+  if (m == NULL) {
+    return 0;
+  }
+  return handle_xattr(&m->inode_removexattr, dentry, name, FILE_ACTIVITY_REMOVEXATTR);
+}
+
 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

@@ -15,6 +15,10 @@
 
 #define LINEAGE_MAX 2
 
+// Matches Linux kernel XATTR_NAME_MAX (255) + null terminator.
+// https://github.com/torvalds/linux/blob/66affa37cfac0aec061cc4bcf4a065b0c52f7e19/include/uapi/linux/limits.h#L15
+#define XATTR_NAME_MAX_LEN 256
+
 #define LPM_SIZE_MAX 256
 
 typedef struct lineage_t {
@@ -64,6 +68,8 @@ typedef enum file_activity_type_t {
   FILE_ACTIVITY_RENAME,
   DIR_ACTIVITY_CREATION,
   DIR_ACTIVITY_UNLINK,
+  FILE_ACTIVITY_SETXATTR,
+  FILE_ACTIVITY_REMOVEXATTR,
 } file_activity_type_t;
 
 struct event_t {
@@ -90,6 +96,9 @@ struct event_t {
       inode_key_t inode;
       monitored_t monitored;
     } rename;
+    struct {
+      char name[XATTR_NAME_MAX_LEN];
+    } xattr;
   };
 };
 
@@ -132,4 +141,6 @@ struct metrics_t {
   struct metrics_by_hook_t path_mkdir;
   struct metrics_by_hook_t d_instantiate;
   struct metrics_by_hook_t path_rmdir;
+  struct metrics_by_hook_t inode_setxattr;
+  struct metrics_by_hook_t inode_removexattr;
 };

fact/src/event/mod.rs

@@ -9,7 +9,9 @@ use std::{
 use globset::GlobSet;
 use serde::Serialize;
 
-use fact_ebpf::{PATH_MAX, event_t, file_activity_type_t, inode_key_t, monitored_t};
+use fact_ebpf::{
+    PATH_MAX, XATTR_NAME_MAX_LEN, event_t, file_activity_type_t, inode_key_t, monitored_t,
+};
 
 use crate::host_info;
 use process::Process;
@@ -131,6 +133,10 @@ impl Event {
         matches!(self.file, FileData::Creation(_) | FileData::MkDir(_))
     }
 
+    pub fn is_xattr(&self) -> bool {
+        matches!(self.file, FileData::SetXattr(_) | FileData::RemoveXattr(_))
+    }
+
     pub fn is_mkdir(&self) -> bool {
         matches!(self.file, FileData::MkDir(_))
     }
@@ -162,6 +168,8 @@ impl Event {
             FileData::Chmod(data) => &data.inner.inode,
             FileData::Chown(data) => &data.inner.inode,
             FileData::Rename(data) => &data.new.inode,
+            FileData::SetXattr(data) => &data.inner.inode,
+            FileData::RemoveXattr(data) => &data.inner.inode,
         }
     }
 
@@ -176,6 +184,8 @@ impl Event {
             FileData::Chmod(data) => &data.inner.parent_inode,
             FileData::Chown(data) => &data.inner.parent_inode,
             FileData::Rename(data) => &data.new.parent_inode,
+            FileData::SetXattr(data) => &data.inner.parent_inode,
+            FileData::RemoveXattr(data) => &data.inner.parent_inode,
         }
     }
 
@@ -199,6 +209,8 @@ impl Event {
             FileData::Chmod(data) => &data.inner.filename,
             FileData::Chown(data) => &data.inner.filename,
             FileData::Rename(data) => &data.new.filename,
+            FileData::SetXattr(data) => &data.inner.filename,
+            FileData::RemoveXattr(data) => &data.inner.filename,
         }
     }
 
@@ -219,6 +231,8 @@ impl Event {
             FileData::Chmod(data) => &data.inner.host_file,
             FileData::Chown(data) => &data.inner.host_file,
             FileData::Rename(data) => &data.new.host_file,
+            FileData::SetXattr(data) => &data.inner.host_file,
+            FileData::RemoveXattr(data) => &data.inner.host_file,
         }
     }
 
@@ -243,6 +257,8 @@ impl Event {
             FileData::Chmod(data) => data.inner.host_file = host_path,
             FileData::Chown(data) => data.inner.host_file = host_path,
             FileData::Rename(data) => data.new.host_file = host_path,
+            FileData::SetXattr(data) => data.inner.host_file = host_path,
+            FileData::RemoveXattr(data) => data.inner.host_file = host_path,
         }
     }
 
@@ -264,6 +280,8 @@ impl Event {
             FileData::Chmod(data) => data.inner.monitored,
             FileData::Chown(data) => data.inner.monitored,
             FileData::Rename(data) => data.new.monitored,
+            FileData::SetXattr(data) => data.inner.monitored,
+            FileData::RemoveXattr(data) => data.inner.monitored,
         }
     }
 
@@ -356,6 +374,8 @@ pub enum FileData {
     Chmod(ChmodFileData),
     Chown(ChownFileData),
     Rename(RenameFileData),
+    SetXattr(XattrFileData),
+    RemoveXattr(XattrFileData),
 }
 
 impl FileData {
@@ -407,6 +427,18 @@ impl FileData {
                 };
                 FileData::Rename(data)
             }
+            file_activity_type_t::FILE_ACTIVITY_SETXATTR => {
+                let xattr_name = slice_to_string(
+                    &unsafe { extra_data.xattr }.name[..XATTR_NAME_MAX_LEN as usize],
+                )?;
+                FileData::SetXattr(XattrFileData { inner, xattr_name })
+            }
+            file_activity_type_t::FILE_ACTIVITY_REMOVEXATTR => {
+                let xattr_name = slice_to_string(
+                    &unsafe { extra_data.xattr }.name[..XATTR_NAME_MAX_LEN as usize],
+                )?;
+                FileData::RemoveXattr(XattrFileData { inner, xattr_name })
+            }
             invalid => unreachable!("Invalid event type: {invalid:?}"),
         };
 
@@ -433,6 +465,14 @@ impl From<FileData> for fact_api::file_activity::File {
             FileData::RmDir(_) => {
                 unreachable!("RmDir event reached protobuf conversion");
             }
+            FileData::SetXattr(event) => {
+                let f_act = fact_api::FileXattrChange::from(event);
+                fact_api::file_activity::File::XattrSet(f_act)
+            }
+            FileData::RemoveXattr(event) => {
+                let f_act = fact_api::FileXattrChange::from(event);
+                fact_api::file_activity::File::XattrRemove(f_act)
+            }
             FileData::Unlink(event) => {
                 let activity = Some(fact_api::FileActivityBase::from(event));
                 let f_act = fact_api::FileUnlink { activity };
@@ -465,6 +505,8 @@ impl PartialEq for FileData {
             (FileData::Unlink(this), FileData::Unlink(other)) => this == other,
             (FileData::Chmod(this), FileData::Chmod(other)) => this == other,
             (FileData::Rename(this), FileData::Rename(other)) => this == other,
+            (FileData::SetXattr(this), FileData::SetXattr(other)) => this == other,
+            (FileData::RemoveXattr(this), FileData::RemoveXattr(other)) => this == other,
             _ => false,
         }
     }
@@ -595,6 +637,29 @@ impl PartialEq for RenameFileData {
     }
 }
 
+#[derive(Debug, Clone, Serialize)]
+pub struct XattrFileData {
+    inner: BaseFileData,
+    xattr_name: String,
+}
+
+impl From<XattrFileData> for fact_api::FileXattrChange {
+    fn from(value: XattrFileData) -> Self {
+        let activity = fact_api::FileActivityBase::from(value.inner);
+        fact_api::FileXattrChange {
+            activity: Some(activity),
+            xattr_name: value.xattr_name,
+        }
+    }
+}
+
+#[cfg(test)]
+impl PartialEq for XattrFileData {
+    fn eq(&self, other: &Self) -> bool {
+        self.xattr_name == other.xattr_name && self.inner == other.inner
+    }
+}
+
 #[cfg(test)]
 mod test_utils {
     use std::os::raw::c_char;

fact/src/metrics/kernel_metrics.rs

@@ -70,4 +70,6 @@ define_kernel_metrics!(
     path_mkdir,
     path_rmdir,
     d_instantiate,
+    inode_setxattr,
+    inode_removexattr,
 );

tests/event.py

@@ -46,6 +46,8 @@ class EventType(Enum):
     PERMISSION = 4
     OWNERSHIP = 5
     RENAME = 6
+    XATTR_SET = 7
+    XATTR_REMOVE = 8
 
 
 class Process:
@@ -233,6 +235,7 @@ def __init__(
         owner_gid: int | None = None,
         old_file: str | Pattern[str] | None = None,
         old_host_path: str | Pattern[str] | None = None,
+        xattr_name: str | None = None,
     ):
         self._type: EventType = event_type
         self._process: Process = process
@@ -243,6 +246,7 @@ def __init__(
         self._owner_gid: int | None = owner_gid
         self._old_file: str | Pattern[str] | None = old_file
         self._old_host_path: str | Pattern[str] | None = old_host_path
+        self._xattr_name: str | None = xattr_name
 
     @property
     def event_type(self) -> EventType:
@@ -280,6 +284,10 @@ def old_file(self) -> str | Pattern[str] | None:
     def old_host_path(self) -> str | Pattern[str] | None:
         return self._old_host_path
 
+    @property
+    def xattr_name(self) -> str | None:
+        return self._xattr_name
+
     @classmethod
     def _diff_field(cls, diff: dict, name: str, expected: Any, actual: Any):
         if expected != actual:
@@ -388,6 +396,13 @@ def diff(self, other: FileActivity) -> dict | None:
                 self.owner_gid,
                 event_field.gid,
             )
+        elif self.event_type in (EventType.XATTR_SET, EventType.XATTR_REMOVE):
+            Event._diff_field(
+                diff,
+                'xattr_name',
+                self.xattr_name,
+                event_field.xattr_name,
+            )
 
         return diff if diff else None
 
@@ -411,6 +426,19 @@ def __str__(self) -> str:
                 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}"'
+
         s += ')'
 
         return s
+
+
+def selinux_xattr(process: Process, host_path: str = '') -> Event:
+    return Event(
+        process=process,
+        event_type=EventType.XATTR_SET,
+        file='',
+        host_path=host_path,
+        xattr_name='security.selinux',
+    )

tests/server.py

@@ -92,6 +92,7 @@ def _wait_events(
         self,
         events: list[Event],
         strict: bool,
+        skip_xattr: bool,
         cancel: ThreadingEvent,
     ):
         while self.is_running() and not cancel.is_set():
@@ -102,6 +103,12 @@ def _wait_events(
 
             print(f'Got event: {msg}')
 
+            if skip_xattr and msg.WhichOneof('file') in (
+                'xattr_set',
+                'xattr_remove',
+            ):
+                continue
+
             # Check if msg matches the next expected event
             diff = events[0].diff(msg)
             if diff is None:
@@ -111,7 +118,12 @@ def _wait_events(
             elif strict:
                 raise ValueError(json.dumps(diff, indent=4))
 
-    def wait_events(self, events: list[Event], strict: bool = True):
+    def wait_events(
+        self,
+        events: list[Event],
+        strict: bool = True,
+        skip_xattr: bool = True,
+    ):
         """
         Continuously checks the server for incoming events until the
         specified events are found.
@@ -125,7 +137,13 @@ def wait_events(self, events: list[Event], strict: bool = True):
         """
         print('Waiting for events:', *events, sep='\n')
         cancel = ThreadingEvent()
-        fs = self.executor.submit(self._wait_events, events, strict, cancel)
+        fs = self.executor.submit(
+            self._wait_events,
+            events,
+            strict,
+            skip_xattr,
+            cancel,
+        )
         try:
             fs.result(timeout=5)
         except TimeoutError: