| Message ID | 20230412043300.360803-6-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | New BPF map and BTF security LSM hooks | expand |
On Tue, Apr 11, 2023 at 09:32:57PM -0700, Andrii Nakryiko wrote: > Add selftests that goes over every known map type and validates that > a combination of privileged/unprivileged modes and allow/reject/pass-through > LSM policy decisions behave as expected. > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > --- > .../selftests/bpf/prog_tests/lsm_map_create.c | 143 ++++++++++++++++++ > .../selftests/bpf/progs/lsm_map_create.c | 32 ++++ > tools/testing/selftests/bpf/test_progs.h | 6 + > 3 files changed, 181 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_map_create.c > create mode 100644 tools/testing/selftests/bpf/progs/lsm_map_create.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_map_create.c b/tools/testing/selftests/bpf/prog_tests/lsm_map_create.c > new file mode 100644 > index 000000000000..fee78b0448c3 > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/lsm_map_create.c > @@ -0,0 +1,143 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */ > +#include "linux/bpf.h" > +#include <test_progs.h> > +#include <bpf/btf.h> > +#include "cap_helpers.h" > +#include "lsm_map_create.skel.h" > + > +static int drop_priv_caps(__u64 *old_caps) > +{ > + return cap_disable_effective((1ULL << CAP_BPF) | > + (1ULL << CAP_PERFMON) | > + (1ULL << CAP_NET_ADMIN) | > + (1ULL << CAP_SYS_ADMIN), old_caps); > +} > + > +static int restore_priv_caps(__u64 old_caps) > +{ > + return cap_enable_effective(old_caps, NULL); > +} > + > +void test_lsm_map_create(void) > +{ > + struct btf *btf = NULL; > + struct lsm_map_create *skel = NULL; > + const struct btf_type *t; > + const struct btf_enum *e; > + int i, n, id, err, ret; > + > + skel = lsm_map_create__open_and_load(); > + if (!ASSERT_OK_PTR(skel, "skel_open_and_load")) > + return; > + > + skel->bss->my_tid = syscall(SYS_gettid); > + skel->bss->decision = 0; > + > + err = lsm_map_create__attach(skel); > + if (!ASSERT_OK(err, "skel_attach")) > + goto cleanup; > + > + btf = btf__parse("/sys/kernel/btf/vmlinux", NULL); > + if (!ASSERT_OK_PTR(btf, "btf_parse")) > + goto cleanup; > + > + /* find enum bpf_map_type and enumerate each value */ > + id = btf__find_by_name_kind(btf, "bpf_map_type", BTF_KIND_ENUM); > + if (!ASSERT_GT(id, 0, "bpf_map_type_id")) > + goto cleanup; > + > + t = btf__type_by_id(btf, id); > + e = btf_enum(t); > + n = btf_vlen(t); > + for (i = 0; i < n; e++, i++) { > + enum bpf_map_type map_type = (enum bpf_map_type)e->val; > + const char *map_type_name; > + __u64 orig_caps; > + bool is_map_priv; > + bool needs_btf; > + > + if (map_type == BPF_MAP_TYPE_UNSPEC) > + continue; > + > + /* this will show which map type we are working with in verbose log */ > + map_type_name = btf__str_by_offset(btf, e->name_off); > + ASSERT_OK_PTR(map_type_name, map_type_name); > + > + switch (map_type) { > + case BPF_MAP_TYPE_ARRAY: > + case BPF_MAP_TYPE_PERCPU_ARRAY: > + case BPF_MAP_TYPE_PROG_ARRAY: > + case BPF_MAP_TYPE_PERF_EVENT_ARRAY: > + case BPF_MAP_TYPE_CGROUP_ARRAY: > + case BPF_MAP_TYPE_ARRAY_OF_MAPS: > + case BPF_MAP_TYPE_HASH: > + case BPF_MAP_TYPE_PERCPU_HASH: > + case BPF_MAP_TYPE_HASH_OF_MAPS: > + case BPF_MAP_TYPE_RINGBUF: > + case BPF_MAP_TYPE_USER_RINGBUF: > + case BPF_MAP_TYPE_CGROUP_STORAGE: > + case BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE: > + is_map_priv = false; > + needs_btf = false; > + break; > + case BPF_MAP_TYPE_SK_STORAGE: > + case BPF_MAP_TYPE_INODE_STORAGE: > + case BPF_MAP_TYPE_TASK_STORAGE: > + case BPF_MAP_TYPE_CGRP_STORAGE: > + is_map_priv = true; > + needs_btf = true; > + break; > + default: > + is_map_priv = true; > + needs_btf = false; > + } > + > + /* make sure we delegate to kernel for final decision */ > + skel->bss->decision = 0; > + > + /* we are normally under sudo, so all maps should succeed */ > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > + ASSERT_EQ(ret, 1, "default_priv_mode"); > + > + /* local storage needs custom BTF to be loaded, which we > + * currently can't do once we drop privileges, so skip few > + * checks for such maps > + */ > + if (needs_btf) > + goto skip_if_needs_btf; > + > + /* now let's drop privileges, and chech that unpriv maps are > + * still possible to create > + */ > + if (!ASSERT_OK(drop_priv_caps(&orig_caps), "drop_caps")) > + goto cleanup; > + > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > + ASSERT_EQ(ret, is_map_priv ? 0 : 1, "default_unpriv_mode"); > + > + /* allow any map creation for our thread */ > + skel->bss->decision = 1; > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > + ASSERT_EQ(ret, 1, "lsm_allow_unpriv_mode"); > + > + /* reject any map creation for our thread */ > + skel->bss->decision = -1; > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > + ASSERT_EQ(ret, 0, "lsm_reject_unpriv_mode"); > + > + /* restore privileges, but keep reject LSM policy */ > + if (!ASSERT_OK(restore_priv_caps(orig_caps), "restore_caps")) > + goto cleanup; > + > +skip_if_needs_btf: > + /* even with all caps map create will fail */ > + skel->bss->decision = -1; > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > + ASSERT_EQ(ret, 0, "lsm_reject_priv_mode"); > + } > + > +cleanup: > + btf__free(btf); > + lsm_map_create__destroy(skel); > +} This test looks good! One meta-comment about testing would be: are you sure each needs to be ASSERT instead of EXPECT? (i.e. should forward progress through this test always be aborted when a check fails?) > diff --git a/tools/testing/selftests/bpf/progs/lsm_map_create.c b/tools/testing/selftests/bpf/progs/lsm_map_create.c > new file mode 100644 > index 000000000000..093825c68459 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/lsm_map_create.c > @@ -0,0 +1,32 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */ > + > +#include "vmlinux.h" > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > +#include <errno.h> > + > +char _license[] SEC("license") = "GPL"; > + > +int my_tid; > +/* LSM enforcement: > + * - 0, delegate to kernel; > + * - 1, allow; > + * - -1, reject. > + */ > +int decision; > + > +SEC("lsm/bpf_map_create_security") > +int BPF_PROG(allow_unpriv_maps, union bpf_attr *attr) > +{ > + if (!my_tid || (u32)bpf_get_current_pid_tgid() != my_tid) > + return 0; /* keep processing LSM hooks */ > + > + if (decision == 0) > + return 0; > + > + if (decision > 0) > + return 1; /* allow */ > + > + return -EPERM; > +} > diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h > index 10ba43250668..12f9c6652d40 100644 > --- a/tools/testing/selftests/bpf/test_progs.h > +++ b/tools/testing/selftests/bpf/test_progs.h > @@ -23,6 +23,7 @@ typedef __u16 __sum16; > #include <linux/perf_event.h> > #include <linux/socket.h> > #include <linux/unistd.h> > +#include <sys/syscall.h> > > #include <sys/ioctl.h> > #include <sys/wait.h> > @@ -176,6 +177,11 @@ void test__skip(void); > void test__fail(void); > int test__join_cgroup(const char *path); > > +static inline int gettid(void) > +{ > + return syscall(SYS_gettid); > +} > + > #define PRINT_FAIL(format...) \ > ({ \ > test__fail(); \ > -- > 2.34.1 >
On Wed, Apr 12, 2023 at 11:23 AM Kees Cook <keescook@chromium.org> wrote: > > On Tue, Apr 11, 2023 at 09:32:57PM -0700, Andrii Nakryiko wrote: > > Add selftests that goes over every known map type and validates that > > a combination of privileged/unprivileged modes and allow/reject/pass-through > > LSM policy decisions behave as expected. > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > --- > > .../selftests/bpf/prog_tests/lsm_map_create.c | 143 ++++++++++++++++++ > > .../selftests/bpf/progs/lsm_map_create.c | 32 ++++ > > tools/testing/selftests/bpf/test_progs.h | 6 + > > 3 files changed, 181 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_map_create.c > > create mode 100644 tools/testing/selftests/bpf/progs/lsm_map_create.c > > [...] > > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > > + ASSERT_EQ(ret, is_map_priv ? 0 : 1, "default_unpriv_mode"); > > + > > + /* allow any map creation for our thread */ > > + skel->bss->decision = 1; > > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > > + ASSERT_EQ(ret, 1, "lsm_allow_unpriv_mode"); > > + > > + /* reject any map creation for our thread */ > > + skel->bss->decision = -1; > > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > > + ASSERT_EQ(ret, 0, "lsm_reject_unpriv_mode"); > > + > > + /* restore privileges, but keep reject LSM policy */ > > + if (!ASSERT_OK(restore_priv_caps(orig_caps), "restore_caps")) > > + goto cleanup; > > + > > +skip_if_needs_btf: > > + /* even with all caps map create will fail */ > > + skel->bss->decision = -1; > > + ret = libbpf_probe_bpf_map_type(map_type, NULL); > > + ASSERT_EQ(ret, 0, "lsm_reject_priv_mode"); > > + } > > + > > +cleanup: > > + btf__free(btf); > > + lsm_map_create__destroy(skel); > > +} > > This test looks good! One meta-comment about testing would be: are you > sure each needs to be ASSERT instead of EXPECT? (i.e. should forward > progress through this test always be aborted when a check fails?) > it's our custom BPF selftests ASSERTs, they don't really do assert() and panic, they really are just a check (so I'm guessing they have EXPECT semantics you are referring to). And if check doesn't pass, we just set a flag notifying our own custom test runner that test failed and proceed. So in short, it already behaves like you would want with EXPECT. We just don't use kselftests's ASSERTs and EXPECTs. > > diff --git a/tools/testing/selftests/bpf/progs/lsm_map_create.c b/tools/testing/selftests/bpf/progs/lsm_map_create.c > > new file mode 100644 > > index 000000000000..093825c68459 > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/progs/lsm_map_create.c > > @@ -0,0 +1,32 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */ > > + > > +#include "vmlinux.h" > > +#include <bpf/bpf_helpers.h> > > +#include <bpf/bpf_tracing.h> > > +#include <errno.h> > > + > > +char _license[] SEC("license") = "GPL"; > > + > > +int my_tid; > > +/* LSM enforcement: > > + * - 0, delegate to kernel; > > + * - 1, allow; > > + * - -1, reject. > > + */ > > +int decision; > > + > > +SEC("lsm/bpf_map_create_security") > > +int BPF_PROG(allow_unpriv_maps, union bpf_attr *attr) > > +{ > > + if (!my_tid || (u32)bpf_get_current_pid_tgid() != my_tid) > > + return 0; /* keep processing LSM hooks */ > > + > > + if (decision == 0) > > + return 0; > > + > > + if (decision > 0) > > + return 1; /* allow */ > > + > > + return -EPERM; > > +} > > diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h > > index 10ba43250668..12f9c6652d40 100644 > > --- a/tools/testing/selftests/bpf/test_progs.h > > +++ b/tools/testing/selftests/bpf/test_progs.h > > @@ -23,6 +23,7 @@ typedef __u16 __sum16; > > #include <linux/perf_event.h> > > #include <linux/socket.h> > > #include <linux/unistd.h> > > +#include <sys/syscall.h> > > > > #include <sys/ioctl.h> > > #include <sys/wait.h> > > @@ -176,6 +177,11 @@ void test__skip(void); > > void test__fail(void); > > int test__join_cgroup(const char *path); > > > > +static inline int gettid(void) > > +{ > > + return syscall(SYS_gettid); > > +} > > + > > #define PRINT_FAIL(format...) \ > > ({ \ > > test__fail(); \ > > -- > > 2.34.1 > > > > -- > Kees Cook
diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_map_create.c b/tools/testing/selftests/bpf/prog_tests/lsm_map_create.c new file mode 100644 index 000000000000..fee78b0448c3 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/lsm_map_create.c @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */ +#include "linux/bpf.h" +#include <test_progs.h> +#include <bpf/btf.h> +#include "cap_helpers.h" +#include "lsm_map_create.skel.h" + +static int drop_priv_caps(__u64 *old_caps) +{ + return cap_disable_effective((1ULL << CAP_BPF) | + (1ULL << CAP_PERFMON) | + (1ULL << CAP_NET_ADMIN) | + (1ULL << CAP_SYS_ADMIN), old_caps); +} + +static int restore_priv_caps(__u64 old_caps) +{ + return cap_enable_effective(old_caps, NULL); +} + +void test_lsm_map_create(void) +{ + struct btf *btf = NULL; + struct lsm_map_create *skel = NULL; + const struct btf_type *t; + const struct btf_enum *e; + int i, n, id, err, ret; + + skel = lsm_map_create__open_and_load(); + if (!ASSERT_OK_PTR(skel, "skel_open_and_load")) + return; + + skel->bss->my_tid = syscall(SYS_gettid); + skel->bss->decision = 0; + + err = lsm_map_create__attach(skel); + if (!ASSERT_OK(err, "skel_attach")) + goto cleanup; + + btf = btf__parse("/sys/kernel/btf/vmlinux", NULL); + if (!ASSERT_OK_PTR(btf, "btf_parse")) + goto cleanup; + + /* find enum bpf_map_type and enumerate each value */ + id = btf__find_by_name_kind(btf, "bpf_map_type", BTF_KIND_ENUM); + if (!ASSERT_GT(id, 0, "bpf_map_type_id")) + goto cleanup; + + t = btf__type_by_id(btf, id); + e = btf_enum(t); + n = btf_vlen(t); + for (i = 0; i < n; e++, i++) { + enum bpf_map_type map_type = (enum bpf_map_type)e->val; + const char *map_type_name; + __u64 orig_caps; + bool is_map_priv; + bool needs_btf; + + if (map_type == BPF_MAP_TYPE_UNSPEC) + continue; + + /* this will show which map type we are working with in verbose log */ + map_type_name = btf__str_by_offset(btf, e->name_off); + ASSERT_OK_PTR(map_type_name, map_type_name); + + switch (map_type) { + case BPF_MAP_TYPE_ARRAY: + case BPF_MAP_TYPE_PERCPU_ARRAY: + case BPF_MAP_TYPE_PROG_ARRAY: + case BPF_MAP_TYPE_PERF_EVENT_ARRAY: + case BPF_MAP_TYPE_CGROUP_ARRAY: + case BPF_MAP_TYPE_ARRAY_OF_MAPS: + case BPF_MAP_TYPE_HASH: + case BPF_MAP_TYPE_PERCPU_HASH: + case BPF_MAP_TYPE_HASH_OF_MAPS: + case BPF_MAP_TYPE_RINGBUF: + case BPF_MAP_TYPE_USER_RINGBUF: + case BPF_MAP_TYPE_CGROUP_STORAGE: + case BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE: + is_map_priv = false; + needs_btf = false; + break; + case BPF_MAP_TYPE_SK_STORAGE: + case BPF_MAP_TYPE_INODE_STORAGE: + case BPF_MAP_TYPE_TASK_STORAGE: + case BPF_MAP_TYPE_CGRP_STORAGE: + is_map_priv = true; + needs_btf = true; + break; + default: + is_map_priv = true; + needs_btf = false; + } + + /* make sure we delegate to kernel for final decision */ + skel->bss->decision = 0; + + /* we are normally under sudo, so all maps should succeed */ + ret = libbpf_probe_bpf_map_type(map_type, NULL); + ASSERT_EQ(ret, 1, "default_priv_mode"); + + /* local storage needs custom BTF to be loaded, which we + * currently can't do once we drop privileges, so skip few + * checks for such maps + */ + if (needs_btf) + goto skip_if_needs_btf; + + /* now let's drop privileges, and chech that unpriv maps are + * still possible to create + */ + if (!ASSERT_OK(drop_priv_caps(&orig_caps), "drop_caps")) + goto cleanup; + + ret = libbpf_probe_bpf_map_type(map_type, NULL); + ASSERT_EQ(ret, is_map_priv ? 0 : 1, "default_unpriv_mode"); + + /* allow any map creation for our thread */ + skel->bss->decision = 1; + ret = libbpf_probe_bpf_map_type(map_type, NULL); + ASSERT_EQ(ret, 1, "lsm_allow_unpriv_mode"); + + /* reject any map creation for our thread */ + skel->bss->decision = -1; + ret = libbpf_probe_bpf_map_type(map_type, NULL); + ASSERT_EQ(ret, 0, "lsm_reject_unpriv_mode"); + + /* restore privileges, but keep reject LSM policy */ + if (!ASSERT_OK(restore_priv_caps(orig_caps), "restore_caps")) + goto cleanup; + +skip_if_needs_btf: + /* even with all caps map create will fail */ + skel->bss->decision = -1; + ret = libbpf_probe_bpf_map_type(map_type, NULL); + ASSERT_EQ(ret, 0, "lsm_reject_priv_mode"); + } + +cleanup: + btf__free(btf); + lsm_map_create__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/lsm_map_create.c b/tools/testing/selftests/bpf/progs/lsm_map_create.c new file mode 100644 index 000000000000..093825c68459 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/lsm_map_create.c @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */ + +#include "vmlinux.h" +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <errno.h> + +char _license[] SEC("license") = "GPL"; + +int my_tid; +/* LSM enforcement: + * - 0, delegate to kernel; + * - 1, allow; + * - -1, reject. + */ +int decision; + +SEC("lsm/bpf_map_create_security") +int BPF_PROG(allow_unpriv_maps, union bpf_attr *attr) +{ + if (!my_tid || (u32)bpf_get_current_pid_tgid() != my_tid) + return 0; /* keep processing LSM hooks */ + + if (decision == 0) + return 0; + + if (decision > 0) + return 1; /* allow */ + + return -EPERM; +} diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h index 10ba43250668..12f9c6652d40 100644 --- a/tools/testing/selftests/bpf/test_progs.h +++ b/tools/testing/selftests/bpf/test_progs.h @@ -23,6 +23,7 @@ typedef __u16 __sum16; #include <linux/perf_event.h> #include <linux/socket.h> #include <linux/unistd.h> +#include <sys/syscall.h> #include <sys/ioctl.h> #include <sys/wait.h> @@ -176,6 +177,11 @@ void test__skip(void); void test__fail(void); int test__join_cgroup(const char *path); +static inline int gettid(void) +{ + return syscall(SYS_gettid); +} + #define PRINT_FAIL(format...) \ ({ \ test__fail(); \
Add selftests that goes over every known map type and validates that a combination of privileged/unprivileged modes and allow/reject/pass-through LSM policy decisions behave as expected. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- .../selftests/bpf/prog_tests/lsm_map_create.c | 143 ++++++++++++++++++ .../selftests/bpf/progs/lsm_map_create.c | 32 ++++ tools/testing/selftests/bpf/test_progs.h | 6 + 3 files changed, 181 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_map_create.c create mode 100644 tools/testing/selftests/bpf/progs/lsm_map_create.c