diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 26254778..a3d30ea9 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -12,18 +12,32 @@ #include // clang-format on -__always_inline static void __submit_event(struct event_t* event, - struct metrics_by_hook_t* m, - file_activity_type_t event_type, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode, +struct submit_event_args_t { + struct event_t* event; + struct metrics_by_hook_t* metrics; + const char* filename; + inode_key_t inode; + inode_key_t parent_inode; + monitored_t monitored; +}; + +__always_inline static bool reserve_event(struct submit_event_args_t* args) { + args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (args->event == NULL) { + args->metrics->ringbuffer_full++; + return false; + } + return true; +} + +__always_inline static void __submit_event(struct submit_event_args_t* args, bool use_bpf_d_path) { - event->type = event_type; + struct event_t* event = args->event; event->timestamp = bpf_ktime_get_boot_ns(); - inode_copy_or_reset(&event->inode, inode); - inode_copy_or_reset(&event->parent_inode, parent_inode); - bpf_probe_read_str(event->filename, PATH_MAX, filename); + 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); struct helper_t* helper = get_helper(); if (helper == NULL) { @@ -36,96 +50,88 @@ __always_inline static void __submit_event(struct event_t* event, goto error; } - m->added++; + args->metrics->added++; bpf_ringbuf_submit(event, 0); return; error: - m->error++; + args->metrics->error++; bpf_ringbuf_discard(event, 0); } -__always_inline static void submit_open_event(struct metrics_by_hook_t* m, - file_activity_type_t event_type, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; +__always_inline static void submit_open_event(struct submit_event_args_t* args, + file_activity_type_t event_type) { + if (!reserve_event(args)) { return; } + args->event->type = event_type; - __submit_event(event, m, event_type, filename, inode, parent_inode, true); + __submit_event(args, true); } -__always_inline static void submit_unlink_event(struct metrics_by_hook_t* m, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; +__always_inline static void submit_unlink_event(struct submit_event_args_t* args) { + if (!reserve_event(args)) { return; } + args->event->type = FILE_ACTIVITY_UNLINK; - __submit_event(event, m, FILE_ACTIVITY_UNLINK, filename, inode, parent_inode, path_hooks_support_bpf_d_path); + __submit_event(args, path_hooks_support_bpf_d_path); } -__always_inline static void submit_mode_event(struct metrics_by_hook_t* m, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode, +__always_inline static void submit_mode_event(struct submit_event_args_t* args, umode_t mode, umode_t old_mode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; + if (!reserve_event(args)) { return; } - event->chmod.new = mode; - event->chmod.old = old_mode; + args->event->type = FILE_ACTIVITY_CHMOD; + args->event->chmod.new = mode; + args->event->chmod.old = old_mode; - __submit_event(event, m, FILE_ACTIVITY_CHMOD, filename, inode, parent_inode, path_hooks_support_bpf_d_path); + __submit_event(args, path_hooks_support_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, - inode_key_t* parent_inode, +__always_inline static void submit_ownership_event(struct submit_event_args_t* args, unsigned long long uid, unsigned long long gid, unsigned long long old_uid, unsigned long long old_gid) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; + if (!reserve_event(args)) { return; } - event->chown.new.uid = uid; - event->chown.new.gid = gid; - event->chown.old.uid = old_uid; - event->chown.old.gid = old_gid; + args->event->type = FILE_ACTIVITY_CHOWN; + args->event->chown.new.uid = uid; + args->event->chown.new.gid = gid; + args->event->chown.old.uid = old_uid; + args->event->chown.old.gid = old_gid; - __submit_event(event, m, FILE_ACTIVITY_CHOWN, filename, inode, parent_inode, path_hooks_support_bpf_d_path); + __submit_event(args, path_hooks_support_bpf_d_path); } -__always_inline static void submit_rename_event(struct metrics_by_hook_t* m, - const char new_filename[PATH_MAX], +__always_inline static void submit_rename_event(struct submit_event_args_t* args, const char old_filename[PATH_MAX], - inode_key_t* new_inode, inode_key_t* old_inode, - inode_key_t* new_parent_inode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; + monitored_t old_monitored) { + if (!reserve_event(args)) { return; } - bpf_probe_read_str(event->rename.old_filename, PATH_MAX, old_filename); - inode_copy_or_reset(&event->rename.old_inode, old_inode); + args->event->type = FILE_ACTIVITY_RENAME; + bpf_probe_read_str(args->event->rename.filename, PATH_MAX, old_filename); + inode_copy(&args->event->rename.inode, old_inode); + args->event->rename.monitored = old_monitored; + + __submit_event(args, path_hooks_support_bpf_d_path); +} + +__always_inline static void submit_mkdir_event(struct submit_event_args_t* args) { + if (!reserve_event(args)) { + return; + } + args->event->type = DIR_ACTIVITY_CREATION; - __submit_event(event, m, FILE_ACTIVITY_RENAME, new_filename, new_inode, new_parent_inode, path_hooks_support_bpf_d_path); + // d_instantiate doesn't support bpf_d_path, so we use false and rely on the stashed path from path_mkdir + __submit_event(args, false); } diff --git a/fact-ebpf/src/bpf/file.h b/fact-ebpf/src/bpf/file.h index d0fdc8b1..942ecbdf 100644 --- a/fact-ebpf/src/bpf/file.h +++ b/fact-ebpf/src/bpf/file.h @@ -27,18 +27,33 @@ __always_inline static bool path_is_monitored(struct bound_path_t* path) { return res; } -__always_inline static inode_monitored_t is_monitored(inode_key_t inode, struct bound_path_t* path, const inode_key_t* parent, inode_key_t** submit) { - const inode_value_t* volatile inode_value = inode_get(&inode); +__always_inline static monitored_t is_monitored(const inode_key_t* inode, struct bound_path_t* path, const inode_key_t* parent) { + const inode_value_t* volatile inode_value = inode_get(inode); const inode_value_t* volatile parent_value = inode_get(parent); - inode_monitored_t status = inode_is_monitored(inode_value, parent_value); + monitored_t status = inode_is_monitored(inode_value, parent_value); if (status != NOT_MONITORED) { return status; } - *submit = NULL; if (path_is_monitored(path)) { - return MONITORED; + return MONITORED_BY_PATH; + } + + return NOT_MONITORED; +} + +// Check if a new directory should be tracked based on its parent and path. +// This is used during mkdir operations where the child inode doesn't exist yet. +__always_inline static monitored_t should_track_mkdir(inode_key_t parent_inode, struct bound_path_t* child_path) { + const inode_value_t* volatile parent_value = inode_get(&parent_inode); + + if (parent_value != NULL) { + return MONITORED_BY_PARENT; + } + + if (path_is_monitored(child_path)) { + return MONITORED_BY_PATH; } return NOT_MONITORED; diff --git a/fact-ebpf/src/bpf/inode.h b/fact-ebpf/src/bpf/inode.h index 481313e3..01f01814 100644 --- a/fact-ebpf/src/bpf/inode.h +++ b/fact-ebpf/src/bpf/inode.h @@ -80,35 +80,36 @@ __always_inline static long inode_remove(struct inode_key_t* inode) { return bpf_map_delete_elem(&inode_map, inode); } -typedef enum inode_monitored_t { - NOT_MONITORED = 0, - MONITORED, - PARENT_MONITORED, -} inode_monitored_t; +__always_inline static void inode_reset(struct inode_key_t* inode) { + inode->inode = 0; + inode->dev = 0; +} + +/* + * Check if the supplied inode is empty + */ +__always_inline static bool inode_is_empty(struct inode_key_t* inode) { + return inode->inode == 0 && inode->dev == 0; +} // Check if the provided inode or its parent is being monitored. -__always_inline static inode_monitored_t inode_is_monitored(const inode_value_t* volatile inode, const inode_value_t* volatile parent_inode) { +__always_inline static monitored_t inode_is_monitored(const inode_value_t* volatile inode, const inode_value_t* volatile parent_inode) { if (inode != NULL) { - return MONITORED; + return MONITORED_BY_INODE; } if (parent_inode != NULL) { - return PARENT_MONITORED; + return MONITORED_BY_PARENT; } return NOT_MONITORED; } -__always_inline static void inode_copy_or_reset(inode_key_t* dst, const inode_key_t* src) { +__always_inline static void inode_copy(inode_key_t* dst, const inode_key_t* src) { if (dst == NULL) { return; } - if (src != NULL) { - dst->inode = src->inode; - dst->dev = src->dev; - } else { - dst->inode = 0; - dst->dev = 0; - } + dst->inode = src->inode; + dst->dev = src->dev; } diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 5ac35159..258aba0c 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -25,8 +25,9 @@ int BPF_PROG(trace_file_open, struct file* file) { if (m == NULL) { return 0; } + struct submit_event_args_t args = {.metrics = &m->file_open}; - m->file_open.total++; + args.metrics->total++; file_activity_type_t event_type = FILE_ACTIVITY_INIT; if ((file->f_mode & FMODE_CREATED) != 0) { @@ -43,25 +44,24 @@ int BPF_PROG(trace_file_open, struct file* file) { m->file_open.error++; return 0; } + args.filename = path->path; - inode_key_t inode_key = inode_to_key(file->f_inode); - inode_key_t* inode_to_submit = &inode_key; + args.inode = inode_to_key(file->f_inode); struct dentry* parent_dentry = BPF_CORE_READ(file, f_path.dentry, d_parent); struct inode* parent_inode_ptr = parent_dentry ? BPF_CORE_READ(parent_dentry, d_inode) : NULL; - inode_key_t parent_key = inode_to_key(parent_inode_ptr); + args.parent_inode = inode_to_key(parent_inode_ptr); - inode_monitored_t status = is_monitored(inode_key, path, &parent_key, &inode_to_submit); - - if (status == PARENT_MONITORED && event_type == FILE_ACTIVITY_CREATION) { - inode_add(&inode_key); + args.monitored = is_monitored(&args.inode, path, &args.parent_inode); + if (args.monitored == NOT_MONITORED) { + goto ignored; } - if (status == NOT_MONITORED) { - goto ignored; + if (args.monitored == MONITORED_BY_PARENT && event_type == FILE_ACTIVITY_CREATION) { + inode_add(&args.inode); } - submit_open_event(&m->file_open, event_type, path->path, inode_to_submit, &parent_key); + submit_open_event(&args, event_type); return 0; @@ -76,8 +76,9 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) { if (m == NULL) { return 0; } + struct submit_event_args_t args = {.metrics = &m->path_unlink}; - m->path_unlink.total++; + args.metrics->total++; struct bound_path_t* path = path_read_append_d_entry(dir, dentry); if (path == NULL) { @@ -85,22 +86,20 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) { m->path_unlink.error++; return 0; } + args.filename = path->path; - inode_key_t inode_key = inode_to_key(dentry->d_inode); - inode_key_t* inode_to_submit = &inode_key; + args.inode = inode_to_key(dentry->d_inode); + args.monitored = is_monitored(&args.inode, path, NULL); - if (is_monitored(inode_key, path, NULL, &inode_to_submit) == NOT_MONITORED) { + if (args.monitored == NOT_MONITORED) { m->path_unlink.ignored++; return 0; } // We only support files with one link for now - inode_remove(&inode_key); + inode_remove(&args.inode); - submit_unlink_event(&m->path_unlink, - path->path, - inode_to_submit, - NULL); + submit_unlink_event(&args); return 0; } @@ -110,31 +109,28 @@ int BPF_PROG(trace_path_chmod, struct path* path, umode_t mode) { if (m == NULL) { return 0; } + struct submit_event_args_t args = {.metrics = &m->path_chmod}; - m->path_chmod.total++; + args.metrics->total++; struct bound_path_t* bound_path = path_read(path); if (bound_path == NULL) { bpf_printk("Failed to read path"); - m->path_chmod.error++; + args.metrics->error++; return 0; } + args.filename = bound_path->path; - inode_key_t inode_key = inode_to_key(path->dentry->d_inode); - inode_key_t* inode_to_submit = &inode_key; + args.inode = inode_to_key(path->dentry->d_inode); + args.monitored = is_monitored(&args.inode, bound_path, NULL); - if (is_monitored(inode_key, bound_path, NULL, &inode_to_submit) == NOT_MONITORED) { - m->path_chmod.ignored++; + if (args.monitored == NOT_MONITORED) { + args.metrics->ignored++; return 0; } umode_t old_mode = BPF_CORE_READ(path, dentry, d_inode, i_mode); - submit_mode_event(&m->path_chmod, - bound_path->path, - inode_to_submit, - NULL, - mode, - old_mode); + submit_mode_event(&args, mode, old_mode); return 0; } @@ -148,21 +144,23 @@ int BPF_PROG(trace_path_chown, struct path* path, unsigned long long uid, unsign if (m == NULL) { return 0; } + struct submit_event_args_t args = {.metrics = &m->path_chown}; - m->path_chown.total++; + args.metrics->total++; struct bound_path_t* bound_path = path_read(path); if (bound_path == NULL) { bpf_printk("Failed to read path"); - m->path_chown.error++; + args.metrics->error++; return 0; } + args.filename = bound_path->path; - inode_key_t inode_key = inode_to_key(path->dentry->d_inode); - inode_key_t* inode_to_submit = &inode_key; + args.inode = inode_to_key(path->dentry->d_inode); + args.monitored = is_monitored(&args.inode, bound_path, NULL); - if (is_monitored(inode_key, bound_path, NULL, &inode_to_submit) == NOT_MONITORED) { - m->path_chown.ignored++; + if (args.monitored == NOT_MONITORED) { + args.metrics->ignored++; return 0; } @@ -170,14 +168,7 @@ int BPF_PROG(trace_path_chown, struct path* path, unsigned long long uid, unsign 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_to_submit, - NULL, - uid, - gid, - old_uid, - old_gid); + submit_ownership_event(&args, uid, gid, old_uid, old_gid); return 0; } @@ -190,14 +181,16 @@ int BPF_PROG(trace_path_rename, struct path* old_dir, if (m == NULL) { return 0; } + struct submit_event_args_t args = {.metrics = &m->path_rename}; - m->path_rename.total++; + args.metrics->total++; struct bound_path_t* new_path = path_read_append_d_entry(new_dir, new_dentry); if (new_path == NULL) { bpf_printk("Failed to read path"); goto error; } + args.filename = new_path->path; struct bound_path_t* old_path = path_read_alt_append_d_entry(old_dir, old_dentry); if (old_path == NULL) { @@ -205,29 +198,177 @@ int BPF_PROG(trace_path_rename, struct path* old_dir, goto error; } + args.inode = inode_to_key(new_dentry->d_inode); + args.parent_inode = inode_to_key(new_dir->dentry->d_inode); + args.monitored = is_monitored(&args.inode, new_path, &args.parent_inode); + inode_key_t old_inode = inode_to_key(old_dentry->d_inode); - inode_key_t new_inode = inode_to_key(new_dentry->d_inode); + monitored_t old_monitored = is_monitored(&old_inode, old_path, NULL); + + // From this point on we need to handle inode tracking. + // + // The result will be a combination of whether we are already tracking + // the old inode or not and whether the target path has an existing + // object that is about to be overwritten and if said object is + // tracked by inode or not. + switch (args.monitored) { + case NOT_MONITORED: + if (old_monitored == NOT_MONITORED) { + m->path_rename.ignored++; + return 0; + } + + if (old_monitored == MONITORED_BY_INODE) { + // Old inode is monitored, new path is not. + // If the old path is a directory userspace will remove any + // subdirectories and files too. + inode_remove(&old_inode); + } + break; + + case MONITORED_BY_PATH: + if (old_monitored == MONITORED_BY_INODE) { + // New path is not inode tracked, old path is. + // + // This implies the inode will be crossing a FS mountpoint, + // which should never happen. When the inode crosses into a new + // mount, a new inode is created altogether. Still, we can cover + // our bases. + inode_remove(&old_inode); + } + break; + + case MONITORED_BY_PARENT: + if (old_monitored != MONITORED_BY_INODE) { + // Old inode is not monitored, new parent is. + if (inode_is_empty(&args.inode)) { + // Landing on an empty path, we track the inode in case we + // need to, userspace will double check in detail. + inode_add(&old_inode); + } + } else if (!inode_is_empty(&args.inode)) { + // Old inode is monitored and will land on a path that has a + // monitored parent but the path itself is not monitored, we + // stop tracking the inode + inode_remove(&old_inode); + } + break; + + case MONITORED_BY_INODE: + // If we landed here, the new path already has an inode that is + // being tracked and is about to be overwritten, we need to remove + // it from the map + inode_remove(&args.inode); + if (old_monitored != MONITORED_BY_INODE) { + // Old inode is not monitored, but is landing in a monitored + // path that uses inode tracking. + inode_add(&old_inode); + } + break; + } + + submit_rename_event(&args, old_path->path, &old_inode, old_monitored); + return 0; + +error: + args.metrics->error++; + return 0; +} + +SEC("lsm/path_mkdir") +int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t mode) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + + m->path_mkdir.total++; - inode_key_t* old_inode_submit = &old_inode; - inode_key_t* new_inode_submit = &new_inode; + struct bound_path_t* path = path_read_append_d_entry(dir, dentry); + if (path == NULL) { + bpf_printk("Failed to read path"); + m->path_mkdir.error++; + return 0; + } - inode_monitored_t old_monitored = is_monitored(old_inode, old_path, NULL, &old_inode_submit); - inode_monitored_t new_monitored = is_monitored(new_inode, new_path, NULL, &new_inode_submit); + struct inode* parent_inode_ptr = BPF_CORE_READ(dir, dentry, d_inode); + inode_key_t parent_inode = inode_to_key(parent_inode_ptr); - if (old_monitored == NOT_MONITORED && new_monitored == NOT_MONITORED) { - m->path_rename.ignored++; + monitored_t monitored = should_track_mkdir(parent_inode, path); + if (monitored != MONITORED_BY_PARENT) { + m->path_mkdir.ignored++; return 0; } - submit_rename_event(&m->path_rename, - new_path->path, - old_path->path, - old_inode_submit, - new_inode_submit, - NULL); + // Stash mkdir context for security_d_instantiate + __u64 pid_tgid = bpf_get_current_pid_tgid(); + struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); + if (mkdir_ctx == NULL) { + static const struct mkdir_context_t empty_ctx = {0}; + if (bpf_map_update_elem(&mkdir_context, &pid_tgid, &empty_ctx, BPF_NOEXIST) != 0) { + bpf_printk("Failed to create mkdir context entry"); + m->path_mkdir.error++; + return 0; + } + mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); + if (mkdir_ctx == NULL) { + bpf_printk("Failed to lookup mkdir context after creation"); + m->path_mkdir.error++; + return 0; + } + } + + long path_copy_len = bpf_probe_read_str(mkdir_ctx->path, PATH_MAX, path->path); + if (path_copy_len < 0) { + bpf_printk("Failed to copy path string"); + m->path_mkdir.error++; + bpf_map_delete_elem(&mkdir_context, &pid_tgid); + return 0; + } + mkdir_ctx->parent_inode = parent_inode; + mkdir_ctx->monitored = monitored; + return 0; +} -error: - m->path_rename.error++; +SEC("lsm/d_instantiate") +int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + struct submit_event_args_t args = {.metrics = &m->d_instantiate}; + + args.metrics->total++; + + __u64 pid_tgid = bpf_get_current_pid_tgid(); + + if (inode == NULL) { + args.metrics->ignored++; + goto cleanup; + } + + struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); + + if (mkdir_ctx == NULL) { + args.metrics->ignored++; + return 0; + } + args.filename = mkdir_ctx->path; + args.parent_inode = mkdir_ctx->parent_inode; + args.monitored = mkdir_ctx->monitored; + + args.inode = inode_to_key(inode); + + if (inode_add(&args.inode) == 0) { + args.metrics->added++; + } else { + args.metrics->error++; + } + + submit_mkdir_event(&args); + +cleanup: + bpf_map_delete_elem(&mkdir_context, &pid_tgid); return 0; } diff --git a/fact-ebpf/src/bpf/maps.h b/fact-ebpf/src/bpf/maps.h index eca822f0..c8595829 100644 --- a/fact-ebpf/src/bpf/maps.h +++ b/fact-ebpf/src/bpf/maps.h @@ -83,6 +83,13 @@ struct { __uint(map_flags, BPF_F_NO_PREALLOC); } inode_map SEC(".maps"); +struct { + __uint(type, BPF_MAP_TYPE_LRU_HASH); + __type(key, __u64); + __type(value, struct mkdir_context_t); + __uint(max_entries, 16384); +} mkdir_context SEC(".maps"); + struct { __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); __type(key, __u32); diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index 55005c00..67d8df49 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -42,6 +42,13 @@ typedef struct inode_key_t { unsigned long dev; } inode_key_t; +typedef enum monitored_t { + NOT_MONITORED = 0, + MONITORED_BY_INODE, + MONITORED_BY_PATH, + MONITORED_BY_PARENT, +} monitored_t; + // We can't use bool here because it is not a standard C type, we would // need to include vmlinux.h but that would explode our Rust bindings. // For the time being we just keep a char. @@ -55,6 +62,7 @@ typedef enum file_activity_type_t { FILE_ACTIVITY_CHMOD, FILE_ACTIVITY_CHOWN, FILE_ACTIVITY_RENAME, + DIR_ACTIVITY_CREATION, } file_activity_type_t; struct event_t { @@ -63,6 +71,7 @@ struct event_t { char filename[PATH_MAX]; inode_key_t inode; inode_key_t parent_inode; + monitored_t monitored; file_activity_type_t type; union { struct { @@ -76,8 +85,9 @@ struct event_t { } old, new; } chown; struct { - char old_filename[PATH_MAX]; - inode_key_t old_inode; + char filename[PATH_MAX]; + inode_key_t inode; + monitored_t monitored; } rename; }; }; @@ -96,6 +106,13 @@ struct path_prefix_t { const char path[LPM_SIZE_MAX]; }; +// Context for correlating mkdir operations +struct mkdir_context_t { + char path[PATH_MAX]; + inode_key_t parent_inode; + monitored_t monitored; +}; + // Metrics types struct metrics_by_hook_t { unsigned long long total; @@ -111,4 +128,6 @@ struct metrics_t { struct metrics_by_hook_t path_chmod; struct metrics_by_hook_t path_chown; struct metrics_by_hook_t path_rename; + struct metrics_by_hook_t path_mkdir; + struct metrics_by_hook_t d_instantiate; }; diff --git a/fact-ebpf/src/lib.rs b/fact-ebpf/src/lib.rs index bd84ee08..5d5984ab 100644 --- a/fact-ebpf/src/lib.rs +++ b/fact-ebpf/src/lib.rs @@ -103,6 +103,27 @@ impl Serialize for inode_key_t { unsafe impl Pod for inode_key_t {} +impl Default for monitored_t { + fn default() -> Self { + monitored_t::NOT_MONITORED + } +} + +impl Serialize for monitored_t { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + match *self { + monitored_t::NOT_MONITORED => "not monitored".serialize(serializer), + monitored_t::MONITORED_BY_INODE => "by inode".serialize(serializer), + monitored_t::MONITORED_BY_PATH => "by path".serialize(serializer), + monitored_t::MONITORED_BY_PARENT => "by parent".serialize(serializer), + _ => unreachable!("Invalid monitored_t value: {self:?}"), + } + } +} + impl metrics_by_hook_t { fn accumulate(&self, other: &metrics_by_hook_t) -> metrics_by_hook_t { let mut m = metrics_by_hook_t { ..*self }; @@ -125,6 +146,8 @@ impl metrics_t { m.path_chmod = m.path_chmod.accumulate(&other.path_chmod); m.path_chown = m.path_chown.accumulate(&other.path_chown); m.path_rename = m.path_rename.accumulate(&other.path_rename); + m.path_mkdir = m.path_mkdir.accumulate(&other.path_mkdir); + m.d_instantiate = m.d_instantiate.accumulate(&other.d_instantiate); m } } diff --git a/fact/src/bpf/mod.rs b/fact/src/bpf/mod.rs index 38fe269d..94fd6a08 100644 --- a/fact/src/bpf/mod.rs +++ b/fact/src/bpf/mod.rs @@ -9,7 +9,7 @@ use aya::{ use checks::Checks; use globset::{Glob, GlobSet, GlobSetBuilder}; use libc::c_char; -use log::{error, info}; +use log::{error, info, warn}; use tokio::{ io::unix::AsyncFd, sync::{mpsc, watch}, @@ -160,7 +160,9 @@ impl Bpf { // Remove old prefixes for p in self.paths.iter().filter(|p| !new_paths.contains(p)) { - path_prefix.remove(&(*p).into())?; + if let Err(e) = path_prefix.remove(&(*p).into()) { + warn!("Failed to remove path prefix: {e:#?}"); + } } self.paths = new_paths; @@ -228,7 +230,12 @@ impl Bpf { let event: &event_t = unsafe { &*(event.as_ptr() as *const _) }; let event = match Event::try_from(event) { Ok(event) => { - if event.is_ignored(&self.paths_globset) { + // If the event is monitored by parent, we need to check + // its host path, but we don't have that context here, + // so we let the event go into HostScanner and make the + // decision there. + if !event.is_monitored_by_parent() && + event.is_ignored(&self.paths_globset) { event_counter.dropped(); continue; } diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index bc9d7897..a5e96c47 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -7,9 +7,10 @@ use std::{ }; use globset::GlobSet; +use log::warn; use serde::Serialize; -use fact_ebpf::{PATH_MAX, event_t, file_activity_type_t, inode_key_t}; +use fact_ebpf::{PATH_MAX, event_t, file_activity_type_t, inode_key_t, monitored_t}; use crate::host_info; use process::Process; @@ -94,6 +95,7 @@ impl Event { host_file, inode: Default::default(), parent_inode: Default::default(), + monitored: Default::default(), }; let file = match data { EventTestData::Creation => FileData::Creation(inner), @@ -127,13 +129,25 @@ impl Event { } pub fn is_creation(&self) -> bool { + matches!(self.file, FileData::Creation(_) | FileData::MkDir(_)) + } + + pub fn is_file_creation(&self) -> bool { matches!(self.file, FileData::Creation(_)) } + pub fn is_mkdir(&self) -> bool { + matches!(self.file, FileData::MkDir(_)) + } + pub fn is_unlink(&self) -> bool { matches!(self.file, FileData::Unlink(_)) } + pub fn is_rename(&self) -> bool { + matches!(self.file, FileData::Rename(_)) + } + /// Unwrap the inner FileData and return the inode that triggered /// the event. /// @@ -143,6 +157,7 @@ impl Event { match &self.file { FileData::Open(data) => &data.inode, FileData::Creation(data) => &data.inode, + FileData::MkDir(data) => &data.inode, FileData::Unlink(data) => &data.inode, FileData::Chmod(data) => &data.inner.inode, FileData::Chown(data) => &data.inner.inode, @@ -155,6 +170,7 @@ impl Event { match &self.file { FileData::Open(data) => &data.parent_inode, FileData::Creation(data) => &data.parent_inode, + FileData::MkDir(data) => &data.parent_inode, FileData::Unlink(data) => &data.parent_inode, FileData::Chmod(data) => &data.inner.parent_inode, FileData::Chown(data) => &data.inner.parent_inode, @@ -176,6 +192,7 @@ impl Event { match &self.file { FileData::Open(data) => &data.filename, FileData::Creation(data) => &data.filename, + FileData::MkDir(data) => &data.filename, FileData::Unlink(data) => &data.filename, FileData::Chmod(data) => &data.inner.filename, FileData::Chown(data) => &data.inner.filename, @@ -183,7 +200,7 @@ impl Event { } } - fn get_old_filename(&self) -> Option<&PathBuf> { + pub fn get_old_filename(&self) -> Option<&PathBuf> { match &self.file { FileData::Rename(data) => Some(&data.old.filename), _ => None, @@ -194,6 +211,7 @@ impl Event { match &self.file { FileData::Open(data) => &data.host_file, FileData::Creation(data) => &data.host_file, + FileData::MkDir(data) => &data.host_file, FileData::Unlink(data) => &data.host_file, FileData::Chmod(data) => &data.inner.host_file, FileData::Chown(data) => &data.inner.host_file, @@ -201,6 +219,13 @@ impl Event { } } + pub fn get_old_host_path(&self) -> Option<&PathBuf> { + match &self.file { + FileData::Rename(data) => Some(&data.old.host_file), + _ => None, + } + } + /// Set the `host_file` field of the event to the one provided. /// /// In the case of operations that involve two paths, like rename, @@ -209,6 +234,7 @@ impl Event { match &mut self.file { FileData::Open(data) => data.host_file = host_path, FileData::Creation(data) => data.host_file = host_path, + FileData::MkDir(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, @@ -224,6 +250,25 @@ impl Event { } } + pub fn get_monitored(&self) -> monitored_t { + match &self.file { + FileData::Open(data) => data.monitored, + FileData::Creation(data) => data.monitored, + FileData::MkDir(data) => data.monitored, + FileData::Unlink(data) => data.monitored, + FileData::Chmod(data) => data.inner.monitored, + FileData::Chown(data) => data.inner.monitored, + FileData::Rename(data) => data.new.monitored, + } + } + + pub fn get_old_monitored(&self) -> Option { + match &self.file { + FileData::Rename(data) => Some(data.old.monitored), + _ => None, + } + } + /// Determine if the event should be ignored. /// /// With wildcards, the kernel can only match on the inode and @@ -235,13 +280,19 @@ impl Event { /// /// We also need to check the old values for rename events. pub fn is_ignored(&self, globset: &GlobSet) -> bool { - self.get_inode().empty() - && self.get_old_inode().is_none_or(|inode| inode.empty()) + self.get_monitored() != monitored_t::MONITORED_BY_INODE + && self + .get_old_monitored() + .is_none_or(|m| m != monitored_t::MONITORED_BY_INODE) && !globset.is_match(self.get_filename()) && self .get_old_filename() .is_none_or(|path| !globset.is_match(path)) } + + pub fn is_monitored_by_parent(&self) -> bool { + self.get_monitored() == monitored_t::MONITORED_BY_PARENT + } } impl TryFrom<&event_t> for Event { @@ -255,6 +306,7 @@ impl TryFrom<&event_t> for Event { value.filename, value.inode, value.parent_inode, + value.monitored, value.__bindgen_anon_1, )?; @@ -293,6 +345,7 @@ impl PartialEq for Event { pub enum FileData { Open(BaseFileData), Creation(BaseFileData), + MkDir(BaseFileData), Unlink(BaseFileData), Chmod(ChmodFileData), Chown(ChownFileData), @@ -305,12 +358,14 @@ impl FileData { filename: [c_char; PATH_MAX as usize], inode: inode_key_t, parent_inode: inode_key_t, + monitored: monitored_t, extra_data: fact_ebpf::event_t__bindgen_ty_1, ) -> anyhow::Result { - let inner = BaseFileData::new(filename, inode, parent_inode)?; + let inner = BaseFileData::new(filename, inode, parent_inode, monitored)?; let file = match event_type { file_activity_type_t::FILE_ACTIVITY_OPEN => FileData::Open(inner), file_activity_type_t::FILE_ACTIVITY_CREATION => FileData::Creation(inner), + file_activity_type_t::DIR_ACTIVITY_CREATION => FileData::MkDir(inner), file_activity_type_t::FILE_ACTIVITY_UNLINK => FileData::Unlink(inner), file_activity_type_t::FILE_ACTIVITY_CHMOD => { let data = ChmodFileData { @@ -331,11 +386,17 @@ impl FileData { FileData::Chown(data) } file_activity_type_t::FILE_ACTIVITY_RENAME => { - let old_filename = unsafe { extra_data.rename.old_filename }; - let old_inode = unsafe { extra_data.rename.old_inode }; + let old_filename = unsafe { extra_data.rename.filename }; + let old_inode = unsafe { extra_data.rename.inode }; + let old_monitored = unsafe { extra_data.rename.monitored }; let data = RenameFileData { new: inner, - old: BaseFileData::new(old_filename, old_inode, Default::default())?, + old: BaseFileData::new( + old_filename, + old_inode, + Default::default(), + old_monitored, + )?, }; FileData::Rename(data) } @@ -359,6 +420,14 @@ impl From for fact_api::file_activity::File { let f_act = fact_api::FileCreation { activity }; fact_api::file_activity::File::Creation(f_act) } + FileData::MkDir(event) => { + warn!( + "MkDir event reached protobuf conversion - converting to Creation (filtering may have failed)" + ); + let activity = Some(fact_api::FileActivityBase::from(event)); + let f_act = fact_api::FileCreation { activity }; + fact_api::file_activity::File::Creation(f_act) + } FileData::Unlink(event) => { let activity = Some(fact_api::FileActivityBase::from(event)); let f_act = fact_api::FileUnlink { activity }; @@ -386,6 +455,7 @@ impl PartialEq for FileData { match (self, other) { (FileData::Open(this), FileData::Open(other)) => this == other, (FileData::Creation(this), FileData::Creation(other)) => this == other, + (FileData::MkDir(this), FileData::MkDir(other)) => this == other, (FileData::Unlink(this), FileData::Unlink(other)) => this == other, (FileData::Chmod(this), FileData::Chmod(other)) => this == other, (FileData::Rename(this), FileData::Rename(other)) => this == other, @@ -400,6 +470,7 @@ pub struct BaseFileData { host_file: PathBuf, inode: inode_key_t, parent_inode: inode_key_t, + monitored: monitored_t, } impl BaseFileData { @@ -407,12 +478,14 @@ impl BaseFileData { filename: [c_char; PATH_MAX as usize], inode: inode_key_t, parent_inode: inode_key_t, + monitored: monitored_t, ) -> anyhow::Result { Ok(BaseFileData { filename: sanitize_d_path(&filename), host_file: PathBuf::new(), // this field is set by HostScanner inode, parent_inode, + monitored, }) } } diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index 19d5b719..af143e44 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -28,7 +28,8 @@ use std::{ use anyhow::{Context, bail}; use aya::maps::MapData; -use fact_ebpf::{inode_key_t, inode_value_t}; +use fact_ebpf::{inode_key_t, inode_value_t, monitored_t}; +use globset::{Glob, GlobSet, GlobSetBuilder}; use log::{debug, info, warn}; use tokio::{ sync::{Notify, broadcast, mpsc, watch}, @@ -54,6 +55,8 @@ pub struct HostScanner { tx: broadcast::Sender>, metrics: HostScannerMetrics, + + paths_globset: GlobSet, } impl HostScanner { @@ -68,6 +71,7 @@ impl HostScanner { let kernel_inode_map = RefCell::new(bpf.take_inode_map()?); let inode_map = RefCell::new(std::collections::HashMap::new()); let (tx, _) = broadcast::channel(100); + let paths_globset = HostScanner::build_globset(paths.borrow().as_slice())?; let host_scanner = HostScanner { kernel_inode_map, @@ -78,6 +82,7 @@ impl HostScanner { rx, tx, metrics, + paths_globset, }; // Run an initial scan to fill in the inode map @@ -86,6 +91,22 @@ impl HostScanner { Ok(host_scanner) } + fn build_globset(paths: &[PathBuf]) -> anyhow::Result { + let mut builder = GlobSetBuilder::new(); + for p in paths.iter() { + let Some(glob_str) = p.to_str() else { + bail!("failed to convert path {} to string", p.display()); + }; + + builder.add( + Glob::new(glob_str) + .with_context(|| format!("invalid glob {}", glob_str)) + .unwrap(), + ); + } + Ok(builder.build()?) + } + fn scan(&self) -> anyhow::Result<()> { debug!("Host scan started"); self.metrics.scan_inc(ScanLabels::Scans); @@ -233,6 +254,127 @@ impl HostScanner { self.metrics.scan_inc(ScanLabels::FileRemoved); } + fn handle_rename_event(&self, event: &mut Event) { + match event.get_monitored() { + monitored_t::MONITORED_BY_INODE => { + // This condition means a file is being renamed and taking the + // place of an existing, tracked file. We need to remove the + // inode we are landing on and put the associated host path in + // the old inode. + let mut inode_map = self.inode_map.borrow_mut(); + let Some(path) = inode_map.remove(event.get_inode()) else { + warn!("Old path was not found for inode tracked event"); + return; + }; + let Some(old_inode) = event.get_old_inode() else { + unreachable!("old inode not found for rename event"); + }; + inode_map.insert(*old_inode, path); + } + monitored_t::NOT_MONITORED + if event.get_old_monitored() == Some(monitored_t::MONITORED_BY_INODE) => + { + // We are landing on a path that is not tracked at all, remove + // the entries for the old path from the map + let Some(old_host_path) = event.get_old_host_path() else { + warn!("Rename event did not have old host path for inode tracked item"); + return; + }; + self.inode_map.borrow_mut().retain(|inode, path| { + if !path.starts_with(old_host_path) { + return true; + } + + let _ = self.kernel_inode_map.borrow_mut().remove(inode); + false + }); + } + monitored_t::NOT_MONITORED => { + // The new path is not monitored and the old path is most likely + // matching by path, we don't need to do anything in this case. + } + monitored_t::MONITORED_BY_PARENT if !event.get_inode().empty() => { + // The parent for the target is monitored, but the file itself + // is not. Remove the entry for the old file from the map. + self.inode_map.borrow_mut().remove( + event + .get_old_inode() + .expect("rename event did not have old inode"), + ); + } + monitored_t::MONITORED_BY_PARENT + if event.get_old_monitored() == Some(monitored_t::MONITORED_BY_INODE) => + { + // The target is monitored by parent and we are landing on a + // path that didn't hold anything, we need to figure out the + // host path and check if we should track it. + let mut inode_map = self.inode_map.borrow_mut(); + let Some(new_host_parent) = inode_map.get(event.get_parent_inode()) else { + warn!("Failed to get parent host path"); + return; + }; + let Some(filename) = event.get_filename().file_name() else { + warn!("Failed to get last component from event: {event:#?}"); + return; + }; + let new_host_path = new_host_parent.join(filename); + let Some(old_host_path) = event.get_old_host_path() else { + unreachable!("Rename event did not have an old host path"); + }; + + if self.paths_globset.is_match(&new_host_path) { + // New path needs to be tracked. + // Move all entries for the old host path to the new one + for (_, path) in inode_map.iter_mut() { + if let Ok(suffix) = path.strip_prefix(old_host_path) { + if suffix == Path::new("") { + *path = new_host_path.clone(); + } else { + *path = new_host_path.join(suffix); + } + } + } + + // Add the new host path to the event + event.set_host_path(new_host_path); + } else { + // New path is not tracked, remove old entries + inode_map.retain(|inode, path| { + if !path.starts_with(old_host_path) { + return true; + } + if let Err(e) = self.kernel_inode_map.borrow_mut().remove(inode) { + warn!("Failed to remove inode kernel entry: {e:?}"); + } + false + }); + } + } + monitored_t::MONITORED_BY_PARENT => { + // In this case, the target location might be monitored, but we + // don't have any information of the host path for the old path, + // best we can do is attempt to scan the file system and fix the + // inode maps that way. + if let Err(e) = self.scan() { + warn!("Scan failed: {e:?}"); + } + + // Attempt to update the host path with the old inode + if let Some(old_inode) = event.get_old_inode() + && let Some(path) = self.inode_map.borrow().get(old_inode) + { + event.set_host_path(path.clone()); + } + } + monitored_t::MONITORED_BY_PATH => { + // Nothing to do here, having one side of the rename monitored + // by path means at best the other side is also monitored by + // path, no inode tracking is involved. + } + _ => unreachable!("Invalid monitored value"), + } + } + /// Periodically notify the host scanner main task that a scan needs /// to happen. /// @@ -280,12 +422,17 @@ impl HostScanner { }; self.metrics.events.added(); - // Handle file creation events by adding new inodes to the map + // Handle file and directory creation events by adding new inodes to the map if event.is_creation() && let Err(e) = self.handle_creation_event(&event) { warn!("Failed to handle creation event: {e}"); } + // Skip directory creation events - we track them internally but don't send to sensor + if event.is_mkdir() { + continue; + } + if let Some(host_path) = self.get_host_path(Some(event.get_inode())) { self.metrics.scan_inc(ScanLabels::InodeHit); event.set_host_path(host_path); @@ -301,6 +448,20 @@ impl HostScanner { self.handle_unlink_event(&event); } + if event.is_rename() { self.handle_rename_event(&mut event); } + + if event.is_monitored_by_parent() && + !self.paths_globset.is_match(event.get_host_path()) { + // The event was monitored by parent, but the host + // path is not to be monitored, so we ignore the + // event and attempt to remove the inode from the + // maps to prevent it from sending more events. + self.inode_map.borrow_mut().remove(event.get_inode()); + let _ = self.kernel_inode_map.borrow_mut().remove(event.get_inode()); + self.metrics.events.ignored(); + continue; + } + let event = Arc::new(event); if let Err(e) = self.tx.send(event) { self.metrics.events.dropped(); @@ -308,7 +469,10 @@ impl HostScanner { } }, _ = scan_trigger.notified() => self.scan()?, - _ = self.paths.changed() => self.scan()?, + _ = self.paths.changed() => { + self.paths_globset = HostScanner::build_globset(self.paths.borrow().as_slice())?; + self.scan()?; + } _ = self.running.changed() => { if !*self.running.borrow() { break; diff --git a/fact/src/metrics/host_scanner.rs b/fact/src/metrics/host_scanner.rs index f5197cf5..81bef60c 100644 --- a/fact/src/metrics/host_scanner.rs +++ b/fact/src/metrics/host_scanner.rs @@ -33,7 +33,12 @@ pub struct HostScannerMetrics { impl HostScannerMetrics { pub(super) fn new() -> Self { - let labels = [EventLabels::Total, EventLabels::Added, EventLabels::Dropped]; + let labels = [ + EventLabels::Total, + EventLabels::Added, + EventLabels::Dropped, + EventLabels::Ignored, + ]; let events = EventCounter::new( "host_scanner_events", "Events processed by the host scanner component", diff --git a/fact/src/metrics/kernel_metrics.rs b/fact/src/metrics/kernel_metrics.rs index d1a3a242..9caa1ff3 100644 --- a/fact/src/metrics/kernel_metrics.rs +++ b/fact/src/metrics/kernel_metrics.rs @@ -13,6 +13,8 @@ pub struct KernelMetrics { path_chmod: EventCounter, path_chown: EventCounter, path_rename: EventCounter, + path_mkdir: EventCounter, + d_instantiate: EventCounter, map: PerCpuArray, } @@ -43,12 +45,24 @@ impl KernelMetrics { "Events processed by the path_rename LSM hook", &[], // Labels are not needed since `collect` will add them all ); + let path_mkdir = EventCounter::new( + "kernel_path_mkdir_events", + "Events processed by the path_mkdir LSM hook", + &[], // Labels are not needed since `collect` will add them all + ); + let d_instantiate = EventCounter::new( + "kernel_d_instantiate_events", + "Events processed by the d_instantiate 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); path_rename.register(reg); + path_mkdir.register(reg); + d_instantiate.register(reg); KernelMetrics { file_open, @@ -56,6 +70,8 @@ impl KernelMetrics { path_chmod, path_chown, path_rename, + path_mkdir, + d_instantiate, map: kernel_metrics, } } @@ -105,6 +121,8 @@ impl KernelMetrics { KernelMetrics::refresh_labels(&self.path_chmod, &metrics.path_chmod); KernelMetrics::refresh_labels(&self.path_chown, &metrics.path_chown); KernelMetrics::refresh_labels(&self.path_rename, &metrics.path_rename); + KernelMetrics::refresh_labels(&self.path_mkdir, &metrics.path_mkdir); + KernelMetrics::refresh_labels(&self.d_instantiate, &metrics.d_instantiate); Ok(()) } diff --git a/fact/src/metrics/mod.rs b/fact/src/metrics/mod.rs index 97213c83..c7a7cd73 100644 --- a/fact/src/metrics/mod.rs +++ b/fact/src/metrics/mod.rs @@ -100,6 +100,19 @@ impl EventCounter { .unwrap() .inc_by(n); } + + /// Increment the counter for the Ignored label. + /// + /// Panics if the counter did not add the Ignored label as part of + /// its creation step. + pub fn ignored(&self) { + self.counter + .get(&MetricEvents { + label: LabelValues::Ignored, + }) + .expect("Ignored label not found") + .inc(); + } } #[derive(Debug, Clone)] diff --git a/tests/test_path_mkdir.py b/tests/test_path_mkdir.py new file mode 100644 index 00000000..88685f8f --- /dev/null +++ b/tests/test_path_mkdir.py @@ -0,0 +1,73 @@ +import os + +import pytest + +from event import Event, EventType, Process + + +@pytest.mark.parametrize("dirname", [ + pytest.param('level3', id='ASCII'), + pytest.param('café', id='French'), + pytest.param('файл', id='Cyrillic'), + pytest.param('日本語', id='Japanese'), +]) +def test_mkdir_nested(monitored_dir, server, dirname): + """ + Tests that creating nested directories tracks all inodes correctly. + + Args: + monitored_dir: Temporary directory path for creating the test directory. + server: The server instance to communicate with. + dirname: Final directory name to test (including UTF-8 variants). + """ + process = Process.from_proc() + + # Create nested directories + test_dir = os.path.join(monitored_dir, 'level1', 'level2', dirname) + os.makedirs(test_dir, exist_ok=True) + + # Create a file in the deepest directory + test_file = os.path.join(test_dir, 'deep_file.txt') + with open(test_file, 'w') as f: + f.write('nested content') + + # Directory creation events are tracked internally but not sent to sensor + # Only the file creation event should be sent + events = [ + Event(process=process, event_type=EventType.CREATION, + file=test_file, host_path=test_file), + ] + + server.wait_events(events) + + +def test_mkdir_ignored(monitored_dir, ignored_dir, server): + """ + Tests that directories created outside monitored paths are ignored. + + Args: + monitored_dir: Temporary directory path that is monitored. + ignored_dir: Temporary directory path that is not monitored. + server: The server instance to communicate with. + """ + process = Process.from_proc() + + # Create directory in ignored path - should not be tracked + ignored_subdir = os.path.join(ignored_dir, 'ignored_subdir') + os.mkdir(ignored_subdir) + ignored_file = os.path.join(ignored_subdir, 'ignored.txt') + with open(ignored_file, 'w') as f: + f.write('ignored') + + # Create directory in monitored path - should be tracked + monitored_subdir = os.path.join(monitored_dir, 'monitored_subdir') + os.mkdir(monitored_subdir) + monitored_file = os.path.join(monitored_subdir, 'monitored.txt') + with open(monitored_file, 'w') as f: + f.write('monitored') + + # Only the monitored file should generate an event (directories are tracked internally) + e = Event(process=process, event_type=EventType.CREATION, + file=monitored_file, host_path=monitored_file) + + server.wait_events([e]) diff --git a/tests/test_path_rename.py b/tests/test_path_rename.py index a3600eff..371f870b 100644 --- a/tests/test_path_rename.py +++ b/tests/test_path_rename.py @@ -28,7 +28,6 @@ def test_rename(monitored_dir, server, filename): fut = join_path_with_filename(monitored_dir, filename) old_fut = os.path.join(monitored_dir, 'file.txt') - # Create a new file, it will have an empty host_path. with open(old_fut, 'w') as f: f.write('This is a test') os.rename(old_fut, fut) @@ -37,23 +36,15 @@ def test_rename(monitored_dir, server, filename): # Convert fut to string for the Event, replacing invalid UTF-8 with U+FFFD fut = path_to_string(fut) - # TODO: Current behavior is incorrect. The inode map should be updated - # during rename events so that host_path reflects the new path. - # Expected correct behavior: - # - First rename: host_path should be `fut` (new path), old_host_path should be `old_fut` - # - Second rename: host_path should be `old_fut`, old_host_path should be `fut` - # Current behavior: host_path remains the original path (old_fut) because - # the inode map is not updated on rename events. old_host_path is empty. - events = [ - Event(process=Process.from_proc(), event_type=EventType.CREATION, + p = Process.from_proc() + server.wait_events([ + Event(process=p, event_type=EventType.CREATION, file=old_fut, host_path=old_fut), - Event(process=Process.from_proc(), event_type=EventType.RENAME, - file=fut, host_path=old_fut, old_file=old_fut, old_host_path=''), - Event(process=Process.from_proc(), event_type=EventType.RENAME, - file=old_fut, host_path=old_fut, old_file=fut, old_host_path=''), - ] - - server.wait_events(events) + Event(process=p, event_type=EventType.RENAME, file=fut, + host_path=fut, old_file=old_fut, old_host_path=old_fut), + Event(process=p, event_type=EventType.RENAME, file=old_fut, + host_path=old_fut, old_file=fut, old_host_path=fut), + ]) def test_ignored(monitored_dir, ignored_dir, server): @@ -67,6 +58,7 @@ def test_ignored(monitored_dir, ignored_dir, server): server: The server instance to communicate with. """ ignored_path = os.path.join(ignored_dir, 'test.txt') + p = Process.from_proc() with open(ignored_path, 'w') as f: f.write('This is to be ignored') @@ -75,26 +67,22 @@ def test_ignored(monitored_dir, ignored_dir, server): # Renaming in between ignored paths should not generate events os.rename(ignored_path, new_ignored_path) - # Renaming to a monitored path generates an event + # Renaming to a monitored path requires a scan, we need to wait for + # it before we can continue modifying the FS new_path = os.path.join(monitored_dir, 'rename.txt') os.rename(new_ignored_path, new_path) + server.wait_events([ + Event(process=p, event_type=EventType.RENAME, + file=new_path, host_path=new_path, old_file=new_ignored_path, old_host_path=''), + ]) # Renaming from a monitored path generates an event too os.rename(new_path, ignored_path) - p = Process.from_proc() - # TODO: Current behavior is incorrect for rename events. - # Expected: When renaming from ignored to monitored, host_path should be new_path. - # When renaming from monitored to ignored, old_host_path should be new_path. - # Current: The inode map is not updated on renames, and old_host_path is not populated. - events = [ - Event(process=p, event_type=EventType.RENAME, - file=new_path, host_path='', old_file=new_ignored_path, old_host_path=''), + server.wait_events([ Event(process=p, event_type=EventType.RENAME, - file=ignored_path, host_path='', old_file=new_path, old_host_path=''), - ] - - server.wait_events(events) + file=ignored_path, host_path='', old_file=new_path, old_host_path=new_path), + ]) def test_rename_dir(monitored_dir, ignored_dir, server): @@ -113,6 +101,20 @@ def test_rename_dir(monitored_dir, ignored_dir, server): ignored_dir: Temporary directory path that is not monitored by fact. server: The server instance to communicate with. """ + def touch_test_files(directory, process=None) -> list[Event]: + events = [] + for i in range(3): + with open(os.path.join(directory, f'{i}.txt'), 'w') as f: + f.write('This is a test') + if process is not None: + events.append( + Event(process=process, + event_type=EventType.OPEN, + file=os.path.join(directory, f'{i}.txt'), + host_path=os.path.join(directory, f'{i}.txt')) + ) + return events + # Directory Under Test dut = os.path.join(monitored_dir, 'some-dir') new_dut = os.path.join(monitored_dir, 'other-dir') @@ -120,32 +122,97 @@ def test_rename_dir(monitored_dir, ignored_dir, server): new_ignored_dut = os.path.join(ignored_dir, 'other-dir') os.mkdir(ignored_dut) - for i in range(3): - with open(os.path.join(ignored_dut, f'{i}.txt'), 'w') as f: - f.write('This is a test') + touch_test_files(ignored_dut) # This rename should generate no events os.rename(ignored_dut, new_ignored_dut) - # The next three renames need to generate one event each + # Going from a non-monitored directory to a monitored one requires a scan of + # the filesystem to add any subdirectories and files, so we need to wait for + # it to end before we can continue modifying the FS. os.rename(new_ignored_dut, dut) - os.rename(dut, new_dut) - os.rename(new_dut, ignored_dut) p = Process.from_proc() - # TODO: Current behavior is incorrect for rename events. - # Expected: host_path should reflect the new path after rename, - # old_host_path should reflect the old path if it was monitored. - # Current: The inode map is not updated on renames, so host_path remains empty - # or shows the wrong path. old_host_path is not populated. - events = [ + server.wait_events([ Event(process=p, event_type=EventType.RENAME, file=dut, - host_path='', old_file=new_ignored_dut, old_host_path=''), - Event(process=p, event_type=EventType.RENAME, - file=new_dut, host_path='', old_file=dut, old_host_path=''), + host_path=dut, old_file=new_ignored_dut, old_host_path=''), + ]) + + events = touch_test_files(dut, p) + + # The following renames should produce full events without scanning the FS. + os.rename(dut, new_dut) + events.extend([ + Event(process=p, event_type=EventType.RENAME, file=new_dut, + host_path=new_dut, old_file=dut, old_host_path=dut), + # Check the renamed subfiles are properly tracked + *touch_test_files(new_dut, p), + ]) + + os.rename(new_dut, ignored_dut) + events.append( Event(process=p, event_type=EventType.RENAME, - file=ignored_dut, host_path='', old_file=new_dut, old_host_path=''), - ] + file=ignored_dut, host_path='', old_file=new_dut, old_host_path=new_dut), + ) + + server.wait_events(events) + + +@pytest.mark.parametrize('from_monitored,to_monitored', [ + pytest.param(True, True, id='both_monitored'), + pytest.param(False, True, id='target_monitored'), + pytest.param(True, False, id='bullet_monitored'), +]) +def test_rename_overwrite(from_monitored, to_monitored, monitored_dir, ignored_dir, server): + events = [] + p = Process.from_proc() + if from_monitored: + bullet = os.path.join(monitored_dir, 'bullet.txt') + events.append( + Event(process=p, + event_type=EventType.CREATION, + file=bullet, + host_path=bullet, + )) + else: + bullet = os.path.join(ignored_dir, 'bullet.txt') + + if to_monitored: + target = os.path.join(monitored_dir, 'target.txt') + events.append( + Event(process=p, + event_type=EventType.CREATION, + file=target, + host_path=target, + )) + else: + target = os.path.join(ignored_dir, 'target.txt') + + # Create both files in the order they are expected in `events` + for path in [bullet, target]: + with open(path, 'w') as f: + f.write('This is a test') + + os.rename(bullet, target) + events.append( + Event(process=p, + event_type=EventType.RENAME, + file=target, + host_path=target if to_monitored else '', + old_file=bullet, + old_host_path=bullet if from_monitored else '', + )) + + if to_monitored: + # One final event to check the mapping is persisted correctly + with open(target, 'w') as f: + f.write('Check mapping') + events.append( + Event(process=p, + event_type=EventType.OPEN, + file=target, + host_path=target, + )) server.wait_events(events) @@ -213,3 +280,69 @@ def test_mounted_dir(test_container, ignored_dir, server): ] server.wait_events(events) + + +def test_cross_mountpoints(test_container, monitored_dir, server): + """ + Attempt to rename files/directories across mountpoints + + This test does not necessarily fit here, since it won't trigger the + path_rename LSM hook, but the test ensures that this hook is + defintely not triggered when an inode crosses a mount point, so it + doesn't fit but it kind of does? ¯\\_(ツ)_/¯ + """ + mounted_file = '/unmonitored/test.txt' + host_path = os.path.join(monitored_dir, 'test.txt') + ovfs_file = '/container-dir/test.txt' + + test_container.exec_run(f'touch {mounted_file}') + # Get owner uid and gid before moving the file + stat = os.stat(host_path) + owner_uid = stat.st_uid + owner_gid = stat.st_gid + mode = stat.st_mode + + test_container.exec_run(f'mv {mounted_file} {ovfs_file}') + test_container.exec_run(f'mv {ovfs_file} {mounted_file}') + + touch = Process.in_container( + exe_path='/usr/bin/touch', + args=f'touch {mounted_file}', + name='touch', + container_id=test_container.id[:12], + ) + first_rename = Process.in_container( + exe_path='/usr/bin/mv', + args=f'mv {mounted_file} {ovfs_file}', + name='mv', + container_id=test_container.id[:12], + ) + second_rename = Process.in_container( + exe_path='/usr/bin/mv', + args=f'mv {ovfs_file} {mounted_file}', + name='mv', + container_id=test_container.id[:12], + ) + + server.wait_events([ + Event(process=touch, event_type=EventType.OPEN, + file=mounted_file, host_path=host_path), + Event(process=first_rename, event_type=EventType.CREATION, + file=ovfs_file, host_path=''), + Event(process=first_rename, event_type=EventType.OPEN, + file=ovfs_file, host_path=''), + Event(process=first_rename, event_type=EventType.OWNERSHIP, + file=ovfs_file, host_path='', owner_uid=owner_uid, owner_gid=owner_gid), + Event(process=first_rename, event_type=EventType.PERMISSION, + file=ovfs_file, host_path='', mode=mode), + Event(process=first_rename, event_type=EventType.UNLINK, + file=mounted_file, host_path=host_path), + Event(process=second_rename, event_type=EventType.CREATION, + file=mounted_file, host_path=host_path), + Event(process=second_rename, event_type=EventType.OWNERSHIP, + file=mounted_file, host_path=host_path, owner_uid=owner_uid, owner_gid=owner_gid), + Event(process=second_rename, event_type=EventType.PERMISSION, + file=mounted_file, host_path=host_path, mode=mode), + Event(process=second_rename, event_type=EventType.UNLINK, + file=ovfs_file, host_path=''), + ])