ROX-30258: Track file ownership changes

.github/workflows/unit-tests.yml

@@ -71,6 +71,7 @@ jobs:
         # need more investigation
         - rhel-8
         - rhcos-412-86-202402272018-0-gcp-x86-64
+        - rhcos-414-92-202407091253-0-gcp-x86-64
         # BPF trampolines are only implemented starting with RHEL 10
         - rhel-9-arm64
         EOF

fact-ebpf/src/bpf/events.h

@@ -73,3 +73,25 @@ __always_inline static void submit_mode_event(struct metrics_by_hook_t* m,
 
   __submit_event(event, m, FILE_ACTIVITY_CHMOD, filename, inode, use_bpf_d_path);
 }
+
+__always_inline static void submit_ownership_event(struct metrics_by_hook_t* m,
+                                                   const char filename[PATH_MAX],
+                                                   inode_key_t* inode,
+                                                   unsigned long long uid,
+                                                   unsigned long long gid,
+                                                   unsigned long long old_uid,
+                                                   unsigned long long old_gid,
+                                                   bool use_bpf_d_path) {
+  struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0);
+  if (event == NULL) {
+    m->ringbuffer_full++;
+    return;
+  }
+
+  event->chown.new.uid = uid;
+  event->chown.new.gid = gid;
+  event->chown.old.uid = old_uid;
+  event->chown.old.gid = old_gid;
+
+  __submit_event(event, m, FILE_ACTIVITY_CHOWN, filename, inode, use_bpf_d_path);
+}

fact-ebpf/src/bpf/main.c

@@ -173,3 +173,59 @@ int BPF_PROG(trace_path_chmod, struct path* path, umode_t mode) {
 
   return 0;
 }
+
+/* path_chown takes _unsigned long long_ for uid and gid because kuid_t and kgid_t (structs)
+   fit in registers and since they contain only one integer, their content is extended to the
+   size of the BPF registers (64 bits) to simplify further arithmetic operations. */
+SEC("lsm/path_chown")
+int BPF_PROG(trace_path_chown, struct path* path, unsigned long long uid, unsigned long long gid) {
+  struct metrics_t* m = get_metrics();
+  if (m == NULL) {
+    return 0;
+  }
+
+  m->path_chown.total++;
+
+  struct bound_path_t* bound_path = NULL;
+  if (path_hooks_support_bpf_d_path) {
+    bound_path = path_read(path);
+  } else {
+    bound_path = path_read_no_d_path(path);
+  }
+
+  if (bound_path == NULL) {
+    bpf_printk("Failed to read path");
+    m->path_chown.error++;
+    return 0;
+  }
+
+  inode_key_t inode_key = inode_to_key(path->dentry->d_inode);
+  const inode_value_t* inode = inode_get(&inode_key);
+
+  switch (inode_is_monitored(inode)) {
+    case NOT_MONITORED:
+      if (!is_monitored(bound_path)) {
+        m->path_chown.ignored++;
+        return 0;
+      }
+      break;
+
+    case MONITORED:
+      break;
+  }
+
+  struct dentry* d = BPF_CORE_READ(path, dentry);
+  unsigned long long old_uid = BPF_CORE_READ(d, d_inode, i_uid.val);
+  unsigned long long old_gid = BPF_CORE_READ(d, d_inode, i_gid.val);
+
+  submit_ownership_event(&m->path_chown,
+                         bound_path->path,
+                         &inode_key,
+                         uid,
+                         gid,
+                         old_uid,
+                         old_gid,
+                         path_hooks_support_bpf_d_path);
+
+  return 0;
+}

fact-ebpf/src/bpf/types.h

@@ -53,6 +53,7 @@ typedef enum file_activity_type_t {
   FILE_ACTIVITY_CREATION,
   FILE_ACTIVITY_UNLINK,
   FILE_ACTIVITY_CHMOD,
+  FILE_ACTIVITY_CHOWN,
 } file_activity_type_t;
 
 struct event_t {
@@ -66,6 +67,12 @@ struct event_t {
       short unsigned int new;
       short unsigned int old;
     } chmod;
+    struct {
+      struct {
+        unsigned int uid;
+        unsigned int gid;
+      } old, new;
+    } chown;
   };
 };
 
@@ -96,4 +103,5 @@ struct metrics_t {
   struct metrics_by_hook_t file_open;
   struct metrics_by_hook_t path_unlink;
   struct metrics_by_hook_t path_chmod;
+  struct metrics_by_hook_t path_chown;
 };

fact-ebpf/src/lib.rs

@@ -112,6 +112,7 @@ impl metrics_t {
         m.file_open = m.file_open.accumulate(&other.file_open);
         m.path_unlink = m.path_unlink.accumulate(&other.path_unlink);
         m.path_chmod = m.path_chmod.accumulate(&other.path_chmod);
+        m.path_chown = m.path_chown.accumulate(&other.path_chown);
         m
     }
 }

fact/src/event/mod.rs

@@ -82,6 +82,7 @@ impl Event {
             FileData::Creation(data) => &data.inode,
             FileData::Unlink(data) => &data.inode,
             FileData::Chmod(data) => &data.inner.inode,
+            FileData::Chown(data) => &data.inner.inode,
         }
     }
 
@@ -91,6 +92,7 @@ impl Event {
             FileData::Creation(data) => data.host_file = host_path,
             FileData::Unlink(data) => data.host_file = host_path,
             FileData::Chmod(data) => data.inner.host_file = host_path,
+            FileData::Chown(data) => data.inner.host_file = host_path,
         }
     }
 }
@@ -145,6 +147,7 @@ pub enum FileData {
     Creation(BaseFileData),
     Unlink(BaseFileData),
     Chmod(ChmodFileData),
+    Chown(ChownFileData),
 }
 
 impl FileData {
@@ -167,6 +170,16 @@ impl FileData {
                 };
                 FileData::Chmod(data)
             }
+            file_activity_type_t::FILE_ACTIVITY_CHOWN => {
+                let data = ChownFileData {
+                    inner,
+                    new_uid: unsafe { extra_data.chown.new.uid },
+                    new_gid: unsafe { extra_data.chown.new.gid },
+                    old_uid: unsafe { extra_data.chown.old.uid },
+                    old_gid: unsafe { extra_data.chown.old.gid },
+                };
+                FileData::Chown(data)
+            }
             invalid => unreachable!("Invalid event type: {invalid:?}"),
         };
 
@@ -196,6 +209,10 @@ impl From<FileData> for fact_api::file_activity::File {
                 let f_act = fact_api::FilePermissionChange::from(event);
                 fact_api::file_activity::File::Permission(f_act)
             }
+            FileData::Chown(event) => {
+                let f_act = fact_api::FileOwnershipChange::from(event);
+                fact_api::file_activity::File::Ownership(f_act)
+            }
         }
     }
 }
@@ -255,29 +272,12 @@ pub struct ChmodFileData {
     old_mode: u16,
 }
 
-impl ChmodFileData {
-    pub fn new(
-        filename: [c_char; PATH_MAX as usize],
-        inode: inode_key_t,
-        new_mode: u16,
-        old_mode: u16,
-    ) -> anyhow::Result<Self> {
-        let inner = BaseFileData::new(filename, inode)?;
-
-        Ok(ChmodFileData {
-            inner,
-            new_mode,
-            old_mode,
-        })
-    }
-}
-
 impl From<ChmodFileData> for fact_api::FilePermissionChange {
     fn from(value: ChmodFileData) -> Self {
         let ChmodFileData {
             inner: file,
             new_mode,
-            old_mode: _,
+            ..
         } = value;
         let activity = fact_api::FileActivityBase::from(file);
         fact_api::FilePermissionChange {
@@ -295,3 +295,31 @@ impl PartialEq for ChmodFileData {
             && self.inner == other.inner
     }
 }
+
+#[derive(Debug, Clone, Serialize)]
+pub struct ChownFileData {
+    inner: BaseFileData,
+    new_uid: u32,
+    new_gid: u32,
+    old_uid: u32,
+    old_gid: u32,
+}
+
+impl From<ChownFileData> for fact_api::FileOwnershipChange {
+    fn from(value: ChownFileData) -> Self {
+        let ChownFileData {
+            inner: file,
+            new_uid,
+            new_gid,
+            ..
+        } = value;
+        let activity = fact_api::FileActivityBase::from(file);
+        fact_api::FileOwnershipChange {
+            activity: Some(activity),
+            uid: new_uid,
+            gid: new_gid,
+            username: "".to_string(),
+            group: "".to_string(),
+        }
+    }
+}

fact/src/metrics/kernel_metrics.rs

@@ -11,6 +11,7 @@ pub struct KernelMetrics {
     file_open: EventCounter,
     path_unlink: EventCounter,
     path_chmod: EventCounter,
+    path_chown: EventCounter,
     map: PerCpuArray<MapData, metrics_t>,
 }
 
@@ -31,15 +32,22 @@ impl KernelMetrics {
             "Events processed by the path_chmod LSM hook",
             &[], // Labels are not needed since `collect` will add them all
         );
+        let path_chown = EventCounter::new(
+            "kernel_path_chown_events",
+            "Events processed by the path_chown LSM hook",
+            &[], // Labels are not needed since `collect` will add them all
+        );
 
         file_open.register(reg);
         path_unlink.register(reg);
         path_chmod.register(reg);
+        path_chown.register(reg);
 
         KernelMetrics {
             file_open,
             path_unlink,
             path_chmod,
+            path_chown,
             map: kernel_metrics,
         }
     }
@@ -87,6 +95,7 @@ impl KernelMetrics {
         KernelMetrics::refresh_labels(&self.file_open, &metrics.file_open);
         KernelMetrics::refresh_labels(&self.path_unlink, &metrics.path_unlink);
         KernelMetrics::refresh_labels(&self.path_chmod, &metrics.path_chmod);
+        KernelMetrics::refresh_labels(&self.path_chown, &metrics.path_chown);
 
         Ok(())
     }

tests/event.py

@@ -31,6 +31,7 @@ class EventType(Enum):
     CREATION = 2
     UNLINK = 3
     PERMISSION = 4
+    OWNERSHIP = 5
 
 
 class Process:
@@ -171,12 +172,16 @@ def __init__(self,
                  event_type: EventType,
                  file: str,
                  host_path: str = '',
-                 mode: int | None = None):
+                 mode: int | None = None,
+                 owner_uid: int | None = None,
+                 owner_gid: int | None = None,):
         self._type: EventType = event_type
         self._process: Process = process
         self._file: str = file
         self._host_path: str = host_path
         self._mode: int | None = mode
+        self._owner_uid: int | None = owner_uid
+        self._owner_gid: int | None = owner_gid
 
     @property
     def event_type(self) -> EventType:
@@ -198,6 +203,14 @@ def host_path(self) -> str:
     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
+
     @override
     def __eq__(self, other: Any) -> bool:
         if isinstance(other, FileActivity):
@@ -217,6 +230,11 @@ def __eq__(self, other: Any) -> bool:
                 return self.file == other.permission.activity.path and \
                     self.host_path == other.permission.activity.host_path and \
                     self.mode == other.permission.mode
+            elif self.event_type == EventType.OWNERSHIP:
+                return self.file == other.ownership.activity.path and \
+                    self.host_path == other.ownership.activity.host_path and \
+                    self.owner_uid == other.ownership.uid and \
+                    self.owner_gid == other.ownership.gid
             return False
         raise NotImplementedError
 
@@ -229,6 +247,9 @@ def __str__(self) -> str:
         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})'
+
         s += ')'
 
         return s