Skip to contentSkip to conversation

Add syscalls for -runas and -chroot to the seccomp sandbox

PATCH In review Namsun Ch'o 4 participants 8 messages Oct 1, 2015 – Oct 8, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add syscalls for -runas and -chroot to the seccomp sandbox

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.

---

qemu-seccomp.c +5 -1
index f9de0d3..5cb1809 100644
@@ -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)
#1 lore raw
Quoted 5 lines · 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.

Should it enable seccomp a bit later?

#2 lore raw
Quoted 9 lines · Markus Armbruster
Quoted 5 lines · 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.

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

Signature · 4 lines
|: 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 :|
#3 lore raw
Quoted 11 lines · Daniel P. Berrange
Quoted 10 lines · Markus Armbruster
Quoted 7 lines · 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.

Quoted 3 lines
Quoted 1 line

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.

Quoted 2 lines

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

Indeed.

#4 lore raw
Quoted 13 lines · Markus Armbruster
Quoted 11 lines · Daniel P. Berrange
Quoted 10 lines · Markus Armbruster
Quoted 7 lines · 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.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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.

Quoted 5 lines
Quoted 3 lines

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

Indeed.

Signature · 2 lines
Eduardo Otubo
ProfitBricks GmbH
#5 lore raw
Quoted 16 lines · Eduardo Otubo
Quoted 15 lines · Markus Armbruster
Quoted 13 lines · Daniel P. Berrange
Quoted 11 lines · Markus Armbruster
Quoted 7 lines · 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.

Should it enable seccomp a bit later?

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

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

Signature · 4 lines
|: 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 :|
#6 lore raw
Quoted 30 lines · Daniel P. Berrange
Quoted 16 lines · Eduardo Otubo
Quoted 15 lines · Markus Armbruster
Quoted 13 lines · Daniel P. Berrange
Quoted 11 lines · Markus Armbruster
Quoted 7 lines · 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.

Should it enable seccomp a bit later?

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

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().

Quoted 7 lines

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).

Signature · 2 lines
Eduardo Otubo
ProfitBricks GmbH
Acked-by on v1 1/1
#7 lore raw
In reply to Namsun Ch'o
Quoted 15 lines · 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.

---

qemu-seccomp.c +0 -0
index f9de0d3..5cb1809 100644
@@ -237,7 +237,11 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(fadvise64), 240 },
     { SCMP_SYS(inotify_init1), 240 },

qemu-seccomp.c static const struct QemuSeccompSyscall seccomp_whitelist[] = { · line 247

Quoted 10 lines
qemu-seccomp.c +5 -1
     { 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.

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.

Signature · 2 lines
Eduardo Otubo
ProfitBricks GmbH