[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v0] qemu: Add sandbox support.





On 09/06/2012 02:27 AM, Daniel Veillard wrote:
On Mon, Sep 03, 2012 at 02:03:39PM +0200, Ján Tomko wrote:
QEMU (since 1.2-rc0) supports setting up a syscall whitelist through
libseccomp on linux kernel from 3.5-rc1. This is enabled by specifying
-sandbox on on qemu command line.

This patch detects this capability by searching for -sandbox in qemu
help output and runs qemu with -sandbox on if sandbox is set to non-zero
in qemu.conf.

---
Should this option be in qemu.conf, or would it be better to set it
per-domain in the XML?
---
  src/qemu/qemu.conf           |    6 ++++++
  src/qemu/qemu_capabilities.c |    3 +++
  src/qemu/qemu_capabilities.h |    1 +
  src/qemu/qemu_command.c      |    3 +++
  src/qemu/qemu_conf.c         |    5 +++++
  src/qemu/qemu_conf.h         |    1 +
  6 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index d3175fa..47e510e 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -375,3 +375,9 @@
  #
  #keepalive_interval = 5
  #keepalive_count = 5
+
+
+
+# Enable this to use seccomp syscall whitelisting in QEMU.
+#
+#sandbox = 1
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ba7956..b0728e8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -176,6 +176,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
                "disable-s3",

                "disable-s4", /* 105 */
+              "sandbox"
      );

  struct qemu_feature_flags {
@@ -1139,6 +1140,8 @@ qemuCapsComputeCmdFlags(const char *help,
      }
      if (strstr(help, "-smbios type"))
          qemuCapsSet(flags, QEMU_CAPS_SMBIOS_TYPE);
+    if (strstr(help, "-sandbox"))
+        qemuCapsSet(flags, QEMU_CAPS_SANDBOX);

      if ((netdev = strstr(help, "-netdev"))) {
          /* Disable -netdev on 0.12 since although it exists,
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index a7b3a06..0066901 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -141,6 +141,7 @@ enum qemuCapsFlags {
      QEMU_CAPS_IOLIMITS           = 103, /* -device ...logical_block_size & co */
      QEMU_CAPS_DISABLE_S3         = 104, /* S3 BIOS Advertisement on/off */
      QEMU_CAPS_DISABLE_S4         = 105, /* S4 BIOS Advertisement on/off */
+    QEMU_CAPS_SANDBOX            = 106, /* -sandbox */

      QEMU_CAPS_LAST,                   /* this must always be the last item */
  };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e739f34..737d4d9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6462,6 +6462,9 @@ qemuBuildCommandLine(virConnectPtr conn,
                                   ? qemucmd->env_value[i] : "");
      }

+    if (driver->sandbox && qemuCapsGet(qemuCaps, QEMU_CAPS_SANDBOX))
+        virCommandAddArgList(cmd, "-sandbox", "on", NULL);
+
      return cmd;

   no_memory:
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e9e15c5..a367fcd 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -129,6 +129,7 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,

      driver->keepAliveInterval = 5;
      driver->keepAliveCount = 5;
+    driver->sandbox = false;

      /* Just check the file is readable before opening it, otherwise
       * libvirt emits an error.
@@ -570,6 +571,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
      CHECK_TYPE("keepalive_count", VIR_CONF_LONG);
      if (p) driver->keepAliveCount = p->l;

+    p = virConfGetValue(conf, "sandbox");
+    CHECK_TYPE("sandbox", VIR_CONF_LONG);
+    if (p) driver->sandbox = p->l;
+
      virConfFree (conf);
      return 0;
  }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index ac285f6..f1b6465 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -152,6 +152,7 @@ struct qemud_driver {

      int keepAliveInterval;
      unsigned int keepAliveCount;
+    bool sandbox;
  };

  typedef struct _qemuDomainCmdlineDef qemuDomainCmdlineDef;

   As-is the patch looks fine to me, now the real question as you pointed
out is do we want to enforce that at the guest level.
   In general, if available sandboxing should be turned on unless we hit
a bug, so if it work as expected, it should always be on, which to me
would be an indication to have that as a global default for the driver
(and on by default).
   If you have to rely on the user explicit setting to activate it, it
won't be activated, if security implementations are good enough they
are better off as default settings IMHO,

I agree, it should be on by default and turned off only if there's a problem with the syscall whitelist. However, note what I said in my previous email that we don't plan to default to on until QEMU 1.3.

--
Regards,
Corey


   So ACK to this, except I would change src/qemu/qemu.conf patch to
enable it by default, i.e. remove the leading # ... then testing will
tell us if we can keep it on.

Daniel



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]