This patch series introduces the Hornet LSM. The goal of Hornet is to provide a signature verification mechanism for eBPF programs.
eBPF has similar requirements to that of modules when it comes to loading: find symbol addresses, fix up ELF relocations, some struct field offset handling stuff called CO-RE (compile-once run-anywhere), and some other miscellaneous bookkeeping. During eBPF program compilation, pseudo-values get written to the immediate operands of instructions. During loading, those pseudo-values get rewritten with concrete addresses or data applicable to the currently running system, e.g., a kallsyms address or an fd for a map. This needs to happen before the instructions for a bpf program are loaded into the kernel via the bpf() syscall. Unlike modules, an in-kernel loader unfortunately doesn't exist. Typically, the instruction rewriting is done dynamically in userspace via libbpf. Since the relocations and instruction modifications are happening in userspace, and their values may change depending upon the running system, this breaks known signature verification mechanisms.
Light skeleton programs were introduced in order to support early loading of eBPF programs along with user-mode drivers. They utilize a separate eBPF program that can load a target eBPF program and perform all necessary relocations in-kernel without needing a working userspace. Light skeletons were mentioned as a possible path forward for signature verification.
Hornet takes a simple approach to light-skeleton-based eBPF signature verification. A PKCS#7 signature of a data buffer containing the raw instructions of an eBPF program, followed by the initial values of any maps used by the program is used. A utility script is provided to parse and extract the contents of autogenerated header files created via bpftool. That payload can then be signed and appended to the light skeleton executable.
Maps are frozen to prevent TOCTOU bugs where a sufficiently privileged user could rewrite map data between the calls to BPF_PROG_LOAD and BPF_PROG_RUN. Additionally, both sparse-array-based and fd_array_cnt-based map fd arrays are supported for signature verification.
References: [1] https://lore.kernel.org/bpf/20220209054315.73833-1-alexei.starovoitov@gmail.... [2] https://lore.kernel.org/bpf/CAADnVQ+wPK1KKZhCgb-Nnf0Xfjk8M1UpX5fnXC=cBzdEYbv...
Change list: - v1 -> v2 - Jargon clarification, maintainer entry and a few cosmetic fixes
Revisions: - v1 https://lore.kernel.org/bpf/20250321164537.16719-1-bboscaccy@linux.microsoft...
Blaise Boscaccy (4): security: Hornet LSM hornet: Introduce sign-ebpf hornet: Add a light-skeleton data extactor script selftests/hornet: Add a selftest for the Hornet LSM
Documentation/admin-guide/LSM/Hornet.rst | 53 +++ 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 + scripts/Makefile | 1 + scripts/hornet/Makefile | 5 + scripts/hornet/extract-skel.sh | 29 ++ scripts/hornet/sign-ebpf.c | 411 +++++++++++++++++++ security/Kconfig | 3 +- security/Makefile | 1 + security/hornet/Kconfig | 11 + security/hornet/Makefile | 4 + security/hornet/hornet_lsm.c | 239 +++++++++++ tools/testing/selftests/Makefile | 1 + tools/testing/selftests/hornet/Makefile | 51 +++ tools/testing/selftests/hornet/loader.c | 21 + tools/testing/selftests/hornet/trivial.bpf.c | 33 ++ 20 files changed, 885 insertions(+), 1 deletion(-) create mode 100644 Documentation/admin-guide/LSM/Hornet.rst create mode 100644 scripts/hornet/Makefile create mode 100755 scripts/hornet/extract-skel.sh create mode 100644 scripts/hornet/sign-ebpf.c create mode 100644 security/hornet/Kconfig create mode 100644 security/hornet/Makefile create mode 100644 security/hornet/hornet_lsm.c create mode 100644 tools/testing/selftests/hornet/Makefile create mode 100644 tools/testing/selftests/hornet/loader.c create mode 100644 tools/testing/selftests/hornet/trivial.bpf.c
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.starovoitov@gmail....
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 bboscaccy@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/Documentation/admin-guide/LSM/Hornet.rst b/Documentation/admin-guide/LSM/Hornet.rst new file mode 100644 index 000000000000..e0d2926c4041 --- /dev/null +++ b/Documentation/admin-guide/LSM/Hornet.rst @@ -0,0 +1,55 @@ +.. SPDX-License-Identifier: GPL-2.0 + +====== +Hornet +====== + +Hornet is a Linux Security Module that provides signature verification +for eBPF programs. This is selectable at build-time with +``CONFIG_SECURITY_HORNET``. + +Overview +======== + +Hornet provides signature verification for eBPF programs by utilizing +the existing PKCS#7 infrastructure that's used for module signature +verification. Hornet works by creating a buffer containing the eBPF +program instructions along with its associated maps and checking a +signature against that buffer. The signature is appended to the end of +the lskel executable file and is extracted at runtime via +get_task_exe_file. Hornet works by hooking into the +security_bpf_prog_load hook. Load invocations that originate from the +kernel (bpf preload, results of bpf_syscall programs, etc.) are +allowed to run unconditionally. Calls that originate from userspace +require signature verification. If signature verification fails, the +program will fail to load. Maps are frozen before the signature check +to prevent TOCTOU exploits where a sufficiently privileged user could +rewrite map data between the calls to BPF_PROG_LOAD and BPF_PROG_RUN. + +Instruction/Map Ordering +======================== + +Hornet supports both sparse-array based maps via map discovery along +with the newly added fd_array_cnt API for continuous map arrays. The +buffer used for signature verification is assumed to be the +instructions followed by all maps used, ordered by their index in +fd_array. + +Tooling +======= + +Some tooling is provided to aid with the development of signed eBPF +lskels. + +extract-skel.sh +--------------- + +This shell script extracts the instructions and map data used by the +light skeleton from the autogenerated header file created by bpftool. + +sign-ebpf +--------- + +sign-ebpf works similarly to the sign-file script with one key +difference: it takes a separate input binary used for signature +verification and will append the signature to a different output file. diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst index ce63be6d64ad..0ecb5016ce1f 100644 --- a/Documentation/admin-guide/LSM/index.rst +++ b/Documentation/admin-guide/LSM/index.rst @@ -48,3 +48,4 @@ subdirectories. Yama SafeSetID ipe + Hornet diff --git a/MAINTAINERS b/MAINTAINERS index 3864d473f52f..a2355059cdf4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10588,6 +10588,15 @@ S: Maintained F: Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml F: drivers/iio/pressure/mprls0025pa*
+HORNET SECURITY MODULE +M: Blaise Boscaccy bboscaccy@linux.microsoft.com +L: linux-security-module@vger.kernel.org +S: Supported +T: git https://github.com/blaiseboscaccy/hornet.git +F: Documentation/admin-guide/LSM/Hornet.rst +F: scripts/hornet/ +F: security/hornet/ + HP BIOSCFG DRIVER M: Jorge Lopez jorge.lopez2@hp.com L: platform-driver-x86@vger.kernel.org diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c index f0d4ff3c20a8..1a5fbb361218 100644 --- a/crypto/asymmetric_keys/pkcs7_verify.c +++ b/crypto/asymmetric_keys/pkcs7_verify.c @@ -428,6 +428,16 @@ int pkcs7_verify(struct pkcs7_message *pkcs7, } /* Authattr presence checked in parser */ break; + case VERIFYING_EBPF_SIGNATURE: + if (pkcs7->data_type != OID_data) { + pr_warn("Invalid ebpf sig (not pkcs7-data)\n"); + return -EKEYREJECTED; + } + if (pkcs7->have_authattrs) { + pr_warn("Invalid ebpf sig (has authattrs)\n"); + return -EKEYREJECTED; + } + break; case VERIFYING_UNSPECIFIED_SIGNATURE: if (pkcs7->data_type != OID_data) { pr_warn("Invalid unspecified sig (not pkcs7-data)\n"); diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h index 90451e2e12bd..7ed9337be542 100644 --- a/include/linux/kernel_read_file.h +++ b/include/linux/kernel_read_file.h @@ -14,6 +14,7 @@ id(KEXEC_INITRAMFS, kexec-initramfs) \ id(POLICY, security-policy) \ id(X509_CERTIFICATE, x509-certificate) \ + id(EBPF, ebpf) \ id(MAX_ID, )
#define __fid_enumify(ENUM, dummy) READING_ ## ENUM, diff --git a/include/linux/verification.h b/include/linux/verification.h index 4f3022d081c3..812be8ad5f74 100644 --- a/include/linux/verification.h +++ b/include/linux/verification.h @@ -35,6 +35,7 @@ enum key_being_used_for { VERIFYING_KEXEC_PE_SIGNATURE, VERIFYING_KEY_SIGNATURE, VERIFYING_KEY_SELF_SIGNATURE, + VERIFYING_EBPF_SIGNATURE, VERIFYING_UNSPECIFIED_SIGNATURE, NR__KEY_BEING_USED_FOR }; diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h index 938593dfd5da..2ff9bcdd551e 100644 --- a/include/uapi/linux/lsm.h +++ b/include/uapi/linux/lsm.h @@ -65,6 +65,7 @@ struct lsm_ctx { #define LSM_ID_IMA 111 #define LSM_ID_EVM 112 #define LSM_ID_IPE 113 +#define LSM_ID_HORNET 114
/* * LSM_ATTR_XXX definitions identify different LSM attributes diff --git a/security/Kconfig b/security/Kconfig index f10dbf15c294..0030f0224c7a 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -230,6 +230,7 @@ source "security/safesetid/Kconfig" source "security/lockdown/Kconfig" source "security/landlock/Kconfig" source "security/ipe/Kconfig" +source "security/hornet/Kconfig"
source "security/integrity/Kconfig"
@@ -273,7 +274,7 @@ config LSM default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,ipe,bpf" if DEFAULT_SECURITY_APPARMOR default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,ipe,bpf" if DEFAULT_SECURITY_TOMOYO default "landlock,lockdown,yama,loadpin,safesetid,ipe,bpf" if DEFAULT_SECURITY_DAC - default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,ipe,bpf" + default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,ipe,hornet,bpf" help A comma-separated list of LSMs, in initialization order. Any LSMs left off this list, except for those with order diff --git a/security/Makefile b/security/Makefile index 22ff4c8bd8ce..e24bccd951f8 100644 --- a/security/Makefile +++ b/security/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_CGROUPS) += device_cgroup.o obj-$(CONFIG_BPF_LSM) += bpf/ obj-$(CONFIG_SECURITY_LANDLOCK) += landlock/ obj-$(CONFIG_SECURITY_IPE) += ipe/ +obj-$(CONFIG_SECURITY_HORNET) += hornet/
# Object integrity file lists obj-$(CONFIG_INTEGRITY) += integrity/ diff --git a/security/hornet/Kconfig b/security/hornet/Kconfig new file mode 100644 index 000000000000..19406aa237ac --- /dev/null +++ b/security/hornet/Kconfig @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0-only +config SECURITY_HORNET + bool "Hornet support" + depends on SECURITY + default n + help + This selects Hornet. + Further information can be found in + Documentation/admin-guide/LSM/Hornet.rst. + + If you are unsure how to answer this question, answer N. diff --git a/security/hornet/Makefile b/security/hornet/Makefile new file mode 100644 index 000000000000..79f4657b215f --- /dev/null +++ b/security/hornet/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0-only +obj-$(CONFIG_SECURITY_HORNET) := hornet.o + +hornet-y := hornet_lsm.o 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 @@ -0,0 +1,239 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Hornet Linux Security Module + * + * Author: Blaise Boscaccy bboscaccy@linux.microsoft.com + * + * Copyright (C) 2025 Microsoft Corporation + */ + +#include <linux/lsm_hooks.h> +#include <uapi/linux/lsm.h> +#include <linux/bpf.h> +#include <linux/verification.h> +#include <crypto/public_key.h> +#include <linux/module_signature.h> +#include <crypto/pkcs7.h> +#include <linux/bpf_verifier.h> +#include <linux/sort.h> + +#define EBPF_SIG_STRING "~eBPF signature appended~\n" + +struct hornet_maps { + u32 used_idx[MAX_USED_MAPS]; + u32 used_map_cnt; + bpfptr_t fd_array; +}; + +static int cmp_idx(const void *a, const void *b) +{ + return *(const u32 *)a - *(const u32 *)b; +} + +static int add_used_map(struct hornet_maps *maps, int idx) +{ + int i; + + for (i = 0; i < maps->used_map_cnt; i++) + if (maps->used_idx[i] == idx) + return i; + + if (maps->used_map_cnt >= MAX_USED_MAPS) + return -E2BIG; + + maps->used_idx[maps->used_map_cnt] = idx; + return maps->used_map_cnt++; +} + +static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) +{ + struct bpf_insn *insn = prog->insnsi; + int insn_cnt = prog->len; + int i; + int err; + + for (i = 0; i < insn_cnt; i++, insn++) { + if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) { + switch (insn[0].src_reg) { + case BPF_PSEUDO_MAP_IDX_VALUE: + case BPF_PSEUDO_MAP_IDX: + err = add_used_map(maps, insn[0].imm); + if (err < 0) + return err; + break; + default: + break; + } + } + } + /* Sort the spare-array indices. This should match the map ordering used during + * signature generation + */ + sort(maps->used_idx, maps->used_map_cnt, sizeof(*maps->used_idx), + cmp_idx, NULL); + + return 0; +} + +static int hornet_populate_fd_array(struct hornet_maps *maps, u32 fd_array_cnt) +{ + int i; + + if (fd_array_cnt > MAX_USED_MAPS) + return -E2BIG; + + for (i = 0; i < fd_array_cnt; i++) + maps->used_idx[i] = i; + + maps->used_map_cnt = fd_array_cnt; + return 0; +} + +/* 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); + +static int hornet_verify_lskel(struct bpf_prog *prog, struct hornet_maps *maps, + void *sig, size_t sig_len) +{ + int fd; + u32 i; + void *buf; + void *new; + size_t buf_sz; + struct bpf_map *map; + int err = 0; + int key = 0; + union bpf_attr attr = {0}; + + buf = kmalloc_array(prog->len, sizeof(struct bpf_insn), GFP_KERNEL); + if (!buf) + return -ENOMEM; + buf_sz = prog->len * sizeof(struct bpf_insn); + memcpy(buf, prog->insnsi, buf_sz); + + for (i = 0; i < maps->used_map_cnt; i++) { + err = copy_from_bpfptr_offset(&fd, maps->fd_array, + maps->used_idx[i] * sizeof(fd), + sizeof(fd)); + if (err < 0) + continue; + if (fd < 1) + continue; + + map = bpf_map_get(fd); + if (IS_ERR(map)) + continue; + + /* don't allow userspace to change map data used for signature verification */ + if (!map->frozen) { + attr.map_fd = fd; + err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr)); + if (err < 0) + goto out; + } + + new = krealloc(buf, buf_sz + map->value_size, GFP_KERNEL); + if (!new) { + err = -ENOMEM; + goto out; + } + buf = new; + new = map->ops->map_lookup_elem(map, &key); + if (!new) { + err = -ENOENT; + goto out; + } + memcpy(buf + buf_sz, new, map->value_size); + buf_sz += map->value_size; + } + + err = verify_pkcs7_signature(buf, buf_sz, sig, sig_len, + VERIFY_USE_SECONDARY_KEYRING, + VERIFYING_EBPF_SIGNATURE, + NULL, NULL); +out: + kfree(buf); + return err; +} + +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; + + 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); +} + +static int hornet_check_signature(struct bpf_prog *prog, union bpf_attr *attr, + struct bpf_token *token) +{ + struct hornet_maps maps = {0}; + int err; + + /* support both sparse arrays and explicit continuous arrays of map fds */ + if (attr->fd_array_cnt) + err = hornet_populate_fd_array(&maps, attr->fd_array_cnt); + else + err = hornet_find_maps(prog, &maps); + + if (err < 0) + return err; + + maps.fd_array = make_bpfptr(attr->fd_array, false); + return hornet_check_binary(prog, attr, &maps); +} + +static int hornet_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, + struct bpf_token *token, bool is_kernel) +{ + if (is_kernel) + return 0; + return hornet_check_signature(prog, attr, token); +} + +static struct security_hook_list hornet_hooks[] __ro_after_init = { + LSM_HOOK_INIT(bpf_prog_load, hornet_bpf_prog_load), +}; + +static const struct lsm_id hornet_lsmid = { + .name = "hornet", + .id = LSM_ID_HORNET, +}; + +static int __init hornet_init(void) +{ + pr_info("Hornet: eBPF signature verification enabled\n"); + security_add_hooks(hornet_hooks, ARRAY_SIZE(hornet_hooks), &hornet_lsmid); + return 0; +} + +DEFINE_LSM(hornet) = { + .name = "hornet", + .init = hornet_init, +};
Hi Blaise,
kernel test robot noticed the following build errors:
[auto build test ERROR on shuah-kselftest/next] [also build test ERROR on shuah-kselftest/fixes herbert-cryptodev-2.6/master herbert-crypto-2.6/master masahiroy-kbuild/for-next masahiroy-kbuild/fixes v6.14] [cannot apply to linus/master next-20250404] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Blaise-Boscaccy/security-Horn... base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next patch link: https://lore.kernel.org/r/20250404215527.1563146-2-bboscaccy%40linux.microso... patch subject: [PATCH v2 security-next 1/4] security: Hornet LSM config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20250406/202504061441.FMnrO665-lkp@i...) compiler: sh4-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250406/202504061441.FMnrO665-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202504061441.FMnrO665-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from security/hornet/hornet_lsm.c:10:
security/hornet/hornet_lsm.c:221:38: error: initialization of 'int (*)(struct bpf_prog *, union bpf_attr *, struct bpf_token *)' from incompatible pointer type 'int (*)(struct bpf_prog *, union bpf_attr *, struct bpf_token *, bool)' {aka 'int (*)(struct bpf_prog *, union bpf_attr *, struct bpf_token *, _Bool)'} [-Wincompatible-pointer-types]
221 | LSM_HOOK_INIT(bpf_prog_load, hornet_bpf_prog_load), | ^~~~~~~~~~~~~~~~~~~~ include/linux/lsm_hooks.h:136:35: note: in definition of macro 'LSM_HOOK_INIT' 136 | .hook = { .NAME = HOOK } \ | ^~~~ security/hornet/hornet_lsm.c:221:38: note: (near initialization for 'hornet_hooks[0].hook.bpf_prog_load') 221 | LSM_HOOK_INIT(bpf_prog_load, hornet_bpf_prog_load), | ^~~~~~~~~~~~~~~~~~~~ include/linux/lsm_hooks.h:136:35: note: in definition of macro 'LSM_HOOK_INIT' 136 | .hook = { .NAME = HOOK } \ | ^~~~
vim +221 security/hornet/hornet_lsm.c
219 220 static struct security_hook_list hornet_hooks[] __ro_after_init = {
221 LSM_HOOK_INIT(bpf_prog_load, hornet_bpf_prog_load),
222 }; 223
Hi Blaise,
kernel test robot noticed the following build errors:
[auto build test ERROR on shuah-kselftest/next] [also build test ERROR on shuah-kselftest/fixes herbert-cryptodev-2.6/master herbert-crypto-2.6/master masahiroy-kbuild/for-next masahiroy-kbuild/fixes v6.14] [cannot apply to linus/master next-20250404] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Blaise-Boscaccy/security-Horn... base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next patch link: https://lore.kernel.org/r/20250404215527.1563146-2-bboscaccy%40linux.microso... patch subject: [PATCH v2 security-next 1/4] security: Hornet LSM config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20250407/202504070413.eDHSjWGP-lkp@i...) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250407/202504070413.eDHSjWGP-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202504070413.eDHSjWGP-lkp@intel.com/
All errors (new ones prefixed by >>):
security/hornet/hornet_lsm.c:221:31: error: incompatible function pointer types initializing 'int (*)(struct bpf_prog *, union bpf_attr *, struct bpf_token *)' with an expression of type 'int (struct bpf_prog *, union bpf_attr *, struct bpf_token *, bool)' (aka 'int (struct bpf_prog *, union bpf_attr *, struct bpf_token *, _Bool)') [-Wincompatible-function-pointer-types]
221 | LSM_HOOK_INIT(bpf_prog_load, hornet_bpf_prog_load), | ^~~~~~~~~~~~~~~~~~~~ include/linux/lsm_hooks.h:136:21: note: expanded from macro 'LSM_HOOK_INIT' 136 | .hook = { .NAME = HOOK } \ | ^~~~ 1 error generated.
vim +221 security/hornet/hornet_lsm.c
219 220 static struct security_hook_list hornet_hooks[] __ro_after_init = {
221 LSM_HOOK_INIT(bpf_prog_load, hornet_bpf_prog_load),
222 }; 223
On 2025-04-04 14:54:50, Blaise Boscaccy wrote:
+static int hornet_verify_lskel(struct bpf_prog *prog, struct hornet_maps *maps,
void *sig, size_t sig_len)+{
- int fd;
- u32 i;
- void *buf;
- void *new;
- size_t buf_sz;
- struct bpf_map *map;
- int err = 0;
- int key = 0;
- union bpf_attr attr = {0};
- buf = kmalloc_array(prog->len, sizeof(struct bpf_insn), GFP_KERNEL);
- if (!buf)
return -ENOMEM;- buf_sz = prog->len * sizeof(struct bpf_insn);
- memcpy(buf, prog->insnsi, buf_sz);
- for (i = 0; i < maps->used_map_cnt; i++) {
err = copy_from_bpfptr_offset(&fd, maps->fd_array,maps->used_idx[i] * sizeof(fd),sizeof(fd));if (err < 0)continue;if (fd < 1)continue;map = bpf_map_get(fd);
I'm not very familiar with BPF map lifetimes but I'd assume we need to have a corresponding bpf_map_put(map) before returning.
if (IS_ERR(map))continue;/* don't allow userspace to change map data used for signature verification */if (!map->frozen) {attr.map_fd = fd;err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));if (err < 0)goto out;}new = krealloc(buf, buf_sz + map->value_size, GFP_KERNEL);if (!new) {err = -ENOMEM;goto out;}buf = new;new = map->ops->map_lookup_elem(map, &key);if (!new) {err = -ENOENT;goto out;}memcpy(buf + buf_sz, new, map->value_size);buf_sz += map->value_size;- }
- err = verify_pkcs7_signature(buf, buf_sz, sig, sig_len,
VERIFY_USE_SECONDARY_KEYRING,VERIFYING_EBPF_SIGNATURE,NULL, NULL);+out:
- kfree(buf);
- return err;
+}
+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);
We should handle get_task_exe_file() returning NULL. I don't think it is likely to happen when passing `current` but kernel_read_file() doesn't protect against it and we'll have a NULL pointer dereference when it calls file_inode(NULL).
- 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);
We are leaking buf in this function. kernel_read_file() allocates the memory for us but we never kfree(buf).
- fput(file);
- if (!buf_sz)
return -1;- prog_len = buf_sz;
- if (prog_len > markerlen &&
memcmp(buf + prog_len - markerlen, EBPF_SIG_STRING, markerlen) == 0)prog_len -= markerlen;
Why is the marker optional? Looking at module_sig_check(), which verifies the signature on kernel modules, I see that it refuses to proceed if the marker is not found. Should we do the same and refuse to operate on any unexpected input?
- memcpy(&sig, buf + (prog_len - sizeof(sig)), sizeof(sig));
We should make sure that prog_len is larger than sizeof(sig) prior to this memcpy(). It is probably not a real issue in practice but it would be good to ensure that we can't be tricked to copy and operate on any bytes proceeding buf.
Tyler
- 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);
+}
Tyler Hicks code@tyhicks.com writes:
On 2025-04-04 14:54:50, Blaise Boscaccy wrote:
+static int hornet_verify_lskel(struct bpf_prog *prog, struct hornet_maps *maps,
void *sig, size_t sig_len)+{
- int fd;
- u32 i;
- void *buf;
- void *new;
- size_t buf_sz;
- struct bpf_map *map;
- int err = 0;
- int key = 0;
- union bpf_attr attr = {0};
- buf = kmalloc_array(prog->len, sizeof(struct bpf_insn), GFP_KERNEL);
- if (!buf)
return -ENOMEM;- buf_sz = prog->len * sizeof(struct bpf_insn);
- memcpy(buf, prog->insnsi, buf_sz);
- for (i = 0; i < maps->used_map_cnt; i++) {
err = copy_from_bpfptr_offset(&fd, maps->fd_array,maps->used_idx[i] * sizeof(fd),sizeof(fd));if (err < 0)continue;if (fd < 1)continue;map = bpf_map_get(fd);I'm not very familiar with BPF map lifetimes but I'd assume we need to have a corresponding bpf_map_put(map) before returning.
if (IS_ERR(map))continue;/* don't allow userspace to change map data used for signature verification */if (!map->frozen) {attr.map_fd = fd;err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));if (err < 0)goto out;}new = krealloc(buf, buf_sz + map->value_size, GFP_KERNEL);if (!new) {err = -ENOMEM;goto out;}buf = new;new = map->ops->map_lookup_elem(map, &key);if (!new) {err = -ENOENT;goto out;}memcpy(buf + buf_sz, new, map->value_size);buf_sz += map->value_size;- }
- err = verify_pkcs7_signature(buf, buf_sz, sig, sig_len,
VERIFY_USE_SECONDARY_KEYRING,VERIFYING_EBPF_SIGNATURE,NULL, NULL);+out:
- kfree(buf);
- return err;
+}
+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);
We should handle get_task_exe_file() returning NULL. I don't think it is likely to happen when passing `current` but kernel_read_file() doesn't protect against it and we'll have a NULL pointer dereference when it calls file_inode(NULL).
- 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);
We are leaking buf in this function. kernel_read_file() allocates the memory for us but we never kfree(buf).
- fput(file);
- if (!buf_sz)
return -1;- prog_len = buf_sz;
- if (prog_len > markerlen &&
memcmp(buf + prog_len - markerlen, EBPF_SIG_STRING, markerlen) == 0)prog_len -= markerlen;Why is the marker optional? Looking at module_sig_check(), which verifies the signature on kernel modules, I see that it refuses to proceed if the marker is not found. Should we do the same and refuse to operate on any unexpected input?
Looking at this again, there doesn't seem to be a good reason to have an optional marker. I'll get that fixed in v3 along with the rest of these suggestions.
- memcpy(&sig, buf + (prog_len - sizeof(sig)), sizeof(sig));
We should make sure that prog_len is larger than sizeof(sig) prior to this memcpy(). It is probably not a real issue in practice but it would be good to ensure that we can't be tricked to copy and operate on any bytes proceeding buf.
Tyler
- 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);
+}
On Apr 4, 2025 Blaise Boscaccy bboscaccy@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.starovoitov@gmail....
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 bboscaccy@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
Paul Moore paul@paul-moore.com writes:
On Apr 4, 2025 Blaise Boscaccy bboscaccy@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.starovoitov@gmail....
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 bboscaccy@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
On Mon, Apr 14, 2025 at 4:46 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Paul Moore paul@paul-moore.com writes:
On Apr 4, 2025 Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
...
+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 ...
My mistake, I spaced out a bit when looking at this and for some reason thought it was reading the BPF program/lskel itself and not the signature, meaning that the BPF that was being checked by Hornet was not necessarily the same BPF that was actually loaded. However, that's not the case here, as you rightly point out it is just the signature that is being read by the kernel_read_file() which shouldn't be an issue.
Thanks for the correction and sorry for the noise :)
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
+static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) +{
struct bpf_insn *insn = prog->insnsi;int insn_cnt = prog->len;int i;int err;for (i = 0; i < insn_cnt; i++, insn++) {if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {switch (insn[0].src_reg) {case BPF_PSEUDO_MAP_IDX_VALUE:case BPF_PSEUDO_MAP_IDX:err = add_used_map(maps, insn[0].imm);if (err < 0)return err;break;default:break;}}}
...
if (!map->frozen) {attr.map_fd = fd;err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));
Sorry for the delay. Still swamped after conferences and the merge window.
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions. Calling into sys_bpf from LSM is plain nack.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
What LSM can do in addition is to say that if the signature is not specified in the prog_load command then deny such request outright. bpf syscall itself will deny program loading if signature is incorrect.
Il giorno sab 12 apr 2025 alle ore 02:19 Alexei Starovoitov alexei.starovoitov@gmail.com ha scritto:
Similar to what I proposed here?
https://lore.kernel.org/bpf/20211203191844.69709-2-mcroce@linux.microsoft.co...
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command.
static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr) { @@ -2302,6 +2306,43 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
It probably should be two new fields in union bpf_attr (signature and length),
@@ -1346,6 +1346,8 @@ union bpf_attr { __aligned_u64 fd_array; /* array of FDs */ __aligned_u64 core_relos; __u32 core_relo_rec_size; /* sizeof(struct bpf_core_relo) */ + __aligned_u64 signature; /* instruction's signature */ + __u32 sig_len; /* signature size */
and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
+ if (err) { + pr_warn("Invalid BPF signature for '%s': %pe\n", + prog->aux->name, ERR_PTR(err)); + goto free_prog_sec; + }
It's been four years since my submission and the discussion was lengthy, what was the problem with the proposed signature in bpf_attr?
Regards,
On Fri, Apr 11, 2025 at 5:30 PM Matteo Croce technoboy85@gmail.com wrote:
Il giorno sab 12 apr 2025 alle ore 02:19 Alexei Starovoitov alexei.starovoitov@gmail.com ha scritto:
Similar to what I proposed here?
https://lore.kernel.org/bpf/20211203191844.69709-2-mcroce@linux.microsoft.co...
...
@@ -1346,6 +1346,8 @@ union bpf_attr { __aligned_u64 fd_array; /* array of FDs */ __aligned_u64 core_relos; __u32 core_relo_rec_size; /* sizeof(struct bpf_core_relo) */
- __aligned_u64 signature; /* instruction's signature */
- __u32 sig_len; /* signature size */
Well, yeah, two fields are obvious. But not like that link from 2021. KP proposed them a year later in 2022 on top of lskel which was much closer to be acceptable. We need to think it through and complete the work, since there are various ways to do it. For example, lskel has a map and a prog. A signature in a prog may cover both, but not necessary it's a good design. A signature for the map plus a signature for the prog that is tied to a map might be a better option. At map creation time the contents can be checked, the map is frozen, and then the verifier can proceed with prog's signature checking. lskel doesn't support all the bpf feature yet, so we need to make sure that the signature verification process is extensible when lskel gains new features.
Attaching was also brought up at lsfmm. Without checking the attach point the whole thing is quite questionable from security pov.
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 11, 2025 at 5:30 PM Matteo Croce technoboy85@gmail.com wrote:
Il giorno sab 12 apr 2025 alle ore 02:19 Alexei Starovoitov alexei.starovoitov@gmail.com ha scritto:
Similar to what I proposed here?
https://lore.kernel.org/bpf/20211203191844.69709-2-mcroce@linux.microsoft.co...
...
@@ -1346,6 +1346,8 @@ union bpf_attr { __aligned_u64 fd_array; /* array of FDs */ __aligned_u64 core_relos; __u32 core_relo_rec_size; /* sizeof(struct bpf_core_relo) */
- __aligned_u64 signature; /* instruction's signature */
- __u32 sig_len; /* signature size */
Well, yeah, two fields are obvious. But not like that link from 2021. KP proposed them a year later in 2022 on top of lskel which was much closer to be acceptable. We need to think it through and complete the work, since there are various ways to do it. For example, lskel has a map and a prog. A signature in a prog may cover both, but not necessary it's a good design. A signature for the map plus a signature for the prog that is tied to a map might be a better option. At map creation time the contents can be checked, the map is frozen, and then the verifier can proceed with prog's signature checking. lskel doesn't support all the bpf feature yet, so we need to make sure that the signature verification process is extensible when lskel gains new features.
Attaching was also brought up at lsfmm. Without checking the attach point the whole thing is quite questionable from security pov.
That statement is quite questionable. Yes, IIRC you brought that up. And again, runtime policy enforcement has nothing to do with proving code provenance. They are completely independent concerns.
That would be akin to saying that having locks on a door is questionable without having surveillance cameras installed.
TAlexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
+static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) +{
struct bpf_insn *insn = prog->insnsi;int insn_cnt = prog->len;int i;int err;for (i = 0; i < insn_cnt; i++, insn++) {if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {switch (insn[0].src_reg) {case BPF_PSEUDO_MAP_IDX_VALUE:case BPF_PSEUDO_MAP_IDX:err = add_used_map(maps, insn[0].imm);if (err < 0)return err;break;default:break;}}}...
if (!map->frozen) {attr.map_fd = fd;err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));Sorry for the delay. Still swamped after conferences and the merge window.
No worries.
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions.
These aren't BPF internals; this is data passed in from userspace. Inspecting userspace function inputs is definitely within the purview of an LSM.
Lskel signature verification doesn't actually need a full disassembly, but it does need all the maps used by the lskel. Due to API design choices, this unfortunately requires disassembling the program to see which array indexes are being used.
Calling into sys_bpf from LSM is plain nack.
kern_sys_bpf is an EXPORT_SYMBOL, which means that it should be callable from a module. Lskels without frozen maps are vulnerable to a TOCTOU attack from a sufficiently privileged user. Lskels currently pass unfrozen maps into the kernel, and there is nothing stopping someone from modifying them between BPF_PROG_LOAD and BPF_PROG_RUN.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
I don't necessarily disagree, but my main concern with this is that previous code signing patchsets seem to get gaslit or have the goalposts moved until they die or are abandoned.
Are you saying that at this point, you would be amenable to an in-tree set of patches that enforce signature verification of lskels during BPF_PROG_LOAD that live in syscall.c, without adding extra non-code signing requirements like attachment point verification, completely eBPF-based solutions, or rich eBPF-based program run-time policy enforcement?
Our entire use case for this is simply "we've signed all code running in ring 0," nothing more. I'm concerned that code signing may be blocked forever while eBPF attempts to reinvent its own runtime policy framework that has absolutely nothing to do with proving code provenance.
What LSM can do in addition is to say that if the signature is not specified in the prog_load command then deny such request outright. bpf syscall itself will deny program loading if signature is incorrect.
On Sat, Apr 12, 2025 at 9:58 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
...
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions.
These aren't BPF internals; this is data passed in from userspace. Inspecting userspace function inputs is definitely within the purview of an LSM.
Lskel signature verification doesn't actually need a full disassembly, but it does need all the maps used by the lskel. Due to API design choices, this unfortunately requires disassembling the program to see which array indexes are being used.
Calling into sys_bpf from LSM is plain nack.
kern_sys_bpf is an EXPORT_SYMBOL, which means that it should be callable from a module. Lskels without frozen maps are vulnerable to a TOCTOU attack from a sufficiently privileged user. Lskels currently pass unfrozen maps into the kernel, and there is nothing stopping someone from modifying them between BPF_PROG_LOAD and BPF_PROG_RUN.
I agree with Blaise on both the issue of iterating through the eBPF program as well as calling into EXPORT_SYMBOL'd functions; I see no reason why these things couldn't be used in a LSM. These are both "public" interfaces; reading/iterating through the eBPF instructions falls under a "don't break userspace" API, and EXPORT_SYMBOL is essentially public by definition.
It is a bit odd that the eBPF code is creating an exported symbol and not providing a declaration in a kernel wide header file, but that's a different issue.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
I don't necessarily disagree, but my main concern with this is that previous code signing patchsets seem to get gaslit or have the goalposts moved until they die or are abandoned.
My understanding from the previous threads is that the recommendation from the BPF devs was that anyone wanting to implement BPF program signature validation at load time should implement a LSM that leverages a light skeleton based loading mechanism and implement a gatekeeper which would authorize BPF program loading based on signatures. From what I can see that is exactly what Blaise has done with Hornet. While Hornet is implemented in C, that alone should not be reason for rejection; from the perspective of the overall LSM framework, we don't accept or reject individual LSMs based on their source language, we have both BPF and C based LSMs today, and we've been working with the Rust folks to ensure we have the right things in place to support Rust in the future. If your response to Hornet is that it isn't acceptable because it is written in C and not BPF, you need to know that such a response isn't an acceptable objection.
Are you saying that at this point, you would be amenable to an in-tree set of patches that enforce signature verification of lskels during BPF_PROG_LOAD that live in syscall.c, without adding extra non-code signing requirements like attachment point verification, completely eBPF-based solutions, or rich eBPF-based program run-time policy enforcement?
I worry that we are now circling back to the original idea of doing BPF program signature validation in the BPF subsystem itself. To be clear, I think that would be okay, if not ultimately preferable, but I think we've all seen this attempted numerous times in the past and it has been delayed, dismissed in favor of alternatives, or simply rejected for one reason or another. If there is a clearly defined path forward for validation of signatures on BPF programs within the context of the BPF subsystem that doesn't require a trusted userspace loader/library/etc. that is one thing, but I don't believe we currently have that, despite user/dev requests for such a feature stretching out over several years.
I believe there are a few questions/issues that have been identified in Hornet's latest round of reviews which may take Blaise a few days (week?) to address; if the BPF devs haven't provided a proposal in which one could acceptably implement in-kernel BPF signature validation by that time, I see no reason why development and review of Hornet shouldn't continue into a v3 revision.
On Sat, Apr 12, 2025 at 6:58 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
TAlexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
+static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) +{
struct bpf_insn *insn = prog->insnsi;int insn_cnt = prog->len;int i;int err;for (i = 0; i < insn_cnt; i++, insn++) {if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {switch (insn[0].src_reg) {case BPF_PSEUDO_MAP_IDX_VALUE:case BPF_PSEUDO_MAP_IDX:err = add_used_map(maps, insn[0].imm);if (err < 0)return err;break;default:break;}}}...
if (!map->frozen) {attr.map_fd = fd;err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));Sorry for the delay. Still swamped after conferences and the merge window.
No worries.
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions.
These aren't BPF internals; this is data passed in from userspace. Inspecting userspace function inputs is definitely within the purview of an LSM.
Lskel signature verification doesn't actually need a full disassembly, but it does need all the maps used by the lskel. Due to API design choices, this unfortunately requires disassembling the program to see which array indexes are being used.
Calling into sys_bpf from LSM is plain nack.
kern_sys_bpf is an EXPORT_SYMBOL, which means that it should be callable from a module.
It's a leftover. kern_sys_bpf() is not something that arbitrary kernel modules should call. It was added to work for cases where kernel modules carry their own lskels. That use case is gone, so EXPORT_SYMBOL will be removed.
Lskels without frozen maps are vulnerable to a TOCTOU attack from a sufficiently privileged user. Lskels currently pass unfrozen maps into the kernel, and there is nothing stopping someone from modifying them between BPF_PROG_LOAD and BPF_PROG_RUN.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
I don't necessarily disagree, but my main concern with this is that previous code signing patchsets seem to get gaslit or have the goalposts moved until they die or are abandoned.
Previous attempts to add signing failed because 1. It's a difficult problem to solve 2. people only cared about their own narrow use case and not considering the needs of bpf ecosystem as a whole.
Are you saying that at this point, you would be amenable to an in-tree set of patches that enforce signature verification of lskels during BPF_PROG_LOAD that live in syscall.c,
that's the only way to do it.
without adding extra non-code signing requirements like attachment point verification, completely eBPF-based solutions, or rich eBPF-based program run-time policy enforcement?
Those are secondary considerations that should also be discussed. Not necessarily a blocker.
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Sat, Apr 12, 2025 at 6:58 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
TAlexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
+static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) +{
struct bpf_insn *insn = prog->insnsi;int insn_cnt = prog->len;int i;int err;for (i = 0; i < insn_cnt; i++, insn++) {if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {switch (insn[0].src_reg) {case BPF_PSEUDO_MAP_IDX_VALUE:case BPF_PSEUDO_MAP_IDX:err = add_used_map(maps, insn[0].imm);if (err < 0)return err;break;default:break;}}}...
if (!map->frozen) {attr.map_fd = fd;err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));Sorry for the delay. Still swamped after conferences and the merge window.
No worries.
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions.
These aren't BPF internals; this is data passed in from userspace. Inspecting userspace function inputs is definitely within the purview of an LSM.
Lskel signature verification doesn't actually need a full disassembly, but it does need all the maps used by the lskel. Due to API design choices, this unfortunately requires disassembling the program to see which array indexes are being used.
Calling into sys_bpf from LSM is plain nack.
kern_sys_bpf is an EXPORT_SYMBOL, which means that it should be callable from a module.
It's a leftover. kern_sys_bpf() is not something that arbitrary kernel modules should call. It was added to work for cases where kernel modules carry their own lskels. That use case is gone, so EXPORT_SYMBOL will be removed.
I'm not following that at all. You recommended using module-based lskels to get around code signing requirements at lsfmmbpf and now you want to nuke that entire feature? And further, skel_internal will no longer be usable from within the kernel and bpf_preload is no longer going to be supported?
Lskels without frozen maps are vulnerable to a TOCTOU attack from a sufficiently privileged user. Lskels currently pass unfrozen maps into the kernel, and there is nothing stopping someone from modifying them between BPF_PROG_LOAD and BPF_PROG_RUN.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
I don't necessarily disagree, but my main concern with this is that previous code signing patchsets seem to get gaslit or have the goalposts moved until they die or are abandoned.
Previous attempts to add signing failed because
- It's a difficult problem to solve
- people only cared about their own narrow use case and not
considering the needs of bpf ecosystem as a whole.
Are you saying that at this point, you would be amenable to an in-tree set of patches that enforce signature verification of lskels during BPF_PROG_LOAD that live in syscall.c,
that's the only way to do it.
So the notion of forcing people into writing bpf-based gatekeeper programs is being abandoned? e.g.
https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeo... https://lore.kernel.org/bpf/61aae2da8c7b0_68de0208dd@john.notmuch/
without adding extra non-code signing requirements like attachment point verification, completely eBPF-based solutions, or rich eBPF-based program run-time policy enforcement?
Those are secondary considerations that should also be discussed. Not necessarily a blocker.
Again, I'm confused here since you recently stated this whole thing was "questionable" without attachment point verification.
On Mon, Apr 14, 2025 at 5:32 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Sat, Apr 12, 2025 at 6:58 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
TAlexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
+static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) +{
struct bpf_insn *insn = prog->insnsi;int insn_cnt = prog->len;int i;int err;for (i = 0; i < insn_cnt; i++, insn++) {if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {switch (insn[0].src_reg) {case BPF_PSEUDO_MAP_IDX_VALUE:case BPF_PSEUDO_MAP_IDX:err = add_used_map(maps, insn[0].imm);if (err < 0)return err;break;default:break;}}}...
if (!map->frozen) {attr.map_fd = fd;err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));Sorry for the delay. Still swamped after conferences and the merge window.
No worries.
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions.
These aren't BPF internals; this is data passed in from userspace. Inspecting userspace function inputs is definitely within the purview of an LSM.
Lskel signature verification doesn't actually need a full disassembly, but it does need all the maps used by the lskel. Due to API design choices, this unfortunately requires disassembling the program to see which array indexes are being used.
Calling into sys_bpf from LSM is plain nack.
kern_sys_bpf is an EXPORT_SYMBOL, which means that it should be callable from a module.
It's a leftover. kern_sys_bpf() is not something that arbitrary kernel modules should call. It was added to work for cases where kernel modules carry their own lskels. That use case is gone, so EXPORT_SYMBOL will be removed.
I'm not following that at all. You recommended using module-based lskels to get around code signing requirements at lsfmmbpf and now you want to nuke that entire feature? And further, skel_internal will no longer be usable from within the kernel and bpf_preload is no longer going to be supported?
It was exported to modules to run lskel-s from modules. It's bpf internal api, but seeing how you want to abuse it the feature has to go. Sadly.
Lskels without frozen maps are vulnerable to a TOCTOU attack from a sufficiently privileged user. Lskels currently pass unfrozen maps into the kernel, and there is nothing stopping someone from modifying them between BPF_PROG_LOAD and BPF_PROG_RUN.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
I don't necessarily disagree, but my main concern with this is that previous code signing patchsets seem to get gaslit or have the goalposts moved until they die or are abandoned.
Previous attempts to add signing failed because
- It's a difficult problem to solve
- people only cared about their own narrow use case and not
considering the needs of bpf ecosystem as a whole.
Are you saying that at this point, you would be amenable to an in-tree set of patches that enforce signature verification of lskels during BPF_PROG_LOAD that live in syscall.c,
that's the only way to do it.
So the notion of forcing people into writing bpf-based gatekeeper programs is being abandoned? e.g.
https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeo... https://lore.kernel.org/bpf/61aae2da8c7b0_68de0208dd@john.notmuch/
Not abandoned. bpf-based tuning of load conditions is still necessary. The bpf_prog_load command will check the signature only. It won't start rejecting progs that don't have a signature. For that a one liner bpf-lsm or C-based lsm would be needed to address your dont-trust-root use case.
without adding extra non-code signing requirements like attachment point verification, completely eBPF-based solutions, or rich eBPF-based program run-time policy enforcement?
Those are secondary considerations that should also be discussed. Not necessarily a blocker.
Again, I'm confused here since you recently stated this whole thing was "questionable" without attachment point verification.
Correct. For fentry prog type the attachment point is checked during the load, but for tracepoints it's not, and anyone who is claiming that their system is secure because the tracepoint prog was signed is simply clueless in how bpf works.
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Mon, Apr 14, 2025 at 5:32 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Sat, Apr 12, 2025 at 6:58 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
TAlexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
+static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) +{
struct bpf_insn *insn = prog->insnsi;int insn_cnt = prog->len;int i;int err;for (i = 0; i < insn_cnt; i++, insn++) {if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {switch (insn[0].src_reg) {case BPF_PSEUDO_MAP_IDX_VALUE:case BPF_PSEUDO_MAP_IDX:err = add_used_map(maps, insn[0].imm);if (err < 0)return err;break;default:break;}}}...
if (!map->frozen) {attr.map_fd = fd;err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));Sorry for the delay. Still swamped after conferences and the merge window.
No worries.
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions.
These aren't BPF internals; this is data passed in from userspace. Inspecting userspace function inputs is definitely within the purview of an LSM.
Lskel signature verification doesn't actually need a full disassembly, but it does need all the maps used by the lskel. Due to API design choices, this unfortunately requires disassembling the program to see which array indexes are being used.
Calling into sys_bpf from LSM is plain nack.
kern_sys_bpf is an EXPORT_SYMBOL, which means that it should be callable from a module.
It's a leftover. kern_sys_bpf() is not something that arbitrary kernel modules should call. It was added to work for cases where kernel modules carry their own lskels. That use case is gone, so EXPORT_SYMBOL will be removed.
I'm not following that at all. You recommended using module-based lskels to get around code signing requirements at lsfmmbpf and now you want to nuke that entire feature? And further, skel_internal will no longer be usable from within the kernel and bpf_preload is no longer going to be supported?
The eBPF dev community has spent what, 4-5 years on this, with little to no progress. I have little faith that this is going to progress on your end in a timely manner or at all, and frankly we (and others) needed this yesterday. Hornet has zero impact on the bpf subsystem, yet you seem viscerally opposed to us doing this. Why are you trying to stop us from securing our cloud?
It was exported to modules to run lskel-s from modules. It's bpf internal api, but seeing how you want to abuse it the feature has to go. Sadly.
Are we in preschool again? You don't like how others are playing with your toys so you want to take them away from everyone. Forever.
Lskels without frozen maps are vulnerable to a TOCTOU attack from a sufficiently privileged user. Lskels currently pass unfrozen maps into the kernel, and there is nothing stopping someone from modifying them between BPF_PROG_LOAD and BPF_PROG_RUN.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
I don't necessarily disagree, but my main concern with this is that previous code signing patchsets seem to get gaslit or have the goalposts moved until they die or are abandoned.
Previous attempts to add signing failed because
- It's a difficult problem to solve
- people only cared about their own narrow use case and not
considering the needs of bpf ecosystem as a whole.
Are you saying that at this point, you would be amenable to an in-tree set of patches that enforce signature verification of lskels during BPF_PROG_LOAD that live in syscall.c,
that's the only way to do it.
So the notion of forcing people into writing bpf-based gatekeeper programs is being abandoned? e.g.
https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeo... https://lore.kernel.org/bpf/61aae2da8c7b0_68de0208dd@john.notmuch/
Not abandoned. bpf-based tuning of load conditions is still necessary. The bpf_prog_load command will check the signature only. It won't start rejecting progs that don't have a signature. For that a one liner bpf-lsm or C-based lsm would be needed to address your dont-trust-root use case.
Since this will require an LSM no matter what, there is zero reason for us not to proceed with Hornet. If or when you actually figure out how to sign an lskel and upstream updated LSM hooks, I can always rework Hornet to use that instead.
without adding extra non-code signing requirements like attachment point verification, completely eBPF-based solutions, or rich eBPF-based program run-time policy enforcement?
Those are secondary considerations that should also be discussed. Not necessarily a blocker.
Again, I'm confused here since you recently stated this whole thing was "questionable" without attachment point verification.
Correct. For fentry prog type the attachment point is checked during the load, but for tracepoints it's not, and anyone who is claiming that their system is secure because the tracepoint prog was signed is simply clueless in how bpf works.
No one is making that claim, although I do appreciate the lovely ad-hominem attack and absolutist standpoint. It's not like we invented code signing last week. All we are trying to do is make our cloud ever-so-slightly more secure and share the results with the community.
The attack vectors I'm looking at are things like CVE-2021-33200. ACLs for attachment points do nothing to stop that whereas code signing is a possible countermeasure. This kind of thing is probably a non-issue with your private cloud, but it's a very real issue with publicly accessible ones.
Blaise Boscaccy bboscaccy@linux.microsoft.com writes:
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Mon, Apr 14, 2025 at 5:32 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Sat, Apr 12, 2025 at 6:58 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
TAlexei Starovoitov alexei.starovoitov@gmail.com writes:
On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote: > + > +static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps) > +{ > + struct bpf_insn *insn = prog->insnsi; > + int insn_cnt = prog->len; > + int i; > + int err; > + > + for (i = 0; i < insn_cnt; i++, insn++) { > + if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) { > + switch (insn[0].src_reg) { > + case BPF_PSEUDO_MAP_IDX_VALUE: > + case BPF_PSEUDO_MAP_IDX: > + err = add_used_map(maps, insn[0].imm); > + if (err < 0) > + return err; > + break; > + default: > + break; > + } > + } > + }
...
> + if (!map->frozen) { > + attr.map_fd = fd; > + err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));
Sorry for the delay. Still swamped after conferences and the merge window.
No worries.
Above are serious layering violations. LSMs should not be looking that deep into bpf instructions.
These aren't BPF internals; this is data passed in from userspace. Inspecting userspace function inputs is definitely within the purview of an LSM.
Lskel signature verification doesn't actually need a full disassembly, but it does need all the maps used by the lskel. Due to API design choices, this unfortunately requires disassembling the program to see which array indexes are being used.
Calling into sys_bpf from LSM is plain nack.
kern_sys_bpf is an EXPORT_SYMBOL, which means that it should be callable from a module.
It's a leftover. kern_sys_bpf() is not something that arbitrary kernel modules should call. It was added to work for cases where kernel modules carry their own lskels. That use case is gone, so EXPORT_SYMBOL will be removed.
I'm not following that at all. You recommended using module-based lskels to get around code signing requirements at lsfmmbpf and now you want to nuke that entire feature? And further, skel_internal will no longer be usable from within the kernel and bpf_preload is no longer going to be supported?
The eBPF dev community has spent what, 4-5 years on this, with little to no progress. I have little faith that this is going to progress on your end in a timely manner or at all, and frankly we (and others) needed this yesterday. Hornet has zero impact on the bpf subsystem, yet you seem viscerally opposed to us doing this. Why are you trying to stop us from securing our cloud?
It was exported to modules to run lskel-s from modules. It's bpf internal api, but seeing how you want to abuse it the feature has to go. Sadly.
In the interest of not harming downstream users that possibly rely on BPF_PRELOAD, it would be completely fine on our end to not call kern_sys_bpf since maps can easily be frozen in userspace by the loader. I'd prefer LSMs to be not mutating things if at all possible as well.
If we agreed to not call that function so-as you can keep that feature for everyone, would you be ammenable to a simple patch in skel_internal.h that freezes maps? e.g
diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h index 4d5fa079b5d6..51e72dc23062 100644 --- a/tools/lib/bpf/skel_internal.h +++ b/tools/lib/bpf/skel_internal.h @@ -263,6 +263,17 @@ static inline int skel_map_delete_elem(int fd, const void *key) return skel_sys_bpf(BPF_MAP_DELETE_ELEM, &attr, attr_sz); }
+static inline int skel_map_freeze(int fd) +{ + const size_t attr_sz = offsetofend(union bpf_attr, map_fd); + union bpf_attr attr; + + memset(&attr, 0, attr_sz); + attr.map_fd = fd; + + return skel_sys_bpf(BPF_MAP_FREEZE, &attr, attr_sz); +} + static inline int skel_map_get_fd_by_id(__u32 id) { const size_t attr_sz = offsetofend(union bpf_attr, flags); @@ -327,6 +338,13 @@ static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts) goto out; }
+ err = skel_map_freeze(map_fd); + if (err < 0) { + opts->errstr = "failed to freeze map"; + set_err; + goto out; + } + memset(&attr, 0, prog_load_attr_sz); attr.prog_type = BPF_PROG_TYPE_SYSCALL; attr.insns = (long) opts->insns;
Lskels without frozen maps are vulnerable to a TOCTOU attack from a sufficiently privileged user. Lskels currently pass unfrozen maps into the kernel, and there is nothing stopping someone from modifying them between BPF_PROG_LOAD and BPF_PROG_RUN.
The verification of module signatures is a job of the module loading process. The same thing should be done by the bpf system. The signature needs to be passed into sys_bpf syscall as a part of BPF_PROG_LOAD command. It probably should be two new fields in union bpf_attr (signature and length), and the whole thing should be processed as part of the loading with human readable error reported back through the verifier log in case of signature mismatch, etc.
I don't necessarily disagree, but my main concern with this is that previous code signing patchsets seem to get gaslit or have the goalposts moved until they die or are abandoned.
Previous attempts to add signing failed because
- It's a difficult problem to solve
- people only cared about their own narrow use case and not
considering the needs of bpf ecosystem as a whole.
Are you saying that at this point, you would be amenable to an in-tree set of patches that enforce signature verification of lskels during BPF_PROG_LOAD that live in syscall.c,
that's the only way to do it.
So the notion of forcing people into writing bpf-based gatekeeper programs is being abandoned? e.g.
https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeo... https://lore.kernel.org/bpf/61aae2da8c7b0_68de0208dd@john.notmuch/
Not abandoned. bpf-based tuning of load conditions is still necessary. The bpf_prog_load command will check the signature only. It won't start rejecting progs that don't have a signature. For that a one liner bpf-lsm or C-based lsm would be needed to address your dont-trust-root use case.
Since this will require an LSM no matter what, there is zero reason for us not to proceed with Hornet. If or when you actually figure out how to sign an lskel and upstream updated LSM hooks, I can always rework Hornet to use that instead.
without adding extra non-code signing requirements like attachment point verification, completely eBPF-based solutions, or rich eBPF-based program run-time policy enforcement?
Those are secondary considerations that should also be discussed. Not necessarily a blocker.
Again, I'm confused here since you recently stated this whole thing was "questionable" without attachment point verification.
Correct. For fentry prog type the attachment point is checked during the load, but for tracepoints it's not, and anyone who is claiming that their system is secure because the tracepoint prog was signed is simply clueless in how bpf works.
No one is making that claim, although I do appreciate the lovely ad-hominem attack and absolutist standpoint. It's not like we invented code signing last week. All we are trying to do is make our cloud ever-so-slightly more secure and share the results with the community.
The attack vectors I'm looking at are things like CVE-2021-33200. ACLs for attachment points do nothing to stop that whereas code signing is a possible countermeasure. This kind of thing is probably a non-issue with your private cloud, but it's a very real issue with publicly accessible ones.
On Tue, Apr 15, 2025 at 3:08 PM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
... would you be ammenable to a simple patch in skel_internal.h that freezes maps? e.g
I have limited network access at the moment, so it is possible I've missed it, but I think it would be helpful to get a verdict on the RFC-esque patch from Blaise below.
diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h index 4d5fa079b5d6..51e72dc23062 100644 --- a/tools/lib/bpf/skel_internal.h +++ b/tools/lib/bpf/skel_internal.h @@ -263,6 +263,17 @@ static inline int skel_map_delete_elem(int fd, const void *key) return skel_sys_bpf(BPF_MAP_DELETE_ELEM, &attr, attr_sz); }
+static inline int skel_map_freeze(int fd) +{
const size_t attr_sz = offsetofend(union bpf_attr, map_fd);union bpf_attr attr;memset(&attr, 0, attr_sz);attr.map_fd = fd;return skel_sys_bpf(BPF_MAP_FREEZE, &attr, attr_sz);+}
static inline int skel_map_get_fd_by_id(__u32 id) { const size_t attr_sz = offsetofend(union bpf_attr, flags); @@ -327,6 +338,13 @@ static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts) goto out; }
err = skel_map_freeze(map_fd);if (err < 0) {opts->errstr = "failed to freeze map";set_err;goto out;}memset(&attr, 0, prog_load_attr_sz); attr.prog_type = BPF_PROG_TYPE_SYSCALL; attr.insns = (long) opts->insns;
On Tue, Apr 15, 2025 at 8:45 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
The eBPF dev community has spent what, 4-5 years on this, with little to no progress. I have little faith that this is going to progress on your end in a timely manner or at all, and frankly we (and others) needed this yesterday.
History repeats itself. 1. the problem is hard. 2. you're only interested in addressing your own use case. There is no end-to-end design here and no attempt to think it through how it will work for others.
Hornet has zero impact on the bpf subsystem, yet you seem viscerally opposed to us doing this.
Hacking into bpf internal objects like maps is not acceptable.
Why are you trying to stop us from securing our cloud?
Keep your lsm hack out-of-tree, please.
Since this will require an LSM no matter what, there is zero reason for us not to proceed with Hornet. If or when you actually figure out how to sign an lskel and upstream updated LSM hooks, I can always rework Hornet to use that instead.
You can do whatever you want out-of-tree including re-exporting kern_sys_bpf.
code signing last week. All we are trying to do is make our cloud ever-so-slightly more secure and share the results with the community.
You're pushing for a custom microsoft specific hack while ignoring community feedback.
The attack vectors I'm looking at are things like CVE-2021-33200.
4 year old bug ? If your kernels are so old you have lots of other vulnerabilities.
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
History repeats itself.
- the problem is hard.
- you're only interested in addressing your own use case.
There is no end-to-end design here and no attempt to think it through how it will work for others.
Well, I suppose anything worth doing is going to be hard :)
The end-to-end design for this is the same end-to-end design that exists for signing kernel modules today. We envisioned it working for others the same way module signing works for others.
Hacking into bpf internal objects like maps is not acceptable.
We've heard your concerns about kern_sys_bpf and we agree that the LSM should not be calling it. The proposal in this email should meet both of our needs https://lore.kernel.org/bpf/874iypjl8t.fsf@microsoft.com/
-blaise
On Wed, Apr 16, 2025 at 10:31 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Hacking into bpf internal objects like maps is not acceptable.
We've heard your concerns about kern_sys_bpf and we agree that the LSM should not be calling it. The proposal in this email should meet both of our needs https://lore.kernel.org/bpf/874iypjl8t.fsf@microsoft.com/
kern_sys_bpf was one example of a layering violation. Calling bpf_map_get() and map->ops->map_lookup_elem() from a module is not ok either.
lskel doing skel_map_freeze is not solving the issue. It is still broken from TOCTOU pov. freeze only makes a map readonly to user space. Any program can still read/write it. That's why freeze wasn't done back then when lskel was introduced. There is still work to do to make signing practical. One needs to think of libbpf equivalent loaders in golang and rust. The solution has to work for them too. In that sense bpf signing is not analogous to kernel module signing. Programs are not distributed as elf files. elf is an intermediate step in a build process. bpftool takes elf and generates skel or lskel and user space does #include <skel.h> to access maps and global variables directly. See how systemd does it. bpf progs are part of various skel.h-s in there. systemd is also using an old style bpf progs written in bpf assembly. We need to figure out how to make them signed too.
The signing problem is big and difficult. It will take time to figure out all these challenges. Introduction of lskel back in 2021 was the first step towards signing (as the commit log clearly states). lskel approach is likely a solution for a large class of bpf users, but not for all. It won't work for bpftrace and bcc. lskel loading is also opaque. The verifier errors are not propagated from the loader prog back to the user. To load normal skeleton the user space can do: LIBBPF_OPTS(bpf_object_open_opts, opts); opts.kernel_log_buf = my_verifier_log_buf; myskel__open_opts(&opts); There is no __open_opts() equivalent for lskel. It needs to be fixed before we can recommend lksel as a solution to signing.
On Mon, Apr 21, 2025 at 4:13 PM Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Wed, Apr 16, 2025 at 10:31 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Hacking into bpf internal objects like maps is not acceptable.
We've heard your concerns about kern_sys_bpf and we agree that the LSM should not be calling it. The proposal in this email should meet both of our needs https://lore.kernel.org/bpf/874iypjl8t.fsf@microsoft.com/
...
Calling bpf_map_get() and map->ops->map_lookup_elem() from a module is not ok either.
A quick look uncovers code living under net/ which calls into these APIs.
lskel doing skel_map_freeze is not solving the issue. It is still broken from TOCTOU pov. freeze only makes a map readonly to user space. Any program can still read/write it.
When you say "any program" you are referring to any BPF program loaded into the kernel, correct? At least that is my understanding of "freezing" a BPF map, while userspace is may be unable to modify the map's contents, it is still possible for a BPF program to modify it. If I'm mistaken, I would appreciate a pointer to a correct description of map freezing.
Assuming the above is correct, that a malicious bit of code running in kernel context could cause mischief, isn't a new concern, and in fact it is one of the reasons why Hornet is valuable. Hornet allows admins/users to have some assurance that the BPF programs they load into their system come from a trusted source (trusted not to intentionally do Bad Things in the kernel) and haven't been modified to do Bad Things (like modify lskel maps).
One needs to think of libbpf equivalent loaders in golang and rust.
...
systemd is also using an old style bpf progs written in bpf assembly.
I've briefly talked with Blaise about the systemd issue in particular, and I believe there are some relatively easy ways to work around the ELF issue in the current version of Hornet. I know Blaise is tied up for the next couple of days on another fire, but I'm sure the next revision will have a solution for this.
Introduction of lskel back in 2021 was the first step towards signing (as the commit log clearly states). lskel approach is likely a solution for a large class of bpf users, but not for all. It won't work for bpftrace and bcc.
As most everyone on the To/CC line already knows, Linux kernel development is often iterative. Not only is it easier for the kernel devs to develop and review incremental additions to functionality, it also enables a feedback loop where users can help drive the direction of the functionality as it is built. I view Hornet as an iterative improvement, building on the lskel concept, that helps users towards their goal of load time verification of code running inside the kernel. Hornet, as currently described, may not be the solution for everything, but it can be the solution for something that users are desperately struggling with today and as far as I'm concerned, that is a good thing worth supporting.
On Mon, Apr 21, 2025 at 3:04 PM Paul Moore paul@paul-moore.com wrote:
On Mon, Apr 21, 2025 at 4:13 PM Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Wed, Apr 16, 2025 at 10:31 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Hacking into bpf internal objects like maps is not acceptable.
We've heard your concerns about kern_sys_bpf and we agree that the LSM should not be calling it. The proposal in this email should meet both of our needs https://lore.kernel.org/bpf/874iypjl8t.fsf@microsoft.com/
...
Calling bpf_map_get() and map->ops->map_lookup_elem() from a module is not ok either.
A quick look uncovers code living under net/ which calls into these APIs.
and your point is ?
Again, Nack to hacking into bpf internals from LSM, module or kernel subsystem.
On Mon, Apr 21, 2025 at 7:48 PM Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Mon, Apr 21, 2025 at 3:04 PM Paul Moore paul@paul-moore.com wrote:
On Mon, Apr 21, 2025 at 4:13 PM Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Wed, Apr 16, 2025 at 10:31 AM Blaise Boscaccy bboscaccy@linux.microsoft.com wrote:
Hacking into bpf internal objects like maps is not acceptable.
We've heard your concerns about kern_sys_bpf and we agree that the LSM should not be calling it. The proposal in this email should meet both of our needs https://lore.kernel.org/bpf/874iypjl8t.fsf@microsoft.com/
...
Calling bpf_map_get() and map->ops->map_lookup_elem() from a module is not ok either.
A quick look uncovers code living under net/ which calls into these APIs.
and your point is ?
Simply the observation that the APIs you've mentioned are currently being used by code living under net/; you're free to take from that whatever you will.
Again, Nack to hacking into bpf internals from LSM, module or kernel subsystem.
I heard you the first time and rest assured I've noted your general objection regarding use of the BPF API. I'm personally still interested in seeing a v3 before deciding on next steps as there were a number of other issues mentioned during the v2 review that need attention. I would encourage you to continue to participate in future reviews of Hornet, but of course that is entirely up to you. In the absence of any additional review feedback, I'll preserve your NACK if we ever get to a point that your comments are worth mentioning.
-- paul-moore.com
On Mon, 2025-04-21 at 13:12 -0700, Alexei Starovoitov wrote: [...]
Calling bpf_map_get() and map->ops->map_lookup_elem() from a module is not ok either.
I don't understand this objection. The program just got passed in to bpf_prog_load() as a set of attributes which, for a light skeleton, directly contain the code as a blob and have the various BTF relocations as a blob in a single element array map. I think everyone agrees that the integrity of the program would be compromised by modifications to the relocations, so the security_bpf_prog_load() hook can't make an integrity determination without examining both. If the hook can't use the bpf_maps.. APIs directly is there some other API it should be using to get the relocations, or are you saying that the security_bpf_prog_load() hook isn't fit for purpose and it should be called after the bpf core has loaded the relocations so they can be provided to the hook as an argument?
The above, by the way, is independent of signing, because it applies to any determination that might be made in the security_bpf_prog_load() hook regardless of purpose.
Regards,
James
On Wed, Apr 23, 2025 at 10:12 AM James Bottomley James.Bottomley@hansenpartnership.com wrote:
On Mon, 2025-04-21 at 13:12 -0700, Alexei Starovoitov wrote: [...]
Calling bpf_map_get() and map->ops->map_lookup_elem() from a module is not ok either.
I don't understand this objection. The program just got passed in to bpf_prog_load() as a set of attributes which, for a light skeleton, directly contain the code as a blob and have the various BTF relocations as a blob in a single element array map. I think everyone agrees that the integrity of the program would be compromised by modifications to the relocations, so the security_bpf_prog_load() hook can't make an integrity determination without examining both. If the hook can't use the bpf_maps.. APIs directly is there some other API it should be using to get the relocations, or are you saying that the security_bpf_prog_load() hook isn't fit for purpose and it should be called after the bpf core has loaded the relocations so they can be provided to the hook as an argument?
The above, by the way, is independent of signing, because it applies to any determination that might be made in the security_bpf_prog_load() hook regardless of purpose.
I've also been worrying that some of the unspoken motivation behind the objection is simply that Hornet is not BPF. If/when we get to a point where Hornet is sent up to Linus and Alexei's objection to the Hornet LSM inspecting BPF maps stands, it seems as though *any* LSM, including BPF LSMs, would need to be prevented from accessing BPF maps. I'm fairly certain no one wants to see it come to that.
On Wed, Apr 23, 2025 at 7:12 AM James Bottomley James.Bottomley@hansenpartnership.com wrote:
On Mon, 2025-04-21 at 13:12 -0700, Alexei Starovoitov wrote: [...]
Calling bpf_map_get() and map->ops->map_lookup_elem() from a module is not ok either.
I don't understand this objection.
Consider an LSM that hooks into security_bprm_*(bprm), parses something in linux_binprm, then struct file *file = fd_file(fdget(some_random_file_descriptor_in_current)); file->f_op->read(..);
Would VFS maintainers approve such usage ?
More so, your LSM does file = get_task_exe_file(current); kernel_read_file(file, ...);
This is even worse. You've corrupted the ELF binary with extra garbage at the end. objdump/elfutils will choke on it and you're lucky that binfmt_elf still loads it. The whole approach is a non-starter.
The program just got passed in to bpf_prog_load() as a set of attributes which, for a light skeleton, directly contain the code as a blob and have the various BTF relocations as a blob in a single element array map. I think everyone agrees that the integrity of the program would be compromised by modifications to the relocations, so the security_bpf_prog_load() hook can't make an integrity determination without examining both. If the hook can't use the bpf_maps.. APIs directly is there some other API it should be using to get the relocations, or are you saying that the security_bpf_prog_load() hook isn't fit for purpose and it should be called after the bpf core has loaded the relocations so they can be provided to the hook as an argument?
No. As I said twice already the only place to verify program signature is a bpf subsystem itself. Hacking into bpf internals from LSM, BPF-LSM program, or any other kernel subsystem is a no go.
The above, by the way, is independent of signing, because it applies to any determination that might be made in the security_bpf_prog_load() hook regardless of purpose.
security_bpf_prog_load() should not access bpf internals. That LSM hook sees the following: security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, struct bpf_token *token, bool kernel);
LSM can look into uapi things there. Like prog->sleepable, prog->tag, prog->aux->name, but things like prog->aux->jit_data or prog->aux->used_maps are not ok to access. If in doubt, ask on the mailing list.
On Fri, 2025-04-04 at 14:54 -0700, Blaise Boscaccy wrote: [...]
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h index 90451e2e12bd..7ed9337be542 100644 --- a/include/linux/kernel_read_file.h +++ b/include/linux/kernel_read_file.h @@ -14,6 +14,7 @@ id(KEXEC_INITRAMFS, kexec-initramfs) \ id(POLICY, security-policy) \ id(X509_CERTIFICATE, x509-certificate) \
- id(EBPF, ebpf) \
This causes a BUILD_BUG_ON for me in security/selinux/hooks.c with CONFIG_SECURITY_SELINUX=y because READING_MAX_ID and LOADING_MAX_ID become 8.
Below is what I had to do to get the compile to work.
Regards,
James
---
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e7a7dcab81db..9a7ed0b4b08d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4133,7 +4133,7 @@ static int selinux_kernel_read_file(struct file *file, { int rc = 0;
- BUILD_BUG_ON_MSG(READING_MAX_ID > 7, + BUILD_BUG_ON_MSG(READING_MAX_ID > 8, "New kernel_read_file_id introduced; update SELinux!");
switch (id) { @@ -4158,6 +4158,10 @@ static int selinux_kernel_read_file(struct file *file, rc = selinux_kernel_load_from_file(file, SYSTEM__X509_CERTIFICATE_LOAD); break; + case READING_EBPF: + rc = selinux_kernel_load_from_file(file, + SYSTEM__EBPF_LOAD); + break; default: break; } @@ -4169,7 +4173,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents) { int rc = 0;
- BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7, + BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8, "New kernel_load_data_id introduced; update SELinux!");
switch (id) { @@ -4195,6 +4199,10 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents) rc = selinux_kernel_load_from_file(NULL, SYSTEM__X509_CERTIFICATE_LOAD); break; + case LOADING_EBPF: + rc = selinux_kernel_load_from_file(NULL, + SYSTEM__EBPF_LOAD); + break; default: break; } diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 04a9b480885e..671db23451df 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -65,7 +65,7 @@ const struct security_class_mapping secclass_map[] = { { "ipc_info", "syslog_read", "syslog_mod", "syslog_console", "module_request", "module_load", "firmware_load", "kexec_image_load", "kexec_initramfs_load", "policy_load", - "x509_certificate_load", NULL } }, + "x509_certificate_load", "ebpf_load", NULL } }, { "capability", { COMMON_CAP_PERMS, NULL } }, { "filesystem", { "mount", "remount", "unmount", "getattr", "relabelfrom",
On Sat, Apr 19, 2025 at 2:43 PM James Bottomley James.Bottomley@hansenpartnership.com wrote:
On Fri, 2025-04-04 at 14:54 -0700, Blaise Boscaccy wrote: [...]
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h index 90451e2e12bd..7ed9337be542 100644 --- a/include/linux/kernel_read_file.h +++ b/include/linux/kernel_read_file.h @@ -14,6 +14,7 @@ id(KEXEC_INITRAMFS, kexec-initramfs) \ id(POLICY, security-policy) \ id(X509_CERTIFICATE, x509-certificate) \
id(EBPF, ebpf) \This causes a BUILD_BUG_ON for me in security/selinux/hooks.c with CONFIG_SECURITY_SELINUX=y because READING_MAX_ID and LOADING_MAX_ID become 8.
Below is what I had to do to get the compile to work.
That code was updated during the v6.15 merge window, depending on what kernel sources Blaise is using for development it's possible he didn't bump into this even if he was building with SELinux enabled.
Otherwise the changes below look reasonable to me.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e7a7dcab81db..9a7ed0b4b08d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4133,7 +4133,7 @@ static int selinux_kernel_read_file(struct file *file, { int rc = 0;
BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
BUILD_BUG_ON_MSG(READING_MAX_ID > 8, "New kernel_read_file_id introduced; update SELinux!"); switch (id) {@@ -4158,6 +4158,10 @@ static int selinux_kernel_read_file(struct file *file, rc = selinux_kernel_load_from_file(file, SYSTEM__X509_CERTIFICATE_LOAD); break;
case READING_EBPF:rc = selinux_kernel_load_from_file(file,SYSTEM__EBPF_LOAD);break; default: break; }@@ -4169,7 +4173,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents) { int rc = 0;
BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8, "New kernel_load_data_id introduced; update SELinux!"); switch (id) {@@ -4195,6 +4199,10 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents) rc = selinux_kernel_load_from_file(NULL, SYSTEM__X509_CERTIFICATE_LOAD); break;
case LOADING_EBPF:rc = selinux_kernel_load_from_file(NULL,SYSTEM__EBPF_LOAD);break; default: break; }diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 04a9b480885e..671db23451df 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -65,7 +65,7 @@ const struct security_class_mapping secclass_map[] = { { "ipc_info", "syslog_read", "syslog_mod", "syslog_console", "module_request", "module_load", "firmware_load", "kexec_image_load", "kexec_initramfs_load", "policy_load",
"x509_certificate_load", NULL } },
"x509_certificate_load", "ebpf_load", NULL } }, { "capability", { COMMON_CAP_PERMS, NULL } }, { "filesystem", { "mount", "remount", "unmount", "getattr", "relabelfrom",
On Mon, 2025-04-21 at 14:52 -0400, Paul Moore wrote:
On Sat, Apr 19, 2025 at 2:43 PM James Bottomley James.Bottomley@hansenpartnership.com wrote:
On Fri, 2025-04-04 at 14:54 -0700, Blaise Boscaccy wrote: [...]
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h index 90451e2e12bd..7ed9337be542 100644 --- a/include/linux/kernel_read_file.h +++ b/include/linux/kernel_read_file.h @@ -14,6 +14,7 @@ id(KEXEC_INITRAMFS, kexec-initramfs) \ id(POLICY, security-policy) \ id(X509_CERTIFICATE, x509-certificate) \ + id(EBPF, ebpf) \
This causes a BUILD_BUG_ON for me in security/selinux/hooks.c with CONFIG_SECURITY_SELINUX=y because READING_MAX_ID and LOADING_MAX_ID become 8.
Below is what I had to do to get the compile to work.
That code was updated during the v6.15 merge window, depending on what kernel sources Blaise is using for development it's possible he didn't bump into this even if he was building with SELinux enabled.
Sorry I should have said I pulled the patches into 6.15-rc2 to play with them (hence I did pick up everything in the recent merge window).
Regards,
James
This introduces the sign-ebpf tool. It is very similar to the existing sign-file script, with one key difference, it will sign a file with with a signature computed off of arbitrary input data. This can used to sign an ebpf light skeleton loader program for verification via Hornet.
Typical usage is to provide a payload containing the light skeleton ebpf syscall program binary and it's associated maps, which can be extracted from the auto-generated skeleton header.
Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com --- scripts/Makefile | 1 + scripts/hornet/Makefile | 5 + scripts/hornet/sign-ebpf.c | 411 +++++++++++++++++++++++++++++++++++++ 3 files changed, 417 insertions(+) create mode 100644 scripts/hornet/Makefile create mode 100644 scripts/hornet/sign-ebpf.c
diff --git a/scripts/Makefile b/scripts/Makefile index 46f860529df5..a2cace05d734 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -57,6 +57,7 @@ subdir-$(CONFIG_GENKSYMS) += genksyms subdir-$(CONFIG_GENDWARFKSYMS) += gendwarfksyms subdir-$(CONFIG_SECURITY_SELINUX) += selinux subdir-$(CONFIG_SECURITY_IPE) += ipe +subdir-$(CONFIG_SECURITY_HORNET) += hornet
# Let clean descend into subdirs subdir- += basic dtc gdb kconfig mod diff --git a/scripts/hornet/Makefile b/scripts/hornet/Makefile new file mode 100644 index 000000000000..ab71dbb8688e --- /dev/null +++ b/scripts/hornet/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 +hostprogs-always-y := sign-ebpf + +HOSTCFLAGS_sign-ebpf.o = $(shell $(HOSTPKG_CONFIG) --cflags libcrypto 2> /dev/null) +HOSTLDLIBS_sign-ebpf = $(shell $(HOSTPKG_CONFIG) --libs libcrypto 2> /dev/null || echo -lcrypto) diff --git a/scripts/hornet/sign-ebpf.c b/scripts/hornet/sign-ebpf.c new file mode 100644 index 000000000000..e9d2cb6d5cfb --- /dev/null +++ b/scripts/hornet/sign-ebpf.c @@ -0,0 +1,411 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Sign ebpf programs and skeletons using the given key. + * + * This program is heavily based on the kernel's sign-file tool + * with some minor additions to support the signing of eBPF lskels. + * + * Copyright © 2014-2016 Red Hat, Inc. All Rights Reserved. + * Copyright © 2015 Intel Corporation. + * Copyright © 2016 Hewlett Packard Enterprise Development LP + * Copyright © 2025 Microsoft Corporation. + */ +#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <stdbool.h> +#include <string.h> +#include <getopt.h> +#include <err.h> +#include <arpa/inet.h> +#include <openssl/opensslv.h> +#include <openssl/bio.h> +#include <openssl/evp.h> +#include <openssl/pem.h> +#include <openssl/err.h> +#if OPENSSL_VERSION_MAJOR >= 3 +# define USE_PKCS11_PROVIDER +# include <openssl/provider.h> +# include <openssl/store.h> +#else +# if !defined(OPENSSL_NO_ENGINE) && !defined(OPENSSL_NO_DEPRECATED_3_0) +# define USE_PKCS11_ENGINE +# include <openssl/engine.h> +# endif +#endif +#include "../ssl-common.h" + +/* + * Use CMS if we have openssl-1.0.0 or newer available - otherwise we have to + * assume that it's not available and its header file is missing and that we + * should use PKCS#7 instead. Switching to the older PKCS#7 format restricts + * the options we have on specifying the X.509 certificate we want. + * + * Further, older versions of OpenSSL don't support manually adding signers to + * the PKCS#7 message so have to accept that we get a certificate included in + * the signature message. Nor do such older versions of OpenSSL support + * signing with anything other than SHA1 - so we're stuck with that if such is + * the case. + */ +#if defined(LIBRESSL_VERSION_NUMBER) || \ + OPENSSL_VERSION_NUMBER < 0x10000000L || \ + defined(OPENSSL_NO_CMS) +#define USE_PKCS7 +#endif +#ifndef USE_PKCS7 +#include <openssl/cms.h> +#else +#include <openssl/pkcs7.h> +#endif + +struct module_signature { + uint8_t algo; /* Public-key crypto algorithm [0] */ + uint8_t hash; /* Digest algorithm [0] */ + uint8_t id_type; /* Key identifier type [PKEY_ID_PKCS7] */ + uint8_t signer_len; /* Length of signer's name [0] */ + uint8_t key_id_len; /* Length of key identifier [0] */ + uint8_t __pad[3]; + uint32_t sig_len; /* Length of signature data */ +}; + +#define PKEY_ID_PKCS7 2 + +static char magic_number[] = "~eBPF signature appended~\n"; + +static __attribute__((noreturn)) +void format(void) +{ + fprintf(stderr, + "Usage: scripts/sign-ebpf [-dp] <hash algo> <key> <x509> <bin> <loader> [<dest>]\n"); + exit(2); +} + +static const char *key_pass; + +static int pem_pw_cb(char *buf, int len, int w, void *v) +{ + int pwlen; + + if (!key_pass) + return -1; + + pwlen = strlen(key_pass); + if (pwlen >= len) + return -1; + + strcpy(buf, key_pass); + + /* If it's wrong, don't keep trying it. */ + key_pass = NULL; + + return pwlen; +} + +static EVP_PKEY *read_private_key_pkcs11(const char *private_key_name) +{ + EVP_PKEY *private_key = NULL; +#ifdef USE_PKCS11_PROVIDER + OSSL_STORE_CTX *store; + + if (!OSSL_PROVIDER_try_load(NULL, "pkcs11", true)) + ERR(1, "OSSL_PROVIDER_try_load(pkcs11)"); + if (!OSSL_PROVIDER_try_load(NULL, "default", true)) + ERR(1, "OSSL_PROVIDER_try_load(default)"); + + store = OSSL_STORE_open(private_key_name, NULL, NULL, NULL, NULL); + ERR(!store, "OSSL_STORE_open"); + + while (!OSSL_STORE_eof(store)) { + OSSL_STORE_INFO *info = OSSL_STORE_load(store); + + if (!info) { + drain_openssl_errors(__LINE__, 0); + continue; + } + if (OSSL_STORE_INFO_get_type(info) == OSSL_STORE_INFO_PKEY) { + private_key = OSSL_STORE_INFO_get1_PKEY(info); + ERR(!private_key, "OSSL_STORE_INFO_get1_PKEY"); + } + OSSL_STORE_INFO_free(info); + if (private_key) + break; + } + OSSL_STORE_close(store); +#elif defined(USE_PKCS11_ENGINE) + ENGINE *e; + + ENGINE_load_builtin_engines(); + drain_openssl_errors(__LINE__, 1); + e = ENGINE_by_id("pkcs11"); + ERR(!e, "Load PKCS#11 ENGINE"); + if (ENGINE_init(e)) + drain_openssl_errors(__LINE__, 1); + else + ERR(1, "ENGINE_init"); + if (key_pass) + ERR(!ENGINE_ctrl_cmd_string(e, "PIN", key_pass, 0), "Set PKCS#11 PIN"); + private_key = ENGINE_load_private_key(e, private_key_name, NULL, NULL); + ERR(!private_key, "%s", private_key_name); +#else + fprintf(stderr, "no pkcs11 engine/provider available\n"); + exit(1); +#endif + return private_key; +} + +static EVP_PKEY *read_private_key(const char *private_key_name) +{ + if (!strncmp(private_key_name, "pkcs11:", 7)) { + return read_private_key_pkcs11(private_key_name); + } else { + EVP_PKEY *private_key; + BIO *b; + + b = BIO_new_file(private_key_name, "rb"); + ERR(!b, "%s", private_key_name); + private_key = PEM_read_bio_PrivateKey(b, NULL, pem_pw_cb, + NULL); + ERR(!private_key, "%s", private_key_name); + BIO_free(b); + + return private_key; + } +} + +static X509 *read_x509(const char *x509_name) +{ + unsigned char buf[2]; + X509 *x509; + BIO *b; + int n; + + b = BIO_new_file(x509_name, "rb"); + ERR(!b, "%s", x509_name); + + /* Look at the first two bytes of the file to determine the encoding */ + n = BIO_read(b, buf, 2); + if (n != 2) { + if (BIO_should_retry(b)) { + fprintf(stderr, "%s: Read wanted retry\n", x509_name); + exit(1); + } + if (n >= 0) { + fprintf(stderr, "%s: Short read\n", x509_name); + exit(1); + } + ERR(1, "%s", x509_name); + } + + ERR(BIO_reset(b) != 0, "%s", x509_name); + + if (buf[0] == 0x30 && buf[1] >= 0x81 && buf[1] <= 0x84) + /* Assume raw DER encoded X.509 */ + x509 = d2i_X509_bio(b, NULL); + else + /* Assume PEM encoded X.509 */ + x509 = PEM_read_bio_X509(b, NULL, NULL, NULL); + + BIO_free(b); + ERR(!x509, "%s", x509_name); + + return x509; +} + +int main(int argc, char **argv) +{ + struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 }; + char *hash_algo = NULL; + char *private_key_name = NULL, *raw_sig_name = NULL; + char *x509_name, *bin_name, *loader_name, *dest_name; + bool save_sig = false, replace_orig; + bool sign_only = false; + bool raw_sig = false; + unsigned char buf[4096]; + unsigned long loader_size, sig_size; + unsigned int use_signed_attrs; + const EVP_MD *digest_algo; + EVP_PKEY *private_key; +#ifndef USE_PKCS7 + CMS_ContentInfo *cms = NULL; + unsigned int use_keyid = 0; +#else + PKCS7 *pkcs7 = NULL; +#endif + X509 *x509; + BIO *bd, *bm, *bl; + int opt, n; + OpenSSL_add_all_algorithms(); + ERR_load_crypto_strings(); + ERR_clear_error(); + + key_pass = getenv("KBUILD_SIGN_PIN"); + +#ifndef USE_PKCS7 + use_signed_attrs = CMS_NOATTR; +#else + use_signed_attrs = PKCS7_NOATTR; +#endif + + do { + opt = getopt(argc, argv, "sdpk"); + switch (opt) { + case 's': raw_sig = true; break; + case 'p': save_sig = true; break; + case 'd': sign_only = true; save_sig = true; break; +#ifndef USE_PKCS7 + case 'k': use_keyid = CMS_USE_KEYID; break; +#endif + case -1: break; + default: format(); + } + } while (opt != -1); + + argc -= optind; + argv += optind; + if (argc < 5 || argc > 6) + format(); + + if (raw_sig) { + raw_sig_name = argv[0]; + hash_algo = argv[1]; + } else { + hash_algo = argv[0]; + private_key_name = argv[1]; + } + x509_name = argv[2]; + bin_name = argv[3]; + loader_name = argv[4]; + if (argc == 6 && strcmp(argv[4], argv[5]) != 0) { + dest_name = argv[5]; + replace_orig = false; + } else { + ERR(asprintf(&dest_name, "%s.~signed~", loader_name) < 0, + "asprintf"); + replace_orig = true; + } + +#ifdef USE_PKCS7 + if (strcmp(hash_algo, "sha1") != 0) { + fprintf(stderr, "sign-file: %s only supports SHA1 signing\n", + OPENSSL_VERSION_TEXT); + exit(3); + } +#endif + + /* Open the bin file */ + bm = BIO_new_file(bin_name, "rb"); + ERR(!bm, "%s", bin_name); + + if (!raw_sig) { + /* Read the private key and the X.509 cert the PKCS#7 message + * will point to. + */ + private_key = read_private_key(private_key_name); + x509 = read_x509(x509_name); + + /* Digest the module data. */ + OpenSSL_add_all_digests(); + drain_openssl_errors(__LINE__, 0); + digest_algo = EVP_get_digestbyname(hash_algo); + ERR(!digest_algo, "EVP_get_digestbyname"); + +#ifndef USE_PKCS7 + /* Load the signature message from the digest buffer. */ + cms = CMS_sign(NULL, NULL, NULL, NULL, + CMS_NOCERTS | CMS_PARTIAL | CMS_BINARY | + CMS_DETACHED | CMS_STREAM); + ERR(!cms, "CMS_sign"); + + ERR(!CMS_add1_signer(cms, x509, private_key, digest_algo, + CMS_NOCERTS | CMS_BINARY | + CMS_NOSMIMECAP | use_keyid | + use_signed_attrs), + "CMS_add1_signer"); + ERR(CMS_final(cms, bm, NULL, CMS_NOCERTS | CMS_BINARY) != 1, + "CMS_final"); + +#else + pkcs7 = PKCS7_sign(x509, private_key, NULL, bm, + PKCS7_NOCERTS | PKCS7_BINARY | + PKCS7_DETACHED | use_signed_attrs); + ERR(!pkcs7, "PKCS7_sign"); +#endif + + if (save_sig) { + char *sig_file_name; + BIO *b; + + ERR(asprintf(&sig_file_name, "%s.p7s", bin_name) < 0, + "asprintf"); + b = BIO_new_file(sig_file_name, "wb"); + ERR(!b, "%s", sig_file_name); +#ifndef USE_PKCS7 + ERR(i2d_CMS_bio_stream(b, cms, NULL, 0) != 1, + "%s", sig_file_name); +#else + ERR(i2d_PKCS7_bio(b, pkcs7) != 1, + "%s", sig_file_name); +#endif + BIO_free(b); + } + + if (sign_only) { + BIO_free(bm); + return 0; + } + } + + /* Open the destination file now so that we can shovel the loader data + * across as we read it. + */ + bd = BIO_new_file(dest_name, "wb"); + ERR(!bd, "%s", dest_name); + + bl = BIO_new_file(loader_name, "rb"); + ERR(!bl, "%s", loader_name); + + + /* Append the marker and the PKCS#7 message to the destination file */ + ERR(BIO_reset(bm) < 0, "%s", bin_name); + ERR(BIO_reset(bl) < 0, "%s", loader_name); + while ((n = BIO_read(bl, buf, sizeof(buf))), + n > 0) { + ERR(BIO_write(bd, buf, n) < 0, "%s", dest_name); + } + BIO_free(bl); + BIO_free(bm); + ERR(n < 0, "%s", loader_name); + loader_size = BIO_number_written(bd); + + if (!raw_sig) { +#ifndef USE_PKCS7 + ERR(i2d_CMS_bio_stream(bd, cms, NULL, 0) != 1, "%s", dest_name); +#else + ERR(i2d_PKCS7_bio(bd, pkcs7) != 1, "%s", dest_name); +#endif + } else { + BIO *b; + + /* Read the raw signature file and write the data to the + * destination file + */ + b = BIO_new_file(raw_sig_name, "rb"); + ERR(!b, "%s", raw_sig_name); + while ((n = BIO_read(b, buf, sizeof(buf))), n > 0) + ERR(BIO_write(bd, buf, n) < 0, "%s", dest_name); + BIO_free(b); + } + + sig_size = BIO_number_written(bd) - loader_size; + sig_info.sig_len = htonl(sig_size); + ERR(BIO_write(bd, &sig_info, sizeof(sig_info)) < 0, "%s", dest_name); + ERR(BIO_write(bd, magic_number, sizeof(magic_number) - 1) < 0, "%s", dest_name); + + ERR(BIO_free(bd) != 1, "%s", dest_name); + + /* Finally, if we're signing in place, replace the original. */ + if (replace_orig) + ERR(rename(dest_name, loader_name) < 0, "%s", dest_name); + + return 0; +}
This script eases light skeleton development against Hornet by generating a data payload which can be used for signing a light skeleton binary using sign-ebpf. The binary payload it generates contains the skeleton's ebpf instructions followed by the skeleton loader's map.
Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com --- scripts/hornet/extract-skel.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100755 scripts/hornet/extract-skel.sh
diff --git a/scripts/hornet/extract-skel.sh b/scripts/hornet/extract-skel.sh new file mode 100755 index 000000000000..9ace78794b85 --- /dev/null +++ b/scripts/hornet/extract-skel.sh @@ -0,0 +1,29 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2025 Microsoft Corporation +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of version 2 of the GNU General Public +# License as published by the Free Software Foundation. + +function usage() { + echo "Sample script for extracting instructions and map data out of" + echo "autogenerated eBPF lskel headers" + echo "" + echo "USAGE: header_file output_file" + exit +} + +ARGC=$# + +EXPECTED_ARGS=2 + +if [ $ARGC -ne $EXPECTED_ARGS ] ; then + usage +else + printf $(gcc -E $1 | grep "static const char opts_insn" | \ + awk -F"=" '{print $2}' | sed 's/;+$//' | sed 's/"//g') > $2 + printf $(gcc -E $1 | grep "static const char opts_data" | \ + awk -F"=" '{print $2}' | sed 's/;+$//' | sed 's/"//g') >> $2 +fi
This selftest contains a testcase that utilizes light skeleton eBPF loaders. One version of the light skeleton is signed with the autogenerated module signing key, another is not. A test driver attempts to load the programs. With Hornet enabled, the signed version should successfully be loaded, and the unsigned version should fail.
Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/hornet/Makefile | 51 ++++++++++++++++++++ tools/testing/selftests/hornet/loader.c | 21 ++++++++ tools/testing/selftests/hornet/trivial.bpf.c | 33 +++++++++++++ 4 files changed, 106 insertions(+) create mode 100644 tools/testing/selftests/hornet/Makefile create mode 100644 tools/testing/selftests/hornet/loader.c create mode 100644 tools/testing/selftests/hornet/trivial.bpf.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 8daac70c2f9d..fce32ee4de32 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -41,6 +41,7 @@ TARGETS += ftrace TARGETS += futex TARGETS += gpio TARGETS += hid +TARGETS += hornet TARGETS += intel_pstate TARGETS += iommu TARGETS += ipc diff --git a/tools/testing/selftests/hornet/Makefile b/tools/testing/selftests/hornet/Makefile new file mode 100644 index 000000000000..93da70f41d40 --- /dev/null +++ b/tools/testing/selftests/hornet/Makefile @@ -0,0 +1,51 @@ +# SPDX-License-Identifier: GPL-2.0 +include ../../../build/Build.include +include ../../../scripts/Makefile.arch +include ../../../scripts/Makefile.include + +CLANG ?= clang +CFLAGS := -g -O2 -Wall +BPFTOOL ?= bpftool +SCRIPTSDIR := $(abspath ../../../../scripts/hornet) +TOOLSDIR := $(abspath ../../..) +LIBDIR := $(TOOLSDIR)/lib +BPFDIR := $(LIBDIR)/bpf +TOOLSINCDIR := $(TOOLSDIR)/include +APIDIR := $(TOOLSINCDIR)/uapi +CERTDIR := $(abspath ../../../../certs) + +TEST_GEN_PROGS_EXTENDED := loader +TEST_GEN_PROGS := signed_loader +TEST_PROGS := fail_loader +TEST_GEN_FILES := vmlinux.h loader.h trivial.bin trivial.bpf.o +$(TEST_GEN_PROGS): LDLIBS += -lbpf +$(TEST_GEN_PROGS): $(TEST_GEN_FILES) + +include ../lib.mk + +BPF_CFLAGS := -target bpf \ + -D__TARGET_ARCH_$(ARCH) \ + -I/usr/include/$(shell uname -m)-linux-gnu \ + $(KHDR_INCLUDES) +vmlinux.h: + $(BPFTOOL) btf dump file /sys/kernel/btf/vmlinux format c > vmlinux.h + +trivial.bpf.o: trivial.bpf.c vmlinux.h + $(CLANG) $(CFLAGS) $(BPF_CFLAGS) -c $< -o $@ + +loader.h: trivial.bpf.o + $(BPFTOOL) gen skeleton -L $< name trivial > $@ + +trivial.bin: loader.h + $(SCRIPTSDIR)/extract-skel.sh $< $@ + +loader: loader.c loader.h + $(CC) $(CFLAGS) -I$(LIBDIR) -I$(APIDIR) $< -o $@ -lbpf + +fail_loader: fail_loader.c loader.h + $(CC) $(CFLAGS) -I$(LIBDIR) -I$(APIDIR) $< -o $@ -lbpf + +signed_loader: trivial.bin loader fail_loader + $(SCRIPTSDIR)/sign-ebpf sha256 $(CERTDIR)/signing_key.pem $(CERTDIR)/signing_key.x509 \ + trivial.bin loader signed_loader + chmod u+x $@ diff --git a/tools/testing/selftests/hornet/loader.c b/tools/testing/selftests/hornet/loader.c new file mode 100644 index 000000000000..9a43bb012d1b --- /dev/null +++ b/tools/testing/selftests/hornet/loader.c @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause + +#include <stdio.h> +#include <unistd.h> +#include <stddef.h> +#include <sys/resource.h> +#include <bpf/libbpf.h> +#include <errno.h> +#include "loader.h" + +int main(int argc, char **argv) +{ + struct trivial *skel; + + skel = trivial__open_and_load(); + if (!skel) + return -1; + + trivial__destroy(skel); + return 0; +} diff --git a/tools/testing/selftests/hornet/trivial.bpf.c b/tools/testing/selftests/hornet/trivial.bpf.c new file mode 100644 index 000000000000..d38c5b53ff93 --- /dev/null +++ b/tools/testing/selftests/hornet/trivial.bpf.c @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause + +#include "vmlinux.h" + +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <bpf/bpf_core_read.h> + +char LICENSE[] SEC("license") = "Dual BSD/GPL"; + +int monitored_pid = 0; + +SEC("tracepoint/syscalls/sys_enter_unlinkat") +int handle_enter_unlink(struct trace_event_raw_sys_enter *ctx) +{ + char filename[128] = { 0 }; + struct task_struct *task; + unsigned long start_time = 0; + int pid = bpf_get_current_pid_tgid() >> 32; + char *pathname_ptr = (char *) BPF_CORE_READ(ctx, args[1]); + + bpf_probe_read_str(filename, sizeof(filename), pathname_ptr); + task = (struct task_struct *)bpf_get_current_task(); + start_time = BPF_CORE_READ(task, start_time); + + bpf_printk("BPF triggered unlinkat by PID: %d, start_time %ld. pathname = %s", + pid, start_time, filename); + + if (monitored_pid == pid) + bpf_printk("target pid found"); + + return 0; +}
linux-kselftest-mirror@lists.linaro.org
-
Alexei Starovoitov -
Blaise Boscaccy -
James Bottomley -
kernel test robot -
Matteo Croce -
Paul Moore -
Tyler Hicks