Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions fact-ebpf/src/bpf/events.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,17 @@ __always_inline static void submit_rename_event(struct metrics_by_hook_t* m,

__submit_event(event, m, FILE_ACTIVITY_RENAME, new_filename, new_inode, new_parent_inode, path_hooks_support_bpf_d_path);
}

__always_inline static void submit_mkdir_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++;
return;
}

// d_instantiate doesn't support bpf_d_path, so we use false and rely on the stashed path from path_mkdir
__submit_event(event, m, DIR_ACTIVITY_CREATION, filename, inode, parent_inode, false);
}
16 changes: 16 additions & 0 deletions fact-ebpf/src/bpf/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,19 @@ __always_inline static inode_monitored_t is_monitored(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 inode_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 PARENT_MONITORED;
}

if (path_is_monitored(child_path)) {
return MONITORED;
}

return NOT_MONITORED;
}
102 changes: 102 additions & 0 deletions fact-ebpf/src/bpf/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ char _license[] SEC("license") = "Dual MIT/GPL";
#define FMODE_PWRITE ((fmode_t)(1 << 4))
#define FMODE_CREATED ((fmode_t)(1 << 20))

// File type constants from linux/stat.h
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a permalink to the definition?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// https://github.com/torvalds/linux/blob/5619b098e2fbf3a23bf13d91897056a1fe238c6d/include/uapi/linux/stat.h
#define S_IFMT 00170000
#define S_IFDIR 0040000
#define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR)

SEC("lsm/file_open")
int BPF_PROG(trace_file_open, struct file* file) {
struct metrics_t* m = get_metrics();
Expand Down Expand Up @@ -228,3 +234,99 @@ int BPF_PROG(trace_path_rename, struct path* old_dir,
m->path_rename.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++;

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;
}

struct inode* parent_inode_ptr = BPF_CORE_READ(dir, dentry, d_inode);
inode_key_t parent_inode = inode_to_key(parent_inode_ptr);

if (should_track_mkdir(parent_inode, path) != PARENT_MONITORED) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally had

if (should_track_mkdir(parent_inode, path) == NOT_MONITORED) {

However, test_nonrecursive_wildcard::test_wildcard.py failed with that change. The reason was that was the inodes of all directories created in that test were tracked and subsequently all files in those directories were tracked regardless if they matched the wildcard pattern or not.

m->path_mkdir.ignored++;
return 0;
}

// Stash mkdir context for security_d_instantiate
__u64 pid_tgid = bpf_get_current_pid_tgid();
struct mkdir_context_t* mkdir_ctx = get_mkdir_context();
if (mkdir_ctx == NULL) {
bpf_printk("Failed to get mkdir context buffer");
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++;
return 0;
}
mkdir_ctx->parent_inode = parent_inode;

if (bpf_map_update_elem(&mkdir_context, &pid_tgid, mkdir_ctx, BPF_ANY) != 0) {
bpf_printk("Failed to stash mkdir context");
m->path_mkdir.error++;
return 0;
}

return 0;
}

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;
}

m->d_instantiate.total++;

__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) {
m->d_instantiate.ignored++;
return 0;
}

if (inode == NULL) {
m->d_instantiate.ignored++;
goto cleanup;
}

umode_t mode = BPF_CORE_READ(inode, i_mode);
if (!S_ISDIR(mode)) {
m->d_instantiate.ignored++;
goto cleanup;
}
Comment on lines +310 to +314
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check if m->d_instantiate.ignored actually increases, I have a feeling this condition should never be met.


inode_key_t inode_key = inode_to_key(inode);

if (inode_add(&inode_key) == 0) {
m->d_instantiate.added++;
} else {
m->d_instantiate.error++;
}

submit_mkdir_event(&m->d_instantiate,
mkdir_ctx->path,
&inode_key,
&mkdir_ctx->parent_inode);

cleanup:
bpf_map_delete_elem(&mkdir_context, &pid_tgid);
return 0;
}
20 changes: 20 additions & 0 deletions fact-ebpf/src/bpf/maps.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,26 @@ struct {
__uint(map_flags, BPF_F_NO_PREALLOC);
} inode_map SEC(".maps");

struct {
__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
__type(key, __u32);
__type(value, struct mkdir_context_t);
__uint(max_entries, 1);
} mkdir_context_heap SEC(".maps");

__always_inline static struct mkdir_context_t* get_mkdir_context() {
unsigned int zero = 0;
return bpf_map_lookup_elem(&mkdir_context_heap, &zero);
}
Comment on lines +86 to +96
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be unused.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


struct {
__uint(type, BPF_MAP_TYPE_HASH);
__type(key, __u64);
__type(value, struct mkdir_context_t);
__uint(max_entries, 16384);
__uint(map_flags, BPF_F_NO_PREALLOC);
} mkdir_context SEC(".maps");

struct {
__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
__type(key, __u32);
Expand Down
9 changes: 9 additions & 0 deletions fact-ebpf/src/bpf/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,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 {
Expand Down Expand Up @@ -96,6 +97,12 @@ 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;
};

// Metrics types
struct metrics_by_hook_t {
unsigned long long total;
Expand All @@ -111,4 +118,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;
};
2 changes: 2 additions & 0 deletions fact-ebpf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,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
}
}
Expand Down
1 change: 1 addition & 0 deletions fact/src/event/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ impl FileData {
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::Creation(inner),
file_activity_type_t::FILE_ACTIVITY_UNLINK => FileData::Unlink(inner),
file_activity_type_t::FILE_ACTIVITY_CHMOD => {
let data = ChmodFileData {
Expand Down
18 changes: 18 additions & 0 deletions fact/src/metrics/kernel_metrics.rs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you also need to add your metrics here:

fact/fact-ebpf/src/lib.rs

Lines 123 to 127 in eb0b3a9

m.file_open = m.file_open.accumulate(&other.file_open);
m.path_unlink = m.path_unlink.accumulate(&other.path_unlink);
m.path_chmod = m.path_chmod.accumulate(&other.path_chmod);
m.path_chown = m.path_chown.accumulate(&other.path_chown);
m.path_rename = m.path_rename.accumulate(&other.path_rename);

I will look into improving how we handle these, it is getting very repetitive and having it scattered among multiple files is very error prone.

Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub struct KernelMetrics {
path_chmod: EventCounter,
path_chown: EventCounter,
path_rename: EventCounter,
path_mkdir: EventCounter,
d_instantiate: EventCounter,
map: PerCpuArray<MapData, metrics_t>,
}

Expand Down Expand Up @@ -43,19 +45,33 @@ 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,
path_unlink,
path_chmod,
path_chown,
path_rename,
path_mkdir,
d_instantiate,
map: kernel_metrics,
}
}
Expand Down Expand Up @@ -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(())
}
Expand Down
67 changes: 67 additions & 0 deletions tests/test_path_mkdir.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import os

import pytest

from event import Event, EventType, Process


def test_mkdir_nested(monitored_dir, server):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can parametrize the final directory with UTF-8 characters?

"""
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.
"""
process = Process.from_proc()

# Create nested directories
level1 = os.path.join(monitored_dir, 'level1')
level2 = os.path.join(level1, 'level2')
level3 = os.path.join(level2, 'level3')

os.makedirs(level3, exist_ok=True)

# Create a file in the deepest directory
test_file = os.path.join(level3, 'deep_file.txt')
with open(test_file, 'w') as f:
f.write('nested content')

events = [
Event(process=process, event_type=EventType.CREATION,
file=level1, host_path=level1),
Event(process=process, event_type=EventType.CREATION,
file=level2, host_path=level2),
Event(process=process, event_type=EventType.CREATION,
file=level3, host_path=level3),
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)

# Create directory in monitored path - should be tracked
monitored_subdir = os.path.join(monitored_dir, 'monitored_subdir')
os.mkdir(monitored_subdir)

# Only the monitored directory should generate an event
e = Event(process=process, event_type=EventType.CREATION,
file=monitored_subdir, host_path=monitored_subdir)

server.wait_events([e])
Loading