On Wed, Jun 11, 2025 at 1:59 PM James Bottomley
[off-list ref] wrote:

On Wed, 2025-06-11 at 00:35 +0200, KP Singh wrote:

quoted

On Tue, Jun 10, 2025 at 11:24 PM James Bottomley
[off-list ref] wrote:

quoted

On Tue, 2025-06-10 at 21:47 +0200, KP Singh wrote:

quoted

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):

quoted

proposed.  Specifically, there already exists a patch set:

https://lore.kernel.org/all/20250528215037.2081066-1-bboscaccy@linux.microsoft.com/ (local)

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.

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 believe we have discussed this enough. Let's have the BPF maintainers decide.

quoted

 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/ (local)

It's this addition in bpf_check_signature():

quoted

+     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 have read and understood the code, there is no technical misalignment.

I am talking about a trusted user space loader. You seem to confuse
the trusted BPF loader program as userspace, no this is not userspace,
it runs in the kernel context.

- KP

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