Re: [RFC PATCH bpf-next 1/2] bpf: Add bpf_current_capable kfunc

[Yonghong Song <yonghong.song@linux.dev>]

On 8/14/23 7:45 PM, Yafang Shao wrote:
> On Tue, Aug 15, 2023 at 8:28 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>>
>>
>> On 8/14/23 7:33 AM, Yafang Shao wrote:
>>> Add a new bpf_current_capable kfunc to check whether the current task
>>> has a specific capability. In our use case, we will use it in a lsm bpf
>>> program to help identify if the user operation is permitted.
>>>
>>> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
>>> ---
>>>    kernel/bpf/helpers.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>>> index eb91cae..bbee7ea 100644
>>> --- a/kernel/bpf/helpers.c
>>> +++ b/kernel/bpf/helpers.c
>>> @@ -2429,6 +2429,11 @@ __bpf_kfunc void bpf_rcu_read_unlock(void)
>>>        rcu_read_unlock();
>>>    }
>>>
>>> +__bpf_kfunc bool bpf_current_capable(int cap)
>>> +{
>>> +     return has_capability(current, cap);
>>> +}
>>
>> Since you are testing against 'current' capabilities, I assume
>> that the context should be process. Otherwise, you are testing
>> against random task which does not make much sense.
> 
> It is in the process context.
> 
>>
>> Since you are testing against 'current' cap, and if the capability
>> for that task is stable, you do not need this kfunc.
>> You can test cap in user space and pass it into the bpf program.
>>
>> But if the cap for your process may change in the middle of
>> run, then you indeed need bpf prog to test capability in real time.
>> Is this your use case and could you describe in in more detail?
> 
> After we convert the capability of our networking bpf program from
> CAP_SYS_ADMIN to CAP_BPF+CAP_NET_ADMIN to enhance the security, we
> encountered the "pointer comparison prohibited" error, because

Could you give a reproducible test case for this verifier failure
with CAP_BPF+CAP_NET_ADMIN capability? Is this due to packet pointer
or something else? Maybe verifier needs to be improved in these
cases without violating the promise of allow_ptr_leaks?

> allow_ptr_leaks is enabled only when CAP_PERFMON is set. However, if
> we enable the CAP_PERFMON for the networking bpf program, it can run
> tracing bpf prog, perf_event bpf prog and etc, that is not expected by
> us.
> 
> Hence we are planning to use a lsm bpf program to disallow it from
> running other bpf programs. In our lsm bpf program we will check the
> capability of processes, if the process has cap_net_admin, cap_bpf and
> cap_perfmon but don't have cap_sys_admin we will refuse it to run
> tracing and perf_event bpf program. While if a process has  cap_bpf
> and cap_perfmon but doesn't have cap_net_admin, that said it is a bpf
> program which wants to run trace bpf, we will allow it.
> 
> We can't use lsm_cgroup because it is supported on cgroup2 only, while
> we're still using cgroup1.
> 
> Another possible solution is enable allow_ptr_leaks for cap_net_admin
> as well, but after I checked the commit which introduces the cap_bpf
> and cap_perfmon [1], I think we wouldn't like to do it.
> 
> [1]. https://lore.kernel.org/bpf/20200513230355.7858-3-alexei.starovoitov@gmail.com/