On Apr 4, 2025 Blaise Boscaccy <bbosca...@linux.microsoft.com> wrote:
>
> This adds the Hornet Linux Security Module which provides signature
> verification of eBPF programs. This allows users to continue to
> maintain an invariant that all code running inside of the kernel has
> been signed.
>
> The primary target for signature verification is light-skeleton based
> eBPF programs which was introduced here:
> https://lore.kernel.org/bpf/20220209054315.73833-1-alexei.starovoi...@gmail.com/
>
> eBPF programs, before loading, undergo a complex set of operations
> which transform pseudo-values within the immediate operands of
> instructions into concrete values based on the running
> system. Typically, this is done by libbpf in
> userspace. Light-skeletons were introduced in order to support
> preloading of bpf programs and user-mode-drivers by removing the
> dependency on libbpf and userspace-based operations.
>
> Userpace modifications, which may change every time a program gets
> loaded or runs on a slightly different kernel, break known signature
> verification algorithms. A method is needed for passing unadulterated
> binary buffers into the kernel in-order to use existing signature
> verification algorithms. Light-skeleton loaders with their support of
> only in-kernel relocations fit that constraint.
>
> Hornet employs a signature verification scheme similar to that of
> kernel modules. A signature is appended to the end of an
> executable file. During an invocation of the BPF_PROG_LOAD subcommand,
> a signature is extracted from the current task's executable file. That
> signature is used to verify the integrity of the bpf instructions and
> maps which were passed into the kernel. Additionally, Hornet
> implicitly trusts any programs which were loaded from inside kernel
> rather than userspace, which allows BPF_PRELOAD programs along with
> outputs for BPF_SYSCALL programs to run.
>
> The validation check consists of checking a PKCS#7 formatted signature
> against a data buffer containing the raw instructions of an eBPF
> program, followed by the initial values of any maps used by the
> program. Maps are frozen before signature verification checking to
> stop TOCTOU attacks.
>
> Signed-off-by: Blaise Boscaccy <bbosca...@linux.microsoft.com>
> ---
> Documentation/admin-guide/LSM/Hornet.rst | 55 ++++++
> Documentation/admin-guide/LSM/index.rst | 1 +
> MAINTAINERS | 9 +
> crypto/asymmetric_keys/pkcs7_verify.c | 10 +
> include/linux/kernel_read_file.h | 1 +
> include/linux/verification.h | 1 +
> include/uapi/linux/lsm.h | 1 +
> security/Kconfig | 3 +-
> security/Makefile | 1 +
> security/hornet/Kconfig | 11 ++
> security/hornet/Makefile | 4 +
> security/hornet/hornet_lsm.c | 239 +++++++++++++++++++++++
> 12 files changed, 335 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/admin-guide/LSM/Hornet.rst
> create mode 100644 security/hornet/Kconfig
> create mode 100644 security/hornet/Makefile
> create mode 100644 security/hornet/hornet_lsm.c
...
> diff --git a/security/hornet/hornet_lsm.c b/security/hornet/hornet_lsm.c
> new file mode 100644
> index 000000000000..d9e36764f968
> --- /dev/null
> +++ b/security/hornet/hornet_lsm.c
...
> +/* kern_sys_bpf is declared as an EXPORT_SYMBOL in kernel/bpf/syscall.c,
> however no definition is
> + * provided in any bpf header files. If/when this function has a proper
> definition provided
> + * somewhere this declaration should be removed
> + */
> +int kern_sys_bpf(int cmd, union bpf_attr *attr, unsigned int size);
I believe the maximum generally accepted line length is now up to 100
characters, but I remain a big fan of the ol' 80 character terminal
width and would encourage you to stick to that if possible. However,
you're the one who is signing on for maintenence of Hornet, not me, so
if you love those >80 char lines, you do you :)
I also understand why you are doing the kern_sys_bpf() declaration here,
but once this lands in Linus' tree I would encourage you to try moving
the declaration into a kernel-wide BPF header where it really belongs.
> +static int hornet_check_binary(struct bpf_prog *prog, union bpf_attr *attr,
> + struct hornet_maps *maps)
> +{
> + struct file *file = get_task_exe_file(current);
> + const unsigned long markerlen = sizeof(EBPF_SIG_STRING) - 1;
> + void *buf = NULL;
> + size_t sz = 0, sig_len, prog_len, buf_sz;
> + int err = 0;
> + struct module_signature sig;
> +
> + buf_sz = kernel_read_file(file, 0, &buf, INT_MAX, &sz, READING_EBPF);
> + fput(file);
> + if (!buf_sz)
> + return -1;
I'm pretty sure I asked you about this already off-list, but I can't
remember the answer so I'm going to bring this up again :)
This file read makes me a bit nervous about a mismatch between the
program copy operation done in the main BPF code and the copy we do
here in kernel_read_file(). Is there not some way to build up the
buffer with the BPF program from the attr passed into this function
(e.g. attr.insns?)?
If there is some clever reason why all of this isn't an issue, it might
be a good idea to put a small comment above the kernel_read_file()
explaining why it is both safe and good.
> + prog_len = buf_sz;
> +
> + if (prog_len > markerlen &&
> + memcmp(buf + prog_len - markerlen, EBPF_SIG_STRING, markerlen) == 0)
> + prog_len -= markerlen;
> +
> + memcpy(&sig, buf + (prog_len - sizeof(sig)), sizeof(sig));
> + sig_len = be32_to_cpu(sig.sig_len);
> + prog_len -= sig_len + sizeof(sig);
> +
> + err = mod_check_sig(&sig, prog->len * sizeof(struct bpf_insn), "ebpf");
> + if (err)
> + return err;
> + return hornet_verify_lskel(prog, maps, buf + prog_len, sig_len);
> +}
--
paul-moore.com
