fix: Systray icon consider restricted privileges of scheduled root-mode or foreign-user backup profiles

[buhtz]

...in progress...

The systray icon do use two technics to determine the name of that user owning/running the current desktop session. First loginctl (systemd) is used. If not not present who and DISPLAY are used.

What is still missing: Disabling context menu entries if needed.

Fix #2237

[samo-sk]

I am not sure if this is correct. Regarding _get_desktop_user_via_loginctl, the active session is not necessarily the session that corresponds to DISPLAY; this is the case if there are multiple sessions running (i.e. multiple X sessions, or a virtual terminal session and an X session).

As for _get_desktop_user_via_x11_who, this does not work if the X server is not launched by a login program (such as sddm), e.g. if the server was launched from shell using startx, or if the server is XWayland.

---

Also, what puzzles me about the fallback display :0.0 is that DISPLAY is not sufficient to launch an X client; XAUTHORITY is also needed. Some programs seem to use ~/.Xauthority if XAUTHORITY is not set. However, if I inspect a systray-icon process (root profile, schedule "Every 5 minutes"), it does have DISPLAY and XAUTHORITY envvars set. I don't know what sets them, but this could mean that the hardcoded fallback may actually never be used. It may be worth investigating what actually sets these envvars.

[samo-sk]

Update: ~/.local/share/backintime/cron_env seems to set DISPLAY and XAUTHORITY variables for scheduled jobs.

Weirdly, this file is only written from Password_Cache.run(), and the only relevant place it is invoked from (during normal operation) is Mount.__init__() (and this happens only when snapshots.<Mode>.password.use_cache is true in the config). This is particularly weird, because none of the envvars saved are directly related to mounting.

(This is true for the dev branch; I haven't investigated the main branch.)

[buhtz]

Puh... This gives me a headache. It is far beyond my skills and expertise. :crying_cat_face:

[samo-sk]

loginctl list-sessions should be able to determine the display of an X login session.(1) Instead of checking if a session is active (as the draft code does now), I would check if the session's Display= from loginctl equals the DISPLAY environment variable.

Maybe it would be easier to only handle ordinary X login sessions. This way, the mechanism using who can stay. Users who use XWayland or run X from a virtual terminal would need to authenticate always, even on their own jobs.

(Actually, the icon doesn't reliably work for me on XWayland on KDE Plasma, because XAUTHORITY changes on every login. That is another reason why I wouldn't try to support XWayland right now.)

[samo-sk]

I wrote:

Instead of checking if a session is active (as the draft code does now), I would check if the session's Display= from loginctl equals the DISPLAY environment variable.

This is how I imagine my version of _get_desktop_user_via_loginctl:

    def _get_desktop_user_via_loginctl(self) -> str | None:
        """Get name of user logged in to the current desktop session using
        loginctl.
        """

        try:
            # get list of sessions
            output = subprocess.check_output(
                ['loginctl', 'list-sessions', '--no-legend', '--json=short'],
                text=True)
        except FileNotFoundError:
            logging.warning('Can not determine user name of current desktop '
                            'session because "loginctl" is not available.')
            return None
        except Exception as exc:
            logging.error('Unexpected error while determining user name of '
                          f'current desktop session: {exc}')
            return None

        # Check each session
        for session in json.loads(output):
            # Ignore none-user sessions
            if session.get('class') != 'user':
                continue

            # properties of the session
            info = subprocess.check_output(
                ['loginctl', 'show-session', str(session['session']),
                '--property=Display', '--property=Name', '--property=Seat'],
                text=True
            ).strip()

            props = dict(line.split('=') for line in info.splitlines())

            # Does the display match?
            if props.get('Display', '') == os.environ.get('DISPLAY', ':0').split('.')[0]:
                # if props['Seat'] == 'seat0':
                logger.info(f'VARIANT - systemd loginctl: {props=}', self)
                return props['Name']

        return None

Like I have mentioned, this doesn't work with XWayland or X manually started from a VT. I have tested this code only very superficially.

[buhtz]

Instead of checking if a session is active (as the draft code does now), I would check if the session's Display= from loginctl equals the DISPLAY environment variable.

I am insecure about both solutions. What do you think about using your solution as first, and in case DISPLAY environment variable is not set using my solution?

We can not be sure today. We have to bring this into the wild and see.

EDIT:

To be honest: I’m groping in the dark and making educated guesses. I’m not an expert here. But I also wonder if I can make it more worse than before. :smile:

I tried to integrate your solution in the current draft. I need to do more tests with it. Because of the security implications I try to be sure about this desktop-user-determination-thing. In unclear situations the code will give None to prevent false positives.

This are the steps the code currently does.

1. Try it with loginctl
   1. If DISPLAY is set, determine the Display of the session. (Your approach?)
   2. Without DISPLAY find an active single(!) session. (My approach.) If multiple sessions are found don't guess. Give None.
2. If no user found (via loginctl) but DISPLAY is set try via who (X11).

[samo-sk]

Looks good to me.

---

I would also suggest another solution that could co-exist with the solution from this PR. However, I do not think it should be implemented now; I would only implement it once IPC mechanisms are fully implemented in BiT.

In this solution, there would be an alternative version of the systray-icon script. The user will configure their desktop environment to launch this script when the DE is launched. The script would wait for a signal from BiT. Once it receives the signal, it will show the icon. Because this script would be launched with all necessary envvars, this solution won't need to guess DISPLAY or the session owner, so it will be more reliable.

[buhtz]

Hello Samo, thanks for the feedback. I will merge the PR and we will see.

I was also thinking about that IPC stuff (see #2260).

I understand the basic concept but never used or implemented something like this before. So I did some research about the possible technologies. The plan is to make a prototyp and play around with that stuff to get to a final decision. Currently I vote for Unix Domain Sockets as a compromise between maintainability and flexibility. I was also looking into DBUS and FIFO-sockets (named pipes). DBUS is to complex, hard to understand and maintain. FIFO is to simple/basic and unflexible.

Regards, Christian