From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Eduardo Otubo" <otubo@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>
Subject: [PATCH 3/5] seccomp: fix blocking of process spawning
Date: Mon, 2 Aug 2021 14:03:01 +0100 [thread overview]
Message-ID: <20210802130303.3300108-4-berrange@redhat.com> (raw)
In-Reply-To: <20210802130303.3300108-1-berrange@redhat.com>
When '-sandbox on,spawn=deny' is given, we are supposed to block the
ability to spawn processes. We naively blocked the 'fork' syscall,
forgetting that any modern libc will use the 'clone' syscall instead.
We can't simply block the 'clone' syscall though, as that will break
thread creation. We thus list the set of flags used to create threads
and block anything that doesn't match this exactly.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
softmmu/qemu-seccomp.c | 101 ++++++++++++++++++++++++++++++++++++++
tests/unit/test-seccomp.c | 3 +-
2 files changed, 102 insertions(+), 2 deletions(-)
diff --git a/softmmu/qemu-seccomp.c b/softmmu/qemu-seccomp.c
index 9f6df8d033..57139cc9ce 100644
--- a/softmmu/qemu-seccomp.c
+++ b/softmmu/qemu-seccomp.c
@@ -46,6 +46,82 @@ const struct scmp_arg_cmp sched_setscheduler_arg[] = {
{ .arg = 1, .op = SCMP_CMP_NE, .datum_a = SCHED_IDLE }
};
+/*
+ * See 'NOTES' in 'man 2 clone' - s390 & cross have 'flags' in
+ * different position to other architectures
+ */
+#if defined(HOST_S390X) || defined(HOST_S390) || defined(HOST_CRIS)
+#define CLONE_FLAGS_ARG 1
+#else
+#define CLONE_FLAGS_ARG 0
+#endif
+
+#ifndef CLONE_PIDFD
+# define CLONE_PIDFD 0x00001000
+#endif
+
+#define REQUIRE_CLONE_FLAG(flag) \
+ const struct scmp_arg_cmp clone_arg ## flag[] = { \
+ { .arg = CLONE_FLAGS_ARG, \
+ .op = SCMP_CMP_MASKED_EQ, \
+ .datum_a = flag, .datum_b = 0 } }
+
+#define FORBID_CLONE_FLAG(flag) \
+ const struct scmp_arg_cmp clone_arg ## flag[] = { \
+ { .arg = CLONE_FLAGS_ARG, \
+ .op = SCMP_CMP_MASKED_EQ, \
+ .datum_a = flag, .datum_b = flag } }
+
+#define RULE_CLONE_FLAG(flag) \
+ { SCMP_SYS(clone), QEMU_SECCOMP_SET_SPAWN, \
+ ARRAY_SIZE(clone_arg ## flag), clone_arg ## flag, SCMP_ACT_TRAP }
+
+/* If no CLONE_* flags are set, except CSIGNAL, deny */
+const struct scmp_arg_cmp clone_arg_none[] = {
+ { .arg = CLONE_FLAGS_ARG,
+ .op = SCMP_CMP_MASKED_EQ,
+ .datum_a = ~(CSIGNAL), .datum_b = 0 }
+};
+
+/*
+ * pthread_create should always set all of these.
+ */
+REQUIRE_CLONE_FLAG(CLONE_VM);
+REQUIRE_CLONE_FLAG(CLONE_FS);
+REQUIRE_CLONE_FLAG(CLONE_FILES);
+REQUIRE_CLONE_FLAG(CLONE_SIGHAND);
+REQUIRE_CLONE_FLAG(CLONE_THREAD);
+REQUIRE_CLONE_FLAG(CLONE_SYSVSEM);
+REQUIRE_CLONE_FLAG(CLONE_SETTLS);
+REQUIRE_CLONE_FLAG(CLONE_PARENT_SETTID);
+REQUIRE_CLONE_FLAG(CLONE_CHILD_CLEARTID);
+/*
+ * Musl sets this in pthread_create too, but it is
+ * obsolete and harmless since its behaviour is
+ * subsumed under CLONE_THREAD
+ */
+/*REQUIRE_CLONE_FLAG(CLONE_DETACHED);*/
+
+
+/*
+ * These all indicate an attempt to spawn a process
+ * instead of a thread, or other undesirable scenarios
+ */
+FORBID_CLONE_FLAG(CLONE_PIDFD);
+FORBID_CLONE_FLAG(CLONE_PTRACE);
+FORBID_CLONE_FLAG(CLONE_VFORK);
+FORBID_CLONE_FLAG(CLONE_PARENT);
+FORBID_CLONE_FLAG(CLONE_NEWNS);
+FORBID_CLONE_FLAG(CLONE_UNTRACED);
+FORBID_CLONE_FLAG(CLONE_NEWCGROUP);
+FORBID_CLONE_FLAG(CLONE_NEWUTS);
+FORBID_CLONE_FLAG(CLONE_NEWIPC);
+FORBID_CLONE_FLAG(CLONE_NEWUSER);
+FORBID_CLONE_FLAG(CLONE_NEWPID);
+FORBID_CLONE_FLAG(CLONE_NEWNET);
+FORBID_CLONE_FLAG(CLONE_IO);
+
+
static const struct QemuSeccompSyscall denylist[] = {
/* default set of syscalls that should get blocked */
{ SCMP_SYS(reboot), QEMU_SECCOMP_SET_DEFAULT,
@@ -143,6 +219,31 @@ static const struct QemuSeccompSyscall denylist[] = {
0, NULL, SCMP_ACT_TRAP },
{ SCMP_SYS(execve), QEMU_SECCOMP_SET_SPAWN,
0, NULL, SCMP_ACT_TRAP },
+ { SCMP_SYS(clone), QEMU_SECCOMP_SET_SPAWN,
+ ARRAY_SIZE(clone_arg_none), clone_arg_none, SCMP_ACT_TRAP },
+ RULE_CLONE_FLAG(CLONE_VM),
+ RULE_CLONE_FLAG(CLONE_FS),
+ RULE_CLONE_FLAG(CLONE_FILES),
+ RULE_CLONE_FLAG(CLONE_SIGHAND),
+ RULE_CLONE_FLAG(CLONE_THREAD),
+ RULE_CLONE_FLAG(CLONE_SYSVSEM),
+ RULE_CLONE_FLAG(CLONE_SETTLS),
+ RULE_CLONE_FLAG(CLONE_PARENT_SETTID),
+ RULE_CLONE_FLAG(CLONE_CHILD_CLEARTID),
+ /*RULE_CLONE_FLAG(CLONE_DETACHED),*/
+ RULE_CLONE_FLAG(CLONE_PIDFD),
+ RULE_CLONE_FLAG(CLONE_PTRACE),
+ RULE_CLONE_FLAG(CLONE_VFORK),
+ RULE_CLONE_FLAG(CLONE_PARENT),
+ RULE_CLONE_FLAG(CLONE_NEWNS),
+ RULE_CLONE_FLAG(CLONE_UNTRACED),
+ RULE_CLONE_FLAG(CLONE_NEWCGROUP),
+ RULE_CLONE_FLAG(CLONE_NEWUTS),
+ RULE_CLONE_FLAG(CLONE_NEWIPC),
+ RULE_CLONE_FLAG(CLONE_NEWUSER),
+ RULE_CLONE_FLAG(CLONE_NEWPID),
+ RULE_CLONE_FLAG(CLONE_NEWNET),
+ RULE_CLONE_FLAG(CLONE_IO),
/* resource control */
{ SCMP_SYS(setpriority), QEMU_SECCOMP_SET_RESOURCECTL,
0, NULL, SCMP_ACT_ERRNO(EPERM) },
diff --git a/tests/unit/test-seccomp.c b/tests/unit/test-seccomp.c
index 10ab3e8fe5..3d7771e46c 100644
--- a/tests/unit/test-seccomp.c
+++ b/tests/unit/test-seccomp.c
@@ -126,8 +126,7 @@ static int doit_fork(void)
static void test_seccomp_fork_on_nospawn(void)
{
- /* XXX fixme - should be killed */
- test_seccomp_passed("on,spawn=deny", doit_fork);
+ test_seccomp_killed("on,spawn=deny", doit_fork);
}
static void test_seccomp_fork_on(void)
--
2.31.1
next prev parent reply other threads:[~2021-08-02 13:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-02 13:02 [PATCH 0/5] seccomp: fix hole in blocking forks Daniel P. Berrangé
2021-08-02 13:02 ` [PATCH 1/5] seccomp: allow action to be customized per syscall Daniel P. Berrangé
2021-08-02 13:03 ` [PATCH 2/5] seccomp: add unit test for seccomp filtering Daniel P. Berrangé
2021-08-02 13:03 ` Daniel P. Berrangé [this message]
2021-08-02 13:03 ` [PATCH 4/5] seccomp: block use of clone3 syscall Daniel P. Berrangé
2021-08-02 13:03 ` [PATCH 5/5] seccomp: block setns, unshare and execveat syscalls Daniel P. Berrangé
2021-08-04 8:05 ` [PATCH 0/5] seccomp: fix hole in blocking forks Eduardo Terrell Ferrari Otubo
2022-01-28 15:42 ` Daniel P. Berrangé
2022-01-31 9:07 ` Eduardo Otubo
2022-01-31 9:09 ` Daniel P. Berrangé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210802130303.3300108-4-berrange@redhat.com \
--to=berrange@redhat.com \
--cc=otubo@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).