[libvirt] [PATCH v0] qemu: Add sandbox support.
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; -- 1.7.8.6
On Mon, Sep 3, 2012 at 7:03 AM, Ján Tomko <jtomko@redhat.com> 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.
<snip> There's a big push to not rely on -help scraping, please work with qemu upstream to get this exposed through the QMP and query for the capability that way. -- Doug Goldstein
On 09/03/2012 03:07 PM, Doug Goldstein wrote:
On Mon, Sep 3, 2012 at 7:03 AM, Ján Tomko <jtomko@redhat.com> 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.
<snip>
There's a big push to not rely on -help scraping, please work with qemu upstream to get this exposed through the QMP and query for the capability that way.
We already agreed upstream that 1.2 and older can use -help scraping, and that 1.3 and newer will assume that all features present in 1.2 are still present, and that QMP queries will supply the rest. Therefore, I'm okay with -help scraping for 1.2, and just blindly assuming that -sandbox exists if we detected version 1.3 through a QMP query. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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, 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 -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Thu, Sep 06, 2012 at 02:27:19PM +0800, 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. [...] 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,
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.
I just asked Chris Evans the vsftpd maintainer since I know he added support for it, except for a couple of bugs on Fedora he activated it by default if the kernel supports it, and things seems to work just fine. So yeah I would keep that a global setting and activated by default, if there are bugs in the kernel or the qemu side we may revisit this but let's see first what kind of bugs pop up, and at the distro level it's easy to switch back to off if there is serious problems. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/06/2012 02:45 AM, Daniel Veillard wrote:
On Thu, Sep 06, 2012 at 02:27:19PM +0800, 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. [...] 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,
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.
I just asked Chris Evans the vsftpd maintainer since I know he added support for it, except for a couple of bugs on Fedora he activated it by default if the kernel supports it, and things seems to work just fine.
So yeah I would keep that a global setting and activated by default, if there are bugs in the kernel or the qemu side we may revisit this but let's see first what kind of bugs pop up, and at the distro level it's easy to switch back to off if there is serious problems.
Daniel
We turned this off by default in QEMU 1.2. The thought was that we'd get more testing coverage. It's a tough feature to test completely, and if a syscall was missed in the whitelist, then your guest is dead. The plan is to turn it on by default in QEMU 1.3. Perhaps you want to do something similar in libvirt? -- Regards, Corey
On Thu, Sep 06, 2012 at 11:53:06AM -0400, Corey Bryant wrote:
On 09/06/2012 02:45 AM, Daniel Veillard wrote:
On Thu, Sep 06, 2012 at 02:27:19PM +0800, 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. [...] 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,
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.
I just asked Chris Evans the vsftpd maintainer since I know he added support for it, except for a couple of bugs on Fedora he activated it by default if the kernel supports it, and things seems to work just fine.
So yeah I would keep that a global setting and activated by default, if there are bugs in the kernel or the qemu side we may revisit this but let's see first what kind of bugs pop up, and at the distro level it's easy to switch back to off if there is serious problems.
Daniel
We turned this off by default in QEMU 1.2. The thought was that we'd get more testing coverage. It's a tough feature to test completely, and if a syscall was missed in the whitelist, then your guest is dead. The plan is to turn it on by default in QEMU 1.3. Perhaps you want to do something similar in libvirt?
The problem is that libvirt and qemu releases are a priori not tied, doing what you suggest would mean to try to guess the actual qemu version used by the guest and then switch on or off, which would somehow be at odd with the overall driver configuration. This also raises the point of the semantic of -sandbox, the code assumes that if it is not present then sandboxing is off, and if it is present sandboxing is on, now what you say seems to imply that sandboxing is on in 1.3 if not present. If right then we need to instead do something like -sandbox=off to make sure we propagate the setting assuming the qemu.conf explicitely states sandbox=0 So we are I think in a tristate configuration: - sandbox=0 in qemu.conf and we need to force it off if supported - sandbox=1 in qemu.conf and we need to force it on if supported - commented out in qemu.conf fallback to the qemu for that guest default Apparently currently -sandbox takes no arguments, any chance to suport for -sandbox=off before 1.3 ? Because otherwise the global settings of libvirt qemu driver will conflict with qemu default setting. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/07/12 05:25, Daniel Veillard wrote:
The problem is that libvirt and qemu releases are a priori not tied, doing what you suggest would mean to try to guess the actual qemu version used by the guest and then switch on or off, which would somehow be at odd with the overall driver configuration. This also raises the point of the semantic of -sandbox, the code assumes that if it is not present then sandboxing is off, and if it is present sandboxing is on, now what you say seems to imply that sandboxing is on in 1.3 if not present. If right then we need to instead do something like -sandbox=off to make sure we propagate the setting assuming the qemu.conf explicitely states sandbox=0
So we are I think in a tristate configuration: - sandbox=0 in qemu.conf and we need to force it off if supported - sandbox=1 in qemu.conf and we need to force it on if supported - commented out in qemu.conf fallback to the qemu for that guest default
Apparently currently -sandbox takes no arguments, any chance to suport for -sandbox=off before 1.3 ? Because otherwise the global settings of libvirt qemu driver will conflict with qemu default setting.
Daniel
-sandbox does require an argument, either on or off, so that tri-state configuration is doable at the moment. I don't think having it on by default is a good idea at this time - I had to add a few syscalls to the whitelist to get it working for me before posting the patch, but somehow I managed to break it since. I'll look into those tests/qemuhelp*. Ján
On Fri, Sep 07, 2012 at 01:29:25PM +0200, Ján Tomko wrote:
On 09/07/12 05:25, Daniel Veillard wrote:
The problem is that libvirt and qemu releases are a priori not tied, doing what you suggest would mean to try to guess the actual qemu version used by the guest and then switch on or off, which would somehow be at odd with the overall driver configuration. This also raises the point of the semantic of -sandbox, the code assumes that if it is not present then sandboxing is off, and if it is present sandboxing is on, now what you say seems to imply that sandboxing is on in 1.3 if not present. If right then we need to instead do something like -sandbox=off to make sure we propagate the setting assuming the qemu.conf explicitely states sandbox=0
So we are I think in a tristate configuration: - sandbox=0 in qemu.conf and we need to force it off if supported - sandbox=1 in qemu.conf and we need to force it on if supported - commented out in qemu.conf fallback to the qemu for that guest default
Apparently currently -sandbox takes no arguments, any chance to suport for -sandbox=off before 1.3 ? Because otherwise the global settings of libvirt qemu driver will conflict with qemu default setting.
Daniel
-sandbox does require an argument, either on or off, so that tri-state configuration is doable at the moment.
Ah, excellent !
I don't think having it on by default is a good idea at this time - I had to add a few syscalls to the whitelist to get it working for me before posting the patch, but somehow I managed to break it since.
We can try to keep commented out then, but we won't get much testing then ...
I'll look into those tests/qemuhelp*.
thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/07/2012 08:06 AM, Daniel Veillard wrote:
On Fri, Sep 07, 2012 at 01:29:25PM +0200, Ján Tomko wrote:
On 09/07/12 05:25, Daniel Veillard wrote:
The problem is that libvirt and qemu releases are a priori not tied, doing what you suggest would mean to try to guess the actual qemu version used by the guest and then switch on or off, which would somehow be at odd with the overall driver configuration. This also raises the point of the semantic of -sandbox, the code assumes that if it is not present then sandboxing is off, and if it is present sandboxing is on, now what you say seems to imply that sandboxing is on in 1.3 if not present. If right then we need to instead do something like -sandbox=off to make sure we propagate the setting assuming the qemu.conf explicitely states sandbox=0
So we are I think in a tristate configuration: - sandbox=0 in qemu.conf and we need to force it off if supported - sandbox=1 in qemu.conf and we need to force it on if supported - commented out in qemu.conf fallback to the qemu for that guest default
Yes, this tristate configuration makes sense to me.
Apparently currently -sandbox takes no arguments, any chance to suport for -sandbox=off before 1.3 ? Because otherwise the global settings of libvirt qemu driver will conflict with qemu default setting.
Daniel
-sandbox does require an argument, either on or off, so that tri-state configuration is doable at the moment.
Ah, excellent !
I don't think having it on by default is a good idea at this time - I had to add a few syscalls to the whitelist to get it working for me before posting the patch, but somehow I managed to break it since.
Jan, What syscalls did you have to add?
We can try to keep commented out then, but we won't get much testing then ...
We want all the testing we can get. At the same time, I think we'd also like to have some more assurance that the whitelist is complete before turning it on by default. The QEMU 1.3 soft feature freeze is on Nov 1st. Should we let this bake for a little bit with default off, and perhaps set a target date of Oct 1st to turn the default on? -- Regards, Corey
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
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(-)
Hi Ján, I think we need a followup patch for the test area, we need to extend tests/qemuhelpdata/ and tests/qemuhelptest.c to detect the new feature, and check it's processed and exposed correctly, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (5)
-
Corey Bryant -
Daniel Veillard -
Doug Goldstein -
Eric Blake -
Ján Tomko