Patchwork [PATCHv3,2/5] seccomp: setting "-sandbox on" as deafult

login
register
mail settings
Submitter Eduardo Otubo
Date Nov. 12, 2012, 7:48 p.m.
Message ID <1352749698-1219-2-git-send-email-otubo@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/198445/
State New
Headers show

Comments

Eduardo Otubo - Nov. 12, 2012, 7:48 p.m.
Now the seccomp filter will be set to "on" even if no argument
"-sandbox" is given.

v3: * Introduced seccomp_states enum and new functions named
      seccomp_set_state() and seccomp_get_state()
     (pbonzini@redhat.com).
    * Merged seccomp_start() and install_seccomp_filter(),
      moved install_seccomp_filter() to qemu-seccomp.c,
      and renamed it.
    * Moved CONFIG_SECCOMP pre-processor checks from Makefile.objs
      to qemu-seccomp.c.
    * Replace qerror_report with fprintf(stderr, "..") in main()
      (lcapitulino@redhat.com).

Note: This support requires libseccomp.  If you don't have access
to libseccomp packages, you can manually build with the following
steps:

  1) git clone git://git.code.sf.net/p/libseccomp/libseccomp
  2) cd libseccomp
  3) ./configure
  4) make
  5) make install
  6) export PKG_CONFIG_PATH="/usr/local/lib/pkgconfig/"

Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
 Makefile.objs  |    2 --
 configure      |    2 +-
 qemu-seccomp.c |   26 ++++++++++++++++++++++++--
 qemu-seccomp.h |   13 +++++++++++--
 vl.c           |   31 ++++++++++++++++---------------
 5 files changed, 52 insertions(+), 22 deletions(-)
Andreas Färber - Nov. 21, 2012, 3:20 p.m.
Am 12.11.2012 20:48, schrieb Eduardo Otubo:
> Now the seccomp filter will be set to "on" even if no argument
> "-sandbox" is given.
> 
> v3: * Introduced seccomp_states enum and new functions named
>       seccomp_set_state() and seccomp_get_state()
>      (pbonzini@redhat.com).
>     * Merged seccomp_start() and install_seccomp_filter(),
>       moved install_seccomp_filter() to qemu-seccomp.c,
>       and renamed it.
>     * Moved CONFIG_SECCOMP pre-processor checks from Makefile.objs
>       to qemu-seccomp.c.
>     * Replace qerror_report with fprintf(stderr, "..") in main()
>       (lcapitulino@redhat.com).
> 
> Note: This support requires libseccomp.  If you don't have access
> to libseccomp packages, you can manually build with the following
> steps:
> 
>   1) git clone git://git.code.sf.net/p/libseccomp/libseccomp
>   2) cd libseccomp
>   3) ./configure
>   4) make
>   5) make install
>   6) export PKG_CONFIG_PATH="/usr/local/lib/pkgconfig/"

To my understanding libseccomp specifically filters Linux syscalls, no?
Are you positive that building and enabling this by default works with
mingw32, bsd, darwin, etc. and makes sense?

Regards,
Andreas
Anthony Liguori - Nov. 27, 2012, 7:01 p.m.
Eduardo Otubo <otubo@linux.vnet.ibm.com> writes:

> Now the seccomp filter will be set to "on" even if no argument
> "-sandbox" is given.
>
> v3: * Introduced seccomp_states enum and new functions named
>       seccomp_set_state() and seccomp_get_state()
>      (pbonzini@redhat.com).
>     * Merged seccomp_start() and install_seccomp_filter(),
>       moved install_seccomp_filter() to qemu-seccomp.c,
>       and renamed it.
>     * Moved CONFIG_SECCOMP pre-processor checks from Makefile.objs
>       to qemu-seccomp.c.
>     * Replace qerror_report with fprintf(stderr, "..") in main()
>       (lcapitulino@redhat.com).

There's no way this can go for 1.3.  Can we just do 1/5 to fix the
libvirt problem for 1.3, and leave the rest for 1.4?

Regards,

Anthony Liguori

>
> Note: This support requires libseccomp.  If you don't have access
> to libseccomp packages, you can manually build with the following
> steps:
>
>   1) git clone git://git.code.sf.net/p/libseccomp/libseccomp
>   2) cd libseccomp
>   3) ./configure
>   4) make
>   5) make install
>   6) export PKG_CONFIG_PATH="/usr/local/lib/pkgconfig/"
>
> Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  Makefile.objs  |    2 --
>  configure      |    2 +-
>  qemu-seccomp.c |   26 ++++++++++++++++++++++++--
>  qemu-seccomp.h |   13 +++++++++++--
>  vl.c           |   31 ++++++++++++++++---------------
>  5 files changed, 52 insertions(+), 22 deletions(-)
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 593a592..682b1e6 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -103,9 +103,7 @@ common-obj-$(CONFIG_SLIRP) += slirp/
>  
>  ######################################################################
>  # libseccomp
> -ifeq ($(CONFIG_SECCOMP),y)
>  common-obj-y += qemu-seccomp.o
> -endif
>  
>  ######################################################################
>  # libuser
> diff --git a/configure b/configure
> index 7290f50..d28f8d5 100755
> --- a/configure
> +++ b/configure
> @@ -221,7 +221,7 @@ guest_agent="yes"
>  want_tools="yes"
>  libiscsi=""
>  coroutine=""
> -seccomp=""
> +seccomp="yes"
>  glusterfs=""
>  
>  # parse CC options first
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index b06a2c6..2386996 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -12,10 +12,28 @@
>   * Contributions after 2012-01-13 are licensed under the terms of the
>   * GNU GPL, version 2 or (at your option) any later version.
>   */
> +#include "config-host.h"
>  #include <stdio.h>
> -#include <seccomp.h>
> +#include "osdep.h"
>  #include "qemu-seccomp.h"
>  
> +#ifdef CONFIG_SECCOMP
> +int seccomp_state = SECCOMP_ON;
> +#else
> +int seccomp_state = SECCOMP_OFF;
> +#endif
> +
> +void seccomp_set_state(int state)
> +{
> +    seccomp_state = state;
> +}
> +
> +int seccomp_get_state(void)
> +{
> +    return seccomp_state;
> +}
> +
> +#ifdef CONFIG_SECCOMP
>  struct QemuSeccompSyscall {
>      int32_t num;
>      uint8_t priority;
> @@ -223,15 +241,18 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
>      { SCMP_SYS(prlimit64), 241 },
>      { SCMP_SYS(waitid), 241 }
>  };
> +#endif
>  
> -int seccomp_start(void)
> +int seccomp_install_filter(void)
>  {
>      int rc = 0;
> +#ifdef CONFIG_SECCOMP
>      unsigned int i = 0;
>      scmp_filter_ctx ctx;
>  
>      ctx = seccomp_init(SCMP_ACT_KILL);
>      if (ctx == NULL) {
> +        rc = -1;
>          goto seccomp_return;
>      }
>  
> @@ -251,5 +272,6 @@ int seccomp_start(void)
>  
>    seccomp_return:
>      seccomp_release(ctx);
> +#endif
>      return rc;
>  }
> diff --git a/qemu-seccomp.h b/qemu-seccomp.h
> index b2fc3f8..fa26d70 100644
> --- a/qemu-seccomp.h
> +++ b/qemu-seccomp.h
> @@ -15,8 +15,17 @@
>  #ifndef QEMU_SECCOMP_H
>  #define QEMU_SECCOMP_H
>  
> +#ifdef CONFIG_SECCOMP
>  #include <seccomp.h>
> -#include "osdep.h"
> +#endif
> +
> +enum seccomp_states {
> +    SECCOMP_OFF,
> +    SECCOMP_ON
> +};
> +
> +void seccomp_set_state(int);
> +int seccomp_get_state(void);
> +int seccomp_install_filter(void);
>  
> -int seccomp_start(void);
>  #endif
> diff --git a/vl.c b/vl.c
> index 4f03a72..cb3d85e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -64,9 +64,7 @@
>  #include <linux/parport.h>
>  #endif
>  
> -#ifdef CONFIG_SECCOMP
>  #include "qemu-seccomp.h"
> -#endif
>  
>  #ifdef __sun__
>  #include <sys/stat.h>
> @@ -772,22 +770,17 @@ static int bt_parse(const char *opt)
>  
>  static int parse_sandbox(QemuOpts *opts, void *opaque)
>  {
> -    /* FIXME: change this to true for 1.3 */
> -    if (qemu_opt_get_bool(opts, "enable", false)) {
>  #ifdef CONFIG_SECCOMP
> -        if (seccomp_start() < 0) {
> -            qerror_report(ERROR_CLASS_GENERIC_ERROR,
> -                          "failed to install seccomp syscall filter in the kernel");
> -            return -1;
> -        }
> -#else
> -        qerror_report(ERROR_CLASS_GENERIC_ERROR,
> -                      "sandboxing request but seccomp is not compiled into this build");
> -        return -1;
> -#endif
> +    /* seccomp sandboxing is on by default */
> +    if (!qemu_opt_get_bool(opts, "enable", true)) {
> +        seccomp_set_state(SECCOMP_OFF);
>      }
> -
>      return 0;
> +#else
> +    fprintf(stderr, "sandbox option specified but seccomp is not compiled "
> +                    "into this build\n");
> +    return -1;
> +#endif
>  }
>  
>  /*********QEMU USB setting******/
> @@ -3489,6 +3482,14 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    if (seccomp_get_state() == SECCOMP_ON) {
> +        if (seccomp_install_filter() < 0) {
> +            fprintf(stderr, "failed to install seccomp syscall "
> +                            "initialization filter\n");
> +            exit(1);
> +        }
> +    }
> +
>  #ifndef _WIN32
>      if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) {
>          exit(1);
> -- 
> 1.7.10.4
Corey Bryant - Nov. 27, 2012, 7:07 p.m.
On 11/27/2012 02:01 PM, Anthony Liguori wrote:
> Eduardo Otubo <otubo@linux.vnet.ibm.com> writes:
>
>> Now the seccomp filter will be set to "on" even if no argument
>> "-sandbox" is given.
>>
>> v3: * Introduced seccomp_states enum and new functions named
>>        seccomp_set_state() and seccomp_get_state()
>>       (pbonzini@redhat.com).
>>      * Merged seccomp_start() and install_seccomp_filter(),
>>        moved install_seccomp_filter() to qemu-seccomp.c,
>>        and renamed it.
>>      * Moved CONFIG_SECCOMP pre-processor checks from Makefile.objs
>>        to qemu-seccomp.c.
>>      * Replace qerror_report with fprintf(stderr, "..") in main()
>>        (lcapitulino@redhat.com).
>
> There's no way this can go for 1.3.  Can we just do 1/5 to fix the
> libvirt problem for 1.3, and leave the rest for 1.4?
>
> Regards,
>
> Anthony Liguori
>

Unfortunately I think that's our only option at this point.  I believe 
we've determined the syscalls to add to patch 1/5 to fix the issue Paul 
was hitting.  (They are syscalls that are being called in spice server 
code.)  Eduardo will be submitting a new patch soon.

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 593a592..682b1e6 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -103,9 +103,7 @@  common-obj-$(CONFIG_SLIRP) += slirp/
 
 ######################################################################
 # libseccomp
-ifeq ($(CONFIG_SECCOMP),y)
 common-obj-y += qemu-seccomp.o
-endif
 
 ######################################################################
 # libuser
diff --git a/configure b/configure
index 7290f50..d28f8d5 100755
--- a/configure
+++ b/configure
@@ -221,7 +221,7 @@  guest_agent="yes"
 want_tools="yes"
 libiscsi=""
 coroutine=""
-seccomp=""
+seccomp="yes"
 glusterfs=""
 
 # parse CC options first
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index b06a2c6..2386996 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -12,10 +12,28 @@ 
  * Contributions after 2012-01-13 are licensed under the terms of the
  * GNU GPL, version 2 or (at your option) any later version.
  */
+#include "config-host.h"
 #include <stdio.h>
-#include <seccomp.h>
+#include "osdep.h"
 #include "qemu-seccomp.h"
 
+#ifdef CONFIG_SECCOMP
+int seccomp_state = SECCOMP_ON;
+#else
+int seccomp_state = SECCOMP_OFF;
+#endif
+
+void seccomp_set_state(int state)
+{
+    seccomp_state = state;
+}
+
+int seccomp_get_state(void)
+{
+    return seccomp_state;
+}
+
+#ifdef CONFIG_SECCOMP
 struct QemuSeccompSyscall {
     int32_t num;
     uint8_t priority;
@@ -223,15 +241,18 @@  static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(prlimit64), 241 },
     { SCMP_SYS(waitid), 241 }
 };
+#endif
 
-int seccomp_start(void)
+int seccomp_install_filter(void)
 {
     int rc = 0;
+#ifdef CONFIG_SECCOMP
     unsigned int i = 0;
     scmp_filter_ctx ctx;
 
     ctx = seccomp_init(SCMP_ACT_KILL);
     if (ctx == NULL) {
+        rc = -1;
         goto seccomp_return;
     }
 
@@ -251,5 +272,6 @@  int seccomp_start(void)
 
   seccomp_return:
     seccomp_release(ctx);
+#endif
     return rc;
 }
diff --git a/qemu-seccomp.h b/qemu-seccomp.h
index b2fc3f8..fa26d70 100644
--- a/qemu-seccomp.h
+++ b/qemu-seccomp.h
@@ -15,8 +15,17 @@ 
 #ifndef QEMU_SECCOMP_H
 #define QEMU_SECCOMP_H
 
+#ifdef CONFIG_SECCOMP
 #include <seccomp.h>
-#include "osdep.h"
+#endif
+
+enum seccomp_states {
+    SECCOMP_OFF,
+    SECCOMP_ON
+};
+
+void seccomp_set_state(int);
+int seccomp_get_state(void);
+int seccomp_install_filter(void);
 
-int seccomp_start(void);
 #endif
diff --git a/vl.c b/vl.c
index 4f03a72..cb3d85e 100644
--- a/vl.c
+++ b/vl.c
@@ -64,9 +64,7 @@ 
 #include <linux/parport.h>
 #endif
 
-#ifdef CONFIG_SECCOMP
 #include "qemu-seccomp.h"
-#endif
 
 #ifdef __sun__
 #include <sys/stat.h>
@@ -772,22 +770,17 @@  static int bt_parse(const char *opt)
 
 static int parse_sandbox(QemuOpts *opts, void *opaque)
 {
-    /* FIXME: change this to true for 1.3 */
-    if (qemu_opt_get_bool(opts, "enable", false)) {
 #ifdef CONFIG_SECCOMP
-        if (seccomp_start() < 0) {
-            qerror_report(ERROR_CLASS_GENERIC_ERROR,
-                          "failed to install seccomp syscall filter in the kernel");
-            return -1;
-        }
-#else
-        qerror_report(ERROR_CLASS_GENERIC_ERROR,
-                      "sandboxing request but seccomp is not compiled into this build");
-        return -1;
-#endif
+    /* seccomp sandboxing is on by default */
+    if (!qemu_opt_get_bool(opts, "enable", true)) {
+        seccomp_set_state(SECCOMP_OFF);
     }
-
     return 0;
+#else
+    fprintf(stderr, "sandbox option specified but seccomp is not compiled "
+                    "into this build\n");
+    return -1;
+#endif
 }
 
 /*********QEMU USB setting******/
@@ -3489,6 +3482,14 @@  int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (seccomp_get_state() == SECCOMP_ON) {
+        if (seccomp_install_filter() < 0) {
+            fprintf(stderr, "failed to install seccomp syscall "
+                            "initialization filter\n");
+            exit(1);
+        }
+    }
+
 #ifndef _WIN32
     if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) {
         exit(1);