Re: [net-next v3 0/2] eBPF seccomp filters

On Feb 26, 2018, at 8:38 PM, Kees Cook [off-list ref] wrote:

On Mon, Feb 26, 2018 at 8:19 PM, Andy Lutomirski [off-list ref] wrote:

quoted

quoted

On Feb 26, 2018, at 3:20 PM, Kees Cook [off-list ref] wrote:

On Mon, Feb 26, 2018 at 3:04 PM, Alexei Starovoitov
[off-list ref] wrote:

quoted

quoted

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.

quoted

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?
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers