Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Mon, Apr 14, 2025 at 5:32 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Sat, Apr 12, 2025 at 6:58 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
TAlexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
+static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) +{
struct bpf_insn *insn = prog->insnsi;
int insn_cnt = prog->len;
int i;
int err;
for (i = 0; i < insn_cnt; i++, insn++) {
if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {
switch (insn[0].src_reg) {
case BPF_PSEUDO_MAP_IDX_VALUE:
case BPF_PSEUDO_MAP_IDX:
err = add_used_map(maps, insn[0].imm);
if (err < 0)
return err;
break;
default:
break;
}
}
}
...
if (!map->frozen) {
attr.map_fd = fd;
err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));
Sorry for the delay. Still swamped after conferences and the merge window.
No worries.
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions.
These aren't BPF internals; this is data passed in from userspace. Inspecting userspace function inputs is definitely within the purview of an LSM.
Lskel signature verification doesn't actually need a full disassembly, but it does need all the maps used by the lskel. Due to API design choices, this unfortunately requires disassembling the program to see which array indexes are being used.
Calling into sys_bpf from LSM is plain nack.
kern_sys_bpf is an EXPORT_SYMBOL, which means that it should be callable from a module.
It's a leftover. kern_sys_bpf() is not something that arbitrary kernel modules should call. It was added to work for cases where kernel modules carry their own lskels. That use case is gone, so EXPORT_SYMBOL will be removed.
I'm not following that at all. You recommended using module-based lskels to get around code signing requirements at lsfmmbpf and now you want to nuke that entire feature? And further, skel_internal will no longer be usable from within the kernel and bpf_preload is no longer going to be supported?
The eBPF dev community has spent what, 4-5 years on this, with little to no progress. I have little faith that this is going to progress on your end in a timely manner or at all, and frankly we (and others) needed this yesterday. Hornet has zero impact on the bpf subsystem, yet you seem viscerally opposed to us doing this. Why are you trying to stop us from securing our cloud?
It was exported to modules to run lskel-s from modules. It's bpf internal api, but seeing how you want to abuse it the feature has to go. Sadly.
Are we in preschool again? You don't like how others are playing with your toys so you want to take them away from everyone. Forever.
Lskels without frozen maps are vulnerable to a TOCTOU attack from a sufficiently privileged user. Lskels currently pass unfrozen maps into the kernel, and there is nothing stopping someone from modifying them between BPF_PROG_LOAD and BPF_PROG_RUN.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
I don't necessarily disagree, but my main concern with this is that previous code signing patchsets seem to get gaslit or have the goalposts moved until they die or are abandoned.
Previous attempts to add signing failed because
- It's a difficult problem to solve
- people only cared about their own narrow use case and not
considering the needs of bpf ecosystem as a whole.
Are you saying that at this point, you would be amenable to an in-tree set of patches that enforce signature verification of lskels during BPF_PROG_LOAD that live in syscall.c,
that's the only way to do it.
So the notion of forcing people into writing bpf-based gatekeeper programs is being abandoned? e.g.
https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeo... https://lore.kernel.org/bpf/61aae2da8c7b0_68de0208dd@john.notmuch/
Not abandoned. bpf-based tuning of load conditions is still necessary. The bpf_prog_load command will check the signature only. It won't start rejecting progs that don't have a signature. For that a one liner bpf-lsm or C-based lsm would be needed to address your dont-trust-root use case.
Since this will require an LSM no matter what, there is zero reason for us not to proceed with Hornet. If or when you actually figure out how to sign an lskel and upstream updated LSM hooks, I can always rework Hornet to use that instead.
without adding extra non-code signing requirements like attachment point verification, completely eBPF-based solutions, or rich eBPF-based program run-time policy enforcement?
Those are secondary considerations that should also be discussed. Not necessarily a blocker.
Again, I'm confused here since you recently stated this whole thing was "questionable" without attachment point verification.
Correct. For fentry prog type the attachment point is checked during the load, but for tracepoints it's not, and anyone who is claiming that their system is secure because the tracepoint prog was signed is simply clueless in how bpf works.
No one is making that claim, although I do appreciate the lovely ad-hominem attack and absolutist standpoint. It's not like we invented code signing last week. All we are trying to do is make our cloud ever-so-slightly more secure and share the results with the community.
The attack vectors I'm looking at are things like CVE-2021-33200. ACLs for attachment points do nothing to stop that whereas code signing is a possible countermeasure. This kind of thing is probably a non-issue with your private cloud, but it's a very real issue with publicly accessible ones.