netdev - Re: [PATCH nf-next] netfilter: nf

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: <87v8q84nlq.fsf@toke.dk>
Date:   Wed, 31 Aug 2022 14:13:53 +0200
From:   Toke Høiland-Jørgensen <toke@...nel.org>
To:     Florian Westphal <fw@...len.de>, netfilter-devel@...r.kernel.org
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH nf-next] netfilter: nf_tables: add ebpf expression

Florian Westphal <fw@...len.de> writes:

> This expression is a native replacement for xtables 'bpf' match "pinned" mode.
> Userspace needs to pass a file descriptor referencing the program (of socket
> filter type).
> Userspace should also pass the original pathname for that fd so userspace can
> print the original filename again.
>
> Tag and program id are dumped to userspace on 'list' to allow to see which
> program is in use in case the filename isn't available/present.

It seems a bit odd to include the file path in the kernel as well. For
one thing, the same object can be pinned multiple times in different
paths (even in different mount namespaces), and there's also nothing
preventing a different program to have been substituted by the pinned
one by the time the value is echoed back.

Also, there's nothing checking that the path attribute actually contains
a path, so it's really just an arbitrary label that the kernel promises
to echo back. But doesn't NFT already have a per-rule comment feature,
so why add another specifically for BPF? Instead we could just teach the
userspace utility to extract metadata from the BPF program (based on the
ID) like bpftool does. This would include the program name, BTW, so it
does have a semantic identifier.

> cbpf bytecode isn't supported.
>
> No new Kconfig option is added: Its included if BPF_SYSCALL is enabled.
>
> Proposed nft userspace syntax is:
>
> add rule ... ebpf pinned "/sys/fs/bpf/myprog"

Any plan to also teach the nft binary to load a BPF program from an ELF
file (instead of relying on pinning)?

-Toke