netdev - Re: [PATCH v6 bpf-next 0/3] Introduce CAP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200513175301.43lxbckootoefrow@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 13 May 2020 10:53:01 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Marek Majkowski <marek@...udflare.com>
Cc:     David Miller <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        network dev <netdev@...r.kernel.org>, bpf@...r.kernel.org,
        kernel-team@...com, linux-security-module@...r.kernel.org,
        acme@...hat.com, jamorris@...ux.microsoft.com,
        Jann Horn <jannh@...gle.com>, kpsingh@...gle.com,
        kernel-team <kernel-team@...udflare.com>
Subject: Re: [PATCH v6 bpf-next 0/3] Introduce CAP_BPF

On Wed, May 13, 2020 at 11:50:42AM +0100, Marek Majkowski wrote:
> On Wed, May 13, 2020 at 4:19 AM Alexei Starovoitov
> <alexei.starovoitov@...il.com> wrote:
> >
> > CAP_BPF solves three main goals:
> > 1. provides isolation to user space processes that drop CAP_SYS_ADMIN and switch to CAP_BPF.
> >    More on this below. This is the major difference vs v4 set back from Sep 2019.
> > 2. makes networking BPF progs more secure, since CAP_BPF + CAP_NET_ADMIN
> >    prevents pointer leaks and arbitrary kernel memory access.
> > 3. enables fuzzers to exercise all of the verifier logic. Eventually finding bugs
> >    and making BPF infra more secure. Currently fuzzers run in unpriv.
> >    They will be able to run with CAP_BPF.
> >
> 
> Alexei, looking at this from a user point of view, this looks fine.
> 
> I'm slightly worried about REUSEPORT_EBPF. Currently without your
> patch, as far as I understand it:
> 
> - You can load SOCKET_FILTER and SO_ATTACH_REUSEPORT_EBPF without any
> permissions

correct.

> - For loading BPF_PROG_TYPE_SK_REUSEPORT program and for SOCKARRAY map
> creation CAP_SYS_ADMIN is needed. But again, no permissions check for
> SO_ATTACH_REUSEPORT_EBPF later.

correct. With clarification that attaching process needs to own
FD of prog and FD of socket.

> If I read the patchset correctly, the former SOCKET_FILTER case
> remains as it is and is not affected in any way by presence or absence
> of CAP_BPF.

correct. As commit log says:
"Existing unprivileged BPF operations are not affected."

> The latter case is different. Presence of CAP_BPF is sufficient for
> map creation, but not sufficient for loading SK_REUSEPORT program. It
> still requires CAP_SYS_ADMIN. 

Not quite.
The patch will allow BPF_PROG_TYPE_SK_REUSEPORT progs to be loaded
with CAP_BPF + CAP_NET_ADMIN.
Since this type of progs is clearly networking type I figured it's
better to be consistent with the rest of networking types.
Two unpriv types SOCKET_FILTER and CGROUP_SKB is the only exception.

> I think it's a good opportunity to relax
> this CAP_SYS_ADMIN requirement. I think the presence of CAP_BPF should
> be sufficient for loading BPF_PROG_TYPE_SK_REUSEPORT.
> 
> Our specific use case is simple - we want an application program -
> like nginx - to control REUSEPORT programs. We will grant it CAP_BPF,
> but we don't want to grant it CAP_SYS_ADMIN.

You'll be able to grant nginx CAP_BPF + CAP_NET_ADMIN to load SK_REUSEPORT
and unpriv child process will be able to attach just like before if
it has right FDs.
I suspect your load balancer needs CAP_NET_ADMIN already anyway due to
use of XDP and TC progs.
So granting CAP_BPF + CAP_NET_ADMIN should cover all bpf prog needs.
Does it address your concern?