[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:52:42 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 08.09.2017 11:50, Eduardo Otubo wrote:
> On Fri, Sep 08, 2017 at 11:43:27AM +0200, Thomas Huth wrote:
>> 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.
>
> The type is exactly the type of the system call on the blacklist array
> below. Being QEMU_SECCOMP_SET_DEFAULT, QEMU_SECCOMP_SET_OBSOLETE, etc.
Sorry, I still do not understand. If that's the case, what's the
difference between the "type" field and the "set" field? Where do you
use the "type" field?
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
[Qemu-devel] [PATCHv5 3/5] seccomp: add elevateprivileges argument to command line, Eduardo Otubo, 2017/09/08