| Message ID | 20200710230107.2528890-2-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | d7d2e5bb9f361dc05c7dedd38d4d9269ad8c05e2 |
| Headers | show |
| Series | selftests/seccomp: SKIP tests requiring root | expand |
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);
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();
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(-)