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@xxxxxxxxxxxxx> 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.
