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

[Benjamin Berg]

Hi,

On Tue, 2021-04-06 at 19:02 +0300, Arseny Maslennikov wrote:
> [SNIP]
> 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. :)

Yes, this is a problem. The _ is really not great, we could add it to
the ignore list.

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

So, what I had been thinking to do in GNOME at one point (but never
did), is to write a file in $XDG_RUNTIME_DIR of all variables that were
set by GNOME (possibly containing the old values).

Then, at logout, unset these variables (or reset to old values). In
GNOME, everything is going through the gnome-session binary, and it
would be quite simple to implement this in a safe manner.

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

Hmm, what I think would be helpful is a mechanism to restart services
after the session has adjusted the environment. Otherwise these
services will not pick up changes in the environment; usually not a big
deal, but it can for example cause issues when the user changes their
language and e.g. pulseaudio/pipewire are not restarted even though the
user has logged out for a short time.

Maybe we could have a feature in systemd that causes services to
restart when certain variables in their environment block have changed?

Benjamin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/systemd-devel/attachments/20210415/deadc867/attachment.sig>