jsauthority: parse groups from GetConnectionCredentials() if available

D-Bus will give us supplementary groups too, using SO_PEERSEC which is
secure and safe against races, so prefer that to parsing the group from
the uid. This is available when using dbus-broker.

Fixes https://gitlab.freedesktop.org/polkit/polkit/-/issues/165

5 files changed, 293 insertions, 63 deletions

diff --git a/src/polkit/polkitsystembusname.c b/src/polkit/polkitsystembusname.c
index 2fbf5f1..9c96747 100644
--- a/src/polkit/polkitsystembusname.c
+++ b/src/polkit/polkitsystembusname.c
@@ -390,26 +390,19 @@ on_retrieved_unix_uid_pid (GObject              *src,
 }
 
 static gboolean
-polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus_name,
+polkit_system_bus_name_get_creds_fallback (PolkitSystemBusName           *system_bus_name,
 				       guint32                       *out_uid,
 				       guint32                       *out_pid,
 				       GCancellable                  *cancellable,
+				       GDBusConnection               *connection,
+				       GMainContext                  *tmp_context,
 				       GError                       **error)
 {
   gboolean ret = FALSE;
-  AsyncGetBusNameCredsData data = { 0, };
-  GDBusConnection *connection = NULL;
-  GMainContext *tmp_context = NULL;
-
-  connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, cancellable, error);
-  if (connection == NULL)
-    goto out;
+  AsyncGetBusNameCredsData data = { };
 
   data.error = error;
 
-  tmp_context = g_main_context_new ();
-  g_main_context_push_thread_default (tmp_context);
-
   dbus_call_respond_fails = 0;
 
   /* Do two async calls as it's basically as fast as one sync call.
@@ -481,6 +474,112 @@ polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus
     }
   if (connection != NULL)
     g_object_unref (connection);
+
+  return ret;
+}
+
+static gboolean
+polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus_name,
+				       guint32                       *out_uid,
+				       GArray                       **out_gids,
+				       guint32                       *out_pid,
+				       GCancellable                  *cancellable,
+				       GError                       **error)
+{
+  gboolean ret = FALSE;
+  GDBusConnection *connection = NULL;
+  GMainContext *tmp_context = NULL;
+  GVariantIter *iter;
+  GVariant *result, *value;
+  GError *dbus_error = NULL;
+  const gchar *key;
+  guint32 uid = G_MAXUINT32, pid = 0;
+  GArray *gids = NULL;
+
+  connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, cancellable, error);
+  if (connection == NULL)
+    goto out;
+
+  tmp_context = g_main_context_new ();
+  g_main_context_push_thread_default (tmp_context);
+
+  /* If the new unified API is available (since dbus-daemon 1.10.4) use it,
+   * or fallback to the old separate calls.
+   */
+  result = g_dbus_connection_call_sync (connection,
+			  "org.freedesktop.DBus",       /* name */
+			  "/org/freedesktop/DBus",      /* object path */
+			  "org.freedesktop.DBus",       /* interface name */
+			  "GetConnectionCredentials",   /* method */
+			  g_variant_new ("(s)", system_bus_name->name),
+			  G_VARIANT_TYPE ("(a{sv})"),
+			  G_DBUS_CALL_FLAGS_NONE,
+			  -1,
+			  cancellable,
+			  &dbus_error);
+
+  if (result == NULL)
+  {
+    if (g_error_matches (dbus_error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_METHOD))
+      {
+        g_error_free (dbus_error);
+        return polkit_system_bus_name_get_creds_fallback(system_bus_name,
+                                                         out_uid,
+                                                         out_pid,
+                                                         cancellable,
+                                                         connection,
+                                                         tmp_context,
+                                                         error);
+      }
+    else
+      goto out;
+  }
+
+  g_variant_get (result, "(a{sv})", &iter);
+
+  while (g_variant_iter_loop (iter, "{&sv}", &key, &value))
+    {
+      if (g_strcmp0 (key, "ProcessID") == 0)
+        pid = g_variant_get_uint32 (value);
+      else if (g_strcmp0 (key, "UnixUserID") == 0)
+        uid = g_variant_get_uint32 (value);
+      else if (g_strcmp0 (key, "UnixGroupIDs") == 0)
+        {
+          GVariantIter *group_iter;
+          gid_t gid;
+
+          gids = g_array_new (FALSE, FALSE, sizeof (gid_t));
+          g_variant_get (value, "au", &group_iter);
+          while (g_variant_iter_loop (group_iter, "u", &gid))
+            g_array_append_val (gids, gid);
+          g_variant_iter_free (group_iter);
+        }
+    }
+
+  g_variant_unref (result);
+
+  if (out_uid)
+    *out_uid = uid;
+  if (out_gids && gids)
+    *out_gids = g_array_ref(gids);
+  if (out_pid)
+    *out_pid = pid;
+  ret = TRUE;
+ out:
+  if (tmp_context)
+    {
+      g_main_context_pop_thread_default (tmp_context);
+      g_main_context_unref (tmp_context);
+    }
+  if (connection != NULL)
+    g_object_unref (connection);
+  if (dbus_error && error)
+    g_propagate_error (error, dbus_error);
+  else if (dbus_error)
+    g_error_free (dbus_error);
+  if (gids)
+    g_array_unref (gids);
+
   return ret;
 }
 
@@ -503,18 +602,22 @@ polkit_system_bus_name_get_process_sync (PolkitSystemBusName  *system_bus_name,
   PolkitSubject *ret = NULL;
   guint32 pid;
   guint32 uid;
+  GArray *gids = NULL;
 
   g_return_val_if_fail (POLKIT_IS_SYSTEM_BUS_NAME (system_bus_name), NULL);
   g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), NULL);
   g_return_val_if_fail (error == NULL || *error == NULL, NULL);
 
-  if (!polkit_system_bus_name_get_creds_sync (system_bus_name, &uid, &pid,
+  if (!polkit_system_bus_name_get_creds_sync (system_bus_name, &uid, &gids, &pid,
 					      cancellable, error))
     goto out;
 
   ret = polkit_unix_process_new_for_owner (pid, 0, uid);
+  polkit_unix_process_set_gids (POLKIT_UNIX_PROCESS (ret), gids);
 
  out:
+  if (gids)
+    g_array_unref (gids);
   return ret;
 }
 
@@ -541,7 +644,7 @@ polkit_system_bus_name_get_user_sync (PolkitSystemBusName  *system_bus_name,
   g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), NULL);
   g_return_val_if_fail (error == NULL || *error == NULL, NULL);
 
-  if (!polkit_system_bus_name_get_creds_sync (system_bus_name, &uid, NULL,
+  if (!polkit_system_bus_name_get_creds_sync (system_bus_name, &uid, NULL, NULL,
 					      cancellable, error))
     goto out;
 
diff --git a/src/polkit/polkitunixprocess.c b/src/polkit/polkitunixprocess.c
index 151085f..be5be2b 100644
--- a/src/polkit/polkitunixprocess.c
+++ b/src/polkit/polkitunixprocess.c
@@ -157,6 +157,7 @@ struct _PolkitUnixProcess
   guint64 start_time;
   gint uid;
   gint pidfd;
+  GArray *gids;
 };
 
 struct _PolkitUnixProcessClass
@@ -171,6 +172,7 @@ enum
   PROP_START_TIME,
   PROP_UID,
   PROP_PIDFD,
+  PROP_GIDS,
 };
 
 static void subject_iface_init (PolkitSubjectIface *subject_iface);
@@ -216,6 +218,10 @@ polkit_unix_process_get_property (GObject    *object,
       g_value_set_int (value, unix_process->uid);
       break;
 
+    case PROP_GIDS:
+      g_value_set_boxed (value, unix_process->gids);
+      break;
+
     case PROP_PIDFD:
       g_value_set_int (value, unix_process->pidfd);
       break;
@@ -248,6 +254,10 @@ polkit_unix_process_set_property (GObject      *object,
       polkit_unix_process_set_uid (unix_process, g_value_get_int (value));
       break;
 
+    case PROP_GIDS:
+      polkit_unix_process_set_gids (unix_process, g_value_get_boxed (value));
+      break;
+
     case PROP_PIDFD:
       polkit_unix_process_set_pidfd (unix_process, g_value_get_int (value));
       break;
@@ -366,6 +376,9 @@ polkit_unix_process_finalize (GObject *object)
       process->pidfd = -1;
     }
 
+  if (process->gids)
+    g_array_unref (process->gids);
+
   if (G_OBJECT_CLASS (polkit_unix_process_parent_class)->finalize != NULL)
     G_OBJECT_CLASS (polkit_unix_process_parent_class)->finalize (object);
 }
@@ -457,6 +470,23 @@ polkit_unix_process_class_init (PolkitUnixProcessClass *klass)
                                                      G_PARAM_STATIC_NAME |
                                                      G_PARAM_STATIC_BLURB |
                                                      G_PARAM_STATIC_NICK));
+
+  /**
+   * PolkitUnixProcess:gids:
+   *
+   * The UNIX group ids of the process.
+   */
+  g_object_class_install_property (gobject_class,
+                                   PROP_GIDS,
+                                   g_param_spec_boxed ("gids",
+                                                       "Group IDs",
+                                                       "The UNIX group IDs",
+                                                       G_TYPE_ARRAY,
+                                                       G_PARAM_CONSTRUCT |
+                                                       G_PARAM_READWRITE |
+                                                       G_PARAM_STATIC_NAME |
+                                                       G_PARAM_STATIC_BLURB |
+                                                       G_PARAM_STATIC_NICK));
 }
 
 /**
@@ -497,6 +527,44 @@ polkit_unix_process_set_uid (PolkitUnixProcess *process,
 }
 
 /**
+ * polkit_unix_process_get_gids:
+ * @process: A #PolkitUnixProcess.
+ *
+ * Gets the group ids for @process. Note that this is the real group-ids,
+ * not the effective group-ids.
+ *
+ * Returns: (element-type GArray) (transfer full) (allow-none): a #GArray
+ *          of #gid_t containing the group ids for @process or NULL if unknown,
+ *          as a new reference to the array, caller must deref it when done.
+ */
+GArray *
+polkit_unix_process_get_gids (PolkitUnixProcess *process)
+{
+  g_return_val_if_fail (POLKIT_IS_UNIX_PROCESS (process), NULL);
+  return process->gids ? g_array_ref (process->gids) : NULL;
+}
+
+/**
+ * polkit_unix_process_set_gids:
+ * @process: A #PolkitUnixProcess.
+ * @gids: (element-type GArray): A #GList of #gid_t containing the group
+ *        ids to set for @process or NULL to unset them.
+ *        A reference to @gids is taken.
+ *
+ * Sets the (real, not effective) group ids for @process.
+ */
+void
+polkit_unix_process_set_gids (PolkitUnixProcess *process,
+                              GArray            *gids)
+{
+  g_return_if_fail (POLKIT_IS_UNIX_PROCESS (process));
+  if (process->gids)
+    g_array_unref (g_steal_pointer (&process->gids));
+  if (gids)
+    process->gids = g_array_ref (gids);
+}
+
+/**
  * polkit_unix_process_get_pid:
  * @process: A #PolkitUnixProcess.
  *
@@ -689,6 +757,7 @@ polkit_unix_process_new_for_owner (gint    pid,
  * polkit_unix_process_new_pidfd:
  * @pidfd: The process id file descriptor.
  * @uid: The (real, not effective) uid of the owner of @pid or -1 to look it up in e.g. <filename>/proc</filename>.
+ * @gids: (element-type gint) (allow-none): The (real, not effective) gids of the owner of @pid or %NULL.
  *
  * Creates a new #PolkitUnixProcess object for @pidfd and @uid.
  *
@@ -696,11 +765,13 @@ polkit_unix_process_new_for_owner (gint    pid,
  */
 PolkitSubject *
 polkit_unix_process_new_pidfd (gint    pidfd,
-                               gint    uid)
+                               gint    uid,
+                               GArray *gids)
 {
   return POLKIT_SUBJECT (g_object_new (POLKIT_TYPE_UNIX_PROCESS,
                                        "pidfd", pidfd,
                                        "uid", uid,
+                                       "gids", gids,
                                        NULL));
 }
 
diff --git a/src/polkit/polkitunixprocess.h b/src/polkit/polkitunixprocess.h
index 9ac5bc9..cc99cf8 100644
--- a/src/polkit/polkitunixprocess.h
+++ b/src/polkit/polkitunixprocess.h
@@ -56,10 +56,14 @@ PolkitSubject  *polkit_unix_process_new_for_owner  (gint               pid,
                                                     guint64            start_time,
                                                     gint               uid);
 PolkitSubject  *polkit_unix_process_new_pidfd      (gint               pidfd,
-                                                    gint               uid);
+                                                    gint               uid,
+                                                    GArray            *gids);
+GArray         *polkit_unix_process_get_gids       (PolkitUnixProcess *process);
 gint            polkit_unix_process_get_pid        (PolkitUnixProcess *process);
 guint64         polkit_unix_process_get_start_time (PolkitUnixProcess *process);
 gint            polkit_unix_process_get_uid        (PolkitUnixProcess *process);
+void            polkit_unix_process_set_gids       (PolkitUnixProcess *process,
+                                                    GArray            *gids);
 void            polkit_unix_process_set_pid        (PolkitUnixProcess *process,
                                                     gint               pid);
 void            polkit_unix_process_set_uid        (PolkitUnixProcess *process,
diff --git a/src/polkitbackend/polkitbackendduktapeauthority.c b/src/polkitbackend/polkitbackendduktapeauthority.c
index d6baf11..f4d8174 100644
--- a/src/polkitbackend/polkitbackendduktapeauthority.c
+++ b/src/polkitbackend/polkitbackendduktapeauthority.c
@@ -372,6 +372,7 @@ push_subject (duk_context               *cx,
   PolkitSubject *process = NULL;
   gchar *user_name = NULL;
   GPtrArray *groups = NULL;
+  GArray *gids_from_dbus = NULL;
   struct passwd *passwd;
   char *seat_str = NULL;
   char *session_str = NULL;
@@ -415,41 +416,64 @@ push_subject (duk_context               *cx,
   uid = polkit_unix_user_get_uid (POLKIT_UNIX_USER (user_for_subject));
 
   groups = g_ptr_array_new_with_free_func (g_free);
+  gids_from_dbus = polkit_unix_process_get_gids (POLKIT_UNIX_PROCESS (process));
 
-  passwd = getpwuid (uid);
-  if (passwd == NULL)
+  /* D-Bus will give us supplementary groups too, so prefer that to looking up
+   * the group from the uid. */
+  if (gids_from_dbus && gids_from_dbus->len > 0)
     {
-      user_name = g_strdup_printf ("%d", (gint) uid);
-      g_warning ("Error looking up info for uid %d: %m", (gint) uid);
+      gint n;
+      for (n = 0; n < gids_from_dbus->len; n++)
+        {
+          struct group *group;
+          group = getgrgid (g_array_index (gids_from_dbus, gid_t, n));
+          if (group == NULL)
+            {
+              g_ptr_array_add (groups, g_strdup_printf ("%d", (gint) g_array_index (gids_from_dbus, gid_t, n)));
+            }
+          else
+            {
+              g_ptr_array_add (groups, g_strdup (group->gr_name));
+            }
+        }
     }
   else
     {
-      gid_t gids[512];
-      int num_gids = 512;
-
-      user_name = g_strdup (passwd->pw_name);
-
-      if (getgrouplist (passwd->pw_name,
-                        passwd->pw_gid,
-                        gids,
-                        &num_gids) < 0)
+      passwd = getpwuid (uid);
+      if (passwd == NULL)
         {
-          g_warning ("Error looking up groups for uid %d: %m", (gint) uid);
+          user_name = g_strdup_printf ("%d", (gint) uid);
+          g_warning ("Error looking up info for uid %d: %m", (gint) uid);
         }
       else
         {
-          gint n;
-          for (n = 0; n < num_gids; n++)
+          gid_t gids[512];
+          int num_gids = 512;
+
+          user_name = g_strdup (passwd->pw_name);
+
+          if (getgrouplist (passwd->pw_name,
+                            passwd->pw_gid,
+                            gids,
+                            &num_gids) < 0)
             {
-              struct group *group;
-              group = getgrgid (gids[n]);
-              if (group == NULL)
-                {
-                  g_ptr_array_add (groups, g_strdup_printf ("%d", (gint) gids[n]));
-                }
-              else
+              g_warning ("Error looking up groups for uid %d: %m", (gint) uid);
+            }
+          else
+            {
+              gint n;
+              for (n = 0; n < num_gids; n++)
                 {
-                  g_ptr_array_add (groups, g_strdup (group->gr_name));
+                  struct group *group;
+                  group = getgrgid (gids[n]);
+                  if (group == NULL)
+                    {
+                      g_ptr_array_add (groups, g_strdup_printf ("%d", (gint) gids[n]));
+                    }
+                  else
+                    {
+                      g_ptr_array_add (groups, g_strdup (group->gr_name));
+                    }
                 }
             }
         }
@@ -498,6 +522,8 @@ push_subject (duk_context               *cx,
   g_free (user_name);
   if (groups != NULL)
     g_ptr_array_unref (groups);
+  if (gids_from_dbus != NULL)
+    g_array_unref (gids_from_dbus);
 
   return ret;
 }
diff --git a/src/polkitbackend/polkitbackendjsauthority.cpp b/src/polkitbackend/polkitbackendjsauthority.cpp
index a577c1c..7920d4d 100644
--- a/src/polkitbackend/polkitbackendjsauthority.cpp
+++ b/src/polkitbackend/polkitbackendjsauthority.cpp
@@ -580,6 +580,7 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
   PolkitSubject *process = NULL;
   gchar *user_name = NULL;
   GPtrArray *groups = NULL;
+  GArray *gids_from_dbus = NULL;
   struct passwd *passwd;
   char *seat_str = NULL;
   char *session_str = NULL;
@@ -635,41 +636,64 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
   uid = polkit_unix_user_get_uid (POLKIT_UNIX_USER (user_for_subject));
 
   groups = g_ptr_array_new_with_free_func (g_free);
+  gids_from_dbus = polkit_unix_process_get_gids (POLKIT_UNIX_PROCESS (process));
 
-  passwd = getpwuid (uid);
-  if (passwd == NULL)
+  /* D-Bus will give us supplementary groups too, so prefer that to looking up
+   * the group from the uid. */
+  if (gids_from_dbus && gids_from_dbus->len > 0)
     {
-      user_name = g_strdup_printf ("%d", (gint) uid);
-      g_warning ("Error looking up info for uid %d: %m", (gint) uid);
+      gint n;
+      for (n = 0; n < gids_from_dbus->len; n++)
+        {
+          struct group *group;
+          group = getgrgid (g_array_index (gids_from_dbus, gid_t, n));
+          if (group == NULL)
+            {
+              g_ptr_array_add (groups, g_strdup_printf ("%d", (gint) g_array_index (gids_from_dbus, gid_t, n)));
+            }
+          else
+            {
+              g_ptr_array_add (groups, g_strdup (group->gr_name));
+            }
+        }
     }
   else
     {
-      gid_t gids[512];
-      int num_gids = 512;
-
-      user_name = g_strdup (passwd->pw_name);
-
-      if (getgrouplist (passwd->pw_name,
-                        passwd->pw_gid,
-                        gids,
-                        &num_gids) < 0)
+      passwd = getpwuid (uid);
+      if (passwd == NULL)
         {
-          g_warning ("Error looking up groups for uid %d: %m", (gint) uid);
+          user_name = g_strdup_printf ("%d", (gint) uid);
+          g_warning ("Error looking up info for uid %d: %m", (gint) uid);
         }
       else
         {
-          gint n;
-          for (n = 0; n < num_gids; n++)
+          gid_t gids[512];
+          int num_gids = 512;
+
+          user_name = g_strdup (passwd->pw_name);
+
+          if (getgrouplist (passwd->pw_name,
+                            passwd->pw_gid,
+                            gids,
+                            &num_gids) < 0)
             {
-              struct group *group;
-              group = getgrgid (gids[n]);
-              if (group == NULL)
-                {
-                  g_ptr_array_add (groups, g_strdup_printf ("%d", (gint) gids[n]));
-                }
-              else
+              g_warning ("Error looking up groups for uid %d: %m", (gint) uid);
+            }
+          else
+            {
+              gint n;
+              for (n = 0; n < num_gids; n++)
                 {
-                  g_ptr_array_add (groups, g_strdup (group->gr_name));
+                  struct group *group;
+                  group = getgrgid (gids[n]);
+                  if (group == NULL)
+                    {
+                      g_ptr_array_add (groups, g_strdup_printf ("%d", (gint) gids[n]));
+                    }
+                  else
+                    {
+                      g_ptr_array_add (groups, g_strdup (group->gr_name));
+                    }
                 }
             }
         }
@@ -703,6 +727,8 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
   g_free (user_name);
   if (groups != NULL)
     g_ptr_array_unref (groups);
+  if (gids_from_dbus != NULL)
+    g_array_unref (gids_from_dbus);
 
   return ret;
 }