netdev - Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf

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]

Date:   Tue, 30 Jul 2019 05:07:25 +0000
From:   Song Liu <songliubraving@...com>
To:     Andy Lutomirski <luto@...capital.net>
CC:     Andy Lutomirski <luto@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        "linux-security@...r.kernel.org" <linux-security@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <Kernel-team@...com>,
        Lorenz Bauer <lmb@...udflare.com>,
        Jann Horn <jannh@...gle.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf

Hi Andy, 

> On Jul 27, 2019, at 11:20 AM, Song Liu <songliubraving@...com> wrote:
> 
> Hi Andy, 
> 
>>>>> 
>>>> 
>>>> Well, yes. sys_bpf() is pretty powerful. 
>>>> 
>>>> The goal of /dev/bpf is to enable special users to call sys_bpf(). In 
>>>> the meanwhile, such users should not take down the whole system easily
>>>> by accident, e.g., with rm -rf /.
>>> 
>>> That’s easy, though — bpftool could learn to read /etc/bpfusers before allowing ruid != 0.
>> 
>> This is a great idea! fscaps + /etc/bpfusers should do the trick. 
> 
> After some discussions and more thinking on this, I have some concerns 
> with the user space only approach.  
> 
> IIUC, your proposal for user space only approach is like: 
> 
> 1. bpftool (and other tools) check /etc/bpfusers and only do 
>   setuid for allowed users:
> 
> 	int main()
> 	{
> 		if (/* uid in /etc/bpfusers */)
> 			setuid(0);
> 		sys_bpf(...);
> 	}
> 
> 2. bpftool (and other tools) is installed with CAP_SETUID:
> 
> 	setcap cap_setuid=e+p /bin/bpftool
> 
> 3. sys admin maintains proper /etc/bpfusers. 
> 
> This approach is not ideal, because we need to trust the tool to give 
> it CAP_SETUID. A hacked tool could easily bypass /etc/bpfusers check
> or use other root only sys calls after setuid(0). 
> 

I would like more comments on this. 

Currently, bpf permission is more or less "root or nothing", which we 
would like to change. 

The short term goal is to separate bpf from root, in other words, it is 
"all or nothing". Special user space utilities, such as systemd, would
benefit from this. Once this is implemented, systemd can call sys_bpf() 
when it is not running as root. 

In longer term, it may be useful to provide finer grain permission of 
sys_bpf(). For example, sys_bpf() should be aware of containers; and
user may only have access to certain bpf maps. Let's call this 
"fine grain" capability. 

Since we are seeing new use cases every year, we will need many 
iterations to implement the fine grain permission. I think we need an 
API that is flexible enough to cover different types of permission 
control. 

For example, bpf_with_cap() can be flexible:

	bpf_with_cap(cmd, attr, size, perm_fd);

We can get different types of permission via different combinations of 
arguments:

    A perm_fd to /dev/bpf gives access to all sys_bpf() commands, so 
    this is "all or nothing" permission. 

    A perm_fd to /sys/fs/cgroup/.../bpf.xxx would only allow some 
    commands to this specific cgroup. 

Alexei raised another idea in offline discussions: instead of adding
bpf_with_cap(), we add a command LOAD_PERM_FD, which enables special
permission for the _next_ sys_bpf() from current task:

    bpf(LOAD_PERM_FD, perm_fd);
    /* the next sys_bpf() uses permission from perm_fd */
    bpf(cmd, attr, size);

This is equivalent to bpf_with_cap(cmd, attr, size, perm_fd), but 
doesn't require the new sys call. 

For both these ideas, we will start with /dev/bpf. As we grow the
fine grain permission control, fewer users/processes will need access 
to /dev/bpf. 

Please let us know your thought on this. Would this make /dev/bpf 
more reasonable? :-)

A few notes for previous discussions:

1. User space only approach doesn't work, even for "all or nothing" 
   permission control. I expanded the discussion in the previous
   email. Please let me know if I missed anything there. 

2. Permission control only at BPF_PROG_ATTACH time is not sufficient.
   We need permission control during BPF_PROG_LOAD, e.g., is_priv in 
   the verifier. 

Thanks,
Song