Message-ID: <5323e010-09df-26d9-15f5-c723faa13224@digikod.net>
Date:   Wed, 28 Feb 2018 00:10:12 +0100
From:   Mickaël Salaün <mic@...ikod.net>
To:     Andy Lutomirski <luto@...capital.net>,
        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>
Subject: Re: [net-next v3 0/2] eBPF seccomp filters


On 27/02/2018 05:54, Andy Lutomirski wrote:
> 
> 
>> 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.

We agreed (including Kees and you, at the Santa Fe LPC) to limit the use
of Landlock to CAP_SYS_ADMIN at first. It is an artificial limitation
that can be re-enabled by removing three explicit checks/lines. Landlock
was designed for unprivileged use from day one and it is still the goal.

> 
> 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?
> 

Landlock is definitely focused on eBPF. It should not be hard to add a
new Landlock program type to mimic the seccomp filter checks (to use
eBPF features like maps), but I'm not sure to get the use case here.



Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists