[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv5 1/5] seccomp: changing from whitelist to black
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCHv5 1/5] seccomp: changing from whitelist to blacklist |
Date: |
Fri, 8 Sep 2017 11:43:27 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 08.09.2017 11:10, Eduardo Otubo wrote:
> 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 <address@hidden>
> ---
> include/sysemu/seccomp.h | 2 +
> qemu-seccomp.c | 264
> ++++++-----------------------------------------
> vl.c | 1 -
> 3 files changed, 35 insertions(+), 232 deletions(-)
>
> diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
> index cfc06008cb..23b9c3c789 100644
> --- a/include/sysemu/seccomp.h
> +++ b/include/sysemu/seccomp.h
> @@ -15,6 +15,8 @@
> #ifndef QEMU_SECCOMP_H
> #define QEMU_SECCOMP_H
>
> +#define QEMU_SECCOMP_SET_DEFAULT (1 << 0)
> +
> #include <seccomp.h>
>
> int seccomp_start(void);
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index df75d9c471..bc9a1f77ff 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -28,232 +28,34 @@
>
> struct QemuSeccompSyscall {
> int32_t num;
> - uint8_t priority;
> + int type;
What's this "type" field good for? I failed to spot the place in the
sources where you are using it...? Anyway, some comments here right
after the struct members would be useful.
Thomas
> + uint8_t set;
> };
>
> -static const struct QemuSeccompSyscall seccomp_whitelist[] = {
> - { SCMP_SYS(timer_settime), 255 },
[...]
> - { SCMP_SYS(memfd_create), 240 },
> -#ifdef HAVE_CACHEFLUSH
> - { SCMP_SYS(cacheflush), 240 },
> -#endif
> - { SCMP_SYS(sysinfo), 240 },
> +static const struct QemuSeccompSyscall blacklist[] = {
> + /* default set of syscalls to blacklist */
> + { SCMP_SYS(reboot), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(swapon), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(swapoff), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(syslog), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(mount), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(umount), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(kexec_load), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(afs_syscall), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(break), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(ftime), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(getpmsg), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(gtty), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(lock), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(mpx), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(prof), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(profil), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(putpmsg), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(security), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(stty), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(tuxcall), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(ulimit), 1, QEMU_SECCOMP_SET_DEFAULT },
> + { SCMP_SYS(vserver), 1, QEMU_SECCOMP_SET_DEFAULT },
> };
>
> int seccomp_start(void)
> @@ -262,19 +64,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);
> - if (rc < 0) {
> - goto seccomp_return;
> + for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
> + switch (blacklist[i].set) {
> + default:
> + break;
> }
> - rc = seccomp_syscall_priority(ctx, seccomp_whitelist[i].num,
> - seccomp_whitelist[i].priority);
> +
> + 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 fb1f05b937..76e0b3a946 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1032,7 +1032,6 @@ static int bt_parse(const char *opt)
>
> 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) {
>
[Qemu-devel] [PATCHv5 2/5] seccomp: add obsolete argument to command line, Eduardo Otubo, 2017/09/08
[Qemu-devel] [PATCHv5 4/5] seccomp: add spawn argument to command line, Eduardo Otubo, 2017/09/08