From 116099ed345c932a8ae4a0d884a8f6cc54fd5fed Mon Sep 17 00:00:00 2001 From: Hu Yadi Date: Tue, 23 Jan 2024 14:26:21 +0800 Subject: [PATCH 1/3] selftests/landlock: Fix net_test build with old libc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One issue comes up while building selftest/landlock/net_test on my side (gcc 7.3/glibc-2.28/kernel-4.19). net_test.c: In function ‘set_service’: net_test.c:91:45: warning: implicit declaration of function ‘gettid’; [-Wimplicit-function-declaration] "_selftests-landlock-net-tid%d-index%d", gettid(), ^~~~~~ getgid net_test.c:(.text+0x4e0): undefined reference to `gettid' Signed-off-by: Hu Yadi Suggested-by: Jiao Reviewed-by: Berlin Fixes: a549d055a22e ("selftests/landlock: Add network tests") Link: https://lore.kernel.org/r/20240123062621.25082-1-hu.yadi@h3c.com [mic: Cosmetic fixes] Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/net_test.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c index ea5f727dd257..efcde123af1f 100644 --- a/tools/testing/selftests/landlock/net_test.c +++ b/tools/testing/selftests/landlock/net_test.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include "common.h" @@ -54,6 +55,11 @@ struct service_fixture { }; }; +static pid_t sys_gettid(void) +{ + return syscall(__NR_gettid); +} + static int set_service(struct service_fixture *const srv, const struct protocol_variant prot, const unsigned short index) @@ -88,7 +94,7 @@ static int set_service(struct service_fixture *const srv, case AF_UNIX: srv->unix_addr.sun_family = prot.domain; sprintf(srv->unix_addr.sun_path, - "_selftests-landlock-net-tid%d-index%d", gettid(), + "_selftests-landlock-net-tid%d-index%d", sys_gettid(), index); srv->unix_addr_len = SUN_LEN(&srv->unix_addr); srv->unix_addr.sun_path[0] = '\0'; From 40b7835e74e0383be308d528c5e0e41b3bf72ade Mon Sep 17 00:00:00 2001 From: Hu Yadi Date: Wed, 24 Jan 2024 10:29:08 +0800 Subject: [PATCH 2/3] selftests/landlock: Fix fs_test build with old libc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One issue comes up while building selftest/landlock/fs_test on my side (gcc 7.3/glibc-2.28/kernel-4.19). gcc -Wall -O2 -isystem fs_test.c -lcap -o selftests/landlock/fs_test fs_test.c:4575:9: error: initializer element is not constant .mnt = mnt_tmp, ^~~~~~~ Signed-off-by: Hu Yadi Suggested-by: Jiao Reviewed-by: Berlin Link: https://lore.kernel.org/r/20240124022908.42100-1-hu.yadi@h3c.com Fixes: 04f9070e99a4 ("selftests/landlock: Add tests for pseudo filesystems") [mic: Factor out mount's data string and make mnt_tmp static] Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/fs_test.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 50818904397c..2d6d9b43d958 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -241,9 +241,11 @@ struct mnt_opt { const char *const data; }; -const struct mnt_opt mnt_tmp = { +#define MNT_TMP_DATA "size=4m,mode=700" + +static const struct mnt_opt mnt_tmp = { .type = "tmpfs", - .data = "size=4m,mode=700", + .data = MNT_TMP_DATA, }; static int mount_opt(const struct mnt_opt *const mnt, const char *const target) @@ -4632,7 +4634,10 @@ FIXTURE_VARIANT(layout3_fs) /* clang-format off */ FIXTURE_VARIANT_ADD(layout3_fs, tmpfs) { /* clang-format on */ - .mnt = mnt_tmp, + .mnt = { + .type = "tmpfs", + .data = MNT_TMP_DATA, + }, .file_path = file1_s1d1, }; From bb6f4dbe2639d5b8a9fde4bfb6fefecfd3f18df3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Thu, 25 Jan 2024 16:32:29 +0100 Subject: [PATCH 3/3] selftests/landlock: Fix capability for net_test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CAP_NET_ADMIN allows to configure network interfaces, not CAP_SYS_ADMIN which only allows to call unshare(2). Without this change, running network tests as a non-root user but with all capabilities would fail at the setup_loopback() step with "RTNETLINK answers: Operation not permitted". The issue is only visible when running tests with non-root users (i.e. only relying on ambient capabilities). Indeed, when configuring the network interface, the "ip" command is called, which may lead to the special handling of capabilities for the root user by execve(2). If root is the caller, then the inherited, permitted and effective capabilities are all reset, which then includes CAP_NET_ADMIN. However, if a non-root user is the caller, then ambient capabilities are masked by the inherited ones, which were explicitly dropped. To make execution deterministic whatever users are running the tests, set the noroot secure bit for each test, and set the inheritable and ambient capabilities to CAP_NET_ADMIN, the only capability that may be required after an execve(2). Factor out _effective_cap() into _change_cap(), and use it to manage ambient capabilities with the new set_ambient_cap() and clear_ambient_cap() helpers. This makes it possible to run all Landlock tests with check-linux.sh from https://github.com/landlock-lsm/landlock-test-tools Cc: Konstantin Meskhidze Fixes: a549d055a22e ("selftests/landlock: Add network tests") Link: https://lore.kernel.org/r/20240125153230.3817165-2-mic@digikod.net [mic: Make sure SECBIT_NOROOT_LOCKED is set] Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/common.h | 48 +++++++++++++++++---- tools/testing/selftests/landlock/net_test.c | 5 ++- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h index 5b79758cae62..e64bbdf0e86e 100644 --- a/tools/testing/selftests/landlock/common.h +++ b/tools/testing/selftests/landlock/common.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -115,11 +116,16 @@ static void _init_caps(struct __test_metadata *const _metadata, bool drop_all) /* clang-format off */ CAP_DAC_OVERRIDE, CAP_MKNOD, + CAP_NET_ADMIN, + CAP_NET_BIND_SERVICE, CAP_SYS_ADMIN, CAP_SYS_CHROOT, - CAP_NET_BIND_SERVICE, /* clang-format on */ }; + const unsigned int noroot = SECBIT_NOROOT | SECBIT_NOROOT_LOCKED; + + if ((cap_get_secbits() & noroot) != noroot) + EXPECT_EQ(0, cap_set_secbits(noroot)); cap_p = cap_get_proc(); EXPECT_NE(NULL, cap_p) @@ -137,6 +143,8 @@ static void _init_caps(struct __test_metadata *const _metadata, bool drop_all) TH_LOG("Failed to cap_set_flag: %s", strerror(errno)); } } + + /* Automatically resets ambient capabilities. */ EXPECT_NE(-1, cap_set_proc(cap_p)) { TH_LOG("Failed to cap_set_proc: %s", strerror(errno)); @@ -145,6 +153,9 @@ static void _init_caps(struct __test_metadata *const _metadata, bool drop_all) { TH_LOG("Failed to cap_free: %s", strerror(errno)); } + + /* Quickly checks that ambient capabilities are cleared. */ + EXPECT_NE(-1, cap_get_ambient(caps[0])); } /* We cannot put such helpers in a library because of kselftest_harness.h . */ @@ -158,8 +169,9 @@ static void __maybe_unused drop_caps(struct __test_metadata *const _metadata) _init_caps(_metadata, true); } -static void _effective_cap(struct __test_metadata *const _metadata, - const cap_value_t caps, const cap_flag_value_t value) +static void _change_cap(struct __test_metadata *const _metadata, + const cap_flag_t flag, const cap_value_t cap, + const cap_flag_value_t value) { cap_t cap_p; @@ -168,7 +180,7 @@ static void _effective_cap(struct __test_metadata *const _metadata, { TH_LOG("Failed to cap_get_proc: %s", strerror(errno)); } - EXPECT_NE(-1, cap_set_flag(cap_p, CAP_EFFECTIVE, 1, &caps, value)) + EXPECT_NE(-1, cap_set_flag(cap_p, flag, 1, &cap, value)) { TH_LOG("Failed to cap_set_flag: %s", strerror(errno)); } @@ -183,15 +195,35 @@ static void _effective_cap(struct __test_metadata *const _metadata, } static void __maybe_unused set_cap(struct __test_metadata *const _metadata, - const cap_value_t caps) + const cap_value_t cap) { - _effective_cap(_metadata, caps, CAP_SET); + _change_cap(_metadata, CAP_EFFECTIVE, cap, CAP_SET); } static void __maybe_unused clear_cap(struct __test_metadata *const _metadata, - const cap_value_t caps) + const cap_value_t cap) { - _effective_cap(_metadata, caps, CAP_CLEAR); + _change_cap(_metadata, CAP_EFFECTIVE, cap, CAP_CLEAR); +} + +static void __maybe_unused +set_ambient_cap(struct __test_metadata *const _metadata, const cap_value_t cap) +{ + _change_cap(_metadata, CAP_INHERITABLE, cap, CAP_SET); + + EXPECT_NE(-1, cap_set_ambient(cap, CAP_SET)) + { + TH_LOG("Failed to set ambient capability %d: %s", cap, + strerror(errno)); + } +} + +static void __maybe_unused clear_ambient_cap( + struct __test_metadata *const _metadata, const cap_value_t cap) +{ + EXPECT_EQ(1, cap_get_ambient(cap)); + _change_cap(_metadata, CAP_INHERITABLE, cap, CAP_CLEAR); + EXPECT_EQ(0, cap_get_ambient(cap)); } /* Receives an FD from a UNIX socket. Returns the received FD, or -errno. */ diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c index efcde123af1f..936cfc879f1d 100644 --- a/tools/testing/selftests/landlock/net_test.c +++ b/tools/testing/selftests/landlock/net_test.c @@ -107,8 +107,11 @@ static void setup_loopback(struct __test_metadata *const _metadata) { set_cap(_metadata, CAP_SYS_ADMIN); ASSERT_EQ(0, unshare(CLONE_NEWNET)); - ASSERT_EQ(0, system("ip link set dev lo up")); clear_cap(_metadata, CAP_SYS_ADMIN); + + set_ambient_cap(_metadata, CAP_NET_ADMIN); + ASSERT_EQ(0, system("ip link set dev lo up")); + clear_ambient_cap(_metadata, CAP_NET_ADMIN); } static bool is_restricted(const struct protocol_variant *const prot,