v2: - check for CONFIG_USER_NS - add review - fix Cc list v1: https://lore.kernel.org/lkml/20200710185156.2437687-1-keescook@chromium.org
Hi,
This fixes the seccomp selftests to pass (with SKIPs) for regular users.
I intend to put this in my for-next/seccomp tree (to avoid further conflicts with the kselftest tree).
(and for those following along, this is effectively based on the -next tree)
-Kees
Kees Cook (2): selftests/seccomp: Add SKIPs for failed unshare() selftests/seccomp: Set NNP for TSYNC ESRCH flag test
tools/testing/selftests/seccomp/config | 1 + tools/testing/selftests/seccomp/seccomp_bpf.c | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-)
Running the seccomp tests as a regular user shouldn't just fail tests that require CAP_SYS_ADMIN (for getting a PID namespace). Instead, detect those cases and SKIP them. Additionally, gracefully SKIP missing CONFIG_USER_NS (and add to "config" since we'd prefer to actually test this case).
Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/seccomp/config | 1 + tools/testing/selftests/seccomp/seccomp_bpf.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/seccomp/config b/tools/testing/selftests/seccomp/config index db1e11b08c8a..64c19d8eba79 100644 --- a/tools/testing/selftests/seccomp/config +++ b/tools/testing/selftests/seccomp/config @@ -1,2 +1,3 @@ CONFIG_SECCOMP=y CONFIG_SECCOMP_FILTER=y +CONFIG_USER_NS=y diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index c0aa46ce14f6..14b038361549 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3439,7 +3439,10 @@ TEST(user_notification_child_pid_ns) struct seccomp_notif req = {}; struct seccomp_notif_resp resp = {};
- ASSERT_EQ(unshare(CLONE_NEWUSER | CLONE_NEWPID), 0); + ASSERT_EQ(unshare(CLONE_NEWUSER | CLONE_NEWPID), 0) { + if (errno == EINVAL) + SKIP(return, "kernel missing CLONE_NEWUSER support"); + };
listener = user_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER); @@ -3504,7 +3507,10 @@ TEST(user_notification_sibling_pid_ns) }
/* Create the sibling ns, and sibling in it. */ - ASSERT_EQ(unshare(CLONE_NEWPID), 0); + ASSERT_EQ(unshare(CLONE_NEWPID), 0) { + if (errno == EPERM) + SKIP(return, "CLONE_NEWPID requires CAP_SYS_ADMIN"); + } ASSERT_EQ(errno, 0);
pid2 = fork();
On Fri, Jul 10, 2020 at 04:01:06PM -0700, Kees Cook wrote:
Running the seccomp tests as a regular user shouldn't just fail tests that require CAP_SYS_ADMIN (for getting a PID namespace). Instead, detect those cases and SKIP them. Additionally, gracefully SKIP missing CONFIG_USER_NS (and add to "config" since we'd prefer to actually test this case).
Signed-off-by: Kees Cook keescook@chromium.org
Just a comment, otherwise: Acked-by: Christian Brauner christian.brauner@ubuntu.com
tools/testing/selftests/seccomp/config | 1 + tools/testing/selftests/seccomp/seccomp_bpf.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/seccomp/config b/tools/testing/selftests/seccomp/config index db1e11b08c8a..64c19d8eba79 100644 --- a/tools/testing/selftests/seccomp/config +++ b/tools/testing/selftests/seccomp/config @@ -1,2 +1,3 @@ CONFIG_SECCOMP=y CONFIG_SECCOMP_FILTER=y +CONFIG_USER_NS=y diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index c0aa46ce14f6..14b038361549 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3439,7 +3439,10 @@ TEST(user_notification_child_pid_ns) struct seccomp_notif req = {}; struct seccomp_notif_resp resp = {};
- ASSERT_EQ(unshare(CLONE_NEWUSER | CLONE_NEWPID), 0);
- ASSERT_EQ(unshare(CLONE_NEWUSER | CLONE_NEWPID), 0) {
if (errno == EINVAL)SKIP(return, "kernel missing CLONE_NEWUSER support");
That would be either CLONE_NEWUSER or CLONE_NEWPID, right? :) Maybe just do: "kernel misses necessary namespace support"
- };
listener = user_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER); @@ -3504,7 +3507,10 @@ TEST(user_notification_sibling_pid_ns) } /* Create the sibling ns, and sibling in it. */
- ASSERT_EQ(unshare(CLONE_NEWPID), 0);
- ASSERT_EQ(unshare(CLONE_NEWPID), 0) {
if (errno == EPERM)SKIP(return, "CLONE_NEWPID requires CAP_SYS_ADMIN");- } ASSERT_EQ(errno, 0);
pid2 = fork(); -- 2.25.1
On Fri, Jul 10, 2020 at 04:01:06PM -0700, Kees Cook wrote:
Running the seccomp tests as a regular user shouldn't just fail tests that require CAP_SYS_ADMIN (for getting a PID namespace). Instead, detect those cases and SKIP them. Additionally, gracefully SKIP missing CONFIG_USER_NS (and add to "config" since we'd prefer to actually test this case).
Signed-off-by: Kees Cook keescook@chromium.org
tools/testing/selftests/seccomp/config | 1 + tools/testing/selftests/seccomp/seccomp_bpf.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/seccomp/config b/tools/testing/selftests/seccomp/config index db1e11b08c8a..64c19d8eba79 100644 --- a/tools/testing/selftests/seccomp/config +++ b/tools/testing/selftests/seccomp/config @@ -1,2 +1,3 @@ CONFIG_SECCOMP=y CONFIG_SECCOMP_FILTER=y +CONFIG_USER_NS=y diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index c0aa46ce14f6..14b038361549 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3439,7 +3439,10 @@ TEST(user_notification_child_pid_ns) struct seccomp_notif req = {}; struct seccomp_notif_resp resp = {};
- ASSERT_EQ(unshare(CLONE_NEWUSER | CLONE_NEWPID), 0);
- ASSERT_EQ(unshare(CLONE_NEWUSER | CLONE_NEWPID), 0) {
if (errno == EINVAL)SKIP(return, "kernel missing CLONE_NEWUSER support");- };
listener = user_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER); @@ -3504,7 +3507,10 @@ TEST(user_notification_sibling_pid_ns) } /* Create the sibling ns, and sibling in it. */
- ASSERT_EQ(unshare(CLONE_NEWPID), 0);
- ASSERT_EQ(unshare(CLONE_NEWPID), 0) {
if (errno == EPERM)SKIP(return, "CLONE_NEWPID requires CAP_SYS_ADMIN");- } ASSERT_EQ(errno, 0);
For this one, I think we can just put an unshare(CLONE_NEWUSER) at the top so the test still runs. This seems works for me unprivileged:
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 252140a52553..65e3642539f9 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3482,6 +3482,11 @@ TEST(user_notification_sibling_pid_ns) TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!"); }
+ ASSERT_EQ(unshare(CLONE_NEWUSER), 0) { + if (errno == EINVAL) + SKIP(return, "kernel missing CLONE_NEWUSER support"); + }; + listener = user_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER); ASSERT_GE(listener, 0);
The TSYNC ESRCH flag test will fail for regular users because NNP was not set yet. Add NNP setting.
Fixes: 51891498f2da ("seccomp: allow TSYNC and USER_NOTIF together") Cc: stable@vger.kernel.org Reviewed-by: Tycho Andersen tycho@tycho.ws Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/seccomp/seccomp_bpf.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 14b038361549..0d29114123fa 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3257,6 +3257,11 @@ TEST(user_notification_with_tsync) int ret; unsigned int flags;
+ ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); + ASSERT_EQ(0, ret) { + TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!"); + } + /* these were exclusive */ flags = SECCOMP_FILTER_FLAG_NEW_LISTENER | SECCOMP_FILTER_FLAG_TSYNC;
On Fri, Jul 10, 2020 at 04:01:07PM -0700, Kees Cook wrote:
The TSYNC ESRCH flag test will fail for regular users because NNP was not set yet. Add NNP setting.
Fixes: 51891498f2da ("seccomp: allow TSYNC and USER_NOTIF together") Cc: stable@vger.kernel.org Reviewed-by: Tycho Andersen tycho@tycho.ws Signed-off-by: Kees Cook keescook@chromium.org
Acked-by: Christian Brauner christian.brauner@ubuntu.com
linux-kselftest-mirror@lists.linaro.org
-
Christian Brauner -
Kees Cook -
Tycho Andersen