Message ID | 20191220154208.15895-13-kpsingh@chromium.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | MAC and Audit policy using eBPF (KRSI) | expand |
On Fri, Dec 20, 2019 at 7:42 AM KP Singh <kpsingh@chromium.org> wrote: > > From: KP Singh <kpsingh@google.com> > > * Load a BPF program that audits mprotect calls > * Attach the program to the "file_mprotect" LSM hook > * Verify if the program is actually loading by reading > securityfs > * Initialize the perf events buffer and poll for audit events > * Do an mprotect on some memory allocated on the heap > * Verify if the audit event was received > > Signed-off-by: KP Singh <kpsingh@google.com> > --- > MAINTAINERS | 2 + > .../bpf/prog_tests/lsm_mprotect_audit.c | 129 ++++++++++++++++++ > .../selftests/bpf/progs/lsm_mprotect_audit.c | 58 ++++++++ > 3 files changed, 189 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c > create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c > [...] > +/* > + * Define some of the structs used in the BPF program. > + * Only the field names and their sizes need to be the > + * same as the kernel type, the order is irrelevant. > + */ > +struct mm_struct { > + unsigned long start_brk, brk, start_stack; > +}; > + > +struct vm_area_struct { > + unsigned long start_brk, brk, start_stack; > + unsigned long vm_start, vm_end; > + struct mm_struct *vm_mm; > + unsigned long vm_flags; > +}; > + > +BPF_TRACE_3("lsm/file_mprotect", mprotect_audit, > + struct vm_area_struct *, vma, > + unsigned long, reqprot, unsigned long, prot) > +{ > + struct mprotect_audit_log audit_log = {}; > + int is_heap = 0; > + > + __builtin_preserve_access_index(({ you don't need __builtin_preserve_access_index, if you mark vm_area_struct and mm_struct with __attribute__((preserve_access_index) > + is_heap = (vma->vm_start >= vma->vm_mm->start_brk && > + vma->vm_end <= vma->vm_mm->brk); > + })); > + > + audit_log.magic = MPROTECT_AUDIT_MAGIC; > + audit_log.is_heap = is_heap; > + bpf_lsm_event_output(&perf_buf_map, BPF_F_CURRENT_CPU, &audit_log, > + sizeof(audit_log)); You test would be much simpler if you use global variables to pass data back to userspace, instead of using perf buffer. Also please see fentry_fexit.c test for example of using BPF skeleton to shorten and simpify userspace part of test. > + return 0; > +} > -- > 2.20.1 >
On 23-Dez 22:49, Andrii Nakryiko wrote: > On Fri, Dec 20, 2019 at 7:42 AM KP Singh <kpsingh@chromium.org> wrote: > > > > From: KP Singh <kpsingh@google.com> > > > > * Load a BPF program that audits mprotect calls > > * Attach the program to the "file_mprotect" LSM hook > > * Verify if the program is actually loading by reading > > securityfs > > * Initialize the perf events buffer and poll for audit events > > * Do an mprotect on some memory allocated on the heap > > * Verify if the audit event was received > > > > Signed-off-by: KP Singh <kpsingh@google.com> > > --- > > MAINTAINERS | 2 + > > .../bpf/prog_tests/lsm_mprotect_audit.c | 129 ++++++++++++++++++ > > .../selftests/bpf/progs/lsm_mprotect_audit.c | 58 ++++++++ > > 3 files changed, 189 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c > > create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c > > > > [...] > > > +/* > > + * Define some of the structs used in the BPF program. > > + * Only the field names and their sizes need to be the > > + * same as the kernel type, the order is irrelevant. > > + */ > > +struct mm_struct { > > + unsigned long start_brk, brk, start_stack; > > +}; > > + > > +struct vm_area_struct { > > + unsigned long start_brk, brk, start_stack; > > + unsigned long vm_start, vm_end; > > + struct mm_struct *vm_mm; > > + unsigned long vm_flags; > > +}; > > + > > +BPF_TRACE_3("lsm/file_mprotect", mprotect_audit, > > + struct vm_area_struct *, vma, > > + unsigned long, reqprot, unsigned long, prot) > > +{ > > + struct mprotect_audit_log audit_log = {}; > > + int is_heap = 0; > > + > > + __builtin_preserve_access_index(({ > > you don't need __builtin_preserve_access_index, if you mark > vm_area_struct and mm_struct with > __attribute__((preserve_access_index) Cool, updated! > > > + is_heap = (vma->vm_start >= vma->vm_mm->start_brk && > > + vma->vm_end <= vma->vm_mm->brk); > > + })); > > + > > + audit_log.magic = MPROTECT_AUDIT_MAGIC; > > + audit_log.is_heap = is_heap; > > + bpf_lsm_event_output(&perf_buf_map, BPF_F_CURRENT_CPU, &audit_log, > > + sizeof(audit_log)); > > You test would be much simpler if you use global variables to pass > data back to userspace, instead of using perf buffer. > > Also please see fentry_fexit.c test for example of using BPF skeleton > to shorten and simpify userspace part of test. Thanks for the skeleton work! This makes using global variables easier and the tests are indeed much simpler, I have updated it for the next revision. One follow up question regarding global variables, let's say I have the following global variable defined in the BPF program: struct result_info { __u32 count; }; struct result_info result = { .count = 0, }; The defintion of result_info needs to be included before the .skel.h as it's not automatically generated or maybe I am missing a trick here? For now, I have defined this in a header which gets included both in the program and the test. - KP > > > + return 0; > > +} > > -- > > 2.20.1 > >
On Fri, 20 Dec 2019, KP Singh wrote: > From: KP Singh <kpsingh@google.com> > > * Load a BPF program that audits mprotect calls > * Attach the program to the "file_mprotect" LSM hook > * Verify if the program is actually loading by reading > securityfs > * Initialize the perf events buffer and poll for audit events > * Do an mprotect on some memory allocated on the heap > * Verify if the audit event was received > > Signed-off-by: KP Singh <kpsingh@google.com> Good to see! Reviewed-by: James Morris <jamorris@linux.microsoft.com>
On Fri, Jan 3, 2020 at 4:09 PM KP Singh <kpsingh@chromium.org> wrote: > > On 23-Dez 22:49, Andrii Nakryiko wrote: > > On Fri, Dec 20, 2019 at 7:42 AM KP Singh <kpsingh@chromium.org> wrote: > > > > > > From: KP Singh <kpsingh@google.com> > > > > > > * Load a BPF program that audits mprotect calls > > > * Attach the program to the "file_mprotect" LSM hook > > > * Verify if the program is actually loading by reading > > > securityfs > > > * Initialize the perf events buffer and poll for audit events > > > * Do an mprotect on some memory allocated on the heap > > > * Verify if the audit event was received > > > > > > Signed-off-by: KP Singh <kpsingh@google.com> > > > --- > > > MAINTAINERS | 2 + > > > .../bpf/prog_tests/lsm_mprotect_audit.c | 129 ++++++++++++++++++ > > > .../selftests/bpf/progs/lsm_mprotect_audit.c | 58 ++++++++ > > > 3 files changed, 189 insertions(+) > > > create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c > > > create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c > > > > > > > [...] > > > > > +/* > > > + * Define some of the structs used in the BPF program. > > > + * Only the field names and their sizes need to be the > > > + * same as the kernel type, the order is irrelevant. > > > + */ > > > +struct mm_struct { > > > + unsigned long start_brk, brk, start_stack; > > > +}; > > > + > > > +struct vm_area_struct { > > > + unsigned long start_brk, brk, start_stack; > > > + unsigned long vm_start, vm_end; > > > + struct mm_struct *vm_mm; > > > + unsigned long vm_flags; > > > +}; > > > + > > > +BPF_TRACE_3("lsm/file_mprotect", mprotect_audit, > > > + struct vm_area_struct *, vma, > > > + unsigned long, reqprot, unsigned long, prot) > > > +{ > > > + struct mprotect_audit_log audit_log = {}; > > > + int is_heap = 0; > > > + > > > + __builtin_preserve_access_index(({ > > > > you don't need __builtin_preserve_access_index, if you mark > > vm_area_struct and mm_struct with > > __attribute__((preserve_access_index) > > Cool, updated! > > > > > > + is_heap = (vma->vm_start >= vma->vm_mm->start_brk && > > > + vma->vm_end <= vma->vm_mm->brk); > > > + })); > > > + > > > + audit_log.magic = MPROTECT_AUDIT_MAGIC; > > > + audit_log.is_heap = is_heap; > > > + bpf_lsm_event_output(&perf_buf_map, BPF_F_CURRENT_CPU, &audit_log, > > > + sizeof(audit_log)); > > > > You test would be much simpler if you use global variables to pass > > data back to userspace, instead of using perf buffer. > > > > Also please see fentry_fexit.c test for example of using BPF skeleton > > to shorten and simpify userspace part of test. > > Thanks for the skeleton work! > > This makes using global variables easier and the tests are indeed much > simpler, I have updated it for the next revision. > > One follow up question regarding global variables, let's say I have > the following global variable defined in the BPF program: > > struct result_info { > __u32 count; > }; > > struct result_info result = { > .count = 0, > }; > > The defintion of result_info needs to be included before the .skel.h > as it's not automatically generated or maybe I am missing a > trick here? > > For now, I have defined this in a header which gets included both in > the program and the test. Yes, ideally all common types should be shared in a common header. My initial implementation actually supported dumping out all the types in a generated skeleton, but that was inconvenient in a lot of cases, so I dropped that. > > - KP > > > > > > + return 0; > > > +} > > > -- > > > 2.20.1 > > >
diff --git a/MAINTAINERS b/MAINTAINERS index 681ae39bb2f0..652c93292ae9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3182,6 +3182,8 @@ L: bpf@vger.kernel.org S: Maintained F: security/bpf/ F: include/linux/bpf_lsm.h +F: tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c +F: tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c BROADCOM B44 10/100 ETHERNET DRIVER M: Michael Chan <michael.chan@broadcom.com> diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c b/tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c new file mode 100644 index 000000000000..953531cec9fd --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Copyright 2019 Google LLC. + */ + +#include <test_progs.h> +#include <sys/mman.h> +#include <unistd.h> +#include <malloc.h> + +#define EXPECTED_PROG_NAME "mprotect_audit" +#define MPROTECT_AUDIT_MAGIC 0xDEAD + +struct mprotect_audit_log { + int is_heap, magic; +}; + +static void on_sample(void *ctx, int cpu, void *data, __u32 size) +{ + struct mprotect_audit_log *audit_log = data; + int duration = 0; + + if (audit_log->magic != MPROTECT_AUDIT_MAGIC) + return; + + if (CHECK(audit_log->is_heap != 1, "mprotect on heap", + "is_heap = %d\n", audit_log->is_heap)) + return; + + *(bool *)ctx = true; +} + +int heap_mprotect(void) +{ + void *buf; + long sz; + + sz = sysconf(_SC_PAGESIZE); + if (sz < 0) + return sz; + + buf = memalign(sz, 2 * sz); + if (buf == NULL) + return -ENOMEM; + + return mprotect(buf, sz, PROT_READ | PROT_EXEC); +} + +void test_lsm_mprotect_audit(void) +{ + struct bpf_prog_load_attr attr = { + .file = "./lsm_mprotect_audit.o", + }; + + struct perf_buffer_opts pb_opts = {}; + struct perf_buffer *pb = NULL; + struct bpf_link *link = NULL; + struct bpf_map *perf_buf_map; + struct bpf_object *prog_obj; + struct bpf_program *prog; + int err, prog_fd, sfs_fd; + char sfs_buf[1024]; + int duration = 0; + bool passed = false; + + err = bpf_prog_load_xattr(&attr, &prog_obj, &prog_fd); + if (CHECK(err, "prog_load lsm/file_mprotect", + "err %d errno %d\n", err, errno)) + goto close_prog; + + prog = bpf_object__find_program_by_title(prog_obj, "lsm/file_mprotect"); + if (CHECK(!prog, "find_prog", "lsm/file_mprotect not found\n")) + goto close_prog; + + link = bpf_program__attach_lsm(prog); + if (CHECK(IS_ERR(link), "attach_lsm file_mprotect", + "err %ld\n", PTR_ERR(link))) + goto close_prog; + + sfs_fd = open("/sys/kernel/security/bpf/file_mprotect", O_RDONLY); + if (CHECK(sfs_fd < 0, "sfs_open file_mprotect", + "err %d errno %d\n", sfs_fd, errno)) + goto close_prog; + + err = read(sfs_fd, sfs_buf, sizeof(sfs_buf)); + if (CHECK(err < 0, "sfs_read file_mprotect", + "err %d errno %d\n", sfs_fd, errno)) + goto close_prog; + + err = strncmp(sfs_buf, EXPECTED_PROG_NAME, strlen(EXPECTED_PROG_NAME)); + if (CHECK(err != 0, + "sfs_read value", "want = %s, got = %s\n", + EXPECTED_PROG_NAME, sfs_buf)) + goto close_prog; + + perf_buf_map = bpf_object__find_map_by_name(prog_obj, "perf_buf_map"); + if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n")) + goto close_prog; + + /* set up perf buffer */ + pb_opts.sample_cb = on_sample; + pb_opts.ctx = &passed; + pb = perf_buffer__new(bpf_map__fd(perf_buf_map), 1, &pb_opts); + if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb))) + goto close_prog; + + err = heap_mprotect(); + if (CHECK(err < 0, "heap_mprotect", + "err %d errno %d\n", err, errno)) + goto close_prog; + + /* read perf buffer */ + err = perf_buffer__poll(pb, 100); + if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err)) + goto close_prog; + + /* + * make sure mprotect_audit program was triggered + * and detected an mprotect on the heap + */ + CHECK_FAIL(!passed); + +close_prog: + perf_buffer__free(pb); + if (!IS_ERR_OR_NULL(link)) + bpf_link__destroy(link); + bpf_object__close(prog_obj); +} diff --git a/tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c b/tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c new file mode 100644 index 000000000000..85048315baae --- /dev/null +++ b/tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Copyright 2019 Google LLC. + */ + +#include <linux/bpf.h> +#include <stdbool.h> +#include "bpf_helpers.h" +#include "bpf_trace_helpers.h" + +char _license[] SEC("license") = "GPL"; +struct { + __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY); + __uint(key_size, sizeof(int)); + __uint(value_size, sizeof(int)); +} perf_buf_map SEC(".maps"); + +#define MPROTECT_AUDIT_MAGIC 0xDEAD + +struct mprotect_audit_log { + int is_heap, magic; +}; + +/* + * Define some of the structs used in the BPF program. + * Only the field names and their sizes need to be the + * same as the kernel type, the order is irrelevant. + */ +struct mm_struct { + unsigned long start_brk, brk, start_stack; +}; + +struct vm_area_struct { + unsigned long start_brk, brk, start_stack; + unsigned long vm_start, vm_end; + struct mm_struct *vm_mm; + unsigned long vm_flags; +}; + +BPF_TRACE_3("lsm/file_mprotect", mprotect_audit, + struct vm_area_struct *, vma, + unsigned long, reqprot, unsigned long, prot) +{ + struct mprotect_audit_log audit_log = {}; + int is_heap = 0; + + __builtin_preserve_access_index(({ + is_heap = (vma->vm_start >= vma->vm_mm->start_brk && + vma->vm_end <= vma->vm_mm->brk); + })); + + audit_log.magic = MPROTECT_AUDIT_MAGIC; + audit_log.is_heap = is_heap; + bpf_lsm_event_output(&perf_buf_map, BPF_F_CURRENT_CPU, &audit_log, + sizeof(audit_log)); + return 0; +}