diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fc8737b..9bd74c68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ possible include a PR number for easier tracking. ## Next +* ROX-34920: track symlink events (#1440) * ROX-33036: add mount-related operations (#1059) * feat(endpoints): add inodes introspection endpoint (#1273) * feat: add --replay mode for JSONL event replay without eBPF (#1010) diff --git a/fact-ebpf/src/bpf/bound_path.h b/fact-ebpf/src/bpf/bound_path.h index cf249b68..fa0dd9dc 100644 --- a/fact-ebpf/src/bpf/bound_path.h +++ b/fact-ebpf/src/bpf/bound_path.h @@ -21,39 +21,6 @@ __always_inline static void path_write_char(char* p, unsigned int offset, char c *path_safe_access(p, offset) = c; } -__always_inline static struct bound_path_t* _path_read(struct path* path, bound_path_buffer_t key, bool use_bpf_d_path) { - struct bound_path_t* bound_path = get_bound_path(key); - if (bound_path == NULL) { - return NULL; - } - - bound_path->len = d_path(path, bound_path->path, PATH_MAX, use_bpf_d_path); - if (bound_path->len <= 0) { - return NULL; - } - - // Ensure length is within PATH_MAX for the verifier - bound_path->len = PATH_LEN_CLAMP(bound_path->len); - - return bound_path; -} - -__always_inline static struct bound_path_t* path_read_unchecked(struct path* path, bool use_bpf_d_path) { - return _path_read(path, BOUND_PATH_MAIN, use_bpf_d_path); -} - -__always_inline static struct bound_path_t* path_read_alt_unchecked(struct path* path, bool use_bpf_d_path) { - return _path_read(path, BOUND_PATH_ALTERNATE, use_bpf_d_path); -} - -__always_inline static struct bound_path_t* path_read(struct path* path) { - return _path_read(path, BOUND_PATH_MAIN, path_hooks_support_bpf_d_path); -} - -__always_inline static struct bound_path_t* path_read_alt(struct path* path) { - return _path_read(path, BOUND_PATH_ALTERNATE, path_hooks_support_bpf_d_path); -} - enum path_append_status_t { PATH_APPEND_SUCCESS = 0, PATH_APPEND_INVALID_LENGTH, @@ -80,26 +47,66 @@ __always_inline static enum path_append_status_t path_append_dentry(struct bound return 0; } -__always_inline static struct bound_path_t* _path_read_append_d_entry(struct path* dir, struct dentry* dentry, bound_path_buffer_t key) { - struct bound_path_t* path = _path_read(dir, key, path_hooks_support_bpf_d_path); +__always_inline static struct bound_path_t* path_read_into(struct path* path, + struct bound_path_t* bound_path, + bool use_bpf_d_path) { + if (path == NULL || bound_path == NULL) { + return NULL; + } + + long len = d_path(path, bound_path->path, PATH_MAX, use_bpf_d_path); + if (len <= 0) { + return NULL; + } + + // Ensure length is within PATH_MAX for the verifier + bound_path->len = PATH_LEN_CLAMP(len); - if (path == NULL) { + return bound_path; +} + +__always_inline static struct bound_path_t* path_read_into_append_d_entry(struct path* path, + struct dentry* dentry, + struct bound_path_t* bound_path, + bool use_bpf_d_path) { + if (path_read_into(path, bound_path, use_bpf_d_path) == NULL) { bpf_printk("Failed to read path"); return NULL; } - path_write_char(path->path, path->len - 1, '/'); + path_write_char(bound_path->path, bound_path->len - 1, '/'); - switch (path_append_dentry(path, dentry)) { + switch (path_append_dentry(bound_path, dentry)) { case PATH_APPEND_SUCCESS: break; case PATH_APPEND_INVALID_LENGTH: - bpf_printk("Invalid path length: %u", path->len); + bpf_printk("Invalid path length: %u", bound_path->len); return NULL; case PATH_APPEND_READ_ERROR: bpf_printk("Failed to read final path component"); return NULL; } - return path; + return bound_path; +} + +__always_inline static struct bound_path_t* _path_read(struct path* path, bound_path_buffer_t key, bool use_bpf_d_path) { + struct bound_path_t* bound_path = get_bound_path(key); + return path_read_into(path, bound_path, use_bpf_d_path); +} + +__always_inline static struct bound_path_t* path_read_unchecked(struct path* path, bool use_bpf_d_path) { + return _path_read(path, BOUND_PATH_MAIN, use_bpf_d_path); +} + +__always_inline static struct bound_path_t* path_read_alt_unchecked(struct path* path, bool use_bpf_d_path) { + return _path_read(path, BOUND_PATH_ALTERNATE, use_bpf_d_path); +} + +__always_inline static struct bound_path_t* path_read(struct path* path) { + return _path_read(path, BOUND_PATH_MAIN, path_hooks_support_bpf_d_path); +} + +__always_inline static struct bound_path_t* path_read_alt(struct path* path) { + return _path_read(path, BOUND_PATH_ALTERNATE, path_hooks_support_bpf_d_path); } /** @@ -110,7 +117,8 @@ __always_inline static struct bound_path_t* _path_read_append_d_entry(struct pat * provides a short way of resolving the full path in one call. */ __always_inline static struct bound_path_t* path_read_append_d_entry(struct path* dir, struct dentry* dentry) { - return _path_read_append_d_entry(dir, dentry, BOUND_PATH_MAIN); + struct bound_path_t* bound_path = get_bound_path(BOUND_PATH_MAIN); + return path_read_into_append_d_entry(dir, dentry, bound_path, path_hooks_support_bpf_d_path); } /** @@ -121,5 +129,6 @@ __always_inline static struct bound_path_t* path_read_append_d_entry(struct path * one path, like path_rename. */ __always_inline static struct bound_path_t* path_read_alt_append_d_entry(struct path* dir, struct dentry* dentry) { - return _path_read_append_d_entry(dir, dentry, BOUND_PATH_ALTERNATE); + struct bound_path_t* bound_path = get_bound_path(BOUND_PATH_ALTERNATE); + return path_read_into_append_d_entry(dir, dentry, bound_path, path_hooks_support_bpf_d_path); } diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 249daf53..a48eb953 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -245,3 +245,16 @@ __always_inline static void submit_move_mount_event(struct submit_event_args_t* __submit_event(args, false); } + +__always_inline static void submit_symlink_event(struct submit_event_args_t* args, + const char from_filename[PATH_MAX]) { + if (!reserve_event(args)) { + return; + } + args->event->type = FILE_ACTIVITY_SYMLINK; + if (bpf_probe_read_str(args->event->from.filename, PATH_MAX, from_filename) <= 0) { + args->event->from.filename[0] = '\0'; + } + + __submit_event(args, false); +} diff --git a/fact-ebpf/src/bpf/file.h b/fact-ebpf/src/bpf/file.h index 942ecbdf..b412fa02 100644 --- a/fact-ebpf/src/bpf/file.h +++ b/fact-ebpf/src/bpf/file.h @@ -42,19 +42,3 @@ __always_inline static monitored_t is_monitored(const inode_key_t* inode, struct 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/main.c b/fact-ebpf/src/bpf/main.c index 12d5e86b..ede0cd4e 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -300,50 +300,33 @@ int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t m->path_mkdir.total++; - struct bound_path_t* path = path_read_append_d_entry(dir, dentry); - if (path == NULL) { + struct d_instantiate_ctx_t* mkdir_ctx = get_or_insert_d_instantiate_ctx(); + if (mkdir_ctx == NULL) { + bpf_printk("Failed to get d_instantiate context entry"); + goto error; + } + + if (path_read_into_append_d_entry(dir, dentry, &mkdir_ctx->path, path_hooks_support_bpf_d_path) == NULL) { bpf_printk("Failed to read path"); - m->path_mkdir.error++; - return 0; + goto error; } struct inode* parent_inode_ptr = BPF_CORE_READ(dir, dentry, d_inode); - inode_key_t parent_inode = inode_to_key(parent_inode_ptr); + mkdir_ctx->parent_inode = inode_to_key(parent_inode_ptr); - monitored_t monitored = should_track_mkdir(parent_inode, path); - if (monitored != MONITORED_BY_PARENT) { + mkdir_ctx->monitored = is_monitored(NULL, &mkdir_ctx->path, &mkdir_ctx->parent_inode); + if (mkdir_ctx->monitored != MONITORED_BY_PARENT) { + delete_d_instantiate_ctx(); m->path_mkdir.ignored++; return 0; } + mkdir_ctx->event_type = DIR_ACTIVITY_CREATION; - // 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: + delete_d_instantiate_ctx(); + m->path_mkdir.error++; return 0; } @@ -364,28 +347,45 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { goto cleanup; } - struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); - - if (mkdir_ctx == NULL) { + struct d_instantiate_ctx_t* d_inst_ctx = get_d_instantiate_ctx(); + if (d_inst_ctx == NULL || d_inst_ctx->event_type == FILE_ACTIVITY_INIT) { args.metrics->ignored++; return 0; } - args.filename = mkdir_ctx->path; - args.parent_inode = mkdir_ctx->parent_inode; - args.monitored = mkdir_ctx->monitored; + args.filename = d_inst_ctx->path.path; + args.parent_inode = d_inst_ctx->parent_inode; + args.monitored = d_inst_ctx->monitored; args.inode = inode_to_key(inode); - if (inode_add(&args.inode) == 0) { - args.metrics->added++; - } else { - args.metrics->error++; - } + switch (d_inst_ctx->event_type) { + case DIR_ACTIVITY_CREATION: + if (inode_add(&args.inode) != 0) { + args.metrics->error++; + } + + submit_mkdir_event(&args); + break; + case FILE_ACTIVITY_SYMLINK: + args.monitored = is_monitored(&args.inode, &d_inst_ctx->path, &args.parent_inode); + if (args.monitored == MONITORED_BY_PARENT) { + if (inode_add(&args.inode) != 0) { + args.metrics->error++; + } + } - submit_mkdir_event(&args); + if (args.monitored != NOT_MONITORED) { + submit_symlink_event(&args, d_inst_ctx->symlink_target); + } + break; + default: + bpf_printk("Unexpected event type: %d", d_inst_ctx->event_type); + args.metrics->error++; + break; + } cleanup: - bpf_map_delete_elem(&mkdir_context, &pid_tgid); + bpf_map_delete_elem(&d_instantiate_ctx, &pid_tgid); return 0; } @@ -601,3 +601,38 @@ int BPF_PROG(trace_move_mount, struct path* from, struct path* to) { args.metrics->error++; return 0; } + +SEC("lsm/path_symlink") +int BPF_PROG(trace_path_symlink, struct path* dir, struct dentry* dentry, const char* old_name) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + m->path_symlink.total++; + + struct d_instantiate_ctx_t* symlink_ctx = get_or_insert_d_instantiate_ctx(); + if (symlink_ctx == NULL) { + bpf_printk("Failed to get d_instantiate context entry"); + goto error; + } + + if (path_read_into_append_d_entry(dir, dentry, &symlink_ctx->path, path_hooks_support_bpf_d_path) == NULL) { + bpf_printk("Failed to read path"); + goto error; + } + + symlink_ctx->parent_inode = inode_to_key(dir->dentry->d_inode); + symlink_ctx->event_type = FILE_ACTIVITY_SYMLINK; + + if (bpf_probe_read_str(symlink_ctx->symlink_target, PATH_MAX, old_name) < 0) { + bpf_printk("Failed to read old_name"); + goto error; + } + + return 0; + +error: + delete_d_instantiate_ctx(); + m->path_symlink.error++; + return 0; +} diff --git a/fact-ebpf/src/bpf/maps.h b/fact-ebpf/src/bpf/maps.h index 88fc6118..57cef10e 100644 --- a/fact-ebpf/src/bpf/maps.h +++ b/fact-ebpf/src/bpf/maps.h @@ -83,12 +83,55 @@ struct { __uint(map_flags, BPF_F_NO_PREALLOC); } inode_map SEC(".maps"); +// Context for correlating operations in d_instantiate +struct d_instantiate_ctx_t { + struct bound_path_t path; + inode_key_t parent_inode; + monitored_t monitored; + file_activity_type_t event_type; + char symlink_target[PATH_MAX]; +}; + struct { __uint(type, BPF_MAP_TYPE_LRU_HASH); __type(key, __u64); - __type(value, struct mkdir_context_t); + __type(value, struct d_instantiate_ctx_t); __uint(max_entries, 16384); -} mkdir_context SEC(".maps"); +} d_instantiate_ctx SEC(".maps"); + +__always_inline static struct d_instantiate_ctx_t* get_d_instantiate_ctx() { + __u64 pid = bpf_get_current_pid_tgid(); + return bpf_map_lookup_elem(&d_instantiate_ctx, &pid); +} + +__always_inline static long delete_d_instantiate_ctx() { + __u64 pid = bpf_get_current_pid_tgid(); + return bpf_map_delete_elem(&d_instantiate_ctx, &pid); +} + +__always_inline static struct d_instantiate_ctx_t* get_or_insert_d_instantiate_ctx() { + static const struct d_instantiate_ctx_t empty_ctx = { + .event_type = FILE_ACTIVITY_INIT, + }; + + __u64 pid = bpf_get_current_pid_tgid(); + struct d_instantiate_ctx_t* ctx = bpf_map_lookup_elem(&d_instantiate_ctx, &pid); + if (ctx != NULL) { + // Clear the event type so `d_instantiate` doesn't trigger by accident + ctx->event_type = FILE_ACTIVITY_INIT; + return ctx; + } + + if (bpf_map_update_elem(&d_instantiate_ctx, &pid, &empty_ctx, BPF_NOEXIST) != 0) { + return NULL; + } + + ctx = bpf_map_lookup_elem(&d_instantiate_ctx, &pid); + if (ctx == NULL) { + return NULL; + } + return ctx; +} struct { __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index 80b94d22..a2fa3449 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -118,6 +118,7 @@ typedef enum file_activity_type_t { FILE_ACTIVITY_MOUNT, FILE_ACTIVITY_UMOUNT, FILE_ACTIVITY_MOVE_MOUNT, + FILE_ACTIVITY_SYMLINK, } file_activity_type_t; struct event_t { @@ -169,13 +170,6 @@ 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; @@ -200,4 +194,5 @@ struct metrics_t { struct metrics_by_hook_t sb_mount; struct metrics_by_hook_t sb_umount; struct metrics_by_hook_t move_mount; + struct metrics_by_hook_t path_symlink; }; diff --git a/fact-ebpf/src/lib.rs b/fact-ebpf/src/lib.rs index 8453c1c0..32f6f7b1 100644 --- a/fact-ebpf/src/lib.rs +++ b/fact-ebpf/src/lib.rs @@ -238,6 +238,7 @@ impl_metrics_t!( sb_mount, sb_umount, move_mount, + path_symlink, ); unsafe impl Pod for metrics_t {} diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 3d7b629c..3bed7a04 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -132,7 +132,10 @@ impl Event { } pub fn is_creation(&self) -> bool { - matches!(self.file, FileData::Creation(_) | FileData::MkDir(_)) + matches!( + self.file, + FileData::Creation(_) | FileData::MkDir(_) | FileData::Symlink { .. } + ) } pub fn is_xattr(&self) -> bool { @@ -162,6 +165,10 @@ impl Event { ) } + pub fn is_symlink(&self) -> bool { + matches!(self.file, FileData::Symlink { .. }) + } + /// Unwrap the inner FileData and return the inode that triggered /// the event. /// @@ -180,6 +187,7 @@ impl Event { | FileData::MoveMount { to: inner, .. } | FileData::Mount(inner) | FileData::Umount(inner) + | FileData::Symlink { inner, .. } | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.inode, @@ -200,6 +208,7 @@ impl Event { | FileData::MoveMount { to: inner, .. } | FileData::Mount(inner) | FileData::Umount(inner) + | FileData::Symlink { inner, .. } | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.parent_inode, @@ -231,6 +240,7 @@ impl Event { | FileData::MoveMount { to: inner, .. } | FileData::Mount(inner) | FileData::Umount(inner) + | FileData::Symlink { inner, .. } | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.filename, @@ -259,6 +269,7 @@ impl Event { | FileData::MoveMount { to: inner, .. } | FileData::Mount(inner) | FileData::Umount(inner) + | FileData::Symlink { inner, .. } | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.host_file, @@ -291,6 +302,7 @@ impl Event { | FileData::MoveMount { to: inner, .. } | FileData::Mount(inner) | FileData::Umount(inner) + | FileData::Symlink { inner, .. } | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => inner.host_file = host_path, @@ -321,6 +333,7 @@ impl Event { | FileData::MoveMount { to: inner, .. } | FileData::Mount(inner) | FileData::Umount(inner) + | FileData::Symlink { inner, .. } | FileData::SetXattr(XattrFileData { inner, .. }) | FileData::RemoveXattr(XattrFileData { inner, .. }) | FileData::AclSet(AclSetFileData { inner, .. }) => inner.monitored, @@ -447,6 +460,10 @@ pub enum FileData { from: BaseFileData, }, Umount(BaseFileData), + Symlink { + inner: BaseFileData, + target: PathBuf, + }, } impl FileData { @@ -528,6 +545,11 @@ impl FileData { let from = read_from_data(extra_data); FileData::MoveMount { to: inner, from } } + file_activity_type_t::FILE_ACTIVITY_SYMLINK => { + let target = unsafe { extra_data.from.filename }; + let target = sanitize_d_path(&target); + FileData::Symlink { inner, target } + } invalid => unreachable!("Invalid event type: {invalid:?}"), }; @@ -551,6 +573,7 @@ impl FileData { FileData::Mount(_) => "mount", FileData::MoveMount { .. } => "move_mount", FileData::Umount(_) => "umount", + FileData::Symlink { .. } => "symlink", } } } @@ -558,7 +581,7 @@ impl FileData { impl From for fact_api::file_activity::File { fn from(event: FileData) -> Self { match event { - FileData::Open(event) => { + FileData::Open(event) | FileData::Symlink { inner: event, .. } => { let activity = Some(fact_api::FileActivityBase::from(event)); let f_act = fact_api::FileOpen { activity }; fact_api::file_activity::File::Open(f_act) @@ -643,6 +666,14 @@ impl From for opentelemetry::logs::AnyValue { } FileData::SetXattr(data) | FileData::RemoveXattr(data) => AnyValue::from(data), FileData::AclSet(data) => AnyValue::from(data), + FileData::Symlink { inner, target } => { + let AnyValue::Map(mut map) = AnyValue::from(inner) else { + unreachable!("to value did not serialize to map"); + }; + map.insert("target".into(), target.to_string_lossy().to_string().into()); + + AnyValue::Map(map) + } }) else { unreachable!("event data did not serialize to map"); }; diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index b5252601..1ea00a5d 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -207,30 +207,63 @@ impl HostScanner { continue; } }; - let metadata = match path.metadata() { - Ok(p) => p, + let metadata = match path.symlink_metadata() { + Ok(m) => m, + Err(e) if e.kind() == io::ErrorKind::NotFound => continue, Err(e) => { - debug!("Failed to get metadata for {}: {e:?}", path.display()); - self.metrics.scan_inc(ScanLabels::FsMetadataFailed); + warn!("Failed to get metadata for {}: {e}", path.display()); continue; } }; if metadata.is_file() { self.metrics.scan_inc(ScanLabels::FileScanned); - self.update_entry(path.as_path(), &metadata) - .with_context(|| format!("Failed to update entry for {}", path.display()))?; + } else if metadata.is_symlink() { + self.metrics.scan_inc(ScanLabels::SymlinkScanned); + self.scan_symlink(&path); } else if metadata.is_dir() { self.metrics.scan_inc(ScanLabels::DirectoryScanned); - self.update_entry(path.as_path(), &metadata) - .with_context(|| format!("Failed to update entry for {}", path.display()))?; } else { self.metrics.scan_inc(ScanLabels::FsItemIgnored); + continue; } + + self.update_entry(&path, &metadata) + .with_context(|| format!("Failed to update entry for {}", path.display()))?; } Ok(()) } + fn scan_symlink(&self, path: &Path) { + let target = match path.read_link() { + Ok(p) => { + if p.has_root() { + &host_info::prepend_host_mount(&p) + } else { + path + } + } + Err(e) => { + warn!("Failed to read symlink path: {e}"); + return; + } + }; + + match target.metadata() { + Ok(metadata) => { + if let Err(e) = self.update_entry(path, &metadata) { + warn!("Failed to update symlink entry for {}: {e}", path.display()); + } + } + Err(e) => { + warn!( + "Failed to read metadata for symlink target {}: {e}", + target.display() + ); + } + } + } + fn update_entry(&self, path: &Path, metadata: &Metadata) -> anyhow::Result<()> { let inode = inode_key_t { inode: metadata.st_ino(), @@ -285,6 +318,19 @@ You can increase this limit with: self.inode_map.borrow().get(inode?).cloned() } + fn build_host_path(&self, event: &Event) -> Option { + let parent_inode = event.get_parent_inode(); + + if !parent_inode.empty() + && let Some(filename) = event.get_filename().file_name() + && let Some(parent_host_path) = self.get_host_path(Some(parent_inode)) + { + Some(parent_host_path.join(filename)) + } else { + None + } + } + /// Handle file creation events by adding new inodes to the map. /// /// We use the parent inode provided by the eBPF code @@ -292,25 +338,22 @@ You can increase this limit with: /// path by appending the new file's name. fn handle_creation_event(&self, event: &Event) -> anyhow::Result<()> { let inode = event.get_inode(); - let parent_inode = event.get_parent_inode(); - if self.get_host_path(Some(inode)).is_some() || parent_inode.empty() { + if self.get_host_path(Some(inode)).is_some() { return Ok(()); } - if let Some(filename) = event.get_filename().file_name() - && let Some(parent_host_path) = self.get_host_path(Some(parent_inode)) - { - let host_path = parent_host_path.join(filename); - self.update_entry_with_inode(*inode, host_path) + match self.build_host_path(event) { + Some(host_path) => self + .update_entry_with_inode(*inode, host_path) .with_context(|| { format!( "Failed to add creation event entry for {}", - filename.display() + event.get_filename().display(), ) - })?; - } + }), - Ok(()) + None => Ok(()), + } } /// Handle unlink events by removing the inode from the inode->path map. @@ -458,6 +501,15 @@ You can increase this limit with: } } + /// Handle symlink events by scanning the filesystem + fn handle_symlink_event(&self) -> anyhow::Result<()> { + // Since `glob` follows symlinks unconditionally, we need to do + // so as well. + // + // TODO: do a partial scan of the symlink, rather than a full scan + self.scan() + } + /// Periodically notify the host scanner main task that a scan needs /// to happen. /// @@ -482,6 +534,19 @@ You can increase this limit with: }); } + /// Check whether an event should be ignored. + /// + /// On top of the check from `Event::is_ignored`, this also checks + /// the host paths for matches in events that are monitored by + /// parent. + fn event_is_ignored(&self, event: &Event) -> bool { + event.is_ignored(&self.paths_globset) + && !self.paths_globset.is_match(event.get_host_path()) + && event + .get_old_host_path() + .is_none_or(|path| !self.paths_globset.is_match(path)) + } + pub fn start(mut self, task_set: &mut JoinSet>) { let scan_interval_value = *self.scan_interval.borrow(); let scan_trigger = Arc::new(Notify::new()); @@ -537,16 +602,23 @@ You can increase this limit with: continue; } + if event.is_symlink() && + let Err(e) = self.handle_symlink_event() { + warn!("Failed to handle symlink event: {e:?}"); + } + 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. + // Before sending the event forward, we need to check + // whether the event is ignored now that we have the + // full inode context. + if self.event_is_ignored(&event) { self.inode_map.borrow_mut().remove(event.get_inode()); let _ = self.kernel_inode_map.borrow_mut().remove(event.get_inode()); + if let Some(old_inode) = event.get_old_inode() { + self.inode_map.borrow_mut().remove(old_inode); + let _ = self.kernel_inode_map.borrow_mut().remove(old_inode); + } self.metrics.events.ignored(); continue; } diff --git a/fact/src/metrics/host_scanner.rs b/fact/src/metrics/host_scanner.rs index 3b106381..6aac0c0e 100644 --- a/fact/src/metrics/host_scanner.rs +++ b/fact/src/metrics/host_scanner.rs @@ -14,6 +14,7 @@ pub enum ScanLabels { InodeHit, DirectoryScanned, FileScanned, + SymlinkScanned, FileRemoved, FileUpdated, FsItemIgnored, diff --git a/fact/src/metrics/kernel_metrics.rs b/fact/src/metrics/kernel_metrics.rs index 50f845a0..10492470 100644 --- a/fact/src/metrics/kernel_metrics.rs +++ b/fact/src/metrics/kernel_metrics.rs @@ -79,4 +79,5 @@ define_kernel_metrics!( sb_mount, sb_umount, move_mount, + path_symlink, ); diff --git a/tests/event.py b/tests/event.py index ca281fed..6de62894 100644 --- a/tests/event.py +++ b/tests/event.py @@ -47,6 +47,7 @@ class EventType(Enum): XATTR_SET = 7 XATTR_REMOVE = 8 ACL = 9 + SYMLINK = 10 # POSIX ACL type values matching the AclType proto enum. @@ -78,12 +79,12 @@ def __init__( container_id: str, loginuid: int, ): - self._pid: int | None = pid + self.pid: int | None = pid self._uid: int = uid self._gid: int = gid - self._exe_path: str = exe_path - self._args: str = args - self._name: str = name + self.exe_path: str = exe_path + self.args: str = args + self.name: str = name self._container_id: str = container_id self._loginuid: int = loginuid @@ -165,22 +166,6 @@ def uid(self) -> int: def gid(self) -> int: return self._gid - @property - def pid(self) -> int | None: - return self._pid - - @property - def exe_path(self) -> str: - return self._exe_path - - @property - def args(self) -> str: - return self._args - - @property - def name(self) -> str: - return self._name - @property def container_id(self) -> str: return self._container_id diff --git a/tests/server.py b/tests/server.py index f274737a..ddb40e6d 100644 --- a/tests/server.py +++ b/tests/server.py @@ -48,6 +48,7 @@ 'xattr_set': EventType.XATTR_SET, 'xattr_remove': EventType.XATTR_REMOVE, 'acl': EventType.ACL, + 'symlink': EventType.SYMLINK, } @@ -387,6 +388,9 @@ def _translate(record: LogRecord) -> Event | None: OtlpServer._acl_entry_translate(entry) for entry in file_data.get('entries', []) ] + elif event_type == EventType.SYMLINK: + # For the time being, symlink events are treated as open events + event_type = EventType.OPEN return Event( process=process, diff --git a/tests/test_path_symlink.py b/tests/test_path_symlink.py new file mode 100644 index 00000000..94fb1afb --- /dev/null +++ b/tests/test_path_symlink.py @@ -0,0 +1,522 @@ +from __future__ import annotations + +import os +import re +import subprocess + +import docker.models.containers +import pytest + +from event import Event, EventType, Process +from server import EventServer +from utils import join_path_with_filename, path_to_string + + +def setup_symlink(file: str | bytes, link: str | bytes) -> list[Event]: + with open(file, 'w') as f: + f.write('This is a test') + + os.symlink(file, link) + + file = path_to_string(file) + link = path_to_string(link) + proc = Process.from_proc() + + return [ + Event( + process=proc, + event_type=EventType.CREATION, + file=file, + host_path=file, + ), + Event( + process=proc, + event_type=EventType.OPEN, + file=link, + host_path=link, + ), + ] + + +def overwrite_symlink(file: str, link: str) -> list[Event]: + """ + Overwrite the provided link with file. + + Returns: + A list of expected events. + """ + # Overwrite the symbolic link with ln -sf + subprocess.run(['ln', '-s', '-f', file, link], check=True) + + proc = Process.from_proc() + + # Build the ln process from the self proc. + proc.exe_path = '/usr/bin/ln' + proc.args = f'ln -s -f {file} {link}' + proc.pid = None + proc.name = 'ln' + + # Sometimes the symlink is moved too fast and we can't read the + # inode in userspace, so the pattern here checks for the actual path + # or an empty string + parent_dir = os.path.dirname(link) + new_link_pattern = rf'{parent_dir}/[0-9a-zA-Z]{{8}}' + new_link = re.compile(new_link_pattern) + return [ + Event( + process=proc, + event_type=EventType.OPEN, + file=new_link, + host_path=new_link, + ), + # The host paths in the following event depend on the order the + # temporary symlink and the actual symlink are scanned, so we + # need to check for some patterns in there. + Event( + process=proc, + event_type=EventType.RENAME, + file=link, + host_path=re.compile(rf'{link}|'), + old_file=new_link, + old_host_path=re.compile(rf'{link}|{new_link_pattern}'), + ), + ] + + +@pytest.mark.parametrize( + ('filename', 'symlink'), + [ + pytest.param('target.txt', 'symlink.txt', id='ASCII'), + pytest.param('café.txt', 'éfac.txt', id='French'), + pytest.param('файл.txt', 'лйаф.txt', id='Cyrillic'), + pytest.param('测试.txt', '试测.txt', id='Chinese'), + pytest.param('🚀rocket.txt', 'rocket🚀.txt', id='Emoji'), + pytest.param(b'test\xff\xfe.txt', b'\xff\xfetest.txt', id='Invalid'), + ], +) +def test_create_symlink( + monitored_dir: str, + server: EventServer, + filename: str | bytes, + symlink: str | bytes, +): + """ + Test creating symlinks in a monitored directory is properly captured. + + Args: + monitored_dir: Temporary directory path for creating the test file. + server: The server instance to communicate with. + filename: Name of the file to create (includes UTF-8 test cases). + """ + file = join_path_with_filename(monitored_dir, filename) + link = join_path_with_filename(monitored_dir, symlink) + + server.wait_events(setup_symlink(file, link)) + + +def test_follow_symlink_to_file( + monitored_dir: str, ignored_dir: str, server: EventServer +): + """ + Creating a symlink to a file that is not monitored should start + monitoring it. + """ + file = os.path.join(ignored_dir, 'file.txt') + link = os.path.join(monitored_dir, 'symlink') + proc = Process.from_proc() + + with open(file, 'w') as f: + f.write('This is a test') + os.symlink(file, link) + + server.wait_events( + [ + Event( + process=proc, + event_type=EventType.OPEN, + file=link, + host_path=link, + ) + ] + ) + + # At this point, modifying the file in the ignored path should + # trigger events + with open(file, 'w') as f: + f.write('This is a test') + + server.wait_events( + [ + Event( + process=proc, + event_type=EventType.OPEN, + file=file, + host_path=link, + ), + ] + ) + + +def test_follow_symlink_to_file_relative( + monitored_dir: str, ignored_dir: str, server: EventServer +): + """ + Creating a symlink to a file that is not monitored should start + monitoring it. + """ + file = os.path.join(ignored_dir, 'file.txt') + link = os.path.join(monitored_dir, 'symlink') + target = os.path.join('..', os.path.basename(ignored_dir), 'file.txt') + proc = Process.from_proc() + + with open(file, 'w') as f: + f.write('This is a test') + os.symlink(target, link) + + server.wait_events( + [ + Event( + process=proc, + event_type=EventType.OPEN, + file=link, + host_path=link, + ) + ] + ) + + # At this point, modifying the file in the ignored path should + # trigger events + with open(file, 'w') as f: + f.write('This is a test') + + server.wait_events( + [ + Event( + process=proc, + event_type=EventType.OPEN, + file=file, + host_path=link, + ), + ] + ) + + +@pytest.mark.skip( + reason='symlinks with absolute paths are broken when ' + + 'running inside container' +) +def test_follow_symlink_to_dir( + monitored_dir: str, ignored_dir: str, server: EventServer +): + """ + Creating a symlink to a directory that is not monitored should start + monitoring it. + """ + file = os.path.join(ignored_dir, 'file.txt') + other_file = os.path.join(ignored_dir, 'other.txt') + link = os.path.join(monitored_dir, 'symlink') + proc = Process.from_proc() + + with open(file, 'w') as f: + f.write('This is a test') + os.symlink(ignored_dir, link) + + server.wait_events( + [ + Event( + process=proc, + event_type=EventType.OPEN, + file=link, + host_path=link, + ) + ] + ) + + # At this point, modifying files in the ignored path should + # trigger events + with open(file, 'w') as f: + f.write('This is a test') + with open(other_file, 'w') as f: + f.write('This is a test') + + server.wait_events( + [ + Event( + process=proc, + event_type=EventType.OPEN, + file=file, + host_path=link, + ), + Event( + process=proc, + event_type=EventType.CREATION, + file=other_file, + host_path=link, + ), + ] + ) + + +def test_follow_symlink_to_dir_relative( + monitored_dir: str, ignored_dir: str, server: EventServer +): + """ + Creating a symlink to a directory that is not monitored should start + monitoring it. + """ + file = os.path.join(ignored_dir, 'file.txt') + other_file = os.path.join(ignored_dir, 'other.txt') + link = os.path.join(monitored_dir, 'symlink') + link_file = os.path.join(link, 'file.txt') + link_other = os.path.join(link, 'other.txt') + target = os.path.join('..', os.path.basename(ignored_dir)) + proc = Process.from_proc() + + with open(file, 'w') as f: + f.write('This is a test') + os.symlink(target, link) + + server.wait_events( + [ + Event( + process=proc, + event_type=EventType.OPEN, + file=link, + host_path=link, + ) + ] + ) + + # At this point, modifying files in the ignored path should + # trigger events + with open(file, 'w') as f: + f.write('This is a test') + with open(other_file, 'w') as f: + f.write('This is a test') + + server.wait_events( + [ + Event( + process=proc, + event_type=EventType.OPEN, + file=file, + host_path=link_file, + ), + Event( + process=proc, + event_type=EventType.CREATION, + file=other_file, + host_path=link_other, + ), + ] + ) + + +def test_overwrite_symlink(monitored_dir: str, server: EventServer): + """ + Test overwriting a symlink in a monitored directory is properly captured. + + Args: + monitored_dir: Temporary directory path for creating the test file. + server: The server instance to communicate with. + """ + file = os.path.join(monitored_dir, 'file.txt') + link = os.path.join(monitored_dir, 'symlink') + + events = setup_symlink(file, link) + events.extend(overwrite_symlink(file, link)) + + server.wait_events(events) + + +def test_multiple(monitored_dir: str, server: EventServer): + """ + Tests creating multiple symlinks is properly captured + + Args: + monitored_dir: Temporary directory path for creating the test file. + server: The server instance to communicate with. + """ + proc = Process.from_proc() + file = os.path.join(monitored_dir, 'file.txt') + with open(file, 'w') as f: + f.write('This is a test') + events = [ + Event( + process=proc, + file=file, + host_path=file, + event_type=EventType.CREATION, + ) + ] + + for i in range(3): + link = os.path.join(monitored_dir, f'symlink{i}') + os.symlink(file, link) + + events.append( + Event( + process=proc, + file=link, + host_path=link, + event_type=EventType.OPEN, + ) + ) + + server.wait_events(events) + + +def test_multiple_overwrite(monitored_dir: str, server: EventServer): + """ + Tests creating multiple symlinks is properly captured + + Args: + monitored_dir: Temporary directory path for creating the test file. + server: The server instance to communicate with. + """ + proc = Process.from_proc() + file = os.path.join(monitored_dir, 'file.txt') + with open(file, 'w') as f: + f.write('This is a test') + events = [ + Event( + process=proc, + file=file, + host_path=file, + event_type=EventType.CREATION, + ) + ] + + for i in range(3): + link = os.path.join(monitored_dir, f'symlink{i}') + os.symlink(file, link) + + events.append( + Event( + process=proc, + file=link, + host_path=link, + event_type=EventType.OPEN, + ) + ) + + events.extend(overwrite_symlink(file, link)) + + server.wait_events(events) + + +def test_ignored(monitored_dir: str, ignored_dir: str, server: EventServer): + """ + Tests that symlink events on ignored file are not captured. + + Args: + monitored_dir: Temporary directory path for creating the test file. + ignored_dir: Temporary directory path that is not monitored by fact. + server: The server instance to communicate with. + """ + file = os.path.join(ignored_dir, 'test.txt') + ignored_link = os.path.join(ignored_dir, 'ignored') + monitored_link = os.path.join(monitored_dir, 'symlink') + + os.symlink(file, ignored_link) + os.symlink(file, monitored_link) + + server.wait_events( + [ + Event( + process=Process.from_proc(), + event_type=EventType.OPEN, + file=monitored_link, + host_path=monitored_link, + ) + ] + ) + + +def test_ovfs( + test_container: docker.models.containers.Container, server: EventServer +): + assert test_container.id is not None + container_id = test_container.id[:12] + file = '/container-dir/test.txt' + link = '/container-dir/symlink' + + res = test_container.exec_run(f'touch {file}') + assert res.exit_code == 0 + res = test_container.exec_run(f'ln -s {file} {link}') + assert res.exit_code == 0 + + touch = Process.in_container( + exe_path='/usr/bin/touch', + args=f'touch {file}', + name='touch', + container_id=container_id, + ) + ln = Process.in_container( + exe_path='/usr/bin/ln', + args=f'ln -s {file} {link}', + name='ln', + container_id=container_id, + ) + + server.wait_events( + [ + Event( + process=touch, + event_type=EventType.CREATION, + file=file, + host_path='', + ), + Event( + process=ln, + event_type=EventType.OPEN, + file=link, + host_path='', + ), + ] + ) + + +def test_mounted_dir( + test_container: docker.models.containers.Container, + ignored_dir: str, + server: EventServer, +): + assert test_container.id is not None + container_id = test_container.id[:12] + file = '/mounted/test.txt' + link = '/mounted/symlink' + + test_container.exec_run(f'touch {file}') + test_container.exec_run(f'ln -s {file} {link}') + + touch = Process.in_container( + exe_path='/usr/bin/touch', + args=f'touch {file}', + name='touch', + container_id=container_id, + ) + ln = Process.in_container( + exe_path='/usr/bin/ln', + args=f'ln -s {file} {link}', + name='ln', + container_id=container_id, + ) + + # ignored_dir is not monitored, so host_path should be blank + server.wait_events( + [ + Event( + process=touch, + event_type=EventType.CREATION, + file=file, + host_path='', + ), + Event( + process=ln, + event_type=EventType.OPEN, + file=link, + host_path='', + ), + ] + )