We are working on making the desktop session work correctly with "systemd --user" as user service manager. This makes one major change: most user code will not run inside the logind session anymore, but will be forked off the per-user singleton service user@.service. This currently confuses Polkit quite a bit, since it wants to make security decisions based on whether processes are in foreground sessions or not, but this way most user processes won't be in any session at all, neither foreground nor background. I'd like to see polkit changed to no longer check whether a process is in an fg session or not, but instead whether a process is owned by a user that has at least one fg session. This is more in-line with traditional UNIX access checks anyway, since on UNIX access is granted to users, never to specific sessions. Moreover, trying to lock down specific sessions is easily circumventable anyway... So, long story short: currently polkit uses sd_pid_get_session() and then sd_session_is_active(), to make its decisions. Instead, please use sd_uid_get_state() and check whether the user is "active" or just "online". See sd_uid_get_state(3) for details. This should actually make the code of polkit quite a bit simpler, too.
This is still a nice-to-have after https://bugs.freedesktop.org/show_bug.cgi?id=78905 right? But not strictly necessary since we're falling back to looking at the primary session?
See also Bug #67728.
Created attachment 116248 [details] [review] sessionmonitor-systemd: Use sd_uid_get_state() to check session activity Instead of using sd_pid_get_session() then sd_session_is_active() to determine whether the user is active, use sd_uid_get_state() directly. This gets the maximum of the states of all the user’s sessions, rather than the state of the session containing the subject process. Since the user is the security boundary, this is fine. This change is necessary for `systemd --user` sessions, where most user code will be forked off user@.service, rather than running inside the logind session (whether that be a foreground/active or background/online session). Policy-wise, the change is from checking whether the subject process is in an active session; to checking whether the subject process is owned by a user with at least one active session.
Comment on attachment 116248 [details] [review] sessionmonitor-systemd: Use sd_uid_get_state() to check session activity Review of attachment 116248 [details] [review]: ----------------------------------------------------------------- Two minor things. Want to trade for review of http://lists.freedesktop.org/archives/polkit-devel/2015-May/000421.html ? I can file as a bug if it's easier. ::: src/polkitbackend/polkitbackendsessionmonitor-systemd.c @@ +399,5 @@ > + g_debug ("Checking whether session %s is active.", session_id); > + > + /* Check whether *any* of the user's current sessions are active. */ > + if (sd_session_get_uid (session_id, &uid) < 0) > + goto fallback; polkit is GNU, 2 space indents please. @@ +404,5 @@ > + > + g_debug ("Session %s has UID %u.", session_id, uid); > + > + if (sd_uid_get_state (uid, &state) < 0) > + goto fallback; In this case we leak `state`
Created attachment 116266 [details] [review] sessionmonitor-systemd: Use sd_uid_get_state() to check session activity Instead of using sd_pid_get_session() then sd_session_is_active() to determine whether the user is active, use sd_uid_get_state() directly. This gets the maximum of the states of all the user’s sessions, rather than the state of the session containing the subject process. Since the user is the security boundary, this is fine. This change is necessary for `systemd --user` sessions, where most user code will be forked off user@.service, rather than running inside the logind session (whether that be a foreground/active or background/online session). Policy-wise, the change is from checking whether the subject process is in an active session; to checking whether the subject process is owned by a user with at least one active session. --- Updated patch to fix the style issue. > @@ +404,5 @@ > > + > > + g_debug ("Session %s has UID %u.", session_id, uid); > > + > > + if (sd_uid_get_state (uid, &state) < 0) > > + goto fallback; > > In this case we leak `state` Nope, sd_uid_get_state() does not return a state on failure (returning < 0).
Merged, http://cgit.freedesktop.org/polkit/commit/?id=a29653ffa99e0809e15aa34afcd7b2df8593871c
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.