linux-kernel - Re: [PATCH bpf-next 2/2] selftests/bpf: Extend test fs

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: <20241016-luxus-winkt-4676cfdf25ff@brauner>
Date: Wed, 16 Oct 2024 16:51:37 +0200
From: Christian Brauner <brauner@...nel.org>
To: Jan Kara <jack@...e.cz>
Cc: Song Liu <songliubraving@...a.com>, 
	Christoph Hellwig <hch@...radead.org>, Song Liu <song@...nel.org>, bpf <bpf@...r.kernel.org>, 
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Kernel Team <kernel-team@...a.com>, Andrii Nakryiko <andrii@...nel.org>, 
	Eduard Zingerman <eddyz87@...il.com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Al Viro <viro@...iv.linux.org.uk>, KP Singh <kpsingh@...nel.org>, 
	Matt Bobrowski <mattbobrowski@...gle.com>
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: Extend test fs_kfuncs to
 cover security.bpf xattr names

On Wed, Oct 16, 2024 at 03:51:55PM +0200, Jan Kara wrote:
> On Tue 15-10-24 05:52:02, Song Liu wrote:
> > > On Oct 14, 2024, at 10:25 PM, Christoph Hellwig <hch@...radead.org> wrote:
> > > On Tue, Oct 15, 2024 at 05:21:48AM +0000, Song Liu wrote:
> > >>>> Extend test_progs fs_kfuncs to cover different xattr names. Specifically:
> > >>>> xattr name "user.kfuncs", "security.bpf", and "security.bpf.xxx" can be
> > >>>> read from BPF program with kfuncs bpf_get_[file|dentry]_xattr(); while
> > >>>> "security.bpfxxx" and "security.selinux" cannot be read.
> > >>> 
> > >>> So you read code from untrusted user.* xattrs?  How can you carve out
> > >>> that space and not known any pre-existing userspace cod uses kfuncs
> > >>> for it's own purpose?
> > >> 
> > >> I don't quite follow the comment here. 
> > >> 
> > >> Do you mean user.* xattrs are untrusted (any user can set it), so we 
> > >> should not allow BPF programs to read them? Or do you mean xattr 
> > >> name "user.kfuncs" might be taken by some use space?
> > > 
> > > All of the above.
> > 
> > This is a selftest, "user.kfunc" is picked for this test. The kfuncs
> > (bpf_get_[file|dentry]_xattr) can read any user.* xattrs. 
> > 
> > Reading untrusted xattrs from trust BPF LSM program can be useful. 
> > For example, we can sign a binary with private key, and save the
> > signature in the xattr. Then the kernel can verify the signature
> > and the binary matches the public key. If the xattr is modified by
> > untrusted user space, the BPF program will just deny the access. 
> 
> So I tend to agree with Christoph that e.g. for the above LSM usecase you
> mention, using user. xattr space is a poor design choice because you have
> to very carefully validate any xattr contents (anybody can provide
> malicious content) and more importantly as different similar usecases
> proliferate the chances of name collisions and resulting funcionality
> issues increase. It is similar as if you decided to store some information
> in a specially named file in each directory. If you choose special enough
> name, it will likely work but long-term someone is going to break you :)
> 
> I think that getting user.* xattrs from bpf hooks can still be useful for
> introspection and other tasks so I'm not convinced we should revert that
> functionality but maybe it is too easy to misuse? I'm not really decided.

Reading user.* xattr is fine. If an LSM decides to built a security
model around it then imho that's their business and since that happens
in out-of-tree LSM programs: shrug.