Bug 215202 - Linuxulator: LOCAL_PEERCRED / xucred does not have process ID
Summary: Linuxulator: LOCAL_PEERCRED / xucred does not have process ID
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Dmitry Chagin
Depends on:
Blocks: 247219
  Show dependency treegraph
Reported: 2016-12-10 22:15 UTC by Greg V
Modified: 2020-12-23 19:03 UTC (History)
5 users (show)

See Also:
jbeich: mfc-stable12?

peercred.patch (2.17 KB, patch)
2017-11-25 11:51 UTC, Greg V
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]

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 freebsd_committer 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 freebsd_committer 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

  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

Comment 5 commit-hook freebsd_committer 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

  Use C11 anonymous unions.

  PR:		215202
  Reported by:	glebius
  MFC after:	2 weeks

Comment 6 Ed Maste freebsd_committer 2020-07-27 19:02:47 UTC
Anything needed here (other than perhaps MFC)?
Comment 7 Greg V 2020-07-28 01:11:02 UTC
(In reply to Ed Maste from comment #6)

Nah, it's solved. Actually back when I first looked at the relevant commits I somehow got to the conclusion that it was special cased for Linuxulator only. But more recently I've discovered that this has been actually properly available to FreeBSD binaries :)

I guess MFC could be nice but I personally don't really care about STABLE/releases.
Comment 8 Jan Beich freebsd_committer 2020-12-23 19:03:23 UTC
I'd prefer MFC to /stable/12 to make porting easier: /stable/11 will reach EOL by the time 12.3 is released. Currently, sysutils/basu, sysutils/seatd, graphics/wayland, lang/rust have to return cr_pid = -1 as a workaround.