On Thu, Mar 1, 2018 at 1:59 PM, Andy Lutomirski <l...@kernel.org> wrote:
> On Thu, Mar 1, 2018 at 9:51 PM, Sargun Dhillon <sar...@sargun.me> wrote:
>> On Thu, Mar 1, 2018 at 9:44 AM, Andy Lutomirski <l...@amacapital.net> wrote:
>>> On Wed, Feb 28, 2018 at 7:56 PM, Daniel Borkmann <dan...@iogearbox.net>
>>> wrote:
>>>> On 02/28/2018 12:55 AM, chris hyser wrote:
>>>>>> On 02/27/2018 04:58 PM, Daniel Borkmann wrote: >> On 02/27/2018 05:59
>>>>>> PM, chris hyser wrote:
>>>>>>>> On 02/27/2018 11:00 AM, Kees Cook wrote:
>>>>>>>>> On Tue, Feb 27, 2018 at 6:53 AM, chris hyser <chris.hy...@oracle.com>
>>>>>>>>> wrote:
>>>>>>>>>> On 02/26/2018 11:38 PM, Kees Cook wrote:
>>>>>>>>>>> On Mon, Feb 26, 2018 at 8:19 PM, Andy Lutomirski
>>>>>>>>>>> <l...@amacapital.net>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> 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
>>>>>>>
>>>>>>> Well, not really. One part of all the Spectre mitigations that went
>>>>>>> upstream
>>>>>>> from BPF side was to have an option to remove interpreter entirely and
>>>>>>> that
>>>>>>> also relates to seccomp eventually. But other than that an attacker
>>>>>>> might
>>>>>>> potentially find as well useful gadgets inside seccomp or any other code
>>>>>>> that is inside the kernel, so it's not a strict necessity either.
>>>>>>>
>>>>>>>>>>> gain. I'm on board for doing it, I just want to be careful. :)
>>>>>>>>>>
>>>>>>>>>> Another option might be to remove c/eBPF from the equation all
>>>>>>>>>> together.
>>>>>>>>>> c/eBPF allows flexibility and that almost always comes at the cost of
>>>>>>>>>> additional security risk. Seccomp is for enhanced security yes? How
>>>>>>>>>> about a
>>>>>>>>>> new seccomp mode that passes in something like a bit vector or
>>>>>>>>>> hashmap for
>>>>>>>>>> "simple" white/black list checks validated by kernel code, versus
>>>>>>>>>> user
>>>>>>>>>> provided interpreted code? Of course this removes a fair number of
>>>>>>>>>> things
>>>>>>>>>> you can currently do or would be able to do with eBPF. Of course,
>>>>>>>>>> restated
>>>>>>>>>> from a security point of view, this removes a fair number of things
>>>>>>>>>> an
>>>>>>>>>> _attacker_ can do. Presumably the performance improvement would also
>>>>>>>>>> be
>>>>>>>>>> significant.
>>>>>>>
>>>>>>> Good luck with not breaking existing applications relying on seccomp out
>>>>>>> there.
>>>>>>
>>>>>> This wasn't in the context of an implementation proposal, but the
>>>>>> assumption would be to add this in addition to the old way. Now, does
>>>>>> that make sense to do? That is the discussion.
>>>>
>>>> I see; didn't read that out from the above when you also mentioned removing
>>>> cBPF, but fair enough.
>>>>
>>>>>>>>>> Is this an idea worth prototyping?
>>>>>>>>>
>>>>>>>>> That was the original prototype for seccomp-filter. :) The discussion
>>>>>>>>> around that from years ago basically boiled down to it being
>>>>>>>>> inflexible. Given all the things people want to do at syscall time,
>>>>>>>>> that continues to be true. So true, in fact, that here we are now,
>>>>>>>>> trying to move to eBPF from cBPF. ;)
>>>>>>>
>>>>>>> Right, agree. cBPF is also pretty much frozen these days and aside from
>>>>>>> that, seccomp/BPF also just uses a proper subset of it. I wouldn't mind
>>>>>>> doing something similar for eBPF side as long as this is reasonably
>>>>>>> maintainable and not making BPF core more complex, but most of it can
>>>>>>> already be set in the verifier anyway based on prog type. Note, that
>>>>>>> performance of seccomp/BPF is definitely a demand as well which is why
>>>>>>> people still extend the old remaining cBPF JITs today such that it can
>>>>>>> be JITed also from there.
>>>>>>>
>>>>>>>> I will try to find that discussion. As someone pointed out here
>>>>>>>> though, eBPF is being used by more and more people in areas where
>>>>>>>> security is not the primary concern. Differing objectives will make
>>>>>>>> this a long term continuing issue. We ourselves were looking at eBPF
>>>>>>>> simply as a means to use a hashmap for a white/blacklist, i.e.
>>>>>>>> performance not flexibility.
>>>>>>>
>>>>>>> Not really, security of verifier and BPF infra in general is on the top
>>>>>>> of the list, it's fundamental to the underlying concept and just because
>>>>>>> it is heavily used also in tracing and networking, it only shows that
>>>>>>> the
>>>>>>> concept is highly flexible that it can be applied in multiple areas.
>>>>>
>>>>> If you're implying that because seccomp would have it's own verifier and
>>>>> could therefore restrict itself to a subset of eBPF, therefore any future
>>>>> additions/features to eBPF would not necessarily make seccomp less
>>>>> secure, I mainly agree. Is that the argument?
>>>>
>>>> Ok, in addition to the current unpriv restrictions imposed by the verifier,
>>>> what additional requirements would you have from your side in order to get
>>>> to semantics that make sense for you wrt seccomp/eBPF? Just trying to
>>>> understand how far we are away from that. Note that not every new feature,
>>>> map or helper is enabled for every program type of course.
>>>>
>>>
>>> I haven't looked at the exact unpriv restrictions lately, but I think
>>> I remember them. Regardless, from my perspective, here's what I would
>>> care about as a seccomp reviewer:
>>>
>>> 1. No extra information should become available to a seccomp filter
>>> program without seccomp's explicit opt-in. In other words, a seccomp
>>> filter program should be able to access the fields in struct
>>> seccomp_data, the return values of BPF_CALL helpers explicitly
>>> authorized by seccomp, and the values in maps authorized by seccomp
>>> (if any!), and that's it. They should not be able to learn the
>>> current time, any kernel pointers, user register state (except that
>>> which is contained in seccomp_data), etc. I believe that this is
>>> already the case except insofar as core BPF_CALL helpers may violate
>>> this. (I'm not sure exactly what the policy is on the use of BPF_CALL
>>> helpers.)
>>>
>> The only calls that were whitelisted was uid/gid, pid/tid, ktime, and
>> prandom. There was no printk, nor perf_events. It didn't give access
>> to maps, or anything else. I have to ask though, why isn't it okay for
>> eBPF filters to have access to time? My use case is that I launch a
>> job, and it has 5 minutes to do some privileged operations, and then I
>> want to deny any privileged operations. Maybe when writeable maps are
>> a thing, we can do something more advanced, but trying to narrow the
>> window of attack has a lot of benefit.
>
> To avoid derailing this discussion: I think that the first incarnation
> of eBPF seccomp in the kernel should allow no BPF_CALL helpers
> whatsoever. Addition of helpers needs to be reviewed carefully. As
> for the "five minutes" use: just kill the process after 5 minutes or
> use user notifiers. That use case is just too weird to be worthy of
> kernel support IMO.
I think that's an okay place to start.
