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

[James Bottomley]

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.

>  Whereas the current approach keeps the UAPI clean while still
> meeting all the use-cases and keeps the implementation flexible
> should it need to change. (no tie into the hash chain approach, if we
> are able to move to stable BPF instruction buffers in the future).
> 
> Blaise's patches also do not handle the trusted user-space loader
> space and the "signature_maps" are not relevant to dynamic generation
> or simple BPF programs like networking, see below.

OK, is this just a technical misreading?  I missed the fact that it
supported both schemes on first reading as well.  If you look in this
patch:

https://lore.kernel.org/all/20250528215037.2081066-2-bboscaccy@linux.microsoft.com/

It's this addition in bpf_check_signature():

> +	if (!attr->signature_maps_size) {
> +		sha256((u8 *)prog->insnsi, prog->len * sizeof(struct bpf_insn), (u8 *)&hash);
> +		err = verify_pkcs7_signature(hash, sizeof(hash), signature, attr->signature_size,
> +				     VERIFY_USE_SECONDARY_KEYRING,
> +				     VERIFYING_EBPF_SIGNATURE,
> +				     NULL, NULL);
> +	} else {
> +		used_maps = kmalloc_array(attr->signature_maps_size,
> +					  sizeof(*used_maps), GFP_KERNEL);
> [...]

The first leg of the if is your use case: a zero map size means the
signature is a single hash of the loader only.  The else clause
encompasses a hash chain over the maps as well.  This means the signer
can choose which scheme they want.

I'll skip responding to the rest since it seems to be assuming that
Blaise's patch excludes your use case (which the above should
demonstrate it doesn't) and we'd be talking past each other.

Regards,

James