Re: [PATCH bpf-next 0/8] New BPF map and BTF security LSM hooks

[Casey Schaufler <casey@schaufler-ca.com>]

On 4/12/2023 6:43 PM, Andrii Nakryiko wrote:
> On Wed, Apr 12, 2023 at 12:07 PM Paul Moore <paul@paul-moore.com> wrote:
>> On Wed, Apr 12, 2023 at 2:28 PM Kees Cook <keescook@chromium.org> wrote:
>>> On Wed, Apr 12, 2023 at 02:06:23PM -0400, Paul Moore wrote:
>>>> On Wed, Apr 12, 2023 at 1:47 PM Kees Cook <keescook@chromium.org> wrote:
>>>>> On Wed, Apr 12, 2023 at 12:49:06PM -0400, Paul Moore wrote:
>>>>>> On Wed, Apr 12, 2023 at 12:33 AM Andrii Nakryiko <andrii@kernel.org> wrote:
>>>>>>> Add new LSM hooks, bpf_map_create_security and bpf_btf_load_security, which
>>>>>>> are meant to allow highly-granular LSM-based control over the usage of BPF
>>>>>>> subsytem. Specifically, to control the creation of BPF maps and BTF data
>>>>>>> objects, which are fundamental building blocks of any modern BPF application.
>>>>>>>
>>>>>>> These new hooks are able to override default kernel-side CAP_BPF-based (and
>>>>>>> sometimes CAP_NET_ADMIN-based) permission checks. It is now possible to
>>>>>>> implement LSM policies that could granularly enforce more restrictions on
>>>>>>> a per-BPF map basis (beyond checking coarse CAP_BPF/CAP_NET_ADMIN
>>>>>>> capabilities), but also, importantly, allow to *bypass kernel-side
>>>>>>> enforcement* of CAP_BPF/CAP_NET_ADMIN checks for trusted applications and use
>>>>>>> cases.
>>>>>> One of the hallmarks of the LSM has always been that it is
>>>>>> non-authoritative: it cannot unilaterally grant access, it can only
>>>>>> restrict what would have been otherwise permitted on a traditional
>>>>>> Linux system.  Put another way, a LSM should not undermine the Linux
>>>>>> discretionary access controls, e.g. capabilities.
>>>>>>
>>>>>> If there is a problem with the eBPF capability-based access controls,
>>>>>> that problem needs to be addressed in how the core eBPF code
>>>>>> implements its capability checks, not by modifying the LSM mechanism
>>>>>> to bypass these checks.
>>>>> I think semantics matter here. I wouldn't view this as _bypassing_
>>>>> capability enforcement: it's just more fine-grained access control.
> Exactly. One of the motivations for this work was the need to move
> some production use cases that are only needing extra privileges so
> that they can use BPF into a more restrictive environment. Granting
> CAP_BPF+CAP_PERFMON+CAP_NET_ADMIN to all such use cases that need them
> for BPF usage is too coarse grained. These caps would allow those
> applications way more than just BPF usage. So the idea here is more
> finer-grained control of BPF-specific operations, granting *effective*
> CAP_BPF+CAP_PERFMON+CAP_NET_ADMIN caps dynamically based on custom
> production logic that would validate the use case.

That's an authoritative model which is in direct conflict with the
design and implementation of both capabilities and LSM.

>
> This *is* an attempt to achieve a more secure production approach.
>
>>>>> For example, in many places we have things like:
>>>>>
>>>>>         if (!some_check(...) && !capable(...))
>>>>>                 return -EPERM;
>>>>>
>>>>> I would expect this is a similar logic. An operation can succeed if the
>>>>> access control requirement is met. The mismatch we have through-out the
>>>>> kernel is that capability checks aren't strictly done by LSM hooks. And
>>>>> this series conceptually, I think, doesn't violate that -- it's changing
>>>>> the logic of the capability checks, not the LSM (i.e. there no LSM hooks
>>>>> yet here).
>>>> Patch 04/08 creates a new LSM hook, security_bpf_map_create(), which
>>>> when it returns a positive value "bypasses kernel checks".  The patch
>>>> isn't based on either Linus' tree or the LSM tree, I'm guessing it is
>>>> based on a eBPF tree, so I can't say with 100% certainty that it is
>>>> bypassing a capability check, but the description claims that to be
>>>> the case.
>>>>
>>>> Regardless of how you want to spin this, I'm not supportive of a LSM
>>>> hook which allows a LSM to bypass a capability check.  A LSM hook can
>>>> be used to provide additional access control restrictions beyond a
>>>> capability check, but a LSM hook should never be allowed to overrule
>>>> an access denial due to a capability check.
>>>>
>>>>> The reason CAP_BPF was created was because there was nothing else that
>>>>> would be fine-grained enough at the time.
>>>> The LSM layer predates CAP_BPF, and one could make a very solid
>>>> argument that one of the reasons LSMs exist is to provide
>>>> supplementary controls due to capability-based access controls being a
>>>> poor fit for many modern use cases.
>>> I generally agree with what you say, but we DO have this code pattern:
>>>
>>>          if (!some_check(...) && !capable(...))
>>>                  return -EPERM;
>> I think we need to make this more concrete; we don't have a pattern in
>> the upstream kernel where 'some_check(...)' is a LSM hook, right?
>> Simply because there is another kernel access control mechanism which
>> allows a capability check to be skipped doesn't mean I want to allow a
>> LSM hook to be used to skip a capability check.
> This work is an attempt to tighten the security of production systems
> by allowing to drop too coarse-grained and permissive capabilities
> (like CAP_BPF, CAP_PERFMON, CAP_NET_ADMIN, which inevitable allow more
> than production use cases are meant to be able to do)

The BPF developers are in complete control of what CAP_BPF controls.
You can easily address the granularity issue by adding addition restrictions
on processes that have CAP_BPF. That is the intended use of LSM.
The whole point of having multiple capabilities is so that you can
grant just those that are required by the system security policy, and
do so safely. That leads to differences of opinion regarding the definition
of the system security policy. BPF chose to set itself up as an element
of security policy (you need CAP_BPF) rather than define elements such that
existing capabilities (CAP_FOWNER, CAP_KILL, CAP_MAC_OVERRIDE, ...) would
control. 

>  and then grant
> specific BPF operations on specific BPF programs/maps based on custom
> LSM security policy,

This is backwards. The correct implementation is to require CAP_BPF and
further restrict BPF operations based on a custom LSM security policy.
That's how LSM is designed.

>  which validates application trustworthiness using
> custom production-specific logic.
>
> Isn't this goal in line with LSMs mission to enhance system security?

We're not arguing the goal, we're discussing the implementation.

>>> It looks to me like this series can be refactored to do the same. I
>>> wouldn't consider that to be a "bypass", but I would agree the current
>>> series looks too much like "bypass", and makes reasoning about the
>>> effect of the LSM hooks too "special". :)
> Sorry, I didn't realize that the current code layout is making things
> more confusing. I'll address feedback to make the intent a bit
> clearer.
>
>> --
>> paul-moore.com