[Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Namsun Ch'o]

The seccomp sandbox doesn't whitelist setuid, setgid, or setgroups, which are
needed for -runas to work. It also doesn't whitelist chroot, which is needed
for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
privileges or chroots, so without these whitelisted, -runas and -chroot cause
QEMU to be killed with -sandbox on. This patch adds those syscalls.

Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
---
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index f9de0d3..5cb1809 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -237,7 +237,11 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(fadvise64), 240 },
     { SCMP_SYS(inotify_init1), 240 },
     { SCMP_SYS(inotify_add_watch), 240 },
-    { SCMP_SYS(mbind), 240 }
+    { SCMP_SYS(mbind), 240 },
+    { SCMP_SYS(setuid), 240 },
+    { SCMP_SYS(setgid), 240 },
+    { SCMP_SYS(chroot), 240 },
+    { SCMP_SYS(setgroups), 240 }
 };

 int seccomp_start(void)

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Markus Armbruster]

"Namsun Ch'o" <namnamc@Safe-mail.net> writes:

> The seccomp sandbox doesn't whitelist setuid, setgid, or setgroups, which are
> needed for -runas to work. It also doesn't whitelist chroot, which is needed
> for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
> privileges or chroots, so without these whitelisted, -runas and -chroot cause
> QEMU to be killed with -sandbox on. This patch adds those syscalls.

Should it enable seccomp a bit later?

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Daniel P. Berrange]

On Thu, Oct 01, 2015 at 02:06:32PM +0200, Markus Armbruster wrote:
> "Namsun Ch'o" <namnamc@Safe-mail.net> writes:
> 
> > The seccomp sandbox doesn't whitelist setuid, setgid, or setgroups, which are
> > needed for -runas to work. It also doesn't whitelist chroot, which is needed
> > for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
> > privileges or chroots, so without these whitelisted, -runas and -chroot cause
> > QEMU to be killed with -sandbox on. This patch adds those syscalls.
> 
> Should it enable seccomp a bit later?

Yeah, I think it would be better to move the seccomp enablement later.
Adding setuid and chroot to the allow list is pretty strongly undesirable
from a security protection POV.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Markus Armbruster]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Thu, Oct 01, 2015 at 02:06:32PM +0200, Markus Armbruster wrote:
>> "Namsun Ch'o" <namnamc@Safe-mail.net> writes:
>> 
>> > The seccomp sandbox doesn't whitelist setuid, setgid, or
>> > setgroups, which are
>> > needed for -runas to work. It also doesn't whitelist chroot, which is needed
>> > for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
>> > privileges or chroots, so without these whitelisted, -runas and
>> > -chroot cause
>> > QEMU to be killed with -sandbox on. This patch adds those syscalls.
>> 
>> Should it enable seccomp a bit later?
>
> Yeah, I think it would be better to move the seccomp enablement later.

Let's do that then.

> Adding setuid and chroot to the allow list is pretty strongly undesirable
> from a security protection POV.

Indeed.

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Eduardo Otubo]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

On Fri, Oct 02, 2015 at 12=05=58PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> 
> > On Thu, Oct 01, 2015 at 02:06:32PM +0200, Markus Armbruster wrote:
> >> "Namsun Ch'o" <namnamc@Safe-mail.net> writes:
> >> 
> >> > The seccomp sandbox doesn't whitelist setuid, setgid, or
> >> > setgroups, which are
> >> > needed for -runas to work. It also doesn't whitelist chroot, which is needed
> >> > for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
> >> > privileges or chroots, so without these whitelisted, -runas and
> >> > -chroot cause
> >> > QEMU to be killed with -sandbox on. This patch adds those syscalls.
> >> 
> >> Should it enable seccomp a bit later?
> >
> > Yeah, I think it would be better to move the seccomp enablement later.
> 
> Let's do that then.

Where exactly you guys think we could call seccomp enablement? Right
it's called (almost) right before cpu_exec_init_all(), on vl.c:4013. I
guess it is as later as it could.

> 
> > Adding setuid and chroot to the allow list is pretty strongly undesirable
> > from a security protection POV.
> 
> Indeed.

-- 
Eduardo Otubo
ProfitBricks GmbH

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Daniel P. Berrange]

On Fri, Oct 02, 2015 at 04:08:20PM +0200, Eduardo Otubo wrote:
> On Fri, Oct 02, 2015 at 12=05=58PM +0200, Markus Armbruster wrote:
> > "Daniel P. Berrange" <berrange@redhat.com> writes:
> > 
> > > On Thu, Oct 01, 2015 at 02:06:32PM +0200, Markus Armbruster wrote:
> > >> "Namsun Ch'o" <namnamc@Safe-mail.net> writes:
> > >> 
> > >> > The seccomp sandbox doesn't whitelist setuid, setgid, or
> > >> > setgroups, which are
> > >> > needed for -runas to work. It also doesn't whitelist chroot, which is needed
> > >> > for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
> > >> > privileges or chroots, so without these whitelisted, -runas and
> > >> > -chroot cause
> > >> > QEMU to be killed with -sandbox on. This patch adds those syscalls.
> > >> 
> > >> Should it enable seccomp a bit later?
> > >
> > > Yeah, I think it would be better to move the seccomp enablement later.
> > 
> > Let's do that then.
> 
> Where exactly you guys think we could call seccomp enablement? Right
> it's called (almost) right before cpu_exec_init_all(), on vl.c:4013. I
> guess it is as later as it could.

It depends on what you consider seccomp to be protecting against. I think
its primary goal is to protect against exploit after a guest OS breakout,
in which case it does not need to be enabled until the guest CPUs start
executing, which IIUC, means immediately before main_loop().

If we intend seccomp to protect against flaws during QEMU setup, then having
it earlier is neccessary. eg QEMU opening a corrupt qcow2 image which might
exploit QEMU before the guest CPUs start.

If the latter is the case, then we could start with a relaxed seccomp
sandbox which included the setuid/chroot features, and then switch to a
more restricted one which blocked them before main_loop() runs.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Eduardo Otubo]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

On Fri, Oct 02, 2015 at 03=15=05PM +0100, Daniel P. Berrange wrote:
> On Fri, Oct 02, 2015 at 04:08:20PM +0200, Eduardo Otubo wrote:
> > On Fri, Oct 02, 2015 at 12=05=58PM +0200, Markus Armbruster wrote:
> > > "Daniel P. Berrange" <berrange@redhat.com> writes:
> > > 
> > > > On Thu, Oct 01, 2015 at 02:06:32PM +0200, Markus Armbruster wrote:
> > > >> "Namsun Ch'o" <namnamc@Safe-mail.net> writes:
> > > >> 
> > > >> > The seccomp sandbox doesn't whitelist setuid, setgid, or
> > > >> > setgroups, which are
> > > >> > needed for -runas to work. It also doesn't whitelist chroot, which is needed
> > > >> > for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
> > > >> > privileges or chroots, so without these whitelisted, -runas and
> > > >> > -chroot cause
> > > >> > QEMU to be killed with -sandbox on. This patch adds those syscalls.
> > > >> 
> > > >> Should it enable seccomp a bit later?
> > > >
> > > > Yeah, I think it would be better to move the seccomp enablement later.
> > > 
> > > Let's do that then.
> > 
> > Where exactly you guys think we could call seccomp enablement? Right
> > it's called (almost) right before cpu_exec_init_all(), on vl.c:4013. I
> > guess it is as later as it could.
> 
> It depends on what you consider seccomp to be protecting against. I think
> its primary goal is to protect against exploit after a guest OS breakout,
> in which case it does not need to be enabled until the guest CPUs start
> executing, which IIUC, means immediately before main_loop().
> 
> If we intend seccomp to protect against flaws during QEMU setup, then having
> it earlier is neccessary. eg QEMU opening a corrupt qcow2 image which might
> exploit QEMU before the guest CPUs start.
> 
> If the latter is the case, then we could start with a relaxed seccomp
> sandbox which included the setuid/chroot features, and then switch to a
> more restricted one which blocked them before main_loop() runs.

Our intention since the beginning was to protect the host from the
illegal guest operations. But you do have an interesting point about
flaws on qemu itself. Perhaps this might be something I could work on to
improve (start a bigger whitelist and get it tighter before guest
launches).

-- 
Eduardo Otubo
ProfitBricks GmbH

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Eduardo Otubo]

[-- Attachment #1: Type: text/plain, Size: 1567 bytes --]

On Thu, Oct 01, 2015 at 12=36=05AM -0400, Namsun Ch'o wrote:
> The seccomp sandbox doesn't whitelist setuid, setgid, or setgroups, which are
> needed for -runas to work. It also doesn't whitelist chroot, which is needed
> for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
> privileges or chroots, so without these whitelisted, -runas and -chroot cause
> QEMU to be killed with -sandbox on. This patch adds those syscalls.
> 
> Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
> ---
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index f9de0d3..5cb1809 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -237,7 +237,11 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
>      { SCMP_SYS(fadvise64), 240 },
>      { SCMP_SYS(inotify_init1), 240 },
>      { SCMP_SYS(inotify_add_watch), 240 },
> -    { SCMP_SYS(mbind), 240 }
> +    { SCMP_SYS(mbind), 240 },
> +    { SCMP_SYS(setuid), 240 },
> +    { SCMP_SYS(setgid), 240 },
> +    { SCMP_SYS(chroot), 240 },
> +    { SCMP_SYS(setgroups), 240 }
>  };
> 
>  int seccomp_start(void)

Breaking a qemu use case is justification enough to whitelist more
syscalls, but we can come up with a better solution for this (continue
the thread) and tighten up this in the future.

Thanks for your contribution.

Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>

ps.: the threads are still being broken by your emails and it's a pain
to track down all of them in order to read. Please fix it.

-- 
Eduardo Otubo
ProfitBricks GmbH

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[namnamc]

> Should it enable seccomp a bit later?

Ideally it should be enabled as late as possible, right before the main loop,
because here's no reason to whitelist syscalls that are only ever needed to
start QEMU up (e.g. chroot, which is only used before the guest even boots).
But for now, the simplest solution to the -chroot and -runas issue I can
think
of is to enable those syscalls in my patch.

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[namnamc]

> ps.: the threads are still being broken by your emails and it's a pain
> to track down all of them in order to read. Please fix it.

I'm really sorry, I am not able to sign up to Google because I don't have a
cell number. I'll try using Sigaint. Does it work now?

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Markus Armbruster]

namnamc@sigaint.org writes:

>> ps.: the threads are still being broken by your emails and it's a pain
>> to track down all of them in order to read. Please fix it.
>
> I'm really sorry, I am not able to sign up to Google because I don't have a
> cell number. I'll try using Sigaint. Does it work now?

Nope.

Most web mail software sucks.  Any particular reason for using a web
browser to send e-mail?  I suggest to pick a provider that provides
IMAP, then use a standalone e-mail application such as Thunderbird,
Claws Mail, Mutt or (if you use Emacs) Gnus.

Re: [Qemu-devel] [PATCH] Add syscalls for -runas and -chroot to the seccomp sandbox

[Eduardo Otubo]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]

On Fri, Oct 09, 2015 at 08=54=33AM +0200, Markus Armbruster wrote:
> namnamc@sigaint.org writes:
> 
> >> ps.: the threads are still being broken by your emails and it's a pain
> >> to track down all of them in order to read. Please fix it.
> >
> > I'm really sorry, I am not able to sign up to Google because I don't have a
> > cell number. I'll try using Sigaint. Does it work now?
> 
> Nope.
> 
> Most web mail software sucks.  Any particular reason for using a web
> browser to send e-mail?  I suggest to pick a provider that provides
> IMAP, then use a standalone e-mail application such as Thunderbird,
> Claws Mail, Mutt or (if you use Emacs) Gnus.

Exactly.

-- 
Eduardo Otubo
ProfitBricks GmbH

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]