[PATCH v4 2/12] bpf: introduce BPF token object

[Paul Moore]

On Thu, Sep 21, 2023 at 6:18 PM Paul Moore <paul at paul-moore.com> wrote:
>

...

> Typically the LSM hook call sites end up being in the same general
> area as the capability checks, usually just after (we want the normal
> Linux discretionary access controls to always come first for the sake
> of consistency).  Sticking with that approach it looks like we would
> end up with a LSM call in bpf_prog_load() right after bpf_capable()
> call, the only gotcha with that is the bpf_prog struct isn't populated
> yet, but how important is that when we have the bpf_attr info (honest
> question, I don't know the answer to this)?
>
> Ignoring the bpf_prog struct, do you think something like this would
> work for a hook call site (please forgive the pseudo code)?
>
>   int bpf_prog_load(...)
>   {
>          ...
>      bpf_cap = bpf_token_capable(token, CAP_BPF);
>      err = security_bpf_token(BPF_PROG_LOAD, attr, uattr_size, token);
>      if (err)
>        return err;
>     ...
>   }
>
> Assuming this type of hook configuration, and an empty/passthrough
> security_bpf() hook, a LSM would first see the various
> capable()/ns_capable() checks present in bpf_token_capable() followed
> by a BPF op check, complete with token, in the security_bpf_token()
> hook.  Further assuming that we convert the bpf_token_new_fd() to use
> anon_inode_getfd_secure() instead of anon_inode_getfd() and the
> security_bpf_token() could still access the token fd via the bpf_attr
> struct I think we could do something like this for the SELinux case
> (more rough pseudo code):
>
>   int selinux_bpf_token(...)
>   {
>     ssid = current_sid();
>     if (token) {
>       /* this could be simplified with better integration
>        * in bpf_token_get_from_fd() */
>       fd = fdget(attr->prog_token_fd);
>       inode = file_inode(fd.file);
>       isec = selinux_inode(inode);
>       tsid = isec->sid;
>       fdput(fd);
>     } else
>       tsid = ssid;
>     switch(cmd) {
>     ...
>     case BPF_PROG_LOAD:
>       rc = avc_has_perm(ssid, tsid, SECCLAS_BPF, BPF__PROG_LOAD);
>       break;
>     default:
>       rc = 0;
>     }
>     return rc;
>   }
>
> This would preserve the current behaviour when a token was not present:
>
>  allow @current @current : bpf { prog_load }
>
> ... but this would change to the following if a token was present:
>
>  allow @current @DELEGATED_ANON_INODE : bpf { prog_load }
>
> That seems reasonable to me, but I've CC'd the SELinux list on this so
> others can sanity check the above :)

I thought it might be helpful to add a bit more background on my
thinking for the SELinux folks, especially since the object label used
in the example above is a bit unusual.  As a reminder, the object
label in the delegated case is not the current domain as it is now for
standard BPF program loads, it is the label of the BPF delegation
token (anonymous inode) that is created by the process/orchestrator
that manages the namespace and explicitly enabled the BPF privilege
delegation.  The BPF token can be labeled using the existing anonymous
inode transition rules.

First off I decided to reuse the existing permission so as not to
break current policies.  We can always separate the PROG_LOAD
permission into a standard and delegated permission if desired, but I
believe we would need to gate that with a policy capability and
preserve some form of access control for the legacy PROG_LOAD-only
case.

Preserving the PROG_LOAD permission does present a challenge with
respect to differentiating the delegated program load from a normal
program load while ensuring that existing policies continue to work
and delegated operations require explicit policy adjustments.
Changing the object label in the delegated case was the only approach
I could think of that would satisfy all of these constraints, but I'm
open to other ideas, tweaks, etc. and I would love to get some other
opinions on this.

Thoughts?

-- 
paul-moore.com