Re: [PATCH v3 0/4] Introducing Hornet LSM

[KP Singh <kpsingh@kernel.org>]

> This patch series introduces the Hornet LSM. The goal of Hornet is to provide
> a signature verification mechanism for eBPF programs.
>

[...]

>
> References: [1]
> https://lore.kernel.org/bpf/20220209054315.73833-1-alexei.starovoitov@gmail.com/
> [2]
> https://lore.kernel.org/bpf/CAADnVQ+wPK1KKZhCgb-Nnf0Xfjk8M1UpX5fnXC=cBzdEYbv_kg@mail.gmail.com/
>
> Change list: - v2 -> v3 - Remove any and all usage of proprietary bpf APIs

BPF APIs are not proprietary, but you cannot implement BPF program signing
for BPF users without aligning with the BPF maintainers and the community.
Signed programs are a UAPI and a key part of how developers experience BPF
and this is not how we would like signing to be experienced by BPF users.

Some more feedback (which should be pretty obvious) but explicitly:

* Hacks like if (current->pid == 1) return 0; also break your threat model
  about root being untrusted. This is all the more reason I think signing
  should be integrated into other LSMs, only a proper LSM policy can breathe
  life into the root / kernel boundary. 

* You also did not take the feedback into account:

   new = map->ops->map_lookup_elem(map, &key);

  This is not okay without having the BPF maintainers aligned, the same way as 

  https://patchwork.kernel.org/project/netdevbpf/patch/20240629084331.3807368-4-kpsingh@kernel.org/#25928981

  was not okay. Let's not have double standards.

* And you copy pasted tools/testing/selftests/hornet/frozen_skel.h which
  is what you expect users to do? Not acceptable either.

So for this approach, it's a:

Nacked-by: KP Singh <kpsingh@kernel.org>

Now if you really care about the use-case and want to work with the maintainers
and implement signing for the community, here's how we think it should be done:

* The core signing logic and the tooling stays in BPF, something that the users
  are already using. No new tooling.
* The policy decision on the effect of signing can be built into various
  existing LSMs. I don't think we need a new LSM for it.
* A simple UAPI (emphasis on UAPI!) change to union bpf_attr in uapi/bpf.h in
  the BPF_PROG_LOAD command:

__aligned_u64 signature; 
__u32 signature_size;