chiark / gitweb /
logind: fix SetLinger to authorize by client's effective User ID
authorAlan Jenkins <alan.christopher.jenkins@gmail.com>
Fri, 15 Sep 2017 16:35:02 +0000 (17:35 +0100)
committerSven Eden <yamakuzure@gmx.net>
Fri, 15 Sep 2017 16:35:02 +0000 (17:35 +0100)
SetLinger is authorized by the PolicyKit action "set-self-linger", if it is
not passed an explicit UID.

According to comments we were determining the default UID from the client's
session.  However, user processes e.g. which are run from a terminal
emulator do not necessarily belong to a session scope unit.  They may
equally be started from the elogind user manager [1][2].  Actually the
comment was wrong, and it would also have worked for processes
started from the elogind user manager.

Nevertheless it seems to involve fetching "augmented credentials" i.e.
it's using a racy method, so we shouldn't have been authenticating based
on it.

We could change the default UID, but that raises issues especially for
consistency between the methods.  Instead we can just use the clients
effective UID for authorization.

This commit also fixes `loginctl enable-linger $USER` to match the docs
that say it was equivalent to `loginctl enable-linger` (given that $USER
matches the callers user and owner_uid).  Previously, the former would not
have suceeded for unpriviliged users in the default configuration.

[1] It seems the main meaning of per-session scopes is tracking the PAM
login process.  Killing that provokes logind to revoke device access.  Less
circularly, killing it provokes getty to hangup the TTY.

[2] User units may be started with an environment which includes
XDG_SESSION_ID (presuambly GNOME does this?).  Or not.

man/loginctl.xml
src/login/logind-dbus.c

index 39534f1159b1888e16e888168c97e78ee63ad534..4d3f701245a86e361bc1c6ac105caafa85503106 100644 (file)
         one or more logged in users, followed by the most recent log
         data from the journal. Takes one or more user names or numeric
         user IDs as parameters. If no parameters are passed, the status
-        of the caller's user is shown. This function is intended to
-        generate human-readable output. If you are looking for
-        computer-parsable output, use <command>show-user</command>
-        instead. Users may be specified by their usernames or numeric
-        user IDs. </para></listitem>
+        is shown for the user of the session of the caller. This
+        function is intended to generate human-readable output. If you
+        are looking for computer-parsable output, use
+        <command>show-user</command> instead.</para></listitem>
       </varlistentry>
 
       <varlistentry>
index 6e42b2b07b9c39d3f33dbbf706e694dbb90e2392..2badab1d3ee8b3c6f71795910d6e277fb9f5248a 100644 (file)
@@ -1131,13 +1131,13 @@ static int method_terminate_seat(sd_bus_message *message, void *userdata, sd_bus
 }
 
 static int method_set_user_linger(sd_bus_message *message, void *userdata, sd_bus_error *error) {
+        _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
         _cleanup_free_ char *cc = NULL;
         Manager *m = userdata;
         int r, b, interactive;
         struct passwd *pw;
         const char *path;
-        uint32_t uid;
-        bool self = false;
+        uint32_t uid, auth_uid;
 
         assert(message);
         assert(m);
@@ -1146,22 +1146,23 @@ static int method_set_user_linger(sd_bus_message *message, void *userdata, sd_bu
         if (r < 0)
                 return r;
 
-        if (!uid_is_valid(uid)) {
-                _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
-
-                /* Note that we get the owner UID of the session, not the actual client UID here! */
-                r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_OWNER_UID|SD_BUS_CREDS_AUGMENT, &creds);
-                if (r < 0)
-                        return r;
+        r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID |
+                                               SD_BUS_CREDS_OWNER_UID|SD_BUS_CREDS_AUGMENT, &creds);
+        if (r < 0)
+                return r;
 
+        if (!uid_is_valid(uid)) {
+                /* Note that we get the owner UID of the session or user unit,
+                 * not the actual client UID here! */
                 r = sd_bus_creds_get_owner_uid(creds, &uid);
                 if (r < 0)
                         return r;
+        }
 
-                self = true;
-
-        } else if (!uid_is_valid(uid))
-                return -EINVAL;
+        /* owner_uid is racy, so for authorization we must use euid */
+        r = sd_bus_creds_get_euid(creds, &auth_uid);
+        if (r < 0)
+                return r;
 
         errno = 0;
         pw = getpwuid(uid);
@@ -1171,7 +1172,8 @@ static int method_set_user_linger(sd_bus_message *message, void *userdata, sd_bu
         r = bus_verify_polkit_async(
                         message,
                         CAP_SYS_ADMIN,
-                        self ? "org.freedesktop.login1.set-self-linger" : "org.freedesktop.login1.set-user-linger",
+                        uid == auth_uid ? "org.freedesktop.login1.set-self-linger" :
+                                          "org.freedesktop.login1.set-user-linger",
                         NULL,
                         interactive,
                         UID_INVALID,