On Tue, Jul 2, 2019 at 2:04 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > On Mon, Jul 01, 2019 at 06:59:13PM -0700, Andy Lutomirski wrote: > > I think I'm understanding your motivation. You're not trying to make > > bpf() generically usable without privilege -- you're trying to create > > a way to allow certain users to access dangerous bpf functionality > > within some limits. > > > > That's a perfectly fine goal, but I think you're reinventing the > > wheel, and the wheel you're reinventing is quite complicated and > > already exists. I think you should teach bpftool to be secure when > > installed setuid root or with fscaps enabled and put your policy in > > bpftool. If you want to harden this a little bit, it would seem > > entirely reasonable to add a new CAP_BPF_ADMIN and change some, but > > not all, of the capable() checks to check CAP_BPF_ADMIN instead of the > > capabilities that they currently check. > > If finer grained controls are wanted, it does seem like the /dev/bpf > path makes the most sense. open, request abilities, use fd. The open can > be mediated by DAC and LSM. The request can be mediated by LSM. This > provides a way to add policy at the LSM level and at the tool level. > (i.e. For tool-level controls: leave LSM wide open, make /dev/bpf owned > by "bpfadmin" and bpftool becomes setuid "bpfadmin". For fine-grained > controls, leave /dev/bpf wide open and add policy to SELinux, etc.) > > With only a new CAP, you don't get the fine-grained controls. (The > "request abilities" part is the key there.) Sure you do: the effective set. It has somewhat bizarre defaults, but I don't think that's a real problem. Also, this wouldn't be like CAP_DAC_READ_SEARCH -- you can't accidentally use your BPF caps. I think that a /dev capability-like object isn't totally nuts, but I think we should do it well, and this patch doesn't really achieve that. But I don't think bpf wants fine-grained controls like this at all -- as I pointed upthread, a fine-grained solution really wants different treatment for the different capable() checks, and a bunch of them won't resemble capabilities or /dev/bpf at all.
