WIP toplevel_info: PID and security context

[ids1024]

The xdg-desktop-portal-gnome background portal uses the sandbox_app_id from the org.gnome.Shell.Introspect protocol to get the app ID of flatpak/snap apps.

Mutter implements this by getting the PID of the Wayland or X11 client, then accessing /proc/$PID/root/.flatpak-info. (Or something else for Snaps.)

If we want to do the same, it's probably easier to have the compositor expose the PID (which may also be useful for other purposes), and then let our portal do any further lookups.

For Wayland apps, security-context-v1 is probably a more robust way to get the sandboxing information, but presumably we still need a PID-based approach for X11 clients. So it would be good to expose that too.

Pid of course is potentially racy. But a pidfd would still be racy in the compositor, since we'd have to create it from a PID, and it wouldn't work on non-Linux platforms. Races probably aren't a problem if the client immediately looks up the process after the compositor adversities the window, and the compositor will remove windows immediately when the client is gone (assuming the process that created the socket is the only one with it open).

We should probably also have a field to indicate that the client uses X11. Maybe an xid field. Though like PID, this is potentially racy.

[ids1024]

PID may also be useful if we want anything to provide an option to force kill an app (or is there a better way to do that?). Not sure what other uses there are for it. I think we talked about wanting PID here for something.

Another thing I'd like to add to toplevel_info somehow is https://wayland.app/protocols/xdg-toplevel-icon-v1, but I don't really know how that would work... seems a bit of a pain to pass through to client shell components. I guess for X11 clients, the XID would allow a shell component to get that sort of thing.

[Consolatis]

Just some random mumbling, feel free to ignore.

PID may also be useful if we want anything to provide an option to force kill an app (or is there a better way to do that?)

IMHO this should be a compositor feature rather than a desktop shell feature. Something like if application didn't react for X seconds to a xdg/xwayland-ping request following a close request: ask the user to force-quit the application at which point the compositor can either disconnect the client or actually kill it via PID from its socket connection peer credentials.

Another note: linux pid namespaces might make the PID irrelevant depending on what component runs in which pid namespace.

Another thing I'd like to add to toplevel_info somehow is https://wayland.app/protocols/xdg-toplevel-icon-v1, but I don't really know how that would work... seems a bit of a pain to pass through to client shell components.

Yes, some kind of solution would be nice for this, also upstream. Its relatively trivial if the client just sends an icon name but if it sends a bunch of wl_buffers it gets a bit annoying to deal with, both client and compositor side.

Edit:
Maybe a compromise here would be to just send the icon name, either if received via the xdg-toplevel-icon protocol or based on the app-id (if the compositor already implements a icon theme lookup based on app-id / WM_CLASS).

[ids1024]

IMHO this should be a compositor feature rather than a desktop shell feature. Something like if application didn't react for X seconds to a xdg/xwayland-ping request following a close request: ask the user to force-quit the application at which point the compositor can either disconnect the client or actually kill it via PID from its socket connection peer credentials.

Yeah, if we wanted to prompt to force quit an app based on response to pings, that will need more cooperation from the compositor.

Another note: linux pid namespaces might make the PID irrelevant depending on what component runs in which pid namespace.

If xdg-desktop-portal-cosmic, cosmic-comp, and XWayland are running without any special sandboxing, then PIDs should match between them, and using the PID to deal with sandboxed apps works fine (though it will be mapped to a different PID within the sandbox.)

But yeah, if any of those things were in a separate PID namespace, this wouldn't work. (I wonder if we could do anything to sandbox XWayland.)

Maybe a compromise here would be to just send the icon name, either if received via the xdg-toplevel-icon protocol or based on the app-id (if the compositor already implements a icon theme lookup based on app-id / WM_CLASS).

That could work in some cases. Though presumably not for all apps. (Particularly sandboxed apps probably can use icons for toplevels that may not be installed at a path the compositor will search?)

We can always fallback to using the icon based on app_id, but this may not provide the best experience for some software.

[Drakulix]

Another thing I'd like to add to toplevel_info somehow is https://wayland.app/protocols/xdg-toplevel-icon-v1, but I don't really know how that would work... seems a bit of a pain to pass through to client shell components. I guess for X11 clients, the XID would allow a shell component to get that sort of thing.

I wonder if the following could work:

• Add an icon_name thingy to cosmic-toplevel
• If we get buffers, write that out into icons files somewhere in /run from cosmic-comp - resembling a typical hicolor icon theme structure.
• Add that folder to XDG_DATA_DIRS - this will be a distro/packaing thing or we modify the env-variable from cosmic-comp/cosmic-session (but I kinda like the former better)
• Either communicate the provided icon-name via cosmic-toplevel or the name of our temporary file.

That would then mean that every non-sandboxed app (sandboxed apps shouldn't have access to cosmic-toplevel anyway and if we ever do sandboxed applets, we can just map the folder into the sandbox) can lookup icons as it is used to.

[Drakulix]

IMHO this should be a compositor feature rather than a desktop shell feature. Something like if application didn't react for X seconds to a xdg/xwayland-ping request following a close request: ask the user to force-quit the application at which point the compositor can either disconnect the client or actually kill it via PID from its socket connection peer credentials.

Yeah, if we wanted to prompt to force quit an app based on response to pings, that will need more cooperation from the compositor.

We could also just expose a ping-timeout or ping-restored event in cosmic-toplevel as well to provide an external daemon with the necessary information.

We should probably also have a field to indicate that the client uses X11. Maybe an xid field. Though like PID, this is potentially racy.

Please lets not do an XID, I already see that getting misused. What do we want to use this for?

[Drakulix]

Looks fine apart from missing descriptions

[Drakulix]

Suggested change

	<arg name="sandbox_engine", type="string" allow-null="true"/>
	<arg name="app_id", type="string" allow-null="true"/>
	<arg name="instance_id", type="string" allow-null="true"/>
	<arg name="sandbox_engine" type="string" allow-null="true"/>
	<arg name="app_id" type="string" allow-null="true"/>
	<arg name="instance_id" type="string" allow-null="true"/>

[Drakulix]

Pid of course is potentially racy. But a pidfd would still be racy in the compositor, since we'd have to create it from a PID, and it wouldn't work on non-Linux platforms. Races probably aren't a problem if the client immediately looks up the process after the compositor adversities the window, and the compositor will remove windows immediately when the client is gone (assuming the process that created the socket is the only one with it open).

As of 6.5 SO_PEERPIDFD exists. I wonder if we should just only expose this with the race-free PIDFDs and nothing else. A client can still convert that into a racy PID, if it needs to do so for older apis, but we could lay the foundation here to be able to get rid of any raciness further down the line.

[ids1024]

So if we use SO_PEERPIDFD, we'd just not have a PID on older kernel versions and non-Linux platforms.

Hopefully anything using COSMIC will have a recent kernel, and BSDs don't currently have Flatpak anyway, so we don't need the PID for that purpose.

I guess cctk can store it as an Arc<OwnedFd> to avoid needing to dup to copy from pending_info to current_info, or anywhere else the application needs the fd. This does mean cctk toplevlel info clients having an additional 1 fd per toplevel, even if they don't actually use them. Probably not an issue as long as where not hitting fd limits, which shouldn't happen...

[Drakulix]

Hopefully anything using COSMIC will have a recent kernel, and BSDs don't currently have Flatpak anyway, so we don't need the PID for that purpose.

Exactly and we can add logic to fallback to other options, if no fd is provided by the compositor. (Looking at e.g. the background portal, which is somewhat functional without it.)

Probably not an issue as long as where not hitting fd limits, which shouldn't happen...

If we hit them, we should just increase the limit. FDs are cheap in recent kernel versions, the folks behind systemd have put a lot of work into making sure they can be used for everything. The only reason for the conservative limit is select being unable to handle higher limits and the commitment to not break old user-space.

[ids1024]

Yeah, using FDs is really the right way to do things like this.

[ids1024]

https://www.corsix.org/content/what-is-a-pidfd

pidfd_send_signal
This function also accepts the result of open("/proc/$pid") as an fd, though it is the only function to do so: open("/proc/$pid") does not give a pidfd, and no other functions accept the result of open("/proc/$pid") in place of a pidfd.

Gotta love that consistency. It would be nice if a pidfd was just the same thing as a proc directory fd...

So as far as I can tell, there's no direct way to convert a pidfd to a /proc/$pid directory. The best I can think of is:

• Use pidfd_getpid (or equivalent pure Rust code, parsing proc/self/fdinfo/$pidfd) to get the PID from the pidfd
• Open /proc/$pid
• Do a non-blocking poll on our pidfd. If it polls as readable, the process is terminated. If not, the process is still running, and presumably our directory must match the PID. Preventing a race.

So I think we can avoid the race, but it still would be an issue to be in a different PID namespace than the compositor. But of course, it makes sense that a process in a PID namespace couldn't access this sort of information about a process outside it.

I guess instead of the poll, something to check that it matches would at least notice the namspace issue, if that somehow happened. I guess calling pidfd_open then comparing stx_ino.

We might actually be able to, and want to, sandbox the portal daemon with systemd sandboxing if we switch to socket activation, but presumably that won't use a PID namespace, or has a way to disable that. We don't expose toplevel-info to Flatpaks regardless.

If we wanted to iterate over the directory multiple times, after opening the directory fd, the standard library doesn't have anything for that, but cap-std has a Dir.

Pidfd seems doable, but a bit more annoying to deal with than I might wish...

[Consolatis]

We could also just expose a ping-timeout or ping-restored event in cosmic-toplevel as well to provide an external daemon with the necessary information.

Just for the record, related work regarding the ping timeout: https://gitlab.freedesktop.org/wlroots/wlr-protocols/-/merge_requests/127.