Re: [RFC PATCH v2] bpf: use bpf_capable() instead of CAP_SYS_ADMIN for blinding decision

From: Daniel Borkmann <daniel@iogearbox.net>
To: "Serge E. Hallyn" <serge@hallyn.com>,
	Yauheni Kaliuta <ykaliuta@redhat.com>
Cc: bpf@vger.kernel.org, andrii@kernel.org,
	alexei.starovoitov@gmail.com, jbenc@redhat.com,
	linux-security-module@vger.kernel.org
Subject: Re: [RFC PATCH v2] bpf: use bpf_capable() instead of CAP_SYS_ADMIN for blinding decision
Date: Wed, 31 Aug 2022 23:15:20 +0200	[thread overview]
Message-ID: <4df80c14-d744-efc6-c043-c70c4c4ab541@iogearbox.net> (raw)
In-Reply-To: <20220831185039.GA20800@mail.hallyn.com>

On 8/31/22 8:50 PM, Serge E. Hallyn wrote:
> On Wed, Aug 31, 2022 at 06:24:14PM +0300, Yauheni Kaliuta wrote:
>> The capability check can cause SELinux denial.
>>
>> For example, in ptp4l, setsockopt() with the SO_ATTACH_FILTER option
>> raises sk_attach_filter() to run a bpf program. SELinux hooks into
>> capable() calls and performs an additional check if the task's
>> SELinux domain has permission to "use" the given capability. ptp4l_t
>> already has CAP_BPF granted by SELinux, so if the function used
>> bpf_capable() as most BPF code does, there would be no change needed
>> in selinux-policy.
> 
> The selinux mentions probably aren't really necessary.  The more
> concise way to say it is that bpf_jit_blinding_enabled() should
> be permitted with CAP_BPF, that full CAP_SYS_ADMIN is not needed.
> (Assuming that that is the case)
> 
>> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
>> ---
>>
>> v2: put the reasoning in the commit message
>>
>> ---
>>   include/linux/filter.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index a5f21dc3c432..3de96b1a736b 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -1100,7 +1100,7 @@ static inline bool bpf_jit_blinding_enabled(struct bpf_prog *prog)
>>   		return false;
>>   	if (!bpf_jit_harden)
>>   		return false;
>> -	if (bpf_jit_harden == 1 && capable(CAP_SYS_ADMIN))
>> +	if (bpf_jit_harden == 1 && bpf_capable())

I think lowering this requirement is fine here. These days given unpriv eBPF is
disabled by default, I see the main users for constant blinding coming from unpriv
in particular via cBPF -> eBPF migration (e.g. old-style socket filters).

>>   		return false;
>>   
>>   	return true;

Please also update Documentation/admin-guide/sysctl/net.rst to clarify cap details.

Thanks,
Daniel