On Sun, Jun 8, 2025 at 4:03 PM James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> [+keyrings]
> On Sat, 2025-06-07 at 01:29 +0200, KP Singh wrote:
> [...]
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index f010295350be..e1dbbca91e34 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -23,6 +23,7 @@
> > #include <linux/err.h>
> > #include <linux/perf_event.h>
> > #include <linux/sizes.h>
> > +#include <linux/keyctl.h>
> >
> > #include <bpf/bpf.h>
> > #include <bpf/btf.h>
> > @@ -1875,6 +1876,8 @@ static int try_loader(struct gen_loader_opts
> > *gen)
> > {
> > struct bpf_load_and_run_opts opts = {};
> > struct bpf_loader_ctx *ctx;
> > + char sig_buf[MAX_SIG_SIZE];
> > + __u8 prog_sha[SHA256_DIGEST_LENGTH];
> > int ctx_sz = sizeof(*ctx) + 64 * max(sizeof(struct
> > bpf_map_desc),
> > sizeof(struct
> > bpf_prog_desc));
> > int log_buf_sz = (1u << 24) - 1;
> > @@ -1898,6 +1901,24 @@ static int try_loader(struct gen_loader_opts
> > *gen)
> > opts.insns = gen->insns;
> > opts.insns_sz = gen->insns_sz;
> > fds_before = count_open_fds();
> > +
> > + if (sign_progs) {
> > + opts.excl_prog_hash = prog_sha;
> > + opts.excl_prog_hash_sz = sizeof(prog_sha);
> > + opts.signature = sig_buf;
> > + opts.signature_sz = MAX_SIG_SIZE;
> > + opts.keyring_id = KEY_SPEC_SESSION_KEYRING;
> > +
>
> This looks wrong on a couple of levels. Firstly, if you want system
> level integrity you can't search the session keyring because any
> process can join (subject to keyring permissions) and the owner, who is
> presumably the one inserting the bpf program, can add any key they
> like.
>
Wanting system level integrity is a security policy question, so this
is something that needs to be implemented at the security layer, the
LSM can deny the keys / keyring IDs they don't trust. Session
keyrings are for sure useful for delegated signing of BPF programs
when dynamically generated.
> The other problem with this scheme is that the keyring_id itself has no
> checked integrity, which means that even if a script was marked as
If an attacker can modify a binary that has permissions to load BPF
programs and update the keyring ID then we have other issues. So, this
does not work in independence, signed BPF programs do not really make
sense without trusted execution).
> system keyring only anyone can binary edit the user space program to
> change it to their preferred keyring and it will still work. If you
> want variable keyrings, they should surely be part of the validated
> policy.
The policy is what I expect to be implemented in the LSM layer. A
variable keyring ID is a critical part of the UAPI to create different
"rings of trust" e.g. LSM can enforce that network programs can be
loaded with a derived key, and have a different keyring for
unprivileged BPF programs.
This patch implements the signing support, not the security policy for it.
- KP
>
> Regards,
>
> James
>
