Re: [libvirt] [PATCHv2 4/4] qemu: deny privilege elevation and spawn in seccomp

2018-04-16 Thread Daniel P . Berrangé 
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

2018-04-15 Thread Daniel P . Berrangé 
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

2018-04-13 Thread John Ferlan 


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 qemus minimal configuration.
> +  This test also tests the description and title elements.
> +  
> +  219100
> +  219100
> +  1
> +  
> +hvm
> +
> +  
> +  
> +  destroy
>

[libvirt] [PATCHv2 4/4] qemu: deny privilege elevation and spawn in seccomp

2018-04-10 Thread Ján Tomko 
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 qemus 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