diff mbox

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

Message ID 1352749698-1219-2-git-send-email-otubo@linux.vnet.ibm.com
State New
Headers show

Commit Message

Eduardo Otubo Nov. 12, 2012, 7:48 p.m. UTC
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(-)

Comments

Andreas Färber Nov. 21, 2012, 3:20 p.m. UTC | #1
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. UTC | #2
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. UTC | #3
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.
diff mbox

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);