Message-ID: <CAHC9VhRjKV4AbSgqb4J_-xhkWAp_VAcKDfLJ4GwhBNPOr+cvpg@mail.gmail.com>
Date: Thu, 8 May 2025 15:23:26 -0400
From: Paul Moore <paul@...l-moore.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, 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 Thu, May 8, 2025 at 1:45 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
> 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.

At this point no, although of course he is welcome to comment if he
likes.  What we've been asking for is some detail on your preferred
solution; all we've seen on the various threads thus far has been a
small number of short sentences that leave far too much ambiguity
around important parts of the design.  Without any clear direction on
your ideal solution, Blaise has been continuing forward with proposals
that we believe solve at least one use case and can be extended in the
future to support others.

Blaise started this most recent effort by attempting to address the
concerns brought up in previous efforts, you and others rejected this
first attempt and directed Blaise towards a light skeleton and LSM
based approach, which is where he is at with Hornet.  Once again, you
reject this approach with minimal guidance on what would be
acceptable, and our response is to ask for clarification on your
preferred design.  We're not asking for a full working solution,
simply a couple of paragraphs outlining the design with enough detail
to put forward a working solution that isn't immediately NACK'd.
We've made this request multiple times in the past, most recently this
past weekend, where KP replied that he would be "happy" to share
designs/code.  Unfortunately, since then all we've received from
either you or KP since then has been effectively just a list of your
objections on repeat; surely typing out a couple of paragraphs
outlining a design would have been quicker, easier, and more
constructive then your latest reply?

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

I guess there are always two sides to every story, but "hacking into
bpf internals" is not how I would characterize the Hornet approach.
Hornet reads the light skeleton BPF loader and the associated BPF maps
(the original BPF program) passed in from userspace into a buffer in
order to verify a signature across the two BPF programs (loader +
original).

Hornet does this to verify the provenance and load time integrity of
BPF programs, two very basic security goals that people have wanted
for BPF programs for years, Blaise is simply the most recent person to
try and get something into Linus' tree.

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

The Hornet proposal is not the first thing to implement a transition
mechanism, but if that is really the blocker you want to go with I'm
sure Blaise would be happy to back out the change.  My understanding
is that it is really about reducing warnings from systemd and nothing
more.

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

Let's just ignore the "borderline" comment on kernel module signing;
the fact is that kernel module signing has been an accepted part of
the upstream Linux kernel for years now, which is part of why Blaise
used that as a starting point for the Hornet approach: keep things
simple and build on something that works.  This is especially
important as we are still largely guessing about the details of your
ideal solution.

Based on some of the vague feedback from you and KP, this week Blaise
has been experimenting with passing a signature via the bpf_attr
struct, but until you provide more detail on-list it is still a
guessing game as to what you might accept.  The kmod inspired
signature-on-ELF approach has the advantage of being much more
self-contained, which is the reason we saw it as a good starting point
that could be augmented with additional schemes in the future if/when
needed.

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

The LSM hook used by Hornet happens well after all of the BPF based
capability and access control checks.  If anything, the ability of a
malicious user to tamper with the BPF program being loaded helps
highlight the importance of validating signatures on BPF programs.  In
the worst case, if the kernel can't allocate a buffer the
kernel_read_file() will fail and an error will be returned to
userspace.

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

Yet BPF was extracted out as a standalone mechanism that has grown
well beyond its initial use as a socket data filter.  From my
perspective either BPF is a standalone entity and the
map_lookup_elem() API is necessary for at least one current user, the
socket fitler, or map_lookup_elem() isn't an API in which case BPF
isn't the general purpose tool that everyone claims.

This touches on another issue that I've brought up before in this
thread which is important.  The LSM subsystem doesn't care about how
the LSM is implemented; C, BPF, and Rust (work is obviously still very
early on the Rust side) are all valid implementation languages, and we
make it clear that if you can do something in one language, you should
be able to do it another.  While I don't believe you or KP have
explicitly stated that your objection to the Hornet approach is
largely due to it being written in C, Daniel did make a comment that
Hornet should be written as a BPF LSM.  From the perspective of the
LSM, something is either "legal" to do in a LSM, or it isn't; we don't
qualify that determination based on the source language of the
implementation.

> Also, do build 'hornet' LSM with LOCKDEP and see how many bpf map
> lifetime rules you violated with that map_lookup_elem() call.

I'm sure Blaise will look into that.  I'm sure that if you, KP,
Daniel, or anyone else in BPF land wanted to provide an alternate API
for map access I'm sure Blaise would be happy to rework his code.  As
we've stated multiple times, don't just say "no" say "no" with a good
description about how to move forward.

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

For this initial use case, we believe that is sufficient and is inline
with kernel module loading, which is a good analogy.  The most
critical part as far as we are concerned is that userspace can not
tamper with the BPF map once the signature has been verified.  Yes,
another in-kernel component might be able to tamper with the BPF map,
but in-kernel components can do a number of bad things; this is yet
one more reason why validating code that executes in kernel context is
important.

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

The current Hornet implementation ignores things it doesn't know
about, sticking to parts of the BPF instruction set that is currently
defined.  Who knows what Hornet might look like in another few
revisions, or if it is replaced with another approach, but if it
becomes part of Linus' tree it should be trivial to update it along
with BPF.  There is precedence for this with other LSMs and other
kernel subsystems.

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

As mentioned *many* times before, please provide more information on
this.  To start, where *exactly* in the BPF subsystem would you accept
a signature check.

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

...

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

In order to work effectively we need a more detailed explanation of
what you want to see with respect to BPF signing.  Thus far we've only
seen rejections with very hand-wavy explanations on how to proceed.
KP indicated almost a week ago that he is happy to share a couple of
paragraphs on a preferred BPF signing approach, but we're still left
waiting with the only feedback being a repeat of previous rejections
and more vitrol.

-- 
paul-moore.com

Powered by blists - more mailing lists