[PATCH 10/12] libbpf: Embed and verify the metadata hash in the loader

[KP Singh]

On Wed, Jun 11, 2025 at 3:12 PM James Bottomley
<James.Bottomley at hansenpartnership.com> wrote:
>
> On Wed, 2025-06-11 at 14:33 +0200, KP Singh wrote:
> > On Wed, Jun 11, 2025 at 1:59 PM James Bottomley
> > <James.Bottomley at hansenpartnership.com> wrote:
> > >
> > > On Wed, 2025-06-11 at 00:35 +0200, KP Singh wrote:
> > > > On Tue, Jun 10, 2025 at 11:24 PM James Bottomley
> > > > <James.Bottomley at hansenpartnership.com> wrote:
> > > > >
> > > > > On Tue, 2025-06-10 at 21:47 +0200, KP Singh wrote:
> > > > > > It's been repeatedly mentioned that trusted loaders (whether
> > > > > > kernel or BPF programs) are the only way because a large
> > > > > > number
> > > > > > of BPF use-cases dynamically generate BPF programs.
> > > > >
> > > > > You keep asserting this, but it isn't supported by patches
> > > > > already
> > > >
> > > > This is supported for sure. But it's not what the patches are
> > > > providing a reference implementation for. The patches provide a
> > > > stand alone reference implementation using in-kernel / BPF
> > > > loaders but you can surely implement this (see below):
> > > >
> > > > > proposed.  Specifically, there already exists a patch set:
> > > > >
> > > > > https://lore.kernel.org/all/20250528215037.2081066-1-bboscaccy@linux.microsoft.com/
> > > >
> > > > The patch-set takes a very narrow view by adding additional UAPI
> > > > and ties us into an implementation.
> > >
> > > What do you mean by this?  When kernel people say UAPI, they think
> > > of the contract between the kernel and userspace.  So for both
> > > patch sets the additional attr. entries which user space adds and
> > > the kernel parses for the signature would conventionally be thought
> > > to extend the UAPI.
> > >
> > > Additionally, the content of the signature (what it's over) is a
> > > UAPI contract.  When adding to the kernel UAPI we don't look not to
> > > change it, we look to change it in a way that is extensible.  It
> > > strikes me that actually only the linked patch does this because
> > > the UAPI addition for your signature scheme doesn't seem to be that
> > > extensible.
> >
> > James, I am adding less attributes, it's always extensible, adding
> > more UAPI than strictly needed is what's not flexible.
>
> To repeat: the object should be extensibility not minimization.  If an
> API is extensible it doesn't tie you to a specific implementation
> regardless of how many arguments it adds.  The attr structure uses the
> standard kernel way of doing this: it can grow but may never lose
> elements and  features added at the end are always optional so an older
> kernel that doesn't see them can still process everything it does
> understand.
>
> > The attributes I proposed remain valid in a world where the BPF
> > instruction set is stable at compile time, for trusted user space
> > loaders (applications like Cilium) that can already have a stable
> > instruction buffer, the attributes Blaise proposed do not.
>
> I don't follow.  For stable compilation (I'm more familiar with the way
> systemd does this but I presume cilium does the same: by constructing
> ebpf byte code on the fly that doesn't require relocation and then
> inserting it directly) you simply program the loader to do the
> restrictions (about insertion point and the like) and sign it, correct?

There is no loader program if the instruction buffer is stable.

> That's covered in the linked patch in the !attr->signature_maps_size
> case, so what Blaise proposed most definitely does do this.
>
> > I believe we have discussed this enough. Let's have the BPF
> > maintainers decide.
>
> But this is obviously an important point otherwise you wouldn't be
> arguing about it.  If pure minimization were all that's required then
> it's easy to do since we're using pkcs7 signatures, the signature can
> contain a data structure with authenticatedAttributes that are
> validated by the signature, so I could do the Blaise patch with fewer
> attr elements than you simply by moving the maps and their count into
> the athenticatedAttributes element of the pkcs7 signature.  I could

Can we discuss this as a follow up as Paul proposed? I have limited
bandwidth to work on this, so this only delays what I think is a solid
baseline implementation.

- KP

> also do the same with your keyring_id and, bonus, it would be integrity
> validated.  Then each of you adds the same number of UAPI attr's so
> there's no argument about who adds fewer attributes.
>
> Regards,
>
> James
>