[systemd-devel] Environment variables are not sanitized when a graphical session dies

[Arseny Maslennikov]

On Fri, Apr 02, 2021 at 07:29:23PM +0300, Mantas Mikulėnas wrote:
> one answer. (Note that stuff like $DISPLAY gets imported into systemd
> --user's environment, to make it available to the services'
> environment, and you can't make $DISPLAY have two values at once.)

Oh. There's a design problem here as well: if session-specific software
sets environment variables for user services, they are not removed from
the user manager env store when that session terminates.

Let's consider the following scenario:
1. A machine is booted up and is ready to establish user sessions.
2. User U logs in (maybe remotely, or locally on a VT/kmscon) and
   establishes an XDG_SESSION_TYPE=tty session Id=1. A systemd --user
   instance is launched.
3. Later on, user U logs into a graphical session Id=2 at a
   seat on the machine. `systemctl import-environment DISPLAY
   XAUTHORITY', etc. Some time afterwards user U logs out; session 1 is
   intact, as well as the systemd --user instance; the now-defunct
   DISPLAY is not expunged from the systemd --user environment
   dictionary.
4. User U logs in into a graphical session Id=3 again, but with a
   Wayland-based setup, e. g. to work around a bug. `systemctl
   import-environment WAYLAND_DISPLAY', etc. The defunct DISPLAY is
   still present in the environment!


The above was a brainstormed reproducer; I did actually encounter this
kind of bug IRL a couple of times this way.
I'd used GDM to log in to GNOME on Xorg, then logged out, then logged in
to Sway (a Wayland compositor/window manager), and some apps e. g.
telegram-desktop malfunction because they infer they're running in GNOME
from the GNOME-specific environment vars that were not unset on logout
from GNOME:

  % systemctl --user show-environment | grep -i GNOME
  DESKTOP_SESSION=gnome
  GDMSESSION=gnome
  GNOME_DESKTOP_SESSION_ID=this-is-deprecated
  XDG_CURRENT_DESKTOP=GNOME
  XDG_MENU_PREFIX=gnome-
  XDG_SESSION_DESKTOP=gnome
  _=/usr/bin/gnome-session

The _, most likely set by a unix shell, is especially funny. :)

The leaked values of XDG_CURRENT_DESKTOP and XDG_SESSION_DESKTOP also
break xdg-desktop-portal in the new session as well, etc. In short,
this is not robust enough and can break in a multitude of ways, most of
them unknown a priori.

To sum it up, if we use the user manager environment store this way,
there should be a way to automatically remove session-specific
environment vars from systemd --user when the respective session is
terminated.

Possible ways to fix:
* Per-session unit managers: this approach was discussed previously and
  deemed hard to implement; unclear relationships between session units
  and user units.
* Tie the systemd--user environment store entries to session IDs.
  When logind emits org.freedesktop.login1.Manager.SessionRemoved on
  /org/freedesktop/login1, remove environment variables tied to that
  session.
* user-specific environment stores for graphical sessions in logind and
  an opt-in mechanism (e. g. UseSessionEnvironment=yes) in unit files to
  use them, API to modify/flush them (on the bus and via loginctl),
  autoflush when the graphical session dies.
  Not too hard to implement, basically 1 hash table per user.
  A session starter would do
    `loginctl import-environment XDG_CURRENT_DESKTOP'
  instead of
    `systemctl import-environment XDG_CURRENT_DESKTOP'
  If I recall correctly, user managers already requires logind, so
  that's not a problem.

I'm not opposed to writing a patch to fix this or just filing an issue
report, but wanted to discuss the general direction we'd like to take
here first.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/systemd-devel/attachments/20210406/aacce448/attachment-0001.sig>