[Qemu-devel] [PULL 00/04] seccomp branch queue

[Qemu-devel] [PULL 00/04] seccomp branch queue

[Eduardo Otubo]

The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13 10:42:06 +0100)

are available in the git repository at:

  git://github.com/otubo/qemu.git tags/pull-seccomp-20151015

for you to fetch changes up to d8e4d1ef78093037fc6483561aee3aaedebbaa37:

  Add syscalls for -runas and -chroot to the seccomp sandbox (2015-10-15 09:41:15 +0200)

----------------------------------------------------------------
seccomp branch queue

----------------------------------------------------------------
Andrew Jones (2):
      libseccomp: add cacheflush to whitelist
      configure: arm/aarch64: allow enable-seccomp

Namsun Ch'o (2):
      Add argument filters to the seccomp sandbox
      Add syscalls for -runas and -chroot to the seccomp sandbox

 configure      | 32 +++++++++++++++++++++++++-------
 qemu-seccomp.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 86 insertions(+), 11 deletions(-)

-- 
2.1.4

[Qemu-devel] [PULL 01/04] libseccomp: add cacheflush to whitelist

[Eduardo Otubo]

From: Andrew Jones <drjones@redhat.com>

cacheflush is an arm-specific syscall that qemu built for arm
uses. Add it to the whitelist.

Signed-off-by: Andrew Jones <drjones@redhat.com>
Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
 qemu-seccomp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index f9de0d3..33644a4 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -237,7 +237,8 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(fadvise64), 240 },
     { SCMP_SYS(inotify_init1), 240 },
     { SCMP_SYS(inotify_add_watch), 240 },
-    { SCMP_SYS(mbind), 240 }
+    { SCMP_SYS(mbind), 240 },
+    { SCMP_SYS(cacheflush), 240 },
 };
 
 int seccomp_start(void)
-- 
2.1.4

[Qemu-devel] [PULL 02/04] configure: arm/aarch64: allow enable-seccomp

[Eduardo Otubo]

From: Andrew Jones <drjones@redhat.com>

This is a revert of ae6e8ef11e6cb, but with a bit of refactoring,
and also specifically adding arm/aarch64, rather than all
architectures. Currently, libseccomp code appears to also support
mips, ppc, and s390. We could therefore allow qemu to enable
seccomp for those platforms as well, with additional configure
patches, given they're tested and proven to work.

Signed-off-by: Andrew Jones <drjones@redhat.com>
Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
 configure | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index f08327e..7d5aab2 100755
--- a/configure
+++ b/configure
@@ -1876,16 +1876,34 @@ fi
 # libseccomp check
 
 if test "$seccomp" != "no" ; then
-    if test "$cpu" = "i386" || test "$cpu" = "x86_64" &&
-        $pkg_config --atleast-version=2.1.1 libseccomp; then
+    case "$cpu" in
+    i386|x86_64)
+        libseccomp_minver="2.1.1"
+        ;;
+    arm|aarch64)
+        libseccomp_minver="2.2.3"
+        ;;
+    *)
+        libseccomp_minver=""
+        ;;
+    esac
+
+    if test "$libseccomp_minver" != "" &&
+       $pkg_config --atleast-version=$libseccomp_minver libseccomp ; then
         libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
         QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
-	seccomp="yes"
+        seccomp="yes"
     else
-	if test "$seccomp" = "yes"; then
-            feature_not_found "libseccomp" "Install libseccomp devel >= 2.1.1"
-	fi
-	seccomp="no"
+        if test "$seccomp" = "yes" ; then
+            if test "$libseccomp_minver" != "" ; then
+                feature_not_found "libseccomp" \
+                    "Install libseccomp devel >= $libseccomp_minver"
+            else
+                feature_not_found "libseccomp" \
+                    "libseccomp is not supported for host cpu $cpu"
+            fi
+        fi
+        seccomp="no"
     fi
 fi
 ##########################################
-- 
2.1.4

[Qemu-devel] [PULL 03/04] Add argument filters to the seccomp sandbox

[Eduardo Otubo]

From: Namsun Ch'o <namnamc@Safe-mail.net>

Here's the v3 patch. I applied it and compiled QEMU, and it worked fine.

Changes so far:
v1
 - Created argument filters for the madvise, shmget, and shmctl syscalls.
v1 -> v2
 - Added 5 new madvise flags which were present in the source code but not in
   the strace which I generated.
 - Added IP_CREAT|0600 to shmget, which Daniel Berrange pointed out was
   present in GTK2, which QEMU uses but does not call directly.
v2 -> v3
 - Replaced include asm/mman-common.h with sys/mman.h which is more proper.
 - Fixed a stupid typo where I had IP_CREAT instead of IPC_CREAT.
 - Removed the comma on the last entry of the madvise_flags array.
 - Removed one madvise flag (MADV_INVALID) which doesn't exist, apparently.

Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
 qemu-seccomp.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 3 deletions(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 33644a4..e7a54e8 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -14,6 +14,8 @@
  */
 #include <stdio.h>
 #include <seccomp.h>
+#include <linux/ipc.h>
+#include <sys/mman.h>
 #include "sysemu/seccomp.h"
 
 struct QemuSeccompSyscall {
@@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(rt_sigreturn), 245 },
     { SCMP_SYS(sync), 245 },
     { SCMP_SYS(pread64), 245 },
-    { SCMP_SYS(madvise), 245 },
     { SCMP_SYS(set_robust_list), 245 },
     { SCMP_SYS(lseek), 245 },
     { SCMP_SYS(pselect6), 245 },
@@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(arch_prctl), 240 },
     { SCMP_SYS(mkdir), 240 },
     { SCMP_SYS(fchmod), 240 },
-    { SCMP_SYS(shmget), 240 },
     { SCMP_SYS(shmat), 240 },
     { SCMP_SYS(shmdt), 240 },
     { SCMP_SYS(timerfd_create), 240 },
-    { SCMP_SYS(shmctl), 240 },
     { SCMP_SYS(mlockall), 240 },
     { SCMP_SYS(mlock), 240 },
     { SCMP_SYS(munlock), 240 },
@@ -265,6 +264,59 @@ int seccomp_start(void)
         }
     }
 
+    /* madvise */
+    static const int madvise_flags[] = {
+        MADV_DODUMP,
+        MADV_DONTDUMP,
+        MADV_UNMERGEABLE,
+        MADV_WILLNEED,
+        MADV_DONTFORK,
+        MADV_DONTNEED,
+        MADV_HUGEPAGE,
+        MADV_MERGEABLE
+    };
+    for (i = 0; i < ARRAY_SIZE(madvise_flags); i++) {
+        rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), 1,
+            SCMP_A2(SCMP_CMP_EQ, madvise_flags[i]));
+        if (rc < 0) {
+            goto seccomp_return;
+        }
+    }
+    rc = seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245);
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+
+    /* shmget */
+    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
+        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
+        SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0777));
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
+        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
+        SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0600));
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+    rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240);
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+
+    /* shmctl */
+    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2,
+        SCMP_A1(SCMP_CMP_EQ, IPC_RMID),
+        SCMP_A2(SCMP_CMP_EQ, 0));
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+    rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240);
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+
     rc = seccomp_load(ctx);
 
   seccomp_return:
-- 
2.1.4

[Qemu-devel] [PULL 04/04] Add syscalls for -runas and -chroot to the seccomp sandbox

[Eduardo Otubo]

From: Namsun Ch'o <namnamc@Safe-mail.net>

The seccomp sandbox doesn't whitelist setuid, setgid, or setgroups, which are
needed for -runas to work. It also doesn't whitelist chroot, which is needed
for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
privileges or chroots, so without these whitelisted, -runas and -chroot cause
QEMU to be killed with -sandbox on. This patch adds those syscalls.

Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
 qemu-seccomp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index e7a54e8..877fd88 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -238,6 +238,10 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(inotify_add_watch), 240 },
     { SCMP_SYS(mbind), 240 },
     { SCMP_SYS(cacheflush), 240 },
+    { SCMP_SYS(setuid), 240 },
+    { SCMP_SYS(setgid), 240 },
+    { SCMP_SYS(chroot), 240 },
+    { SCMP_SYS(setgroups), 240 },
 };
 
 int seccomp_start(void)
-- 
2.1.4

Re: [Qemu-devel] [PULL 00/04] seccomp branch queue

[Peter Maydell]

On 15 October 2015 at 08:57, Eduardo Otubo
<eduardo.otubo@profitbricks.com> wrote:
> The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13 10:42:06 +0100)
>
> are available in the git repository at:
>
>   git://github.com/otubo/qemu.git tags/pull-seccomp-20151015
>
> for you to fetch changes up to d8e4d1ef78093037fc6483561aee3aaedebbaa37:
>
>   Add syscalls for -runas and -chroot to the seccomp sandbox (2015-10-15 09:41:15 +0200)
>
> ----------------------------------------------------------------
> seccomp branch queue
>
> ----------------------------------------------------------------
> Andrew Jones (2):
>       libseccomp: add cacheflush to whitelist
>       configure: arm/aarch64: allow enable-seccomp
>
> Namsun Ch'o (2):
>       Add argument filters to the seccomp sandbox
>       Add syscalls for -runas and -chroot to the seccomp sandbox

The commit messages in this are a bit of a mess. In particular,
there's lots of chatty information about "I compiled QEMU and it worked fine"
and "changes between v2 and v3" information, which should all
go below the '---' divider so it doesn't appear in the final
commit logs. "Add argument filters to the seccomp sandbox" should
really also have a longer and more explanatory commit message
giving rationale for the change, and so on, since it's adding
a fair bit of new code.

Can you either clean them up or ask the submitters to do so,
please?

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/04] seccomp branch queue

[Eduardo Otubo]

[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]

On Fri, Oct 16, 2015 at 05=12=18PM +0100, Peter Maydell wrote:
> On 15 October 2015 at 08:57, Eduardo Otubo
> <eduardo.otubo@profitbricks.com> wrote:
> > The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:
> >
> >   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13 10:42:06 +0100)
> >
> > are available in the git repository at:
> >
> >   git://github.com/otubo/qemu.git tags/pull-seccomp-20151015
> >
> > for you to fetch changes up to d8e4d1ef78093037fc6483561aee3aaedebbaa37:
> >
> >   Add syscalls for -runas and -chroot to the seccomp sandbox (2015-10-15 09:41:15 +0200)
> >
> > ----------------------------------------------------------------
> > seccomp branch queue
> >
> > ----------------------------------------------------------------
> > Andrew Jones (2):
> >       libseccomp: add cacheflush to whitelist
> >       configure: arm/aarch64: allow enable-seccomp
> >
> > Namsun Ch'o (2):
> >       Add argument filters to the seccomp sandbox
> >       Add syscalls for -runas and -chroot to the seccomp sandbox
> 
> The commit messages in this are a bit of a mess. In particular,
> there's lots of chatty information about "I compiled QEMU and it worked fine"
> and "changes between v2 and v3" information, which should all
> go below the '---' divider so it doesn't appear in the final
> commit logs. "Add argument filters to the seccomp sandbox" should
> really also have a longer and more explanatory commit message
> giving rationale for the change, and so on, since it's adding
> a fair bit of new code.
> 
> Can you either clean them up or ask the submitters to do so,
> please?

I guess this is addressed to Namsun's patches.

Namsun, can you rewrite the commit messages according to Peter's
suggestions and resend?

Thanks,

> 
> thanks
> -- PMM

-- 
Eduardo Otubo
ProfitBricks GmbH

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PULL 00/04] seccomp branch queue

[Namsun Ch'o]

> I guess this is addressed to Namsun's patches.

> Namsun, can you rewrite the commit messages according to Peter's
> suggestions and resend?

Sure. I used a longer commit message, and put the chatty stuff below the 
---.
I posted it as a new top-level thread as a v3 patch.

[Qemu-devel] [PULL 00/04] seccomp branch queue

[Eduardo Otubo]

The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13 10:42:06 +0100)

are available in the git repository at:

  git://github.com/otubo/qemu.git tags/pull-seccomp-20151014

for you to fetch changes up to dcebff271f7134265e8c627088ccab86bfcd4867:

  Add syscalls for -runas and -chroot to the seccomp sandbox (2015-10-14 11:08:46 +0200)

----------------------------------------------------------------
seccomp branch queue

----------------------------------------------------------------
Andrew Jones (2):
      libseccomp: add cacheflush to whitelist
      configure: arm/aarch64: allow enable-seccomp

Namsun Ch'o (2):
      Add argument filters to the seccomp sandbox
      Add syscalls for -runas and -chroot to the seccomp sandbox

 configure      | 32 +++++++++++++++++++++++++-------
 qemu-seccomp.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 87 insertions(+), 11 deletions(-)


-- 
2.1.4