linux-kernel - Re: [PATCH v4 04/11] net: reserve prefix

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: <20250508214850.62973-1-kuniyu@amazon.com>
Date: Thu, 8 May 2025 14:47:45 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <brauner@...nel.org>
CC: <alexander@...alicyn.com>, <bluca@...ian.org>, <daan.j.demeyer@...il.com>,
	<davem@...emloft.net>, <david@...dahead.eu>, <edumazet@...gle.com>,
	<horms@...nel.org>, <jack@...e.cz>, <jannh@...gle.com>, <kuba@...nel.org>,
	<kuniyu@...zon.com>, <lennart@...ttering.net>,
	<linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<me@...dnzj.com>, <netdev@...r.kernel.org>, <oleg@...hat.com>,
	<pabeni@...hat.com>, <viro@...iv.linux.org.uk>, <zbyszek@...waw.pl>
Subject: Re: [PATCH v4 04/11] net: reserve prefix

From: Christian Brauner <brauner@...nel.org>
Date: Thu, 8 May 2025 08:16:29 +0200
> On Wed, May 07, 2025 at 03:45:52PM -0700, Kuniyuki Iwashima wrote:
> > From: Christian Brauner <brauner@...nel.org>
> > Date: Wed, 07 May 2025 18:13:37 +0200
> > > Add the reserved "linuxafsk/" prefix for AF_UNIX sockets and require
> > > CAP_NET_ADMIN in the owning user namespace of the network namespace to
> > > bind it. This will be used in next patches to support the coredump
> > > socket but is a generally useful concept.
> > 
> > I really think we shouldn't reserve address and it should be
> > configurable by users via core_pattern as with the other
> > coredump types.
> > 
> > AF_UNIX doesn't support SO_REUSEPORT, so once the socket is
> > dying, user can't start the new coredump listener until it's
> > fully cleaned up, which adds unnecessary drawback.
> 
> This really doesn't matter.
> 
> > The semantic should be same with other types, and the todo
> > for the coredump service is prepare file (file, process, socket)
> > that can receive data and set its name to core_pattern.
> 
> We need to perform a capability check during bind() for the host's
> coredump socket. Otherwise if the coredump server crashes an
> unprivileged attacker can simply bind the address and receive all
> coredumps from suid binaries.

As I mentioned in the previous thread, this can be better
handled by BPF LSM with more fine-grained rule.

1. register a socket with its name to BPF map
2. check if the destination socket is registered at connect

Even when LSM is not availalbe, the cgroup BPF prog can make
connect() fail if the destination name is not registered
in the map.

> 
> This is also a problem for legitimate coredump server updates. To change
> the coredump address the coredump server must first setup a new socket
> and then update core_pattern and then shutdown the old coredump socket.

So, for completeness, the server should set up a cgroup BPF
prog to route the request for the old name to the new one.

Here, the bpf map above can be reused to check if the socket
name is registered in the map or route to another socket in
the map.

Then, the unprivileged issue below and the non-dumpable issue
mentioned in the cover letter can also be resolved.

The server is expected to have CAP_SYS_ADMIN, so BPF should
play a role.


> 
> Now an unprivileged attacker can rebind the old coredump socket address
> but there's still a crashing task that got scheduled out after it copied
> the old coredump server address but before it connected to the coredump
> server. The new server is now up and the old server's address has been
> reused by the attacker. Now the crashing task gets scheduled back in and
> connects to the unprivileged attacker and forwards its suid dump to the
> attacker.
> 
> The name of the socket needs to be protected. This can be done by prefix
> but the simplest way is what I did in my earlier version and to just use
> a well-known name. The name really doesn't matter and all it adds is
> potential for subtle bugs. I want the coredump code I have to maintain
> to have as little moving parts as possible.
> 
> I'm happy to drop the patch to reserve the prefix as that seems to
> bother you. But the coredump socket name won't be configurable. It'd be
> good if we could just compromise here. Without the capability check on
> bind we can just throw this all out as that's never going to be safe.