Thread (79 messages) 79 messages, 9 authors, 2025-07-14

Re: [PATCH 11/12] bpftool: Add support for signing BPF programs

From: Blaise Boscaccy <hidden>
Date: 2025-06-10 16:34:49
Also in: bpf, keyrings

KP Singh [off-list ref] writes:
On Sun, Jun 8, 2025 at 4:03 PM James Bottomley
[off-list ref] wrote:
quoted
[+keyrings]
On Sat, 2025-06-07 at 01:29 +0200, KP Singh wrote:
[...]
quoted
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.
quoted
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).
Untrusted userspace/root is precisely the issue I solved with previous
patchsets for this effort. Signed BPF programs absolutely work without
trusted execution.

-blaise
quoted
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
quoted
Regards,

James
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help