[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5j+64WzxjBnpQxYCU50ak+VqVw1y0W+MWygFodxsDqEZRw@mail.gmail.com>
Date: Mon, 26 Feb 2018 20:38:18 -0800
From: Kees Cook <keescook@...omium.org>
To: Andy Lutomirski <luto@...capital.net>
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>
Subject: Re: [net-next v3 0/2] eBPF seccomp filters
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. :)
-Kees
--
Kees Cook
Pixel Security
Powered by blists - more mailing lists