On Fri, Oct 13, 2023 at 2:15 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Oct 12, 2023 Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > Add basic support of BPF token to BPF_PROG_LOAD. Wire through a set of > > allowed BPF program types and attach types, derived from BPF FS at BPF > > token creation time. Then make sure we perform bpf_token_capable() > > checks everywhere where it's relevant. > > > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > --- > > include/linux/bpf.h | 6 ++ > > include/uapi/linux/bpf.h | 2 + > > kernel/bpf/core.c | 1 + > > kernel/bpf/inode.c | 6 +- > > kernel/bpf/syscall.c | 87 ++++++++++++++----- > > kernel/bpf/token.c | 27 ++++++ > > tools/include/uapi/linux/bpf.h | 2 + > > .../selftests/bpf/prog_tests/libbpf_probes.c | 2 + > > .../selftests/bpf/prog_tests/libbpf_str.c | 3 + > > 9 files changed, 110 insertions(+), 26 deletions(-) > > ... > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index a2c9edcbcd77..c6b00aee3b62 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -2584,13 +2584,15 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type) > > } > > > > /* last field in 'union bpf_attr' used by this command */ > > -#define BPF_PROG_LOAD_LAST_FIELD log_true_size > > +#define BPF_PROG_LOAD_LAST_FIELD prog_token_fd > > > > static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size) > > { > > enum bpf_prog_type type = attr->prog_type; > > struct bpf_prog *prog, *dst_prog = NULL; > > struct btf *attach_btf = NULL; > > + struct bpf_token *token = NULL; > > + bool bpf_cap; > > int err; > > char license[128]; > > > > @@ -2606,10 +2608,31 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size) > > BPF_F_XDP_DEV_BOUND_ONLY)) > > return -EINVAL; > > > > + bpf_prog_load_fixup_attach_type(attr); > > + > > + if (attr->prog_token_fd) { > > + token = bpf_token_get_from_fd(attr->prog_token_fd); > > + if (IS_ERR(token)) > > + return PTR_ERR(token); > > + /* if current token doesn't grant prog loading permissions, > > + * then we can't use this token, so ignore it and rely on > > + * system-wide capabilities checks > > + */ > > + if (!bpf_token_allow_cmd(token, BPF_PROG_LOAD) || > > + !bpf_token_allow_prog_type(token, attr->prog_type, > > + attr->expected_attach_type)) { > > + bpf_token_put(token); > > + token = NULL; > > + } > > At the start of this effort I mentioned how we wanted to have LSM > control points when the token is created and when it is used. It is > for this reason that we still want a hook inside the > bpf_token_allow_cmd() function as it allows us to enable/disable use > of the token when its use is first attempted. If the LSM decides to > disallow use of the token in this particular case then the token is > disabled (set to NULL) while the operation is still allowed to move > forward, simply without the token. It's a much cleaner and well > behaved approach as it allows the normal BPF access controls to do > their work. I see, ok, so you want to be able to say "no BPF token for you", but not just error out the entire operation. Makes sense. > > > + } > > + > > + bpf_cap = bpf_token_capable(token, CAP_BPF); > > Similar to the above comment, we want to a LSM control point in > bpf_token_capable() so that the LSM can control the token's > ability to delegate capability privileges when they are used. Having > to delay this access control point to security_bpf_prog_load() is not > only awkward but it requires either manual synchronization between > all of the different LSMs and the the capability checks in the > bpf_prog_load() function or a completely different set of LSM > permissions for a token-based BPF program load over a normal BPF > program load. > > We really need these hooks Andrii, I wouldn't have suggested them if > I didn't believe they were important. No problem, I'll add both of them. I really didn't want to add hooks for allow_{maps,progs,attachs} (which you agreed shouldn't be added, so we are good), but I think allow_cmds and capable checks are fine. Will add in the next revision. > > > + err = -EPERM; > > + > > if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && > > (attr->prog_flags & BPF_F_ANY_ALIGNMENT) && > > - !bpf_capable()) > > - return -EPERM; > > + !bpf_cap) > > + goto put_token; > > [...]