|Summary:||Linuxulator: LOCAL_PEERCRED / xucred does not have process ID|
|Product:||Base System||Reporter:||Greg V <greg>|
|Component:||kern||Assignee:||Dmitry Chagin <dchagin>|
|Severity:||Affects Some People||CC:||dchagin, hselasky, jilles|
Description Greg V 2016-12-10 22:15:44 UTC
Applications like the 'sway' Wayland compositor rely on getting the socket peer's pid for IPC security. It would be nice to get FreeBSD to report the pid, not just uid and gids, through the LOCAL_PEERCRED option. From the commit that fixed bug #102956 (SO_PEERCRED in Linux emulation): "Temporarily use 0 for pid member as the FreeBSD does not cache remote UNIX domain socket peer pid" So adding pid to LOCAL_PEERCRED would allow a simple improvement to Linux compatibility as well.
Comment 1 Greg V 2017-11-25 11:51:38 UTC
Created attachment 188271 [details] peercred.patch Well, that was easy. Here's a tiny patch that fixes the problem. Should XUCRED_VERSION be raised? Adding an extra field at the end of the struct doesn't break backwards compatibility…
Comment 2 Jilles Tjoelker 2017-11-29 22:58:58 UTC
The pid in LOCAL_PEERCRED is not a security feature but only a feature to enforce that a proper security feature can be added later. This is because there is nothing to enforce that the pid refers to the same process or that it has not executed some other binary; also, applications are not isolated in a way that makes different Wayland privileges useful for security. Adding pid to struct ucred seems wrong since it is shared between processes that do not change their credentials. This would make more sense as a property of the socket. The pid would then be the pid of the process that called listen() or connect().
Comment 3 Greg V 2017-12-09 17:02:53 UTC
(In reply to Jilles Tjoelker from comment #2) sway just uses the pid to read /proc/PID/file and check the file against a list of allowed files. The pid is in ucred on Linux, I think it makes sense to match what they do.
Comment 4 commit-hook 2019-05-30 14:24:45 UTC
A commit references this bug: Author: dchagin Date: Thu May 30 14:24:28 UTC 2019 New revision: 348419 URL: https://svnweb.freebsd.org/changeset/base/348419 Log: Complete LOCAL_PEERCRED support. Cache pid of the remote process in the struct xucred. Do not bump XUCRED_VERSION as struct layout is not changed. PR: 215202 Reviewed by: tijl MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D20415 Changes: head/crypto/heimdal/lib/ipc/server.c head/share/man/man4/unix.4 head/sys/compat/linux/linux_socket.c head/sys/kern/kern_prot.c head/sys/kern/uipc_usrreq.c head/sys/sys/ucred.h head/usr.sbin/mountd/mountd.c
Comment 5 commit-hook 2019-06-10 05:28:38 UTC
A commit references this bug: Author: dchagin Date: Mon Jun 10 05:28:04 UTC 2019 New revision: 348847 URL: https://svnweb.freebsd.org/changeset/base/348847 Log: Use C11 anonymous unions. PR: 215202 Reported by: glebius MFC after: 2 weeks Changes: head/sys/sys/ucred.h