Re: Potential issues (security and otherwise) with the current cgroup-bpf API

From: Andy Lutomirski <luto@amacapital.net>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: "Tejun Heo" <tj@kernel.org>, "Michal Hocko" <mhocko@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"David Ahern" <dsahern@gmail.com>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Daniel Mack" <daniel@zonque.org>,
	"Mickaël Salaün" <mic@digikod.net>,
	"Kees Cook" <keescook@chromium.org>, "Jann Horn" <jann@thejh.net>,
	"David S. Miller" <davem@davemloft.net>,
	"Thomas Graf" <tgraf@suug.ch>,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	"Linux API" <linux-api@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Network Development" <netdev@vger.kernel.org>
Subject: Re: Potential issues (security and otherwise) with the current cgroup-bpf API
Date: Fri, 3 Feb 2017 13:07:39 -0800	[thread overview]
Message-ID: <CALCETrVQyzoCDV5u_mui7KKCwLZ4+gJQFpjvz3XuNtvVTOqRcQ@mail.gmail.com> (raw)
In-Reply-To: <CALCETrXK65itdDqYTdhB-Td8d-Hzj00dcDScUOUh9psCZN_cLA@mail.gmail.com>

On Mon, Jan 23, 2017 at 12:20 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Sun, Jan 22, 2017 at 8:31 PM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Thu, Jan 19, 2017 at 08:04:59PM -0800, Andy Lutomirski wrote:
>>> restricting the types of sockets that can be created, then you do want
>>> the filter to work across namespaces, but seccomp can do that too and
>>> the current code doesn't handle netns correctly.
>>
>> are you saying that seccomp supports netns filtering? please show the proof.
>
> It can trivially restruct the types of sockets that are created by
> filtering on socket(2) syscall parameters, at least on sane
> architectures that don't use socketcall().

I think this is actually wrong -- the socket creation filter appears
to be called only on inet sockets.  Is there a good reason for that?

>
>> To summarize, I think the 'prog override is not allowed' flag would be
>> ok feature to have and I wouldn't mind making it the default when no 'flag'
>> field is passed to bpf syscall, but it's not acceptable to remove current
>> 'prog override is allowed' behavior.
>> So if you insist on changing the default, please implement the flag asap.
>> Though from security point of view and ABI point of view there is absolutely
>> no difference whether this flag is added today or a year later while
>> the default is kept as-is.
>
> It's too late and I have too little time.  I'll try to write a patch
> to change the default to just deny attempts to override.  Better
> behavior can be added later.
>
> IMO your suggestions about priorities are overcomplicated.  For your
> instrumentation needs, I can imagine that a simple "make this hook not
> run if a descendent has a hook" would do it.  For everything else, run
> them all in tree order (depending on filter type) and let the eBPF
> code do whatever it wants to do.

Is there any plan to address this?  If not, I'll try to write that
patch this weekend.