* [PATCH for-next/seccomp v2 0/2] selftests/seccomp: SKIP tests requiring root @ 2020-07-10 23:01 Kees Cook 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 1/2] selftests/seccomp: Add SKIPs for failed unshare() Kees Cook 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 2/2] selftests/seccomp: Set NNP for TSYNC ESRCH flag test Kees Cook 0 siblings, 2 replies; 6+ messages in thread From: Kees Cook @ 2020-07-10 23:01 UTC (permalink / raw) To: Will Deacon Cc: Kees Cook, Tycho Andersen, Christian Brauner, Shuah Khan, Andy Lutomirski, Will Drewry, linux-kselftest, linux-kernel 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(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH for-next/seccomp v2 1/2] selftests/seccomp: Add SKIPs for failed unshare() 2020-07-10 23:01 [PATCH for-next/seccomp v2 0/2] selftests/seccomp: SKIP tests requiring root Kees Cook @ 2020-07-10 23:01 ` Kees Cook 2020-07-11 16:01 ` Christian Brauner 2020-07-11 16:05 ` Tycho Andersen 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 2/2] selftests/seccomp: Set NNP for TSYNC ESRCH flag test Kees Cook 1 sibling, 2 replies; 6+ messages in thread From: Kees Cook @ 2020-07-10 23:01 UTC (permalink / raw) To: Will Deacon Cc: Kees Cook, Tycho Andersen, Christian Brauner, Shuah Khan, Andy Lutomirski, Will Drewry, linux-kselftest, linux-kernel 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(); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH for-next/seccomp v2 1/2] selftests/seccomp: Add SKIPs for failed unshare() 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 1/2] selftests/seccomp: Add SKIPs for failed unshare() Kees Cook @ 2020-07-11 16:01 ` Christian Brauner 2020-07-11 16:05 ` Tycho Andersen 1 sibling, 0 replies; 6+ messages in thread From: Christian Brauner @ 2020-07-11 16:01 UTC (permalink / raw) To: Kees Cook Cc: Will Deacon, Tycho Andersen, Shuah Khan, Andy Lutomirski, Will Drewry, linux-kselftest, linux-kernel 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-next/seccomp v2 1/2] selftests/seccomp: Add SKIPs for failed unshare() 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 1/2] selftests/seccomp: Add SKIPs for failed unshare() Kees Cook 2020-07-11 16:01 ` Christian Brauner @ 2020-07-11 16:05 ` Tycho Andersen 1 sibling, 0 replies; 6+ messages in thread From: Tycho Andersen @ 2020-07-11 16:05 UTC (permalink / raw) To: Kees Cook Cc: Will Deacon, Christian Brauner, Shuah Khan, Andy Lutomirski, Will Drewry, linux-kselftest, linux-kernel 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); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH for-next/seccomp v2 2/2] selftests/seccomp: Set NNP for TSYNC ESRCH flag test 2020-07-10 23:01 [PATCH for-next/seccomp v2 0/2] selftests/seccomp: SKIP tests requiring root Kees Cook 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 1/2] selftests/seccomp: Add SKIPs for failed unshare() Kees Cook @ 2020-07-10 23:01 ` Kees Cook 2020-07-11 16:01 ` Christian Brauner 1 sibling, 1 reply; 6+ messages in thread From: Kees Cook @ 2020-07-10 23:01 UTC (permalink / raw) To: Will Deacon Cc: Kees Cook, stable, Tycho Andersen, Christian Brauner, Shuah Khan, Andy Lutomirski, Will Drewry, linux-kselftest, linux-kernel 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; -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH for-next/seccomp v2 2/2] selftests/seccomp: Set NNP for TSYNC ESRCH flag test 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 2/2] selftests/seccomp: Set NNP for TSYNC ESRCH flag test Kees Cook @ 2020-07-11 16:01 ` Christian Brauner 0 siblings, 0 replies; 6+ messages in thread From: Christian Brauner @ 2020-07-11 16:01 UTC (permalink / raw) To: Kees Cook Cc: Will Deacon, stable, Tycho Andersen, Shuah Khan, Andy Lutomirski, Will Drewry, linux-kselftest, linux-kernel 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> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-11 16:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-07-10 23:01 [PATCH for-next/seccomp v2 0/2] selftests/seccomp: SKIP tests requiring root Kees Cook 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 1/2] selftests/seccomp: Add SKIPs for failed unshare() Kees Cook 2020-07-11 16:01 ` Christian Brauner 2020-07-11 16:05 ` Tycho Andersen 2020-07-10 23:01 ` [PATCH for-next/seccomp v2 2/2] selftests/seccomp: Set NNP for TSYNC ESRCH flag test Kees Cook 2020-07-11 16:01 ` Christian Brauner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox