* [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring
@ 2017-03-14 11:32 Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 1/5] seccomp: changing from whitelist to blacklist Eduardo Otubo
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange, thuth
Hi all,
This is first attempt of refactoring the seccomp feature, following Daniel's ideas:
https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03348.html
Best regards,
Eduardo Otubo (5):
seccomp: changing from whitelist to blacklist
seccomp: add obsolete argument to command line
seccomp: add elevateprivileges argument to command line
seccomp: add spawn argument to command line
seccomp: add resourcecontrol argument to command line
include/sysemu/seccomp.h | 7 +-
qemu-options.hx | 19 ++-
qemu-seccomp.c | 375 ++++++++++++++++++-----------------------------
vl.c | 49 ++++++-
4 files changed, 217 insertions(+), 233 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 1/5] seccomp: changing from whitelist to blacklist
2017-03-14 11:32 [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring Eduardo Otubo
@ 2017-03-14 11:32 ` Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line Eduardo Otubo
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange, thuth
This patch changes the default behavior of the seccomp filter from
whitelist to blacklist. By default now all system calls are allowed and
a small black list of definitely forbidden ones was created.
Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
qemu-seccomp.c | 256 +++++++--------------------------------------------------
1 file changed, 28 insertions(+), 228 deletions(-)
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index df75d9c471..f8877b07b5 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -31,229 +31,29 @@ struct QemuSeccompSyscall {
uint8_t priority;
};
-static const struct QemuSeccompSyscall seccomp_whitelist[] = {
- { SCMP_SYS(timer_settime), 255 },
- { SCMP_SYS(timer_gettime), 254 },
- { SCMP_SYS(futex), 253 },
- { SCMP_SYS(select), 252 },
- { SCMP_SYS(recvfrom), 251 },
- { SCMP_SYS(sendto), 250 },
- { SCMP_SYS(socketcall), 250 },
- { SCMP_SYS(read), 249 },
- { SCMP_SYS(io_submit), 249 },
- { SCMP_SYS(brk), 248 },
- { SCMP_SYS(clone), 247 },
- { SCMP_SYS(mmap), 247 },
- { SCMP_SYS(mprotect), 246 },
- { SCMP_SYS(execve), 245 },
- { SCMP_SYS(open), 245 },
- { SCMP_SYS(ioctl), 245 },
- { SCMP_SYS(socket), 245 },
- { SCMP_SYS(setsockopt), 245 },
- { SCMP_SYS(recvmsg), 245 },
- { SCMP_SYS(sendmsg), 245 },
- { SCMP_SYS(accept), 245 },
- { SCMP_SYS(connect), 245 },
- { SCMP_SYS(socketpair), 245 },
- { SCMP_SYS(bind), 245 },
- { SCMP_SYS(listen), 245 },
- { SCMP_SYS(semget), 245 },
- { SCMP_SYS(ipc), 245 },
- { SCMP_SYS(gettimeofday), 245 },
- { SCMP_SYS(readlink), 245 },
- { SCMP_SYS(access), 245 },
- { SCMP_SYS(prctl), 245 },
- { SCMP_SYS(signalfd), 245 },
- { SCMP_SYS(getrlimit), 245 },
- { SCMP_SYS(getrusage), 245 },
- { SCMP_SYS(set_tid_address), 245 },
- { SCMP_SYS(statfs), 245 },
- { SCMP_SYS(unlink), 245 },
- { SCMP_SYS(wait4), 245 },
- { SCMP_SYS(fcntl64), 245 },
- { SCMP_SYS(fstat64), 245 },
- { SCMP_SYS(stat64), 245 },
- { SCMP_SYS(getgid32), 245 },
- { SCMP_SYS(getegid32), 245 },
- { SCMP_SYS(getuid32), 245 },
- { SCMP_SYS(geteuid32), 245 },
- { SCMP_SYS(sigreturn), 245 },
- { SCMP_SYS(_newselect), 245 },
- { SCMP_SYS(_llseek), 245 },
- { SCMP_SYS(mmap2), 245 },
- { SCMP_SYS(sigprocmask), 245 },
- { SCMP_SYS(sched_getparam), 245 },
- { SCMP_SYS(sched_getscheduler), 245 },
- { SCMP_SYS(fstat), 245 },
- { SCMP_SYS(clock_getres), 245 },
- { SCMP_SYS(sched_get_priority_min), 245 },
- { SCMP_SYS(sched_get_priority_max), 245 },
- { SCMP_SYS(stat), 245 },
- { SCMP_SYS(uname), 245 },
- { SCMP_SYS(eventfd2), 245 },
- { SCMP_SYS(io_getevents), 245 },
- { SCMP_SYS(dup), 245 },
- { SCMP_SYS(dup2), 245 },
- { SCMP_SYS(dup3), 245 },
- { SCMP_SYS(gettid), 245 },
- { SCMP_SYS(getgid), 245 },
- { SCMP_SYS(getegid), 245 },
- { SCMP_SYS(getuid), 245 },
- { SCMP_SYS(geteuid), 245 },
- { SCMP_SYS(timer_create), 245 },
- { SCMP_SYS(times), 245 },
- { SCMP_SYS(exit), 245 },
- { SCMP_SYS(clock_gettime), 245 },
- { SCMP_SYS(time), 245 },
- { SCMP_SYS(restart_syscall), 245 },
- { SCMP_SYS(pwrite64), 245 },
- { SCMP_SYS(nanosleep), 245 },
- { SCMP_SYS(chown), 245 },
- { SCMP_SYS(openat), 245 },
- { SCMP_SYS(getdents), 245 },
- { SCMP_SYS(timer_delete), 245 },
- { SCMP_SYS(exit_group), 245 },
- { SCMP_SYS(rt_sigreturn), 245 },
- { SCMP_SYS(sync), 245 },
- { SCMP_SYS(pread64), 245 },
- { SCMP_SYS(madvise), 245 },
- { SCMP_SYS(set_robust_list), 245 },
- { SCMP_SYS(lseek), 245 },
- { SCMP_SYS(pselect6), 245 },
- { SCMP_SYS(fork), 245 },
- { SCMP_SYS(rt_sigprocmask), 245 },
- { SCMP_SYS(write), 244 },
- { SCMP_SYS(fcntl), 243 },
- { SCMP_SYS(tgkill), 242 },
- { SCMP_SYS(kill), 242 },
- { SCMP_SYS(rt_sigaction), 242 },
- { SCMP_SYS(pipe2), 242 },
- { SCMP_SYS(munmap), 242 },
- { SCMP_SYS(mremap), 242 },
- { SCMP_SYS(fdatasync), 242 },
- { SCMP_SYS(close), 242 },
- { SCMP_SYS(rt_sigpending), 242 },
- { SCMP_SYS(rt_sigtimedwait), 242 },
- { SCMP_SYS(readv), 242 },
- { SCMP_SYS(writev), 242 },
- { SCMP_SYS(preadv), 242 },
- { SCMP_SYS(pwritev), 242 },
- { SCMP_SYS(setrlimit), 242 },
- { SCMP_SYS(ftruncate), 242 },
- { SCMP_SYS(lstat), 242 },
- { SCMP_SYS(pipe), 242 },
- { SCMP_SYS(umask), 242 },
- { SCMP_SYS(chdir), 242 },
- { SCMP_SYS(setitimer), 242 },
- { SCMP_SYS(setsid), 242 },
- { SCMP_SYS(poll), 242 },
- { SCMP_SYS(epoll_create), 242 },
- { SCMP_SYS(epoll_ctl), 242 },
- { SCMP_SYS(epoll_wait), 242 },
- { SCMP_SYS(waitpid), 242 },
- { SCMP_SYS(getsockname), 242 },
- { SCMP_SYS(getpeername), 242 },
- { SCMP_SYS(accept4), 242 },
- { SCMP_SYS(timerfd_settime), 242 },
- { SCMP_SYS(newfstatat), 241 },
- { SCMP_SYS(shutdown), 241 },
- { SCMP_SYS(getsockopt), 241 },
- { SCMP_SYS(semop), 241 },
- { SCMP_SYS(semtimedop), 241 },
- { SCMP_SYS(epoll_ctl_old), 241 },
- { SCMP_SYS(epoll_wait_old), 241 },
- { SCMP_SYS(epoll_pwait), 241 },
- { SCMP_SYS(epoll_create1), 241 },
- { SCMP_SYS(ppoll), 241 },
- { SCMP_SYS(creat), 241 },
- { SCMP_SYS(link), 241 },
- { SCMP_SYS(getpid), 241 },
- { SCMP_SYS(getppid), 241 },
- { SCMP_SYS(getpgrp), 241 },
- { SCMP_SYS(getpgid), 241 },
- { SCMP_SYS(getsid), 241 },
- { SCMP_SYS(getdents64), 241 },
- { SCMP_SYS(getresuid), 241 },
- { SCMP_SYS(getresgid), 241 },
- { SCMP_SYS(getgroups), 241 },
- { SCMP_SYS(getresuid32), 241 },
- { SCMP_SYS(getresgid32), 241 },
- { SCMP_SYS(getgroups32), 241 },
- { SCMP_SYS(signal), 241 },
- { SCMP_SYS(sigaction), 241 },
- { SCMP_SYS(sigsuspend), 241 },
- { SCMP_SYS(sigpending), 241 },
- { SCMP_SYS(truncate64), 241 },
- { SCMP_SYS(ftruncate64), 241 },
- { SCMP_SYS(fchown32), 241 },
- { SCMP_SYS(chown32), 241 },
- { SCMP_SYS(lchown32), 241 },
- { SCMP_SYS(statfs64), 241 },
- { SCMP_SYS(fstatfs64), 241 },
- { SCMP_SYS(fstatat64), 241 },
- { SCMP_SYS(lstat64), 241 },
- { SCMP_SYS(sendfile64), 241 },
- { SCMP_SYS(ugetrlimit), 241 },
- { SCMP_SYS(alarm), 241 },
- { SCMP_SYS(rt_sigsuspend), 241 },
- { SCMP_SYS(rt_sigqueueinfo), 241 },
- { SCMP_SYS(rt_tgsigqueueinfo), 241 },
- { SCMP_SYS(sigaltstack), 241 },
- { SCMP_SYS(signalfd4), 241 },
- { SCMP_SYS(truncate), 241 },
- { SCMP_SYS(fchown), 241 },
- { SCMP_SYS(lchown), 241 },
- { SCMP_SYS(fchownat), 241 },
- { SCMP_SYS(fstatfs), 241 },
- { SCMP_SYS(getitimer), 241 },
- { SCMP_SYS(syncfs), 241 },
- { SCMP_SYS(fsync), 241 },
- { SCMP_SYS(fchdir), 241 },
- { SCMP_SYS(msync), 241 },
- { SCMP_SYS(sched_setparam), 241 },
- { SCMP_SYS(sched_setscheduler), 241 },
- { SCMP_SYS(sched_yield), 241 },
- { SCMP_SYS(sched_rr_get_interval), 241 },
- { SCMP_SYS(sched_setaffinity), 241 },
- { SCMP_SYS(sched_getaffinity), 241 },
- { SCMP_SYS(readahead), 241 },
- { SCMP_SYS(timer_getoverrun), 241 },
- { SCMP_SYS(unlinkat), 241 },
- { SCMP_SYS(readlinkat), 241 },
- { SCMP_SYS(faccessat), 241 },
- { SCMP_SYS(get_robust_list), 241 },
- { SCMP_SYS(splice), 241 },
- { SCMP_SYS(vmsplice), 241 },
- { SCMP_SYS(getcpu), 241 },
- { SCMP_SYS(sendmmsg), 241 },
- { SCMP_SYS(recvmmsg), 241 },
- { SCMP_SYS(prlimit64), 241 },
- { SCMP_SYS(waitid), 241 },
- { SCMP_SYS(io_cancel), 241 },
- { SCMP_SYS(io_setup), 241 },
- { SCMP_SYS(io_destroy), 241 },
- { SCMP_SYS(arch_prctl), 240 },
- { SCMP_SYS(mkdir), 240 },
- { SCMP_SYS(fchmod), 240 },
- { SCMP_SYS(shmget), 240 },
- { SCMP_SYS(shmat), 240 },
- { SCMP_SYS(shmdt), 240 },
- { SCMP_SYS(timerfd_create), 240 },
- { SCMP_SYS(shmctl), 240 },
- { SCMP_SYS(mlockall), 240 },
- { SCMP_SYS(mlock), 240 },
- { SCMP_SYS(munlock), 240 },
- { SCMP_SYS(semctl), 240 },
- { SCMP_SYS(fallocate), 240 },
- { SCMP_SYS(fadvise64), 240 },
- { SCMP_SYS(inotify_init1), 240 },
- { SCMP_SYS(inotify_add_watch), 240 },
- { SCMP_SYS(mbind), 240 },
- { SCMP_SYS(memfd_create), 240 },
-#ifdef HAVE_CACHEFLUSH
- { SCMP_SYS(cacheflush), 240 },
-#endif
- { SCMP_SYS(sysinfo), 240 },
+static const struct QemuSeccompSyscall blacklist[] = {
+ { SCMP_SYS(reboot), 255 },
+ { SCMP_SYS(swapon), 255 },
+ { SCMP_SYS(swapoff), 255 },
+ { SCMP_SYS(syslog), 255 },
+ { SCMP_SYS(mount), 255 },
+ { SCMP_SYS(umount), 255 },
+ { SCMP_SYS(kexec_load), 255 },
+ { SCMP_SYS(afs_syscall), 255 },
+ { SCMP_SYS(break), 255 },
+ { SCMP_SYS(ftime), 255 },
+ { SCMP_SYS(getpmsg), 255 },
+ { SCMP_SYS(gtty), 255 },
+ { SCMP_SYS(lock), 255 },
+ { SCMP_SYS(mpx), 255 },
+ { SCMP_SYS(prof), 255 },
+ { SCMP_SYS(profil), 255 },
+ { SCMP_SYS(putpmsg), 255 },
+ { SCMP_SYS(security), 255 },
+ { SCMP_SYS(stty), 255 },
+ { SCMP_SYS(tuxcall), 255 },
+ { SCMP_SYS(ulimit), 255 },
+ { SCMP_SYS(vserver), 255 },
};
int seccomp_start(void)
@@ -262,19 +62,19 @@ int seccomp_start(void)
unsigned int i = 0;
scmp_filter_ctx ctx;
- ctx = seccomp_init(SCMP_ACT_KILL);
+ ctx = seccomp_init(SCMP_ACT_ALLOW);
if (ctx == NULL) {
rc = -1;
goto seccomp_return;
}
- for (i = 0; i < ARRAY_SIZE(seccomp_whitelist); i++) {
- rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, seccomp_whitelist[i].num, 0);
+ for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
+ rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, blacklist[i].num, 0);
if (rc < 0) {
goto seccomp_return;
}
- rc = seccomp_syscall_priority(ctx, seccomp_whitelist[i].num,
- seccomp_whitelist[i].priority);
+ rc = seccomp_syscall_priority(ctx, blacklist[i].num,
+ blacklist[i].priority);
if (rc < 0) {
goto seccomp_return;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line
2017-03-14 11:32 [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 1/5] seccomp: changing from whitelist to blacklist Eduardo Otubo
@ 2017-03-14 11:32 ` Eduardo Otubo
2017-03-14 11:47 ` Paolo Bonzini
2017-03-14 12:01 ` Thomas Huth
2017-03-14 11:32 ` [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges " Eduardo Otubo
` (3 subsequent siblings)
5 siblings, 2 replies; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange, thuth
This patch introduces the argument [,obsolete=allow] to the `-sandbox on'
option. It allows Qemu to run safely on old system that still relies on
old system calls.
Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
include/sysemu/seccomp.h | 4 +++-
qemu-options.hx | 9 +++++++--
qemu-seccomp.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
vl.c | 16 +++++++++++++++-
4 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index cfc06008cb..7a7bde246b 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -15,7 +15,9 @@
#ifndef QEMU_SECCOMP_H
#define QEMU_SECCOMP_H
+#define OBSOLETE 0x0001
+
#include <seccomp.h>
-int seccomp_start(void);
+int seccomp_start(uint8_t seccomp_opts);
#endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 8dd8ee34a6..1403d0c85f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3732,13 +3732,18 @@ Old param mode (ARM only).
ETEXI
DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
- "-sandbox <arg> Enable seccomp mode 2 system call filter (default 'off').\n",
+ "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
+ " obsolete: Allow obsolete system calls",
QEMU_ARCH_ALL)
STEXI
-@item -sandbox @var{arg}
+@item -sandbox @var{arg}[,obsolete=@var{string}]
@findex -sandbox
Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering and 'off' will
disable it. The default is 'off'.
+@table @option
+@item obsolete=@var{string}
+Enable Obsolete system calls
+@end table
ETEXI
DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index f8877b07b5..5ef36890da 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -31,6 +31,35 @@ struct QemuSeccompSyscall {
uint8_t priority;
};
+static const struct QemuSeccompSyscall obsolete[] = {
+ { SCMP_SYS(readdir), 255 },
+ { SCMP_SYS(_sysctl), 255 },
+ { SCMP_SYS(afs_syscall), 255 },
+ { SCMP_SYS(bdflush), 255 },
+ { SCMP_SYS(break), 255 },
+ { SCMP_SYS(create_module), 255 },
+ { SCMP_SYS(ftime), 255 },
+ { SCMP_SYS(get_kernel_syms), 255 },
+ { SCMP_SYS(getpmsg), 255 },
+ { SCMP_SYS(gtty), 255 },
+ { SCMP_SYS(lock), 255 },
+ { SCMP_SYS(mpx), 255 },
+ { SCMP_SYS(prof), 255 },
+ { SCMP_SYS(profil), 255 },
+ { SCMP_SYS(putpmsg), 255 },
+ { SCMP_SYS(query_module), 255 },
+ { SCMP_SYS(security), 255 },
+ { SCMP_SYS(sgetmask), 255 },
+ { SCMP_SYS(ssetmask), 255 },
+ { SCMP_SYS(stty), 255 },
+ { SCMP_SYS(sysfs), 255 },
+ { SCMP_SYS(tuxcall), 255 },
+ { SCMP_SYS(ulimit), 255 },
+ { SCMP_SYS(uselib), 255 },
+ { SCMP_SYS(ustat), 255 },
+ { SCMP_SYS(vserver), 255 },
+};
+
static const struct QemuSeccompSyscall blacklist[] = {
{ SCMP_SYS(reboot), 255 },
{ SCMP_SYS(swapon), 255 },
@@ -56,7 +85,20 @@ static const struct QemuSeccompSyscall blacklist[] = {
{ SCMP_SYS(vserver), 255 },
};
-int seccomp_start(void)
+static int is_obsolete(int syscall)
+{
+ unsigned int i = 0;
+
+ for (i = 0; i < ARRAY_SIZE(obsolete); i++) {
+ if (syscall == obsolete[i].num) {
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+int seccomp_start(uint8_t seccomp_opts)
{
int rc = 0;
unsigned int i = 0;
@@ -69,6 +111,9 @@ int seccomp_start(void)
}
for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
+ if ((seccomp_opts & OBSOLETE) && is_obsolete(blacklist[i].num)) {
+ continue;
+ }
rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, blacklist[i].num, 0);
if (rc < 0) {
goto seccomp_return;
diff --git a/vl.c b/vl.c
index 1a95500ac7..7b08b3383b 100644
--- a/vl.c
+++ b/vl.c
@@ -269,6 +269,10 @@ static QemuOptsList qemu_sandbox_opts = {
.name = "enable",
.type = QEMU_OPT_BOOL,
},
+ {
+ .name = "obsolete",
+ .type = QEMU_OPT_STRING,
+ },
{ /* end of list */ }
},
};
@@ -1031,7 +1035,17 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
/* FIXME: change this to true for 1.3 */
if (qemu_opt_get_bool(opts, "enable", false)) {
#ifdef CONFIG_SECCOMP
- if (seccomp_start() < 0) {
+ uint8_t seccomp_opts = 0x0000;
+ const char * value = NULL;
+
+ value = qemu_opt_get(opts,"obsolete");
+ if (value) {
+ if (strcmp(value, "allow") == 0) {
+ seccomp_opts |= OBSOLETE;
+ }
+ }
+
+ if (seccomp_start(seccomp_opts) < 0) {
error_report("failed to install seccomp syscall filter "
"in the kernel");
return -1;
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 11:32 [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 1/5] seccomp: changing from whitelist to blacklist Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line Eduardo Otubo
@ 2017-03-14 11:32 ` Eduardo Otubo
2017-03-14 11:47 ` Paolo Bonzini
2017-03-14 11:32 ` [Qemu-devel] [PATCH 4/5] seccomp: add spawn " Eduardo Otubo
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange, thuth
This patch introduces the new argument [,elevateprivileges=deny] to the
`-sandbox on'. It avoids Qemu process to elevate its privileges by
blacklisting all set*uid|gid system calls
Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
include/sysemu/seccomp.h | 1 +
qemu-options.hx | 8 ++++++--
qemu-seccomp.c | 28 ++++++++++++++++++++++++++++
vl.c | 11 +++++++++++
4 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index 7a7bde246b..e6e78d85ce 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -16,6 +16,7 @@
#define QEMU_SECCOMP_H
#define OBSOLETE 0x0001
+#define PRIVILEGED 0x0010
#include <seccomp.h>
diff --git a/qemu-options.hx b/qemu-options.hx
index 1403d0c85f..47018db5aa 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3732,8 +3732,10 @@ Old param mode (ARM only).
ETEXI
DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
- "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
- " obsolete: Allow obsolete system calls",
+ "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
+ " Enable seccomp mode 2 system call filter (default 'off').\n" \
+ " obsolete: Allow obsolete system calls\n" \
+ " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
QEMU_ARCH_ALL)
STEXI
@item -sandbox @var{arg}[,obsolete=@var{string}]
@@ -3743,6 +3745,8 @@ disable it. The default is 'off'.
@table @option
@item obsolete=@var{string}
Enable Obsolete system calls
+@item elevateprivileges=@var{string}
+Disable set*uid|gid systema calls
@end table
ETEXI
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 5ef36890da..5aa6590386 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -31,6 +31,19 @@ struct QemuSeccompSyscall {
uint8_t priority;
};
+static const struct QemuSeccompSyscall privileged_syscalls[] = {
+ { SCMP_SYS(setuid), 255 },
+ { SCMP_SYS(setgid), 255 },
+ { SCMP_SYS(setpgid), 255 },
+ { SCMP_SYS(setsid), 255 },
+ { SCMP_SYS(setreuid), 255 },
+ { SCMP_SYS(setregid), 255 },
+ { SCMP_SYS(setresuid), 255 },
+ { SCMP_SYS(setresgid), 255 },
+ { SCMP_SYS(setfsuid), 255 },
+ { SCMP_SYS(setfsgid), 255 },
+};
+
static const struct QemuSeccompSyscall obsolete[] = {
{ SCMP_SYS(readdir), 255 },
{ SCMP_SYS(_sysctl), 255 },
@@ -125,6 +138,21 @@ int seccomp_start(uint8_t seccomp_opts)
}
}
+ if (seccomp_opts & PRIVILEGED) {
+ for (i = 0; i < ARRAY_SIZE(privileged_syscalls); i++) {
+ rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, privileged_syscalls[i].num, 0);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ rc = seccomp_syscall_priority(ctx, privileged_syscalls[i].num,
+ privileged_syscalls[i].priority);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ }
+ }
+
+
rc = seccomp_load(ctx);
seccomp_return:
diff --git a/vl.c b/vl.c
index 7b08b3383b..d071e240b0 100644
--- a/vl.c
+++ b/vl.c
@@ -273,6 +273,10 @@ static QemuOptsList qemu_sandbox_opts = {
.name = "obsolete",
.type = QEMU_OPT_STRING,
},
+ {
+ .name = "elevateprivileges",
+ .type = QEMU_OPT_STRING,
+ },
{ /* end of list */ }
},
};
@@ -1045,6 +1049,13 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
}
}
+ value = qemu_opt_get(opts,"elevateprivileges");
+ if (value) {
+ if (strcmp(value, "deny") == 0) {
+ seccomp_opts |= PRIVILEGED;
+ }
+ }
+
if (seccomp_start(seccomp_opts) < 0) {
error_report("failed to install seccomp syscall filter "
"in the kernel");
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 4/5] seccomp: add spawn argument to command line
2017-03-14 11:32 [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring Eduardo Otubo
` (2 preceding siblings ...)
2017-03-14 11:32 ` [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges " Eduardo Otubo
@ 2017-03-14 11:32 ` Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 5/5] seccomp: add resourcecontrol " Eduardo Otubo
2017-03-14 11:40 ` [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring no-reply
5 siblings, 0 replies; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange, thuth
This patch adds [,spawn=deny] argument to `-sandbox on' option. It
blacklists fork and execve syste calls, avoiding Qemu to spawn new
threads or processes.
Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
include/sysemu/seccomp.h | 1 +
qemu-options.hx | 7 +++++--
qemu-seccomp.c | 18 ++++++++++++++++++
vl.c | 11 +++++++++++
4 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index e6e78d85ce..f1614d6514 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -17,6 +17,7 @@
#define OBSOLETE 0x0001
#define PRIVILEGED 0x0010
+#define SPAWN 0x0100
#include <seccomp.h>
diff --git a/qemu-options.hx b/qemu-options.hx
index 47018db5aa..53f4f8cfd2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3732,10 +3732,11 @@ Old param mode (ARM only).
ETEXI
DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
- "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
+ "-sandbox on[,obsolete=allow][,elevateprivileges=deny][,spawn=deny]" \
" Enable seccomp mode 2 system call filter (default 'off').\n" \
" obsolete: Allow obsolete system calls\n" \
- " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
+ " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls\n" \
+ " spawn: avoids Qemu to spawn new threads or processes by blacklisting *fork and execve\n"
QEMU_ARCH_ALL)
STEXI
@item -sandbox @var{arg}[,obsolete=@var{string}]
@@ -3747,6 +3748,8 @@ disable it. The default is 'off'.
Enable Obsolete system calls
@item elevateprivileges=@var{string}
Disable set*uid|gid systema calls
+@item spawn=@var{string}
+Disable *fork and execve
@end table
ETEXI
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 5aa6590386..4c1f7b41ba 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -31,6 +31,12 @@ struct QemuSeccompSyscall {
uint8_t priority;
};
+static const struct QemuSeccompSyscall spawn_syscalls[] = {
+ { SCMP_SYS(fork), 255 },
+ { SCMP_SYS(vfork), 255 },
+ { SCMP_SYS(execve), 255 },
+};
+
static const struct QemuSeccompSyscall privileged_syscalls[] = {
{ SCMP_SYS(setuid), 255 },
{ SCMP_SYS(setgid), 255 },
@@ -152,6 +158,18 @@ int seccomp_start(uint8_t seccomp_opts)
}
}
+ if (seccomp_opts & SPAWN) {
+ for (i = 0; i < ARRAY_SIZE(spawn_syscalls); i++) {
+ rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, spawn_syscalls[i].num, 0);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ rc = seccomp_syscall_priority(ctx, spawn_syscalls[i].num, spawn_syscalls[i].priority);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ }
+ }
rc = seccomp_load(ctx);
diff --git a/vl.c b/vl.c
index d071e240b0..6a6e9a69bf 100644
--- a/vl.c
+++ b/vl.c
@@ -277,6 +277,10 @@ static QemuOptsList qemu_sandbox_opts = {
.name = "elevateprivileges",
.type = QEMU_OPT_STRING,
},
+ {
+ .name = "spawn",
+ .type = QEMU_OPT_STRING,
+ },
{ /* end of list */ }
},
};
@@ -1056,6 +1060,13 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
}
}
+ value = qemu_opt_get(opts,"spawn");
+ if (value) {
+ if (strcmp(value, "deny") == 0) {
+ seccomp_opts |= SPAWN;
+ }
+ }
+
if (seccomp_start(seccomp_opts) < 0) {
error_report("failed to install seccomp syscall filter "
"in the kernel");
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 5/5] seccomp: add resourcecontrol argument to command line
2017-03-14 11:32 [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring Eduardo Otubo
` (3 preceding siblings ...)
2017-03-14 11:32 ` [Qemu-devel] [PATCH 4/5] seccomp: add spawn " Eduardo Otubo
@ 2017-03-14 11:32 ` Eduardo Otubo
2017-03-14 11:40 ` [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring no-reply
5 siblings, 0 replies; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 11:32 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange, thuth
This patch adds [,resourcecontrol=deny] to `-sandbox on' option. It
blacklists all process affinity and scheduler priority system calls to
avoid any bigger of the process.
Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
include/sysemu/seccomp.h | 1 +
qemu-options.hx | 5 ++++-
qemu-seccomp.c | 26 ++++++++++++++++++++++++++
vl.c | 11 +++++++++++
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index f1614d6514..c7003dd197 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -18,6 +18,7 @@
#define OBSOLETE 0x0001
#define PRIVILEGED 0x0010
#define SPAWN 0x0100
+#define RESOURCECTL 0x1000
#include <seccomp.h>
diff --git a/qemu-options.hx b/qemu-options.hx
index 53f4f8cfd2..5784ffe4b1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3732,11 +3732,12 @@ Old param mode (ARM only).
ETEXI
DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
- "-sandbox on[,obsolete=allow][,elevateprivileges=deny][,spawn=deny]" \
+ "-sandbox on[,obsolete=allow][,elevateprivileges=deny][,spawn=deny][,resourcecontrol=deny]\n" \
" Enable seccomp mode 2 system call filter (default 'off').\n" \
" obsolete: Allow obsolete system calls\n" \
" elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls\n" \
" spawn: avoids Qemu to spawn new threads or processes by blacklisting *fork and execve\n"
+ " resourcecontrol: disable process affinity and schedular priority\n",
QEMU_ARCH_ALL)
STEXI
@item -sandbox @var{arg}[,obsolete=@var{string}]
@@ -3750,6 +3751,8 @@ Enable Obsolete system calls
Disable set*uid|gid systema calls
@item spawn=@var{string}
Disable *fork and execve
+@item resourcecontrol=@var{string}
+Disable process affinity and schedular priority
@end table
ETEXI
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 4c1f7b41ba..dec47e9a74 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -31,6 +31,19 @@ struct QemuSeccompSyscall {
uint8_t priority;
};
+static const struct QemuSeccompSyscall resourcecontrol_syscalls[] = {
+ { SCMP_SYS(getpriority), 255 },
+ { SCMP_SYS(setpriority), 255 },
+ { SCMP_SYS(sched_setparam), 255 },
+ { SCMP_SYS(sched_getparam), 255 },
+ { SCMP_SYS(sched_setscheduler), 255 },
+ { SCMP_SYS(sched_getscheduler), 255 },
+ { SCMP_SYS(sched_setaffinity), 255 },
+ { SCMP_SYS(sched_getaffinity), 255 },
+ { SCMP_SYS(sched_get_priority_max), 255 },
+ { SCMP_SYS(sched_get_priority_min), 255 },
+};
+
static const struct QemuSeccompSyscall spawn_syscalls[] = {
{ SCMP_SYS(fork), 255 },
{ SCMP_SYS(vfork), 255 },
@@ -171,6 +184,19 @@ int seccomp_start(uint8_t seccomp_opts)
}
}
+ if (seccomp_opts & RESOURCECTL) {
+ for (i = 0; i < ARRAY_SIZE(resourcecontrol_syscalls); i++) {
+ rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, resourcecontrol_syscalls[i].num, 0);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ rc = seccomp_syscall_priority(ctx, resourcecontrol_syscalls[i].num, resourcecontrol_syscalls[i].priority);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ }
+ }
+
rc = seccomp_load(ctx);
seccomp_return:
diff --git a/vl.c b/vl.c
index 6a6e9a69bf..3ceffef094 100644
--- a/vl.c
+++ b/vl.c
@@ -281,6 +281,10 @@ static QemuOptsList qemu_sandbox_opts = {
.name = "spawn",
.type = QEMU_OPT_STRING,
},
+ {
+ .name = "resourcecontrol",
+ .type = QEMU_OPT_STRING,
+ },
{ /* end of list */ }
},
};
@@ -1067,6 +1071,13 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
}
}
+ value = qemu_opt_get(opts,"resourcecontrol");
+ if (value) {
+ if (strcmp(value, "deny") == 0) {
+ seccomp_opts |= RESOURCECTL;
+ }
+ }
+
if (seccomp_start(seccomp_opts) < 0) {
error_report("failed to install seccomp syscall filter "
"in the kernel");
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring
2017-03-14 11:32 [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring Eduardo Otubo
` (4 preceding siblings ...)
2017-03-14 11:32 ` [Qemu-devel] [PATCH 5/5] seccomp: add resourcecontrol " Eduardo Otubo
@ 2017-03-14 11:40 ` no-reply
5 siblings, 0 replies; 20+ messages in thread
From: no-reply @ 2017-03-14 11:40 UTC (permalink / raw)
To: eduardo.otubo; +Cc: famz, qemu-devel, thuth
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring
Message-id: 20170314113209.12025-1-eduardo.otubo@profitbricks.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1489491125-23648-1-git-send-email-peter.maydell@linaro.org -> patchew/1489491125-23648-1-git-send-email-peter.maydell@linaro.org
* [new tag] patchew/20170314113209.12025-1-eduardo.otubo@profitbricks.com -> patchew/20170314113209.12025-1-eduardo.otubo@profitbricks.com
Switched to a new branch 'test'
3f499bf seccomp: add resourcecontrol argument to command line
3ffba6f seccomp: add spawn argument to command line
a830fdd seccomp: add elevateprivileges argument to command line
54c6390 seccomp: add obsolete argument to command line
7759286 seccomp: changing from whitelist to blacklist
=== OUTPUT BEGIN ===
Checking PATCH 1/5: seccomp: changing from whitelist to blacklist...
Checking PATCH 2/5: seccomp: add obsolete argument to command line...
ERROR: "foo * bar" should be "foo *bar"
#146: FILE: vl.c:1039:
+ const char * value = NULL;
ERROR: space required after that ',' (ctx:VxV)
#148: FILE: vl.c:1041:
+ value = qemu_opt_get(opts,"obsolete");
^
total: 2 errors, 0 warnings, 123 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/5: seccomp: add elevateprivileges argument to command line...
WARNING: line over 80 characters
#81: FILE: qemu-seccomp.c:143:
+ rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, privileged_syscalls[i].num, 0);
ERROR: space required after that ',' (ctx:VxV)
#116: FILE: vl.c:1052:
+ value = qemu_opt_get(opts,"elevateprivileges");
^
total: 1 errors, 1 warnings, 90 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/5: seccomp: add spawn argument to command line...
ERROR: line over 90 characters
#79: FILE: qemu-seccomp.c:167:
+ rc = seccomp_syscall_priority(ctx, spawn_syscalls[i].num, spawn_syscalls[i].priority);
ERROR: space required after that ',' (ctx:VxV)
#107: FILE: vl.c:1063:
+ value = qemu_opt_get(opts,"spawn");
^
total: 2 errors, 0 warnings, 81 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 5/5: seccomp: add resourcecontrol argument to command line...
WARNING: line over 80 characters
#82: FILE: qemu-seccomp.c:189:
+ rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, resourcecontrol_syscalls[i].num, 0);
ERROR: line over 90 characters
#86: FILE: qemu-seccomp.c:193:
+ rc = seccomp_syscall_priority(ctx, resourcecontrol_syscalls[i].num, resourcecontrol_syscalls[i].priority);
ERROR: space required after that ',' (ctx:VxV)
#115: FILE: vl.c:1074:
+ value = qemu_opt_get(opts,"resourcecontrol");
^
total: 2 errors, 1 warnings, 89 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 11:32 ` [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges " Eduardo Otubo
@ 2017-03-14 11:47 ` Paolo Bonzini
2017-03-14 11:52 ` Daniel P. Berrange
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2017-03-14 11:47 UTC (permalink / raw)
To: Eduardo Otubo, qemu-devel; +Cc: thuth
On 14/03/2017 12:32, Eduardo Otubo wrote:
> This patch introduces the new argument [,elevateprivileges=deny] to the
> `-sandbox on'. It avoids Qemu process to elevate its privileges by
> blacklisting all set*uid|gid system calls
>
> Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> ---
> include/sysemu/seccomp.h | 1 +
> qemu-options.hx | 8 ++++++--
> qemu-seccomp.c | 28 ++++++++++++++++++++++++++++
> vl.c | 11 +++++++++++
> 4 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
> index 7a7bde246b..e6e78d85ce 100644
> --- a/include/sysemu/seccomp.h
> +++ b/include/sysemu/seccomp.h
> @@ -16,6 +16,7 @@
> #define QEMU_SECCOMP_H
>
> #define OBSOLETE 0x0001
> +#define PRIVILEGED 0x0010
>
> #include <seccomp.h>
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1403d0c85f..47018db5aa 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3732,8 +3732,10 @@ Old param mode (ARM only).
> ETEXI
>
> DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
> - "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
> - " obsolete: Allow obsolete system calls",
> + "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
> + " Enable seccomp mode 2 system call filter (default 'off').\n" \
> + " obsolete: Allow obsolete system calls\n" \
> + " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
Why allow these by default?
Paolo
> QEMU_ARCH_ALL)
> STEXI
> @item -sandbox @var{arg}[,obsolete=@var{string}]
> @@ -3743,6 +3745,8 @@ disable it. The default is 'off'.
> @table @option
> @item obsolete=@var{string}
> Enable Obsolete system calls
> +@item elevateprivileges=@var{string}
> +Disable set*uid|gid systema calls
> @end table
> ETEXI
>
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index 5ef36890da..5aa6590386 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -31,6 +31,19 @@ struct QemuSeccompSyscall {
> uint8_t priority;
> };
>
> +static const struct QemuSeccompSyscall privileged_syscalls[] = {
> + { SCMP_SYS(setuid), 255 },
> + { SCMP_SYS(setgid), 255 },
> + { SCMP_SYS(setpgid), 255 },
> + { SCMP_SYS(setsid), 255 },
> + { SCMP_SYS(setreuid), 255 },
> + { SCMP_SYS(setregid), 255 },
> + { SCMP_SYS(setresuid), 255 },
> + { SCMP_SYS(setresgid), 255 },
> + { SCMP_SYS(setfsuid), 255 },
> + { SCMP_SYS(setfsgid), 255 },
> +};
> +
> static const struct QemuSeccompSyscall obsolete[] = {
> { SCMP_SYS(readdir), 255 },
> { SCMP_SYS(_sysctl), 255 },
> @@ -125,6 +138,21 @@ int seccomp_start(uint8_t seccomp_opts)
> }
> }
>
> + if (seccomp_opts & PRIVILEGED) {
> + for (i = 0; i < ARRAY_SIZE(privileged_syscalls); i++) {
> + rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, privileged_syscalls[i].num, 0);
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> + rc = seccomp_syscall_priority(ctx, privileged_syscalls[i].num,
> + privileged_syscalls[i].priority);
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> + }
> + }
> +
> +
> rc = seccomp_load(ctx);
>
> seccomp_return:
> diff --git a/vl.c b/vl.c
> index 7b08b3383b..d071e240b0 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -273,6 +273,10 @@ static QemuOptsList qemu_sandbox_opts = {
> .name = "obsolete",
> .type = QEMU_OPT_STRING,
> },
> + {
> + .name = "elevateprivileges",
> + .type = QEMU_OPT_STRING,
> + },
> { /* end of list */ }
> },
> };
> @@ -1045,6 +1049,13 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
> }
> }
>
> + value = qemu_opt_get(opts,"elevateprivileges");
> + if (value) {
> + if (strcmp(value, "deny") == 0) {
> + seccomp_opts |= PRIVILEGED;
> + }
> + }
> +
> if (seccomp_start(seccomp_opts) < 0) {
> error_report("failed to install seccomp syscall filter "
> "in the kernel");
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line
2017-03-14 11:32 ` [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line Eduardo Otubo
@ 2017-03-14 11:47 ` Paolo Bonzini
2017-03-14 12:01 ` Thomas Huth
1 sibling, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2017-03-14 11:47 UTC (permalink / raw)
To: Eduardo Otubo, qemu-devel; +Cc: thuth
On 14/03/2017 12:32, Eduardo Otubo wrote:
> +static const struct QemuSeccompSyscall obsolete[] = {
> + { SCMP_SYS(readdir), 255 },
> + { SCMP_SYS(_sysctl), 255 },
> + { SCMP_SYS(afs_syscall), 255 },
> + { SCMP_SYS(bdflush), 255 },
> + { SCMP_SYS(break), 255 },
> + { SCMP_SYS(create_module), 255 },
> + { SCMP_SYS(ftime), 255 },
> + { SCMP_SYS(get_kernel_syms), 255 },
> + { SCMP_SYS(getpmsg), 255 },
> + { SCMP_SYS(gtty), 255 },
> + { SCMP_SYS(lock), 255 },
> + { SCMP_SYS(mpx), 255 },
> + { SCMP_SYS(prof), 255 },
> + { SCMP_SYS(profil), 255 },
> + { SCMP_SYS(putpmsg), 255 },
> + { SCMP_SYS(query_module), 255 },
> + { SCMP_SYS(security), 255 },
> + { SCMP_SYS(sgetmask), 255 },
> + { SCMP_SYS(ssetmask), 255 },
> + { SCMP_SYS(stty), 255 },
> + { SCMP_SYS(sysfs), 255 },
> + { SCMP_SYS(tuxcall), 255 },
> + { SCMP_SYS(ulimit), 255 },
> + { SCMP_SYS(uselib), 255 },
> + { SCMP_SYS(ustat), 255 },
> + { SCMP_SYS(vserver), 255 },
> +};
Some of these are already blacklisted in patch 1.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 11:47 ` Paolo Bonzini
@ 2017-03-14 11:52 ` Daniel P. Berrange
2017-03-14 12:13 ` Paolo Bonzini
0 siblings, 1 reply; 20+ messages in thread
From: Daniel P. Berrange @ 2017-03-14 11:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Eduardo Otubo, qemu-devel, thuth
On Tue, Mar 14, 2017 at 12:47:10PM +0100, Paolo Bonzini wrote:
>
>
> On 14/03/2017 12:32, Eduardo Otubo wrote:
> > This patch introduces the new argument [,elevateprivileges=deny] to the
> > `-sandbox on'. It avoids Qemu process to elevate its privileges by
> > blacklisting all set*uid|gid system calls
> >
> > Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> > ---
> > include/sysemu/seccomp.h | 1 +
> > qemu-options.hx | 8 ++++++--
> > qemu-seccomp.c | 28 ++++++++++++++++++++++++++++
> > vl.c | 11 +++++++++++
> > 4 files changed, 46 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
> > index 7a7bde246b..e6e78d85ce 100644
> > --- a/include/sysemu/seccomp.h
> > +++ b/include/sysemu/seccomp.h
> > @@ -16,6 +16,7 @@
> > #define QEMU_SECCOMP_H
> >
> > #define OBSOLETE 0x0001
> > +#define PRIVILEGED 0x0010
> >
> > #include <seccomp.h>
> >
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 1403d0c85f..47018db5aa 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -3732,8 +3732,10 @@ Old param mode (ARM only).
> > ETEXI
> >
> > DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
> > - "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
> > - " obsolete: Allow obsolete system calls",
> > + "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
> > + " Enable seccomp mode 2 system call filter (default 'off').\n" \
> > + " obsolete: Allow obsolete system calls\n" \
> > + " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
>
> Why allow these by default?
The goal is that '-sandbox on' should not break *any* QEMU feature. It
needs to be a safe thing that people can unconditionally turn on without
thinking about it. The QEMU bridge helper requires setuid privs, hence
elevated privileges needs to be permitted by default. Similarly I could
see the qemu ifup scripts, or migrate 'exec' command wanting to elevate
privs via setuid binaries if QEMU was running unprivileged.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line
2017-03-14 11:32 ` [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line Eduardo Otubo
2017-03-14 11:47 ` Paolo Bonzini
@ 2017-03-14 12:01 ` Thomas Huth
2017-03-14 12:22 ` Eduardo Otubo
1 sibling, 1 reply; 20+ messages in thread
From: Thomas Huth @ 2017-03-14 12:01 UTC (permalink / raw)
To: Eduardo Otubo, qemu-devel
On 14.03.2017 12:32, Eduardo Otubo wrote:
> This patch introduces the argument [,obsolete=allow] to the `-sandbox on'
> option. It allows Qemu to run safely on old system that still relies on
> old system calls.
>
> Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> ---
> include/sysemu/seccomp.h | 4 +++-
> qemu-options.hx | 9 +++++++--
> qemu-seccomp.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> vl.c | 16 +++++++++++++++-
> 4 files changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
> index cfc06008cb..7a7bde246b 100644
> --- a/include/sysemu/seccomp.h
> +++ b/include/sysemu/seccomp.h
> @@ -15,7 +15,9 @@
> #ifndef QEMU_SECCOMP_H
> #define QEMU_SECCOMP_H
>
> +#define OBSOLETE 0x0001
> +
> #include <seccomp.h>
>
> -int seccomp_start(void);
> +int seccomp_start(uint8_t seccomp_opts);
> #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8dd8ee34a6..1403d0c85f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3732,13 +3732,18 @@ Old param mode (ARM only).
> ETEXI
>
> DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
> - "-sandbox <arg> Enable seccomp mode 2 system call filter (default 'off').\n",
> + "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
> + " obsolete: Allow obsolete system calls",
> QEMU_ARCH_ALL)
> STEXI
> -@item -sandbox @var{arg}
> +@item -sandbox @var{arg}[,obsolete=@var{string}]
> @findex -sandbox
> Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering and 'off' will
> disable it. The default is 'off'.
> +@table @option
> +@item obsolete=@var{string}
> +Enable Obsolete system calls
> +@end table
> ETEXI
>
> DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index f8877b07b5..5ef36890da 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -31,6 +31,35 @@ struct QemuSeccompSyscall {
> uint8_t priority;
> };
>
> +static const struct QemuSeccompSyscall obsolete[] = {
> + { SCMP_SYS(readdir), 255 },
> + { SCMP_SYS(_sysctl), 255 },
> + { SCMP_SYS(afs_syscall), 255 },
> + { SCMP_SYS(bdflush), 255 },
> + { SCMP_SYS(break), 255 },
> + { SCMP_SYS(create_module), 255 },
> + { SCMP_SYS(ftime), 255 },
> + { SCMP_SYS(get_kernel_syms), 255 },
> + { SCMP_SYS(getpmsg), 255 },
> + { SCMP_SYS(gtty), 255 },
> + { SCMP_SYS(lock), 255 },
> + { SCMP_SYS(mpx), 255 },
> + { SCMP_SYS(prof), 255 },
> + { SCMP_SYS(profil), 255 },
> + { SCMP_SYS(putpmsg), 255 },
> + { SCMP_SYS(query_module), 255 },
> + { SCMP_SYS(security), 255 },
> + { SCMP_SYS(sgetmask), 255 },
> + { SCMP_SYS(ssetmask), 255 },
> + { SCMP_SYS(stty), 255 },
> + { SCMP_SYS(sysfs), 255 },
> + { SCMP_SYS(tuxcall), 255 },
> + { SCMP_SYS(ulimit), 255 },
> + { SCMP_SYS(uselib), 255 },
> + { SCMP_SYS(ustat), 255 },
> + { SCMP_SYS(vserver), 255 },
> +};
> +
> static const struct QemuSeccompSyscall blacklist[] = {
> { SCMP_SYS(reboot), 255 },
> { SCMP_SYS(swapon), 255 },
> @@ -56,7 +85,20 @@ static const struct QemuSeccompSyscall blacklist[] = {
> { SCMP_SYS(vserver), 255 },
> };
>
> -int seccomp_start(void)
> +static int is_obsolete(int syscall)
> +{
> + unsigned int i = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(obsolete); i++) {
> + if (syscall == obsolete[i].num) {
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +int seccomp_start(uint8_t seccomp_opts)
> {
> int rc = 0;
> unsigned int i = 0;
> @@ -69,6 +111,9 @@ int seccomp_start(void)
> }
>
> for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
> + if ((seccomp_opts & OBSOLETE) && is_obsolete(blacklist[i].num)) {
> + continue;
> + }
> rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, blacklist[i].num, 0);
> if (rc < 0) {
> goto seccomp_return;
> diff --git a/vl.c b/vl.c
> index 1a95500ac7..7b08b3383b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -269,6 +269,10 @@ static QemuOptsList qemu_sandbox_opts = {
> .name = "enable",
> .type = QEMU_OPT_BOOL,
> },
> + {
> + .name = "obsolete",
> + .type = QEMU_OPT_STRING,
> + },
Since this is basically a boolean switch, could you maybe rather use
"QEMU_OPT_BOOL" here, too (and also in the following patches)? I think
it is also easier for the users to memorize "obsolete=on" instead of
yet-another-parameter-syntax like "obsolete=allow".
Thomas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 11:52 ` Daniel P. Berrange
@ 2017-03-14 12:13 ` Paolo Bonzini
2017-03-14 12:24 ` Daniel P. Berrange
2017-03-14 12:32 ` Eduardo Otubo
0 siblings, 2 replies; 20+ messages in thread
From: Paolo Bonzini @ 2017-03-14 12:13 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Eduardo Otubo, qemu-devel, thuth, Andy Lutomirski
On 14/03/2017 12:52, Daniel P. Berrange wrote:
>>> DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
>>> - "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
>>> - " obsolete: Allow obsolete system calls",
>>> + "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
>>> + " Enable seccomp mode 2 system call filter (default 'off').\n" \
>>> + " obsolete: Allow obsolete system calls\n" \
>>> + " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
>> Why allow these by default?
> The goal is that '-sandbox on' should not break *any* QEMU feature. It
> needs to be a safe thing that people can unconditionally turn on without
> thinking about it.
Sure, but what advantages would it provide if the default blacklist does
not block anything meaningful? At the very least, spawn=deny should
default elevateprivileges to deny too.
I think there should be a list (as small as possible) of features that
are sacrificed by "-sandbox on".
> The QEMU bridge helper requires setuid privs, hence
> elevated privileges needs to be permitted by default.
QEMU itself should not be getting additional privileges, only the helper
and in turn the helper or ifup scripts can be limited through MAC. The
issue is that seccomp persists across execve.
Currently, unprivileged users are only allowed to install seccomp
filters if no_new_privs is set. Would it make sense if seccomp filters
without no_new_privs succeeded, except that the filter would not persist
across execve of binaries with setuid, setgid or file capabilities?
Then the spawn option could be a tri-state with the choice of allow,
deny and no_new_privs:
- elevateprivileges=allow,spawn=allow: legacy for old kernels
- elevateprivileges=deny,spawn=allow: can run privileged helpers
- elevateprivileges=deny,spawn=deny: cannot run helpers at all
- elevateprivileges=deny,spawn=no_new_privs: can run unprivileged
helpers only
Paolo
> Similarly I could
> see the qemu ifup scripts, or migrate 'exec' command wanting to elevate
> privs via setuid binaries if QEMU was running unprivileged.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line
2017-03-14 12:01 ` Thomas Huth
@ 2017-03-14 12:22 ` Eduardo Otubo
0 siblings, 0 replies; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 12:22 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, berrange
On Tue, Mar 14, 2017 at 01=01=11PM +0100, Thomas Huth wrote:
> On 14.03.2017 12:32, Eduardo Otubo wrote:
> > This patch introduces the argument [,obsolete=allow] to the `-sandbox on'
> > option. It allows Qemu to run safely on old system that still relies on
> > old system calls.
> >
> > Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> > ---
> > include/sysemu/seccomp.h | 4 +++-
> > qemu-options.hx | 9 +++++++--
> > qemu-seccomp.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> > vl.c | 16 +++++++++++++++-
> > 4 files changed, 71 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
> > index cfc06008cb..7a7bde246b 100644
> > --- a/include/sysemu/seccomp.h
> > +++ b/include/sysemu/seccomp.h
> > @@ -15,7 +15,9 @@
> > #ifndef QEMU_SECCOMP_H
> > #define QEMU_SECCOMP_H
> >
> > +#define OBSOLETE 0x0001
> > +
> > #include <seccomp.h>
> >
> > -int seccomp_start(void);
> > +int seccomp_start(uint8_t seccomp_opts);
> > #endif
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 8dd8ee34a6..1403d0c85f 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -3732,13 +3732,18 @@ Old param mode (ARM only).
> > ETEXI
> >
> > DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
> > - "-sandbox <arg> Enable seccomp mode 2 system call filter (default 'off').\n",
> > + "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
> > + " obsolete: Allow obsolete system calls",
> > QEMU_ARCH_ALL)
> > STEXI
> > -@item -sandbox @var{arg}
> > +@item -sandbox @var{arg}[,obsolete=@var{string}]
> > @findex -sandbox
> > Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering and 'off' will
> > disable it. The default is 'off'.
> > +@table @option
> > +@item obsolete=@var{string}
> > +Enable Obsolete system calls
> > +@end table
> > ETEXI
> >
> > DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
> > diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> > index f8877b07b5..5ef36890da 100644
> > --- a/qemu-seccomp.c
> > +++ b/qemu-seccomp.c
> > @@ -31,6 +31,35 @@ struct QemuSeccompSyscall {
> > uint8_t priority;
> > };
> >
> > +static const struct QemuSeccompSyscall obsolete[] = {
> > + { SCMP_SYS(readdir), 255 },
> > + { SCMP_SYS(_sysctl), 255 },
> > + { SCMP_SYS(afs_syscall), 255 },
> > + { SCMP_SYS(bdflush), 255 },
> > + { SCMP_SYS(break), 255 },
> > + { SCMP_SYS(create_module), 255 },
> > + { SCMP_SYS(ftime), 255 },
> > + { SCMP_SYS(get_kernel_syms), 255 },
> > + { SCMP_SYS(getpmsg), 255 },
> > + { SCMP_SYS(gtty), 255 },
> > + { SCMP_SYS(lock), 255 },
> > + { SCMP_SYS(mpx), 255 },
> > + { SCMP_SYS(prof), 255 },
> > + { SCMP_SYS(profil), 255 },
> > + { SCMP_SYS(putpmsg), 255 },
> > + { SCMP_SYS(query_module), 255 },
> > + { SCMP_SYS(security), 255 },
> > + { SCMP_SYS(sgetmask), 255 },
> > + { SCMP_SYS(ssetmask), 255 },
> > + { SCMP_SYS(stty), 255 },
> > + { SCMP_SYS(sysfs), 255 },
> > + { SCMP_SYS(tuxcall), 255 },
> > + { SCMP_SYS(ulimit), 255 },
> > + { SCMP_SYS(uselib), 255 },
> > + { SCMP_SYS(ustat), 255 },
> > + { SCMP_SYS(vserver), 255 },
> > +};
> > +
> > static const struct QemuSeccompSyscall blacklist[] = {
> > { SCMP_SYS(reboot), 255 },
> > { SCMP_SYS(swapon), 255 },
> > @@ -56,7 +85,20 @@ static const struct QemuSeccompSyscall blacklist[] = {
> > { SCMP_SYS(vserver), 255 },
> > };
> >
> > -int seccomp_start(void)
> > +static int is_obsolete(int syscall)
> > +{
> > + unsigned int i = 0;
> > +
> > + for (i = 0; i < ARRAY_SIZE(obsolete); i++) {
> > + if (syscall == obsolete[i].num) {
> > + return 1;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int seccomp_start(uint8_t seccomp_opts)
> > {
> > int rc = 0;
> > unsigned int i = 0;
> > @@ -69,6 +111,9 @@ int seccomp_start(void)
> > }
> >
> > for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
> > + if ((seccomp_opts & OBSOLETE) && is_obsolete(blacklist[i].num)) {
> > + continue;
> > + }
> > rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, blacklist[i].num, 0);
> > if (rc < 0) {
> > goto seccomp_return;
> > diff --git a/vl.c b/vl.c
> > index 1a95500ac7..7b08b3383b 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -269,6 +269,10 @@ static QemuOptsList qemu_sandbox_opts = {
> > .name = "enable",
> > .type = QEMU_OPT_BOOL,
> > },
> > + {
> > + .name = "obsolete",
> > + .type = QEMU_OPT_STRING,
> > + },
>
> Since this is basically a boolean switch, could you maybe rather use
> "QEMU_OPT_BOOL" here, too (and also in the following patches)? I think
> it is also easier for the users to memorize "obsolete=on" instead of
> yet-another-parameter-syntax like "obsolete=allow".
>
Yes, that's definitely in my mind. I just wanted to make the first
draft the same way Daniel designed and evolve from there.
--
Eduardo Otubo
ProfitBricks GmbH
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 12:13 ` Paolo Bonzini
@ 2017-03-14 12:24 ` Daniel P. Berrange
2017-03-14 12:42 ` Eduardo Otubo
2017-03-14 12:32 ` Eduardo Otubo
1 sibling, 1 reply; 20+ messages in thread
From: Daniel P. Berrange @ 2017-03-14 12:24 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Eduardo Otubo, qemu-devel, thuth, Andy Lutomirski
On Tue, Mar 14, 2017 at 01:13:15PM +0100, Paolo Bonzini wrote:
>
>
> On 14/03/2017 12:52, Daniel P. Berrange wrote:
> >>> DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
> >>> - "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
> >>> - " obsolete: Allow obsolete system calls",
> >>> + "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
> >>> + " Enable seccomp mode 2 system call filter (default 'off').\n" \
> >>> + " obsolete: Allow obsolete system calls\n" \
> >>> + " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
> >> Why allow these by default?
> > The goal is that '-sandbox on' should not break *any* QEMU feature. It
> > needs to be a safe thing that people can unconditionally turn on without
> > thinking about it.
>
> Sure, but what advantages would it provide if the default blacklist does
> not block anything meaningful? At the very least, spawn=deny should
> default elevateprivileges to deny too.
Yep, having spawn=deny imply elevateprivileges=deny is reasonable IMHO.
> I think there should be a list (as small as possible) of features that
> are sacrificed by "-sandbox on".
That breaks the key goal that '-sandbox on' should never break a valid
QEMU configuration, no matter how obscure, and would thus continue to
discourage people from turning it on by default.
Yes, a bare '-sandbox on' is very loose, but think of it as just being
a building block. 90% of the time the user or mgmt app would want to
turn on extra flags to lock it down more meaningfully, by explicitly
blocking ability to use feature they know won't be needed.
> > The QEMU bridge helper requires setuid privs, hence
> > elevated privileges needs to be permitted by default.
>
> QEMU itself should not be getting additional privileges, only the helper
> and in turn the helper or ifup scripts can be limited through MAC. The
> issue is that seccomp persists across execve.
That's true.
> Currently, unprivileged users are only allowed to install seccomp
> filters if no_new_privs is set. Would it make sense if seccomp filters
> without no_new_privs succeeded, except that the filter would not persist
> across execve of binaries with setuid, setgid or file capabilities?
>
> Then the spawn option could be a tri-state with the choice of allow,
> deny and no_new_privs:
>
> - elevateprivileges=allow,spawn=allow: legacy for old kernels
> - elevateprivileges=deny,spawn=allow: can run privileged helpers
> - elevateprivileges=deny,spawn=deny: cannot run helpers at all
> - elevateprivileges=deny,spawn=no_new_privs: can run unprivileged
> helpers only
That could work, but I think that syntax is making it uneccessarily
complex to understand. I don't like how it introduces a semantic
dependancy between the elevateprivileges & spawn flags i.e. the
interpretation of elevateprivileges=deny, varies according to what
you set for spawn= option.
I'd be more inclined to make elevateprivileges be a tri-state instead
e.g.
elevateprivileges=allow|deny|children
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 12:13 ` Paolo Bonzini
2017-03-14 12:24 ` Daniel P. Berrange
@ 2017-03-14 12:32 ` Eduardo Otubo
2017-03-14 12:48 ` Paolo Bonzini
1 sibling, 1 reply; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 12:32 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Daniel P. Berrange, qemu-devel, thuth, Andy Lutomirski
On Tue, Mar 14, 2017 at 01=13=15PM +0100, Paolo Bonzini wrote:
>
>
> On 14/03/2017 12:52, Daniel P. Berrange wrote:
> >>> DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
> >>> - "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
> >>> - " obsolete: Allow obsolete system calls",
> >>> + "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
> >>> + " Enable seccomp mode 2 system call filter (default 'off').\n" \
> >>> + " obsolete: Allow obsolete system calls\n" \
> >>> + " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
> >> Why allow these by default?
> > The goal is that '-sandbox on' should not break *any* QEMU feature. It
> > needs to be a safe thing that people can unconditionally turn on without
> > thinking about it.
>
> Sure, but what advantages would it provide if the default blacklist does
> not block anything meaningful? At the very least, spawn=deny should
> default elevateprivileges to deny too.
>
> I think there should be a list (as small as possible) of features that
> are sacrificed by "-sandbox on".
Can you give an example of such a list?
>
> > The QEMU bridge helper requires setuid privs, hence
> > elevated privileges needs to be permitted by default.
>
> QEMU itself should not be getting additional privileges, only the helper
> and in turn the helper or ifup scripts can be limited through MAC. The
> issue is that seccomp persists across execve.
>
> Currently, unprivileged users are only allowed to install seccomp
> filters if no_new_privs is set. Would it make sense if seccomp filters
> without no_new_privs succeeded, except that the filter would not persist
> across execve of binaries with setuid, setgid or file capabilities?
Yes, it does make sense. Using seccomp_attr_set() and the flag
SCMP_FLTATR_CTL_NNP we can disable NO_NEW_PRIVS.
>
> Then the spawn option could be a tri-state with the choice of allow,
> deny and no_new_privs:
>
> - elevateprivileges=allow,spawn=allow: legacy for old kernels
> - elevateprivileges=deny,spawn=allow: can run privileged helpers
> - elevateprivileges=deny,spawn=deny: cannot run helpers at all
> - elevateprivileges=deny,spawn=no_new_privs: can run unprivileged
> helpers only
I think it does look interesting to me this model.
--
Eduardo Otubo
ProfitBricks GmbH
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 12:24 ` Daniel P. Berrange
@ 2017-03-14 12:42 ` Eduardo Otubo
0 siblings, 0 replies; 20+ messages in thread
From: Eduardo Otubo @ 2017-03-14 12:42 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Paolo Bonzini, qemu-devel, thuth, Andy Lutomirski
On Tue, Mar 14, 2017 at 12=24=55PM +0000, Daniel P. Berrange wrote:
> On Tue, Mar 14, 2017 at 01:13:15PM +0100, Paolo Bonzini wrote:
> >
> >
> > On 14/03/2017 12:52, Daniel P. Berrange wrote:
> > >>> DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
> > >>> - "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
> > >>> - " obsolete: Allow obsolete system calls",
> > >>> + "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
> > >>> + " Enable seccomp mode 2 system call filter (default 'off').\n" \
> > >>> + " obsolete: Allow obsolete system calls\n" \
> > >>> + " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
> > >> Why allow these by default?
> > > The goal is that '-sandbox on' should not break *any* QEMU feature. It
> > > needs to be a safe thing that people can unconditionally turn on without
> > > thinking about it.
> >
> > Sure, but what advantages would it provide if the default blacklist does
> > not block anything meaningful? At the very least, spawn=deny should
> > default elevateprivileges to deny too.
>
> Yep, having spawn=deny imply elevateprivileges=deny is reasonable IMHO.
>
> > I think there should be a list (as small as possible) of features that
> > are sacrificed by "-sandbox on".
>
> That breaks the key goal that '-sandbox on' should never break a valid
> QEMU configuration, no matter how obscure, and would thus continue to
> discourage people from turning it on by default.
>
> Yes, a bare '-sandbox on' is very loose, but think of it as just being
> a building block. 90% of the time the user or mgmt app would want to
> turn on extra flags to lock it down more meaningfully, by explicitly
> blocking ability to use feature they know won't be needed.
>
> > > The QEMU bridge helper requires setuid privs, hence
> > > elevated privileges needs to be permitted by default.
> >
> > QEMU itself should not be getting additional privileges, only the helper
> > and in turn the helper or ifup scripts can be limited through MAC. The
> > issue is that seccomp persists across execve.
>
> That's true.
>
> > Currently, unprivileged users are only allowed to install seccomp
> > filters if no_new_privs is set. Would it make sense if seccomp filters
> > without no_new_privs succeeded, except that the filter would not persist
> > across execve of binaries with setuid, setgid or file capabilities?
> >
> > Then the spawn option could be a tri-state with the choice of allow,
> > deny and no_new_privs:
> >
> > - elevateprivileges=allow,spawn=allow: legacy for old kernels
> > - elevateprivileges=deny,spawn=allow: can run privileged helpers
> > - elevateprivileges=deny,spawn=deny: cannot run helpers at all
> > - elevateprivileges=deny,spawn=no_new_privs: can run unprivileged
> > helpers only
>
> That could work, but I think that syntax is making it uneccessarily
> complex to understand. I don't like how it introduces a semantic
> dependancy between the elevateprivileges & spawn flags i.e. the
> interpretation of elevateprivileges=deny, varies according to what
> you set for spawn= option.
>
> I'd be more inclined to make elevateprivileges be a tri-state instead
> e.g.
>
> elevateprivileges=allow|deny|children
>
Still weird but better than the combination of elevateprivileges and
spawn. Perhaps this has a better semantics?
elevateprivileges=allow|deny|no_new_privs
--
Eduardo Otubo
ProfitBricks GmbH
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 12:32 ` Eduardo Otubo
@ 2017-03-14 12:48 ` Paolo Bonzini
2017-03-14 13:02 ` Daniel P. Berrange
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2017-03-14 12:48 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel, thuth, Andy Lutomirski
On 14/03/2017 13:32, Eduardo Otubo wrote:
> On Tue, Mar 14, 2017 at 01=13=15PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 14/03/2017 12:52, Daniel P. Berrange wrote:
>>>>> DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
>>>>> - "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
>>>>> - " obsolete: Allow obsolete system calls",
>>>>> + "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
>>>>> + " Enable seccomp mode 2 system call filter (default 'off').\n" \
>>>>> + " obsolete: Allow obsolete system calls\n" \
>>>>> + " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
>>>> Why allow these by default?
>>> The goal is that '-sandbox on' should not break *any* QEMU feature. It
>>> needs to be a safe thing that people can unconditionally turn on without
>>> thinking about it.
>>
>> Sure, but what advantages would it provide if the default blacklist does
>> not block anything meaningful? At the very least, spawn=deny should
>> default elevateprivileges to deny too.
>>
>> I think there should be a list (as small as possible) of features that
>> are sacrificed by "-sandbox on".
>
> Can you give an example of such a list?
Well, anything that makes "-sandbox on" more than security theater... I
would consider it a necessary evil, not a good thing to have such a list.
Due to NO_NEW_PRIVS, I think non-migration fork/exec should be on the
list, but hopefully nothing else.
>>> The QEMU bridge helper requires setuid privs, hence
>>> elevated privileges needs to be permitted by default.
>>
>> QEMU itself should not be getting additional privileges, only the helper
>> and in turn the helper or ifup scripts can be limited through MAC. The
>> issue is that seccomp persists across execve.
>>
>> Currently, unprivileged users are only allowed to install seccomp
>> filters if no_new_privs is set. Would it make sense if seccomp filters
>> without no_new_privs succeeded, except that the filter would not persist
>> across execve of binaries with setuid, setgid or file capabilities?
>
> Yes, it does make sense. Using seccomp_attr_set() and the flag
> SCMP_FLTATR_CTL_NNP we can disable NO_NEW_PRIVS.
That however in turn requires CAP_SYS_ADMIN. Without kernel changes,
it's not possible.
Paolo
>>
>> Then the spawn option could be a tri-state with the choice of allow,
>> deny and no_new_privs:
>>
>> - elevateprivileges=allow,spawn=allow: legacy for old kernels
>> - elevateprivileges=deny,spawn=allow: can run privileged helpers
>> - elevateprivileges=deny,spawn=deny: cannot run helpers at all
>> - elevateprivileges=deny,spawn=no_new_privs: can run unprivileged
>> helpers only
>
> I think it does look interesting to me this model.
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 12:48 ` Paolo Bonzini
@ 2017-03-14 13:02 ` Daniel P. Berrange
2017-03-14 13:05 ` Paolo Bonzini
0 siblings, 1 reply; 20+ messages in thread
From: Daniel P. Berrange @ 2017-03-14 13:02 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, thuth, Andy Lutomirski
On Tue, Mar 14, 2017 at 01:48:59PM +0100, Paolo Bonzini wrote:
>
>
> On 14/03/2017 13:32, Eduardo Otubo wrote:
> > On Tue, Mar 14, 2017 at 01=13=15PM +0100, Paolo Bonzini wrote:
> >>
> >>
> >> On 14/03/2017 12:52, Daniel P. Berrange wrote:
> >>>>> DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
> >>>>> - "-sandbox on[,obsolete=allow] Enable seccomp mode 2 system call filter (default 'off').\n" \
> >>>>> - " obsolete: Allow obsolete system calls",
> >>>>> + "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
> >>>>> + " Enable seccomp mode 2 system call filter (default 'off').\n" \
> >>>>> + " obsolete: Allow obsolete system calls\n" \
> >>>>> + " elevateprivileges: avoids Qemu process to elevate its privileges by blacklisting all set*uid|gid system calls",
> >>>> Why allow these by default?
> >>> The goal is that '-sandbox on' should not break *any* QEMU feature. It
> >>> needs to be a safe thing that people can unconditionally turn on without
> >>> thinking about it.
> >>
> >> Sure, but what advantages would it provide if the default blacklist does
> >> not block anything meaningful? At the very least, spawn=deny should
> >> default elevateprivileges to deny too.
> >>
> >> I think there should be a list (as small as possible) of features that
> >> are sacrificed by "-sandbox on".
> >
> > Can you give an example of such a list?
>
> Well, anything that makes "-sandbox on" more than security theater... I
> would consider it a necessary evil, not a good thing to have such a list.
> Due to NO_NEW_PRIVS, I think non-migration fork/exec should be on the
> list, but hopefully nothing else.
The current semantics of '-sandbox on' are that it is documented to not
break any valid QEMU features. By blocking fork/exec, we make a semantic
change to the existing behaviour of '-sandbox on'. So any application
which has been using '-sandbox on' historically, is at risk of being
broken when upgrading to QEMU 2.10. It would force the application to
generate a different CLI for new QEMU - ie to avoid being broken they
would have to now add elevateprivs=allow, spawn=allow. I think we have
to maintain semantic compat with existing usage, which means that
'-sandbox on' has to remain security theatre.
So if we want a default config to be more restrictive, then I think you
realistically have to turn the default parameter into a tri-state
instead of bool, ie allow '-sandbox default' as an alternative to
'-sandbox on' that was slightly stronger by blocking fork/exec.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 13:02 ` Daniel P. Berrange
@ 2017-03-14 13:05 ` Paolo Bonzini
2017-03-14 13:19 ` Daniel P. Berrange
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2017-03-14 13:05 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: qemu-devel, thuth, Andy Lutomirski
On 14/03/2017 14:02, Daniel P. Berrange wrote:
>>> Can you give an example of such a list?
>>
>> Well, anything that makes "-sandbox on" more than security theater... I
>> would consider it a necessary evil, not a good thing to have such a list.
>>
>> Due to NO_NEW_PRIVS, I think non-migration fork/exec should be on the
>> list, but hopefully nothing else.
>
> The current semantics of '-sandbox on' are that it is documented to not
> break any valid QEMU features. By blocking fork/exec, we make a semantic
> change to the existing behaviour of '-sandbox on'.
I don't propose to block fork/exec. I propose not to whitelist setuid,
as is the case now too. It wouldn't be backwards-incompatible.
> So any application
> which has been using '-sandbox on' historically, is at risk of being
> broken when upgrading to QEMU 2.10. It would force the application to
> generate a different CLI for new QEMU - ie to avoid being broken they
> would have to now add elevateprivs=allow, spawn=allow. I think we have
> to maintain semantic compat with existing usage, which means that
> '-sandbox on' has to remain security theatre.
>
> So if we want a default config to be more restrictive, then I think you
> realistically have to turn the default parameter into a tri-state
> instead of bool, ie allow '-sandbox default' as an alternative to
> '-sandbox on' that was slightly stronger by blocking fork/exec.
Or just print a warning that "-sandbox on" is useless. I guess that
would be okay too if we want to be "stronger" than backwards-compatible.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line
2017-03-14 13:05 ` Paolo Bonzini
@ 2017-03-14 13:19 ` Daniel P. Berrange
0 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2017-03-14 13:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, thuth, Andy Lutomirski
On Tue, Mar 14, 2017 at 02:05:32PM +0100, Paolo Bonzini wrote:
>
>
> On 14/03/2017 14:02, Daniel P. Berrange wrote:
> >>> Can you give an example of such a list?
> >>
> >> Well, anything that makes "-sandbox on" more than security theater... I
> >> would consider it a necessary evil, not a good thing to have such a list.
> >>
> >> Due to NO_NEW_PRIVS, I think non-migration fork/exec should be on the
> >> list, but hopefully nothing else.
> >
> > The current semantics of '-sandbox on' are that it is documented to not
> > break any valid QEMU features. By blocking fork/exec, we make a semantic
> > change to the existing behaviour of '-sandbox on'.
>
> I don't propose to block fork/exec. I propose not to whitelist setuid,
> as is the case now too. It wouldn't be backwards-incompatible.
That we don't whitelist setuid is a bug in the current impl vs the intended
semantics of '-sandbox on', where we are denying valid features.
> > So any application
> > which has been using '-sandbox on' historically, is at risk of being
> > broken when upgrading to QEMU 2.10. It would force the application to
> > generate a different CLI for new QEMU - ie to avoid being broken they
> > would have to now add elevateprivs=allow, spawn=allow. I think we have
> > to maintain semantic compat with existing usage, which means that
> > '-sandbox on' has to remain security theatre.
> >
> > So if we want a default config to be more restrictive, then I think you
> > realistically have to turn the default parameter into a tri-state
> > instead of bool, ie allow '-sandbox default' as an alternative to
> > '-sandbox on' that was slightly stronger by blocking fork/exec.
>
> Or just print a warning that "-sandbox on" is useless. I guess that
> would be okay too if we want to be "stronger" than backwards-compatible.
It isn't totally useless - it is blocking access to a number of system
calls that QEMU should never use. eg things like reboot, load_module,
etc. In the absence of other security protections it is useless, but
if you combine with other mechanisms like DAC or LSM, then the default
seccomp rules offer some extra protection. Admittedly not alot, but
it is non-zero. The extra opt-in protecton flags then increase the
value at cost of killing some features.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-03-14 13:19 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-14 11:32 [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 1/5] seccomp: changing from whitelist to blacklist Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 2/5] seccomp: add obsolete argument to command line Eduardo Otubo
2017-03-14 11:47 ` Paolo Bonzini
2017-03-14 12:01 ` Thomas Huth
2017-03-14 12:22 ` Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges " Eduardo Otubo
2017-03-14 11:47 ` Paolo Bonzini
2017-03-14 11:52 ` Daniel P. Berrange
2017-03-14 12:13 ` Paolo Bonzini
2017-03-14 12:24 ` Daniel P. Berrange
2017-03-14 12:42 ` Eduardo Otubo
2017-03-14 12:32 ` Eduardo Otubo
2017-03-14 12:48 ` Paolo Bonzini
2017-03-14 13:02 ` Daniel P. Berrange
2017-03-14 13:05 ` Paolo Bonzini
2017-03-14 13:19 ` Daniel P. Berrange
2017-03-14 11:32 ` [Qemu-devel] [PATCH 4/5] seccomp: add spawn " Eduardo Otubo
2017-03-14 11:32 ` [Qemu-devel] [PATCH 5/5] seccomp: add resourcecontrol " Eduardo Otubo
2017-03-14 11:40 ` [Qemu-devel] [PATCH 0/5] seccomp: feature refactoring no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.