diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 2df2428c..44887c5f 100644 --- a/.github/workflows/unit-tests.yml +++ b/.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 diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 4e9e51bf..9cc6baae 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/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); +} diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 2e4d64c0..3b448128 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/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; +} diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index f0fc2a94..8fce112e 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/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; }; diff --git a/fact-ebpf/src/lib.rs b/fact-ebpf/src/lib.rs index 5ba66dc5..655d48d7 100644 --- a/fact-ebpf/src/lib.rs +++ b/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 } } diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 784458d7..e43d8b58 100644 --- a/fact/src/event/mod.rs +++ b/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 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 { - let inner = BaseFileData::new(filename, inode)?; - - Ok(ChmodFileData { - inner, - new_mode, - old_mode, - }) - } -} - impl From 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 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(), + } + } +} diff --git a/fact/src/metrics/kernel_metrics.rs b/fact/src/metrics/kernel_metrics.rs index 720943a9..1a6fc977 100644 --- a/fact/src/metrics/kernel_metrics.rs +++ b/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, } @@ -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(()) } diff --git a/tests/event.py b/tests/event.py index 5422833b..389c01e0 100644 --- a/tests/event.py +++ b/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 diff --git a/tests/test_path_chown.py b/tests/test_path_chown.py new file mode 100644 index 00000000..d318f4eb --- /dev/null +++ b/tests/test_path_chown.py @@ -0,0 +1,268 @@ +import os + +from event import Event, EventType, Process + +# Tests here have to use a container to do 'chown', +# otherwise they would require to run as root. + +# UID and GID values for chown tests +TEST_UID = 1234 +TEST_GID = 2345 + + +def test_chown(fact, test_container, server): + """ + Execute a chown operation on a file and verifies the corresponding event is + captured by the server. + + Args: + fact: Fixture for file activity (only required to be running). + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + # File Under Test + fut = '/container-dir/test.txt' + + # Create the file and chown it + touch_cmd = f'touch {fut}' + chown_cmd = f'chown {TEST_UID}:{TEST_GID} {fut}' + test_container.exec_run(touch_cmd) + test_container.exec_run(chown_cmd) + + loginuid = pow(2, 32) - 1 + touch = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/touch', + args=touch_cmd, + name='touch', + container_id=test_container.id[:12], + loginuid=loginuid) + chown = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/chown', + args=chown_cmd, + name='chown', + container_id=test_container.id[:12], + loginuid=loginuid) + events = [ + Event(process=touch, event_type=EventType.CREATION, file=fut, + host_path=''), + Event(process=chown, event_type=EventType.OWNERSHIP, file=fut, + host_path='', owner_uid=TEST_UID, owner_gid=TEST_GID), + ] + + for e in events: + print(f'Waiting for event: {e}') + + server.wait_events(events) + + +def test_multiple(fact, test_container, server): + """ + Tests ownership operations on multiple files and verifies the corresponding + events are captured by the server. + + Args: + fact: Fixture for file activity (only required to be running). + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + events = [] + loginuid = pow(2, 32) - 1 + + # File Under Test + for i in range(3): + fut = f'/container-dir/{i}.txt' + touch_cmd = f'touch {fut}' + chown_cmd = f'chown {TEST_UID}:{TEST_GID} {fut}' + test_container.exec_run(touch_cmd) + test_container.exec_run(chown_cmd) + + touch = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/touch', + args=touch_cmd, + name='touch', + container_id=test_container.id[:12], + loginuid=loginuid) + chown = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/chown', + args=chown_cmd, + name='chown', + container_id=test_container.id[:12], + loginuid=loginuid) + + events.extend([ + Event(process=touch, event_type=EventType.CREATION, file=fut, + host_path=''), + Event(process=chown, event_type=EventType.OWNERSHIP, file=fut, + host_path='', owner_uid=TEST_UID, owner_gid=TEST_GID), + ]) + + for e in events: + print(f'Waiting for event: {e}') + + server.wait_events(events) + + +def test_ignored(fact, test_container, server): + """ + Tests that ownership events on ignored files are not captured by the + server. + + Args: + fact: Fixture for file activity (only required to be running). + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + loginuid = pow(2, 32) - 1 + + ignored_file = '/test.txt' + monitored_file = '/container-dir/test.txt' + + ignored_touch_cmd = f'touch {ignored_file}' + ignored_chown_cmd = f'chown {TEST_UID}:{TEST_GID} {ignored_file}' + monitored_touch_cmd = f'touch {monitored_file}' + monitored_chown_cmd = f'chown {TEST_UID}:{TEST_GID} {monitored_file}' + + test_container.exec_run(ignored_touch_cmd) + test_container.exec_run(ignored_chown_cmd) + test_container.exec_run(monitored_touch_cmd) + test_container.exec_run(monitored_chown_cmd) + + ignored_touch = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/touch', + args=ignored_touch_cmd, + name='touch', + container_id=test_container.id[:12], + loginuid=loginuid) + ignored_chown = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/chown', + args=ignored_chown_cmd, + name='chown', + container_id=test_container.id[:12], + loginuid=loginuid) + reported_touch = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/touch', + args=monitored_touch_cmd, + name='touch', + container_id=test_container.id[:12], + loginuid=loginuid) + reported_chown = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/chown', + args=monitored_chown_cmd, + name='chown', + container_id=test_container.id[:12], + loginuid=loginuid) + ignored_events = [ + Event(process=ignored_touch, + event_type=EventType.CREATION, + file=ignored_file, + host_path=''), + Event(process=ignored_chown, + event_type=EventType.OWNERSHIP, + file=ignored_file, + host_path='', + owner_uid=TEST_UID, + owner_gid=TEST_GID), + ] + expected_events = [ + Event(process=reported_touch, + event_type=EventType.CREATION, + file=monitored_file, + host_path=''), + Event(process=reported_chown, + event_type=EventType.OWNERSHIP, + file=monitored_file, + host_path='', + owner_uid=TEST_UID, + owner_gid=TEST_GID), + ] + + for e in ignored_events: + print(f'Events that should be ignored: {e}') + + for e in expected_events: + print(f'Waiting for event: {e}') + + server.wait_events(events=expected_events, ignored=ignored_events) + + +def test_no_change(fact, test_container, server): + """ + Tests that chown to the same UID/GID triggers events for all calls. + + Args: + fact: Fixture for file activity (only required to be running). + test_container: A container for running commands in. + server: The server instance to communicate with. + """ + # File Under Test + fut = '/container-dir/test.txt' + + touch_cmd = f'touch {fut}' + chown_cmd = f'chown {TEST_UID}:{TEST_GID} {fut}' + + # Create the file + test_container.exec_run(touch_cmd) + + # First chown to TEST_UID:TEST_GID - this should trigger an event + test_container.exec_run(chown_cmd) + + # Second chown to the same UID/GID - this should ALSO trigger an event + test_container.exec_run(chown_cmd) + + loginuid = pow(2, 32) - 1 + touch = Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/touch', + args=touch_cmd, + name='touch', + container_id=test_container.id[:12], + loginuid=loginuid) + chown = [ + Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/chown', + args=chown_cmd, + name='chown', + container_id=test_container.id[:12], + loginuid=loginuid), + Process(pid=None, + uid=0, + gid=0, + exe_path='/usr/bin/chown', + args=chown_cmd, + name='chown', + container_id=test_container.id[:12], + loginuid=loginuid) + ] + + # Expect both chown events (all calls to chown trigger events) + events = [ + Event(process=touch, event_type=EventType.CREATION, file=fut, + host_path=''), + *(Event(process=p, event_type=EventType.OWNERSHIP, file=fut, + host_path='', owner_uid=TEST_UID, owner_gid=TEST_GID) for p in chown), + ] + + for e in events: + print(f'Waiting for event: {e}') + + server.wait_events(events) +