Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: new(modern_bpf): add security_file_mprotect #1601

Closed
wants to merge 4 commits into from
Closed

RFC: new(modern_bpf): add security_file_mprotect #1601

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f4143e
new(modern_bpf): add security_file_mprotect
dwindsor Dec 22, 2023
077f8f1
Don't assert result param in security_file_mprotect test
dwindsor Dec 22, 2023
eb0dcf4
Readjust parameters in tests
dwindsor Dec 22, 2023
015c64c
Only push 4 params
dwindsor Dec 22, 2023
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
2 changes: 1 addition & 1 deletion driver/event_stats.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ or GPL2.txt for full copies of the license.
#define TRACEPOINT_EVENTS_NUM 6
#define METAEVENTS_NUM 20
#define PLUGIN_EVENTS_NUM 1
#define UNKNOWN_EVENTS_NUM 21
#define UNKNOWN_EVENTS_NUM 23
2 changes: 2 additions & 0 deletions driver/event_table.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,8 @@ const struct ppm_event_info g_event_info[] = {
[PPME_SYSCALL_MKNOD_X] = {"mknod", EC_OTHER | EC_SYSCALL, EF_NONE, 4, {{"res", PT_ERRNO, PF_DEC}, {"path", PT_FSPATH, PF_NA},{"mode", PT_MODE, PF_OCT, mknod_mode},{"dev", PT_UINT32, PF_DEC}}},
[PPME_SYSCALL_MKNODAT_E] = {"mknodat", EC_OTHER | EC_SYSCALL, EF_NONE, 0},
[PPME_SYSCALL_MKNODAT_X] = {"mknodat", EC_OTHER | EC_SYSCALL, EF_USES_FD, 5, {{"res", PT_ERRNO, PF_DEC}, {"dirfd", PT_FD, PF_DEC}, {"path", PT_FSRELPATH, PF_NA, DIRFD_PARAM(1)},{"mode", PT_MODE, PF_OCT, mknod_mode},{"dev", PT_UINT32, PF_DEC}}},
[PPME_LSM_SECURITY_FILE_MPROTECT_E] = {"security_file_mprotect", EC_OTHER, EF_SKIPPARSERESET | EF_NONE, 4, {{"addr_start", PT_UINT64, PF_HEX}, {"addr_end", PT_UINT64, PF_HEX}, {"reqprot", PT_FLAGS32, PF_HEX, prot_flags}, {"prot", PT_FLAGS32, PF_HEX, prot_flags}}},
[PPME_LSM_SECURITY_FILE_MPROTECT_X] = {"NA", EC_UNKNOWN, EF_UNUSED | EF_OLD_VERSION, 0},
};
#pragma GCC diagnostic pop

Expand Down
3 changes: 3 additions & 0 deletions driver/modern_bpf/definitions/events_dimensions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,9 @@
#define MKNOD_E_SIZE HEADER_LEN
#define MKNODAT_E_SIZE HEADER_LEN

/* kprobes */
#define SECURITY_FILE_MPROTECT_SIZE HEADER_LEN + sizeof(uint64_t) * 2 + sizeof(uint32_t) * 2 + PARAM_LEN * 4

/* Generic tracepoints events. */
#define SCHED_SWITCH_SIZE HEADER_LEN + sizeof(int64_t) + sizeof(uint64_t) * 2 + sizeof(uint32_t) * 3 + PARAM_LEN * 6
#define PAGE_FAULT_SIZE HEADER_LEN + sizeof(uint64_t) * 2 + sizeof(uint32_t) + PARAM_LEN * 3
Expand Down
14 changes: 14 additions & 0 deletions driver/modern_bpf/helpers/extract/extract_from_kernel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,20 @@ static __always_inline void extract__pgft_min(struct task_struct *task, unsigned
READ_TASK_FIELD_INTO(pgft_min, task, min_flt);
}

static __always_inline unsigned long extract__vm_start(struct vm_area_struct *vma)
{
unsigned long vm_start = 0;
BPF_CORE_READ_INTO(&vm_start, vma, vm_start);
return vm_start;
}

static __always_inline unsigned long extract__vm_end(struct vm_area_struct *vma)
{
unsigned long vm_end = 0;
BPF_CORE_READ_INTO(&vm_end, vma, vm_end);
return vm_end;
}

/**
* @brief Extract total page size
*
Expand Down
51 changes: 51 additions & 0 deletions driver/modern_bpf/programs/attached/events/security_file_mprotect.bpf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// SPDX-License-Identifier: GPL-2.0-only OR MIT
/*
* Copyright (C) 2023 The Falco Authors.
*
* This file is dual licensed under either the MIT or GPL 2. See MIT.txt
* or GPL2.txt for full copies of the license.
*/

/*=============================== ENTER EVENT ===========================*/

#include <helpers/interfaces/variable_size_event.h>
#include <helpers/extract/extract_from_kernel.h>

SEC("lsm/file_mprotect")
int BPF_PROG(file_mprotect,
struct vm_area_struct *vma,
unsigned long reqprot,
unsigned long prot,
int ret)
{
struct auxiliary_map *auxmap = auxmap__get();
if(!auxmap)
{
return 0;
}

auxmap__preload_event_header(auxmap, PPME_LSM_SECURITY_FILE_MPROTECT_E);

/*=============================== COLLECT PARAMETERS ===========================*/

/* Parameter 1: vm_start (PT_UINT64) */
unsigned long vm_start = extract__vm_start(vma);
auxmap__store_u64_param(auxmap, vm_start);

/* Parameter 2: vm_end (PT_UINT64) */
unsigned long vm_end = extract__vm_end(vma);
auxmap__store_u64_param(auxmap, vm_end);

/* Parameter 3: reqprot (type: PT_FLAGS32) */
auxmap__store_u32_param(auxmap, reqprot);

/* Parameter 4: prot (type: PT_FLAGS32)*/
auxmap__store_u32_param(auxmap, prot);

/*=============================== COLLECT PARAMETERS ===========================*/
//auxmap__submit_event(auxmap, ctx);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The auxmap API is wrong (we should be using fixed-size ringbuf), but to illustrate the problem I'm facing, let's just go with this for now.

If this line is not commented out, the verifier fails:

libbpf: prog 'file_mprotect': BPF program load failed: Invalid argument
libbpf: prog 'file_mprotect': -- BEGIN PROG LOAD LOG --
processed 623 insns (limit 1000000) max_states_per_insn 4 total_states 50 peak_states 48 mark_read 3
-- END PROG LOAD LOG --
libbpf: prog 'file_mprotect': failed to load: -22
libbpf: failed to load object 'bpf_probe'
libbpf: failed to load BPF skeleton 'bpf_probe': -22
libpman: failed to load BPF object (errno: 22 | message: Invalid argument)/home/dave/src/libs/test/libscap/test_suites/engines/modern_bpf/modern_bpf.cpp:148: Failure
Value of: !h || ret != 0
  Actual: true
Expected: false

Rather than digging deeper into this now, could I get a sanity check to see if this is the correct approach generally given the guidance in #963? @FedeDP

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with this approach; i don't know why this is failing though ;)
Anyway, as shared privately, we surely need more work to understand how to integrate LSM/kprobes inside current libs architecture:

  • we will have a single event (instead of pushing a fake empty exit event), while we always had enter/exit events until now
  • kprobes/LSM don't carry a return code, therefore we need to understand how to deal with this too
  • how to avoid enriching same state from different hooks (ie: both kprobe and syscalls)?

In the end, there are multiple questions looking for a good answer before we can start.
Hopefully maintainers will share some proposals soonish :)


return 0;
}

/*=============================== EXIT EVENT ===========================*/
7 changes: 5 additions & 2 deletions driver/ppm_events_public.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,9 @@ typedef enum {
PPME_SYSCALL_MKNOD_X = 415,
PPME_SYSCALL_MKNODAT_E = 416,
PPME_SYSCALL_MKNODAT_X = 417,
PPM_EVENT_MAX = 418
PPME_LSM_SECURITY_FILE_MPROTECT_E = 418,
PPME_LSM_SECURITY_FILE_MPROTECT_X = 419,
PPM_EVENT_MAX = 420
} ppm_event_code;
/*@}*/

Expand Down Expand Up @@ -1892,7 +1894,8 @@ enum extra_event_prog_code
PPM_SC_X(VM86, 433) \
PPM_SC_X(OLDOLDUNAME, 434) \
PPM_SC_X(SUBPAGE_PROT, 435) \
PPM_SC_X(PCICONFIG_IOBASE, 436)
PPM_SC_X(PCICONFIG_IOBASE, 436) \
PPM_SC_X(SECURITY_FILE_MPROTECT, 437)

typedef enum {
#define PPM_SC_X(name, value) PPM_SC_##name = (value),
Expand Down
3 changes: 2 additions & 1 deletion driver/ppm_tp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
X(KMOD_PROG_PAGE_FAULT_KERNEL, "page_fault_kernel") \
X(KMOD_PROG_SIGNAL_DELIVER, "signal_deliver") \
X(KMOD_PROG_SCHED_PROC_FORK, "sched_process_fork") \
X(KMOD_PROG_SCHED_PROC_EXEC, "sched_process_exec")
X(KMOD_PROG_SCHED_PROC_EXEC, "sched_process_exec") \
X(KMOD_PROG_SECUTIRY_FILE_MPROTECT, "security_file_mprotect")

typedef enum
{
Expand Down
69 changes: 69 additions & 0 deletions test/drivers/test_suites/generic_tracepoints_suite/security_file_mprotect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#include "../../event_class/event_class.h"
#include "../../helpers/proc_parsing.h"

#if defined(__NR_mprotect)

#include <linux/sched.h>
#include <sys/mman.h>

TEST(GenericTracepoints, security_file_mprotect)
{
auto evt_test = get_syscall_event_test(__NR_execve, EXIT_EVENT);

evt_test->enable_capture();

/*=============================== TRIGGER SYSCALL ===========================*/

char buffer[1024];
int ret = syscall(__NR_mprotect, buffer, 1024, PROT_READ);
if (ret < 0)
{
exit(EXIT_FAILURE);
}
assert_syscall_state(SYSCALL_SUCCESS, "mprotect", ret, NOT_EQUAL, -1);

evt_test->disable_capture();

evt_test->assert_event_presence(ret);

/*=============================== TRIGGER SYSCALL ===========================*/

evt_test->disable_capture();

/* We search for a child event. */
evt_test->assert_event_presence(ret);

if(HasFatalFailure())
{
return;
}

evt_test->parse_event();

evt_test->assert_header();

/*=============================== ASSERT PARAMETERS ===========================*/

/* Please note here we cannot assert all the params, we check only the possible ones. */

/* Parameter 1: res (type: PT_ERRNO)*/
evt_test->assert_numeric_param(1, (int64_t)0);

/* Parameter 2: addr_start (type: PT_UINT64) */
evt_test->assert_only_param_len(2, sizeof(uint64_t));

/* Parameter 3: addr_end (type: PT_UINT64) */
evt_test->assert_only_param_len(3, sizeof(uint64_t));

/* Parameter 4: reqprot (type: PT_FLAGS32) */
evt_test->assert_numeric_param(4, (int32_t)PROT_READ);

/* Parameter 5: prot (type: PT_FLAGS32) */
evt_test->assert_numeric_param(5, (int32_t)PROT_READ);

/*=============================== ASSERT PARAMETERS ===========================*/

evt_test->assert_num_params_pushed(5);
}

#endif
4 changes: 3 additions & 1 deletion userspace/libpman/src/events_prog_names.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,9 @@ static const char* event_prog_names[PPM_EVENT_MAX] = {
[PPME_SYSCALL_MKNOD_E] = "mknod_e",
[PPME_SYSCALL_MKNOD_X] = "mknod_x",
[PPME_SYSCALL_MKNODAT_E] = "mknodat_e",
[PPME_SYSCALL_MKNODAT_X] = "mknodat_x"
[PPME_SYSCALL_MKNODAT_X] = "mknodat_x",
[PPME_LSM_SECURITY_FILE_MPROTECT_E] = "security_file_mprotect_e",
[PPME_LSM_SECURITY_FILE_MPROTECT_X] = "security_file_mprotect_x"
};

/* Some events can require more than one bpf program to collect all the data. */
Expand Down
Loading
Loading