Re: [libvirt] [PATCHv2 4/4] qemu: deny privilege elevation and spawn in seccomp
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 > --- > 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é 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 4/4] qemu: deny privilege elevation and spawn in seccomp
On Fri, Apr 13, 2018 at 10:08:34AM -0400, John Ferlan wrote: > > > 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 > > --- > > 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? Yes, setting this to '1' just means "enable use of seccomp". We explicitly never defined what kind of seccomp rules would be enabled - only that something seccomp related is on. Whether its a blacklist or a whitelist is a low level impl detail that we don't expect users to care about. > 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. Essentially the default (-1) means "do the best thing". On old QEMU the best thing was to disable it because it was horribly unreliable with a whitelist. On modern QEMU the best thing is to enable it because the blacklist is much saner 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 4/4] qemu: deny privilege elevation and spawn in seccomp
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 > --- > 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 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 0..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 0..9ef92f8fe > --- /dev/null > +++ b/tests/qemuxml2argvdata/minimal-sandbox.xml > @@ -0,0 +1,34 @@ > + > + QEMUGuest1 > + c7a5fdbd-edaf-9455-926a-d65c16db1809 > + A description of the test machine. > + > + A test of qemu's minimal configuration. > + This test also tests the description and title elements. > + > + 219100 > + 219100 > + 1 > + > +hvm > + > + > + > + destroy > + restart > + destroy > + > +/u
[libvirt] [PATCHv2 4/4] qemu: deny privilege elevation and spawn in seccomp
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 --- 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. # #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 0..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 0..9ef92f8fe --- /dev/null +++ b/tests/qemuxml2argvdata/minimal-sandbox.xml @@ -0,0 +1,34 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + A description of the test machine. + + A test of qemu's minimal configuration. + This test also tests the description and title elements. + + 219100 + 219100 + 1 + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu-system-i686 + + + + + + + + + + + + + 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); -- 2.16.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list