Message-ID: <CAADnVQ+C2KNR1ryRtBGOZTNk961pF+30FnU9n3dt3QjaQu_N6Q@mail.gmail.com>
Date: Thu, 8 May 2025 10:44:55 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Paul Moore <paul@...l-moore.com>, Linus Torvalds <torvalds@...ux-foundation.org>
Cc: KP Singh <kpsingh@...nel.org>, 
	James Bottomley <James.Bottomley@...senpartnership.com>, 
	Blaise Boscaccy <bboscaccy@...ux.microsoft.com>, bpf <bpf@...r.kernel.org>, code@...icks.com, 
	Jonathan Corbet <corbet@....net>, "David S. Miller" <davem@...emloft.net>, 
	David Howells <dhowells@...hat.com>, Günther Noack <gnoack@...gle.com>, 
	Herbert Xu <herbert@...dor.apana.org.au>, Jarkko Sakkinen <jarkko@...nel.org>, 
	James Morris <jmorris@...ei.org>, Jan Stancek <jstancek@...hat.com>, 
	Justin Stitt <justinstitt@...gle.com>, keyrings@...r.kernel.org, 
	Linux Crypto Mailing List <linux-crypto@...r.kernel.org>, 
	"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>, 
	Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, 
	LSM List <linux-security-module@...r.kernel.org>, 
	clang-built-linux <llvm@...ts.linux.dev>, Masahiro Yamada <masahiroy@...nel.org>, 
	Mickaël Salaün <mic@...ikod.net>, 
	Bill Wendling <morbo@...gle.com>, Nathan Chancellor <nathan@...nel.org>, Neal Gompa <neal@...pa.dev>, 
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Nicolas Schier <nicolas@...sle.eu>, nkapron@...gle.com, 
	Roberto Sassu <roberto.sassu@...wei.com>, "Serge E . Hallyn" <serge@...lyn.com>, 
	Shuah Khan <shuah@...nel.org>, Matteo Croce <teknoraver@...a.com>, 
	Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: [PATCH v3 0/4] Introducing Hornet LSM

On Wed, May 7, 2025 at 4:24 PM Paul Moore <paul@...l-moore.com> wrote:
>
> On Wed, May 7, 2025 at 1:48 PM James Bottomley
> <James.Bottomley@...senpartnership.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.microsoft.com/

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.com/

"
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.

Powered by blists - more mailing lists