Paul Moore <p...@paul-moore.com> writes: > 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?)? >
There is. That would require modifying the BPF_PROG_LOAD subcommand along with modifying the skeletobn generator to use it. I don't know if there is enough buy-in from the ebpf developers to do that currently. Tacking the signature to the end of of the light-skeleton binary allows Hornet to operate without modifying the bpf subsystem and is very similar to how module signing currently works. Modules have the advantage of having a working in-kernel loader, which makes all of this a non-issue with modules. > 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. > Will do. I don't see this being an issue. In practice it's not much different than auth schemes that use a separate passkey. The instructions and maps are passed into the kernel during BPF_PROG_LOAD via a syscall, they aren't copied from the binary. The only part that gets copied during kernel_read_file() is the signature. If there was a mismatch between what was on-disk and what was passed in via the syscall, the signature verification would fail. As long as a signature can be found somewhere for the loader program and map, that signature is valid, and that program and map can't be modified by the user after the signature is checked, it means that someone trusted signed that blob at some point in time and only signed blobs are going to run. It shouldn't matter from a math standpoint where that signature physically lives, be it in a binary image, a buffer in a syscall or even an additional map. >> + 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
