On Wed, May 7, 2025 at 4:24 PM Paul Moore paul@paul-moore.com wrote:
On Wed, May 7, 2025 at 1:48 PM James Bottomley James.Bottomley@hansenpartnership.com wrote:
I'm with Paul on this: if you could share your design ideas more fully than you have above that would help make this debate way more technical.
I think it would also help some of us, at the very least me, put your objections into context. I believe the more durable solutions that end up in Linus' tree are combinations of designs created out of compromise, and right now we are missing the context and detail of your ideal solution to be able to do that compromise and get to a design and implementation we can all begrudgingly accept. In the absence of a detailed alternate design, and considering that BPF signature validation efforts have sputtered along for years without any real success, we'll continue to push forward on-list with refinements to the current proposal in an effort to drive this to some form of resolution.
It sounds like you're asking for Linus's opinion.
This 'hornet' LSM attempts to implement signed bpf programs by hacking into bpf internals: https://lore.kernel.org/bpf/20250502184421.1424368-2-bboscaccy@linux.microso...
It got 3 Nacks from bpf maintainers. Let me recap our objections:
1. Your definition of attack surface says that root is untrusted. Yet this "hornet" LSM allowlists systemd as trusted. Allegedly, it's an intermediate solution. What it really means that as presented the security is broken, since an untrusted root can poke into systemd and make it load whatever bpf programs it wants.
2. you propose to mangle every binary in the system that needs to load bpf programs with an extra "signature" at the end of the binary that breaks ELF format. I already explained earlier that such an approach was a border line acceptable solution for kernel modules, but certainly not ok as a general approach for all binaries. The kernel modules are consumed by the kernel and user space doesn't touch them. It's not ok to mangle general purpose binaries. The user space tooling relies on well formed ELF files. 'perf record' and any profiling tool will parse various ELF binaries to symbolize addresses. If ELF is corrupted such tools may crash. So far you've been lucky that ld-linux.so was able to interpret such corrupted ELF. It's not something to rely on.
3. To read this mangled ELF you do: file = get_task_exe_file(current); buf_sz = kernel_read_file(file, 0, &buf, INT_MAX, &sz, READING_EBPF); A malicious user can give you a multi gigabyte file and your LSM will happily read it into the kernel memory. It's an obvious DoS vector.
4. I said multiple times it's not ok to do bpf_map->ops->map_lookup_elem() outside of the bpf subsystem. You did 'git grep' and argued that something in the net/ directory is doing that. Well, ./scripts/get_maintainer.pl `git grep -l -- '->map_lookup_elem' net/` is your answer. net/core/filter.c is a part of the bpf subsystem. The bpf originated in networking. Also, do build 'hornet' LSM with LOCKDEP and see how many bpf map lifetime rules you violated with that map_lookup_elem() call.
5. I also explained that map->frozen == true doesn't guarantee that the map is immutable. It only freezes the map from user space writes. You argued that when all bpf programs are signed then the non-signed attacker cannot mangle the map. That's broken for two reasons: - you allow systemd to load bpf without signatures - even when all bpf progs are signed, the program actions are not controlled after loading, so signed bpf prog that uses map-in-map is subject to an attack where a malicious root can add loader-map as inner map-in-map and an unsuspecting program will mangle it.
6. Though bpf instructions have standardized format LSMs shouldn't not be in the business of parsing them. New instructions are being added. We don't need a headache that an LSM will start crashing/misbehaving when a new instruction is added or extended. bpf instruction parsing belongs in the bpf subsystem.
7. You do: copy_from_bpfptr_offset(&map_fd, ...) then proceed with content extraction, but this is a user controlled address. It's an obvious TOCTOU bug. The user can replace that map_fd after your LSM read it and before bpf verifier reads it. So the signature checking from LSM is fundamentally broken. I already explained that the signature check has to be done within a bpf subsystem.
In the last kernel release we added 'bool kernel' parameter to security_bpf_prog_load() LSM hook assuming that you're going to work with us on actual solution for signed bpf programs, but so far you ignored our feedback and accused us of artificially delaying a solution to signed programs, though we told you that the "light skeleton" (that you incorrectly attempt to use here) was designed specifically as a building block towards signed bpf programs:
See the cover letter from 2021: https://lore.kernel.org/bpf/20210514003623.28033-1-alexei.starovoitov@gmail....
" This is a first step towards signed bpf programs and the third approach of that kind. ... Future steps: - support CO-RE in the kernel - generate light skeleton for kernel - finally do the signing of the loader program " Later in 2022-23 we implemented "CORE in the kernel" and "light skel for kernel" steps. There are a few more steps to do that we didn't anticipate back in 2021. Like the exclusive map <-> prog relationship and error propagation through the loading process.
When Blaise volunteered to work on signed progs we pointed him to light skeleton assuming that you're going to work with us to finish this complex task. Instead you went with this broken LSM and now argue that your insecure approach somehow should be accepted by upstream and microsoft users. Sorry, but users deserve better than this.
The only path forward here is to: - stop pushing broken code - internalize the feedback - work with the bpf community
The problem is difficult and a truly secure solution will take time. You cannot short circuit this path.