netdev - Re: [PATCH v1 bpf-next 4/5] bpf: Add kfunc to scrub SCM_RIGHTS at security_unix_may

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: <20250506.ahjae2Beemuo@digikod.net>
Date: Tue, 6 May 2025 10:25:09 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Kuniyuki Iwashima <kuniyu@...zon.com>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Daniel Borkmann <daniel@...earbox.net>, 
	John Fastabend <john.fastabend@...il.com>, Alexei Starovoitov <ast@...nel.org>, 
	Andrii Nakryiko <andrii@...nel.org>, Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Günther Noack <gnoack@...gle.com>, Paul Moore <paul@...l-moore.com>, 
	James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>, 
	Stephen Smalley <stephen.smalley.work@...il.com>, Ondrej Mosnacek <omosnace@...hat.com>, 
	Casey Schaufler <casey@...aufler-ca.com>, Christian Brauner <brauner@...nel.org>, 
	Kuniyuki Iwashima <kuni1840@...il.com>, bpf <bpf@...r.kernel.org>, 
	Network Development <netdev@...r.kernel.org>, LSM List <linux-security-module@...r.kernel.org>, 
	selinux@...r.kernel.org, Kees Cook <keescook@...omium.org>, Jann Horn <jannh@...gle.com>, 
	linux-api@...r.kernel.org
Subject: Re: [PATCH v1 bpf-next 4/5] bpf: Add kfunc to scrub SCM_RIGHTS at
 security_unix_may_send().

On Mon, May 05, 2025 at 05:13:32PM -0700, Alexei Starovoitov wrote:
> On Mon, May 5, 2025 at 3:00 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
> >
> > As Christian Brauner said [0], systemd calls cmsg_close_all() [1] after
> > each recvmsg() to close() unwanted file descriptors sent via SCM_RIGHTS.
> >
> > However, this cannot work around the issue that close() for unwanted file
> > descriptors could block longer because the last fput() could occur on
> > the receiver side once sendmsg() with SCM_RIGHTS succeeds.
> >
> > Also, even filtering by LSM at recvmsg() does not work for the same reason.
> >
> > Thus, we need a better way to filter SCM_RIGHTS on the sender side.
> >
> > Let's add a new kfunc to scrub all file descriptors from skb in
> > sendmsg().
> >
> > This allows the receiver to keep recv()ing the bare data and disallows
> > the sender to impose the potential slowness of the last fput().
> >
> > If necessary, we can add more granular filtering per file descriptor
> > after refactoring GC code and adding some fd-to-file helpers for BPF.
> >
> > Sample:
> >
> > SEC("lsm/unix_may_send")
> > int BPF_PROG(unix_scrub_scm_rights,
> >              struct socket *sock, struct socket *other, struct sk_buff *skb)
> > {
> >         struct unix_skb_parms *cb;
> >
> >         if (skb && bpf_unix_scrub_fds(skb))
> >                 return -EPERM;
> >
> >         return 0;
> > }
> 
> Any other programmability do you need there?
> 
> If not and above is all that is needed then what Jann proposed
> sounds like better path to me:
> "
> I think the thorough fix would probably be to introduce a socket
> option (controlled via setsockopt()) that already blocks the peer's
> sendmsg().
> "
> 
> Easier to operate and upriv process can use such setsockopt() too.

Adding a flag with setsockopt() will enable any program to protect
themselves instead of requiring the capability to load an eBPF program.
For the systemd use case, I think a flag would be enough, and it would
benefit more than only/mainly systemd.

Another thing is that we should have a consistent user space error code
if passing file descriptors is denied, to avoid confusing senders, to
not silently ignore dropped file descriptors, and to let the sender know
that it can send again but without passed file descriptors or maybe with
a maximum number of file descriptors.  The ENFILE errno (file table
overflow) looks like a good candidate.

I guess both approaches are valuable, but this series should add this
new flag as well, and specify the expected error handling.

If we want to be able to handle legacy software not using this new flag,
we can leverage seccomp unotify.