Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shouldn't add XDG_SESSION_ID to dbus and systemd activation environment #271

Open
smcv opened this issue Mar 30, 2021 · 5 comments · May be fixed by #282
Open

Shouldn't add XDG_SESSION_ID to dbus and systemd activation environment #271

smcv opened this issue Mar 30, 2021 · 5 comments · May be fixed by #282

Comments

@smcv
Copy link

smcv commented Mar 30, 2021

Steps to reproduce the behaviour

  • Install a Debian 11 system with gdm 3.38, GNOME 3.38, MATE 1.24.x and systemd
  • Log in via ssh (for example assume systemd-logind assigns login session ID 2 for this)
  • Log in to MATE, as the same user (for example assume systemd-logind assigns login session ID 4 for this)
  • Log out from MATE
  • Log in to GNOME, as the same user, without rebooting (for example assume systemd-logind assigns login session ID 6 for this)
  • Lock the screen
  • Try to unlock the screen

Expected behaviour

  • When I log in to MATE, mate-session-manager uploads some environment variables to dbus-daemon --session via UpdateActivationEnvironment, and either mate-session-manager itself or dbus-daemon passes on those environment variables to systemd --user via SetEnvironment
  • Because the systemd --user and dbus-daemon --session processes are shared between multiple login sessions, these environment variables should not include things that are highly specific to one login session, and in particular XDG_SEAT, XDG_SESSION_ID and XDG_VTNR
  • Processes run by systemd --user, including most of GNOME, do not inherit XDG_SESSION_ID
  • In particular, GNOME Shell does not inherit XDG_SESSION_ID, correctly determines its own systemd-logind session ID (in my example, session ID 6) by querying systemd-logind, and can lock and unlock correctly

Actual behaviour

  • mate-session-manager sends all environment variables to systemd --user, including XDG_SESSION_ID=4 in my example
  • Because the ssh login session keeps systemd --user alive, XDG_SESSION_ID=4 stays in the activation environment and is "leaked" into the subsequent GNOME session
  • GNOME Shell inherits XDG_SESSION_ID=4 from systemd --user, even though that login session has already ended
  • This makes GNOME Shell try to communicate with a logind session that no longer exists, breaking the ability to unlock the screen
    • I contributed a change to gnome-session to work around this, but I don't think it's really a GNOME bug

MATE general version

1.24.1 (not tested with 1.24.2, but none of the changes since 1.24.1 seem like they would affect this)

Package version

1.24.1-1

Linux Distribution

Debian 11

Link to bugreport of your Distribution (requirement)


linuxmint/cinnamon-session#141 (comment) lists some other related fixes in the ancestor project gnome-session. It would be a good idea to incorporate those into mate-session-manager at the same time.

Related bug reports:

@smcv
Copy link
Author

smcv commented Mar 30, 2021

A minimal solution is to apply the equivalent of GNOME/gnome-session@646b9bc to mate-session-manager, but all the commits mentioned in linuxmint/cinnamon-session#141 (comment) touch the same file and would be good fixes to have.

sunweaver added a commit to sunweaver/mate-session-manager that referenced this issue May 17, 2021
Things like XDG_SESSION_ID should not be uploaded to the environment.
For example this is broken currently:

  1. SSH to your machine
  2. Log in to MATE Shell
  3. Log out
  4. Log in again
  5. Lock the screen
  6. Try to unlock

You can't, and this is because the XDG_SESSION_ID from the first session
(step 2) has leaked through to the second one (step 4), and so MATE
Shell is listening to the `logind` `UnlockSession` signal for the wrong
session. The SSH session established in step 1 serves to keep the
`systemd --user` instance alive, so that the state is not torn down
between logins.

Original patch for GNOME by Iain Lane <[email protected]>.

Patch ported over to MATE's session manager by Mike Gabriel
<[email protected]>.

Fixes mate-desktop#271
@sunweaver sunweaver linked a pull request May 17, 2021 that will close this issue
@L-U-T-i
Copy link

L-U-T-i commented May 17, 2021

@smcv
based on your posts, I've made a test build of the latest git commit of mate-session-manager 1.25 with the following patches applied (in an order as listed here):

1 - Remove some session-specific environment variables from activation environment:
mate-session-manager-1.25-271-fix-1.patch.txt

2 - Fix environment variables patterns that are accepted:
mate-session-manager-1.25-271-fix-2.patch.txt

3 - Never try to autostart systemd:
mate-session-manager-1.25-271-fix-3.patch.txt

4 - Continue if a client exits during EndSession signal:
mate-session-manager-1.25-continue-if-a-client-exits-during-EndSession-signal.patch.txt

5 - Log a critical when our SIGTERM/SIGINT handler fails to log out:
mate-session-manager-1.25-log-a-critical-when-our-SIGTERM-SIGINT-handler-fails-to-log-out.patch.txt

6 - Stop dbus-daemon instead of restarting it:
mate-session-manager-1.25-stop-dbus-daemon-instead-of-restarting-it.patch.txt

7 - Warn about failures to update the environment:
mate-session-manager-1.25-warn-about-failures-to-update-the-environment.patch.txt

8 - Log variables excluded from environment upload:
mate-session-manager-1.25-log-variables-excluded-from-environment-upload.patch.txt

9 - Create the config directory with 0700:
mate-session-manager-1.25-create-the-config-directory-with-0700.patch.txt

10 - Fix typos in DBus documentation:
mate-session-manager-1.25-fix-typos-in-DBus-documentation.patch.txt

11 - Mention that session saving does not work on systemd:
mate-session-manager-1.25-mention-that-session-saving-does-not-work-on-systemd.patch.txt

12 - Find user's graphical session, not the current pid's session:
mate-session-manager-1.25-gsm-systemd-1.patch.txt

All those patches are ported from gnome-session git commits (and, modified as needed, in some rare cases).

There are more details about each patch within the patch file itself (copied from gnome-session commit descriptions).

I haven't noticed any issues by now (built, installed and launched without any issues...). As I've noticed sessions were not always properly saved / restored in my Mate Desktop (some rare cases, but still...), I've decided to give all this a try. I know some of those patches (a majority of them, in fact ;-)) are not related to this issue at all, but considering I went through all recent git commits of gnome-session, I've decided to simply pull anything that seemd to me appropriate for mate. I am publishing all that here, in case anyone is interested for any part of that...

@smcv
Copy link
Author

smcv commented May 17, 2021

I'm not a MATE developer or user, so someone from MATE should review those changes, but I think the general principle of sharing non-GNOME-specific fixes between GNOME and MATE is a good one.

@raveit65
Copy link
Member

Pull request please.

@sunweaver
Copy link
Member

@smcv @raveit65 @L-U-T-i I have already filed a PR for the minimal required changes here: #282

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants