[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <A20EA7DD-94E9-488A-B9FF-D8E2C9F26611@amacapital.net>
Date: Mon, 26 Feb 2018 20:54:39 -0800
From: Andy Lutomirski <luto@...capital.net>
To: Kees Cook <keescook@...omium.org>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>,
Sargun Dhillon <sargun@...gun.me>,
Network Development <netdev@...r.kernel.org>,
Linux Containers <containers@...ts.linux-foundation.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Will Drewry <wad@...omium.org>,
Jessica Frazelle <me@...sfraz.com>,
Brian Goff <cpuguy83@...il.com>,
Tom Hromatka <tom.hromatka@...cle.com>,
James Morris <jmorris@...ei.org>,
Tycho Andersen <tycho@...ho.ws>, mic@...ikod.net
Subject: Re: [net-next v3 0/2] eBPF seccomp filters
> On Feb 26, 2018, at 8:38 PM, Kees Cook <keescook@...omium.org> wrote:
>
> On Mon, Feb 26, 2018 at 8:19 PM, Andy Lutomirski <luto@...capital.net> wrote:
>>> On Feb 26, 2018, at 3:20 PM, Kees Cook <keescook@...omium.org> wrote:
>>>
>>> On Mon, Feb 26, 2018 at 3:04 PM, Alexei Starovoitov
>>> <alexei.starovoitov@...il.com> wrote:
>>>>> On Mon, Feb 26, 2018 at 07:26:54AM +0000, Sargun Dhillon wrote:
>>>>> This patchset enables seccomp filters to be written in eBPF. Although, this
>>>>> [...]
>>>> The main statement I want to hear from seccomp maintainers before
>>>> proceeding any further on this that enabling eBPF in seccomp won't lead
>>>> to seccomp folks arguing against changes in bpf core (like verifier)
>>>> just because it's used by seccomp.
>>>> It must be spelled out in the commit log with explicit Ack.
>>>
>>> The primary thing I'm concerned about with eBPF and seccomp is
>>> side-effects from eBPF programs running at syscall time. This is an
>>> extremely sensitive area, and I want to be sure there won't be
>>> feature-creep here that leads to seccomp getting into a bad state.
>>>
>>> As long as seccomp can continue have its own verifier, I *think* this
>>> will be fine, though, again I remain concerned about maps, etc. I'm
>>> still reviewing these patches and how they might provide overlap with
>>> Tycho's needs too, etc.
>>
>> I'm not sure I see this as a huge problem. As far as I can see, there
>> are three ways that a verifier change could be problematic:
>>
>> 1. Addition of a new type of map. But seccomp would just not allow
>> new map types by default, right?
>>
>> 2. Addition of a new BPF_CALLable helper. Seccomp wants a way to
>> whitelist BPF_CALL targets. That should be straightforward.
>
> Yup, agreed on 1 and 2.
>
>> 3. Straight-up bugs. Those are exactly as problematic as verifier
>> bugs in any other unprivileged eBPF program type, right? I don't see
>> why seccomp is special here.
>
> My concern is more about unintended design mistakes or other feature
> creep with side-effects, especially when it comes to privileges and
> synchronization. Getting no-new-privs done correctly, for example,
> took some careful thought and discussion, and I'm shy from how painful
> TSYNC was on the process locking side, and eBPF has had some rather
> ugly flaws in the past (and recently: it was nice to be able to say
> for Spectre that seccomp filters couldn't be constructed to make
> attacks but eBPF could). Adding the complexity needs to be worth the
> gain. I'm on board for doing it, I just want to be careful. :)
>
I agree. I think that, if we do this right, we get a clean version of Tycho's notifiers. We can also very easily build on that to send a non-blocking message to the notifier fd, which gets us a version of seccomp logging that works for things like Chromium and even strace. I think this is worth it.
I also think this sort of argument is why Mickaƫl's privileged-first Landlock is the wrong approach. By getting the unprivileged parts right from day one, we can carefully extend the mechanism and keep it usable by unprivileged apps. But, if we'd started as root-only, fixing up everything needed to make it safe for unprivileged users after the fact would have been quite messy.
And the considerations for making eBPF safe for use by unprivileged tasks to filter their descendents are more or less the same for seccomp and Landlock. Can we please arrange things so we solve this problem only once?
Powered by blists - more mailing lists