* [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
* [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 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 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
* 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.