diff mbox series

[v2,security-next,1/4] security: Hornet LSM

Message ID 20250404215527.1563146-2-bboscaccy@linux.microsoft.com (mailing list archive)
State New
Headers show
Series Introducing Hornet LSM | expand

Commit Message

Blaise Boscaccy April 4, 2025, 9:54 p.m. UTC
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.com/

eBPF programs, before loading, undergo a complex set of operations
which transform pseudo-values within the immediate operands of
instructions into concrete values based on the running
system. Typically, this is done by libbpf in
userspace. Light-skeletons were introduced in order to support
preloading of bpf programs and user-mode-drivers by removing the
dependency on libbpf and userspace-based operations.

Userpace modifications, which may change every time a program gets
loaded or runs on a slightly different kernel, break known signature
verification algorithms. A method is needed for passing unadulterated
binary buffers into the kernel in-order to use existing signature
verification algorithms. Light-skeleton loaders with their support of
only in-kernel relocations fit that constraint.

Hornet employs a signature verification scheme similar to that of
kernel modules. A signature is appended to the end of an
executable file. During an invocation of the BPF_PROG_LOAD subcommand,
a signature is extracted from the current task's executable file. That
signature is used to verify the integrity of the bpf instructions and
maps which were passed into the kernel. Additionally, Hornet
implicitly trusts any programs which were loaded from inside kernel
rather than userspace, which allows BPF_PRELOAD programs along with
outputs for BPF_SYSCALL programs to run.

The validation check consists of checking a PKCS#7 formatted signature
against a data buffer containing the raw instructions of an eBPF
program, followed by the initial values of any maps used by the
program. Maps are frozen before signature verification checking to
stop TOCTOU attacks.

Signed-off-by: Blaise Boscaccy <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

Comments

kernel test robot April 6, 2025, 4:27 a.m. UTC | #1
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-Hornet-LSM/20250405-055741
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.microsoft.com
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@intel.com/config)
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@intel.com/reproduce)

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
kernel test robot April 6, 2025, 8:42 p.m. UTC | #2
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-Hornet-LSM/20250405-055741
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.microsoft.com
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@intel.com/config)
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@intel.com/reproduce)

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
Tyler Hicks April 11, 2025, 7:09 p.m. UTC | #3
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);
> +}
Paul Moore April 11, 2025, 11:16 p.m. UTC | #4
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.com/
> 
> eBPF programs, before loading, undergo a complex set of operations
> which transform pseudo-values within the immediate operands of
> instructions into concrete values based on the running
> system. Typically, this is done by libbpf in
> userspace. Light-skeletons were introduced in order to support
> preloading of bpf programs and user-mode-drivers by removing the
> dependency on libbpf and userspace-based operations.
> 
> Userpace modifications, which may change every time a program gets
> loaded or runs on a slightly different kernel, break known signature
> verification algorithms. A method is needed for passing unadulterated
> binary buffers into the kernel in-order to use existing signature
> verification algorithms. Light-skeleton loaders with their support of
> only in-kernel relocations fit that constraint.
> 
> Hornet employs a signature verification scheme similar to that of
> kernel modules. A signature is appended to the end of an
> executable file. During an invocation of the BPF_PROG_LOAD subcommand,
> a signature is extracted from the current task's executable file. That
> signature is used to verify the integrity of the bpf instructions and
> maps which were passed into the kernel. Additionally, Hornet
> implicitly trusts any programs which were loaded from inside kernel
> rather than userspace, which allows BPF_PRELOAD programs along with
> outputs for BPF_SYSCALL programs to run.
> 
> The validation check consists of checking a PKCS#7 formatted signature
> against a data buffer containing the raw instructions of an eBPF
> program, followed by the initial values of any maps used by the
> program. Maps are frozen before signature verification checking to
> stop TOCTOU attacks.
> 
> Signed-off-by: Blaise Boscaccy <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
Alexei Starovoitov April 12, 2025, 12:09 a.m. UTC | #5
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.
Matteo Croce April 12, 2025, 12:29 a.m. UTC | #6
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.com/

> 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,
Alexei Starovoitov April 12, 2025, 12:57 a.m. UTC | #7
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.com/
...
> @@ -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.
Blaise Boscaccy April 12, 2025, 1:57 p.m. UTC | #8
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.
Blaise Boscaccy April 12, 2025, 2:11 p.m. UTC | #9
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.com/
> ...
>> @@ -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.
Paul Moore April 14, 2025, 4:08 p.m. UTC | #10
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.
Blaise Boscaccy April 14, 2025, 8:11 p.m. UTC | #11
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);
>> +}
Blaise Boscaccy April 14, 2025, 8:46 p.m. UTC | #12
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.com/
>> 
>> eBPF programs, before loading, undergo a complex set of operations
>> which transform pseudo-values within the immediate operands of
>> instructions into concrete values based on the running
>> system. Typically, this is done by libbpf in
>> userspace. Light-skeletons were introduced in order to support
>> preloading of bpf programs and user-mode-drivers by removing the
>> dependency on libbpf and userspace-based operations.
>> 
>> Userpace modifications, which may change every time a program gets
>> loaded or runs on a slightly different kernel, break known signature
>> verification algorithms. A method is needed for passing unadulterated
>> binary buffers into the kernel in-order to use existing signature
>> verification algorithms. Light-skeleton loaders with their support of
>> only in-kernel relocations fit that constraint.
>> 
>> Hornet employs a signature verification scheme similar to that of
>> kernel modules. A signature is appended to the end of an
>> executable file. During an invocation of the BPF_PROG_LOAD subcommand,
>> a signature is extracted from the current task's executable file. That
>> signature is used to verify the integrity of the bpf instructions and
>> maps which were passed into the kernel. Additionally, Hornet
>> implicitly trusts any programs which were loaded from inside kernel
>> rather than userspace, which allows BPF_PRELOAD programs along with
>> outputs for BPF_SYSCALL programs to run.
>> 
>> The validation check consists of checking a PKCS#7 formatted signature
>> against a data buffer containing the raw instructions of an eBPF
>> program, followed by the initial values of any maps used by the
>> program. Maps are frozen before signature verification checking to
>> stop TOCTOU attacks.
>> 
>> Signed-off-by: Blaise Boscaccy <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
Alexei Starovoitov April 14, 2025, 8:56 p.m. UTC | #13
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.
Blaise Boscaccy April 15, 2025, 12:32 a.m. UTC | #14
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
> 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.
>

So the notion of forcing people into writing bpf-based gatekeeper programs
is being abandoned? e.g.

https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeol7@nkbxnrhpn7qr/#t
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.
Paul Moore April 15, 2025, 1:37 a.m. UTC | #15
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 :)
Alexei Starovoitov April 15, 2025, 1:38 a.m. UTC | #16
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
> > 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.
> >
>
> So the notion of forcing people into writing bpf-based gatekeeper programs
> is being abandoned? e.g.
>
> https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeol7@nkbxnrhpn7qr/#t
> 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.
Blaise Boscaccy April 15, 2025, 3:45 p.m. UTC | #17
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
>> > 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.
>> >
>>
>> So the notion of forcing people into writing bpf-based gatekeeper programs
>> is being abandoned? e.g.
>>
>> https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeol7@nkbxnrhpn7qr/#t
>> 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 April 15, 2025, 7:08 p.m. UTC | #18
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
>>> > 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.
>>> >
>>>
>>> So the notion of forcing people into writing bpf-based gatekeeper programs
>>> is being abandoned? e.g.
>>>
>>> https://lore.kernel.org/bpf/bqxgv2tqk3hp3q3lcdqsw27btmlwqfkhyg6kohsw7lwdgbeol7@nkbxnrhpn7qr/#t
>>> 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.
Alexei Starovoitov April 15, 2025, 9:48 p.m. UTC | #19
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.
Blaise Boscaccy April 16, 2025, 5:31 p.m. UTC | #20
Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> 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.
>

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
diff mbox series

Patch

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,
+};