linux-kernel - Re: [PATCH v2 security-next 1/4] security: Hornet LSM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAADnVQ+JGfwRgsoe2=EHkXdTyQ8ycn0D9nh1k49am++4oXUPHg@mail.gmail.com>
Date: Mon, 14 Apr 2025 13:56:42 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Blaise Boscaccy <bboscaccy@...ux.microsoft.com>
Cc: Jonathan Corbet <corbet@....net>, David Howells <dhowells@...hat.com>, 
	Herbert Xu <herbert@...dor.apana.org.au>, "David S. Miller" <davem@...emloft.net>, 
	Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>, 
	"Serge E. Hallyn" <serge@...lyn.com>, Masahiro Yamada <masahiroy@...nel.org>, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, Shuah Khan <shuah@...nel.org>, 
	Mickaël Salaün <mic@...ikod.net>, 
	Günther Noack <gnoack@...gle.com>, 
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>, 
	Justin Stitt <justinstitt@...gle.com>, Jarkko Sakkinen <jarkko@...nel.org>, 
	Jan Stancek <jstancek@...hat.com>, Neal Gompa <neal@...pa.dev>, 
	"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	keyrings@...r.kernel.org, 
	Linux Crypto Mailing List <linux-crypto@...r.kernel.org>, 
	LSM List <linux-security-module@...r.kernel.org>, 
	Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, bpf <bpf@...r.kernel.org>, 
	clang-built-linux <llvm@...ts.linux.dev>, nkapron@...gle.com, 
	Matteo Croce <teknoraver@...a.com>, Roberto Sassu <roberto.sassu@...wei.com>, 
	Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: [PATCH v2 security-next 1/4] security: Hornet LSM

On Sat, Apr 12, 2025 at 6:58 AM Blaise Boscaccy
<bboscaccy@...ux.microsoft.com> wrote:
>
> TAlexei Starovoitov <alexei.starovoitov@...il.com> writes:
>
> > On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy
> > <bboscaccy@...ux.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.