On Tue, Apr 10, 2018 at 04:49:39PM +0200, Ján Tomko wrote:

QEMU commit 1bd6152 changed the default behavior from whitelist to blacklist and introduced a few sets of system calls.

Use the 'elevateprivileges' parameter of -sandbox as a witness of this change.

https://bugzilla.redhat.com/show_bug.cgi?id=1492597

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 7 files changed, 8 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 35905e993..729e29e20 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -468,6 +468,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "virtio-tablet-ccw", "qcow2-luks", "pcie-pci-bridge", + "seccomp-blacklist", );

@@ -3214,6 +3215,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "loadparm", QEMU_CAPS_LOADPARM }, { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, + { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, };

static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index bec28cae9..d88102f34 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -452,6 +452,7 @@ typedef enum { QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW, /* -device virtio-tablet-ccw */ QEMU_CAPS_QCOW2_LUKS, /* qcow2 format support LUKS encryption */ QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE, /* -device pcie-pci-bridge */ + QEMU_CAPS_SECCOMP_BLACKLIST, /* -sandbox.elevateprivileges */

QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index cbd645ae9..3861666e5 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -151,6 +151,7 @@ <flag name='virtio-mouse-ccw'/> <flag name='virtio-tablet-ccw'/> <flag name='qcow2-luks'/> + <flag name='seccomp-blacklist'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342058</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index 66629ff5b..39238a9b6 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -189,6 +189,7 @@ <flag name='dump-completed'/> <flag name='qcow2-luks'/> <flag name='pcie-pci-bridge'/> + <flag name='seccomp-blacklist'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342346</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index 1122d6408..6bf293b9d 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -186,6 +186,7 @@ <flag name='machine.pseries.max-cpu-compat'/> <flag name='dump-completed'/> <flag name='qcow2-luks'/> + <flag name='seccomp-blacklist'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>419215</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 191b1e0e3..b77aec9c9 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -151,6 +151,7 @@ <flag name='virtio-mouse-ccw'/> <flag name='virtio-tablet-ccw'/> <flag name='qcow2-luks'/> + <flag name='seccomp-blacklist'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 4ed2e1ea9..1bb825c9b 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -227,6 +227,7 @@ <flag name='dump-completed'/> <flag name='qcow2-luks'/> <flag name='pcie-pci-bridge'/> + <flag name='seccomp-blacklist'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>390060</microcodeVersion> -- 2.16.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Tue, Apr 10, 2018 at 04:49:41PM +0200, Ján Tomko wrote:

Exit early if possible to simplify the logic.

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dfeba54ee..ba279e640 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9974,16 +9974,22 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd, virQEMUDriverConfigPtr cfg, virQEMUCapsPtr qemuCaps) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_SANDBOX)) { - if (cfg->seccompSandbox == 0) - virCommandAddArgList(cmd, "-sandbox", "off", NULL); - else if (cfg->seccompSandbox > 0) - virCommandAddArgList(cmd, "-sandbox", "on", NULL); - } else if (cfg->seccompSandbox > 0) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_SANDBOX) && + cfg->seccompSandbox > 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("QEMU does not support seccomp sandboxes")); return -1; } + + if (cfg->seccompSandbox == 0) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_SANDBOX)) + virCommandAddArgList(cmd, "-sandbox", "off", NULL); + return 0; + } + + if (cfg->seccompSandbox > 0) + virCommandAddArgList(cmd, "-sandbox", "on", NULL); + return 0;

} -- 2.16.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 04/10/2018 10:49 AM, Ján Tomko wrote:

If QEMU uses a seccomp blacklist (since 2.11), -sandbox on no longer tries to whitelist all the calls, but uses sets of blacklists: default (always blacklisted with -sandbox on) obsolete (defaults to deny) elevateprivileges (setuid & co, default: allow) spawn (fork & execve, default: allow) resourcecontrol (setaffinity, setscheduler, default: allow)

If these are supported, default to sandbox with all four categories blacklisted.

https://bugzilla.redhat.com/show_bug.cgi?id=1492597

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu.conf | 7 +++--- src/qemu/qemu_command.c | 10 +++++++++ tests/qemuxml2argvdata/minimal-sandbox.args | 29 ++++++++++++++++++++++++ tests/qemuxml2argvdata/minimal-sandbox.xml | 34 +++++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++++++++ 5 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/minimal-sandbox.args create mode 100644 tests/qemuxml2argvdata/minimal-sandbox.xml

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 07eab7eff..740129cf5 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -669,9 +669,10 @@

-# Use seccomp syscall whitelisting in QEMU. -# 1 = on, 0 = off, -1 = use QEMU default -# Defaults to -1. +# Use seccomp syscall sandbox in QEMU. +# 1 = on, 0 = off, -1 = use the default +# For QEMUs using a whitelist, the default (-1) is off. +# For QEMUs using a blacklist, the default (-1) is on.

Not sure it's even possible to provide any sort of details, but suffice to say the description here is really lacking. One of those things that if you know and care, then you use, if you don't you ignore. Maybe it's just me being dense ;-). Still if someone supplies 0 or 1 does that now mean the opposite of what it did before 2.11? That is if I had this set to 1 in my qemu.conf - does that mean that now I'm using a blacklist instead of a whitelist? As an Admin trying to decipher this - what would each setting mean to me and if going with the new -1 default, then that means libvirt is going to set "on" w/ a list of 4 to deny. With at least a few more details or shreds of information that may help someone decipher what really changed... Reviewed-by: John Ferlan <jferlan@redhat.com> John

# #seccomp_sandbox = 1

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba279e640..fa5906d0b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9987,6 +9987,16 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd, return 0; }

+ /* Use blacklist by default if supported */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_BLACKLIST)) { + virCommandAddArgList(cmd, "-sandbox", + "on,obsolete=deny,elevateprivileges=deny," + "spawn=deny,resourcecontrol=deny", + NULL); + return 0; + } + + /* Seccomp whitelist is opt-in */ if (cfg->seccompSandbox > 0) virCommandAddArgList(cmd, "-sandbox", "on", NULL);

diff --git a/tests/qemuxml2argvdata/minimal-sandbox.args b/tests/qemuxml2argvdata/minimal-sandbox.args new file mode 100644 index 000000000..c9d71fe8f --- /dev/null +++ b/tests/qemuxml2argvdata/minimal-sandbox.args @@ -0,0 +1,29 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny diff --git a/tests/qemuxml2argvdata/minimal-sandbox.xml b/tests/qemuxml2argvdata/minimal-sandbox.xml new file mode 100644 index 000000000..9ef92f8fe --- /dev/null +++ b/tests/qemuxml2argvdata/minimal-sandbox.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements. + </description> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static' cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9a22fe5f4..cf69135e8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -729,6 +729,17 @@ mymain(void) unsetenv("SDL_AUDIODRIVER");

DO_TEST("minimal", NONE); + DO_TEST("minimal-sandbox", + QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_MONITOR_JSON, + QEMU_CAPS_NO_USER_CONFIG, + QEMU_CAPS_RTC, + QEMU_CAPS_NO_SHUTDOWN, + QEMU_CAPS_DUMP_GUEST_CORE, + QEMU_CAPS_DISPLAY, + QEMU_CAPS_MACHINE_USB_OPT, + QEMU_CAPS_SECCOMP_SANDBOX, + QEMU_CAPS_SECCOMP_BLACKLIST); DO_TEST_PARSE_ERROR("minimal-no-memory", NONE); DO_TEST("minimal-msg-timestamp", QEMU_CAPS_MSG_TIMESTAMP); DO_TEST("machine-aliases1", NONE);

On Tue, Apr 10, 2018 at 04:49:42PM +0200, Ján Tomko wrote:

If QEMU uses a seccomp blacklist (since 2.11), -sandbox on no longer tries to whitelist all the calls, but uses sets of blacklists: default (always blacklisted with -sandbox on) obsolete (defaults to deny) elevateprivileges (setuid & co, default: allow) spawn (fork & execve, default: allow) resourcecontrol (setaffinity, setscheduler, default: allow)

If these are supported, default to sandbox with all four categories blacklisted.

https://bugzilla.redhat.com/show_bug.cgi?id=1492597

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu.conf | 7 +++--- src/qemu/qemu_command.c | 10 +++++++++ tests/qemuxml2argvdata/minimal-sandbox.args | 29 ++++++++++++++++++++++++ tests/qemuxml2argvdata/minimal-sandbox.xml | 34 +++++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++++++++ 5 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/minimal-sandbox.args create mode 100644 tests/qemuxml2argvdata/minimal-sandbox.xml

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 07eab7eff..740129cf5 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -669,9 +669,10 @@

-# Use seccomp syscall whitelisting in QEMU. -# 1 = on, 0 = off, -1 = use QEMU default -# Defaults to -1. +# Use seccomp syscall sandbox in QEMU. +# 1 = on, 0 = off, -1 = use the default +# For QEMUs using a whitelist, the default (-1) is off. +# For QEMUs using a blacklist, the default (-1) is on.

I'd suggest rewriting this a bit: # 1 == seccomp enabled, 0 == seccomp disabled # # If it is unset (or -1), then seccomp will be enabled # only if QEMU >= 2.11.0 is detected, otherwise it is # left disabled. This ensures the default config gets # protection for new QEMU using the blacklist approach.

# #seccomp_sandbox = 1

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba279e640..fa5906d0b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9987,6 +9987,16 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd, return 0; }

+ /* Use blacklist by default if supported */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_BLACKLIST)) { + virCommandAddArgList(cmd, "-sandbox", + "on,obsolete=deny,elevateprivileges=deny," + "spawn=deny,resourcecontrol=deny", + NULL); + return 0; + } + + /* Seccomp whitelist is opt-in */ if (cfg->seccompSandbox > 0) virCommandAddArgList(cmd, "-sandbox", "on", NULL);

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|