PolicyKit: Branch 'master' - 2 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Fri Sep 8 16:29:53 UTC 2023


 docs/man/polkit.xml                               |   41 +++++
 src/polkit/meson.build                            |    1 
 src/polkit/polkitsystembusname.c                  |  155 ++++++++++++++++++++--
 src/polkit/polkitunixprocess.c                    |  124 +++++++++++++++++
 src/polkit/polkitunixprocess.h                    |    7 
 src/polkitbackend/polkitbackendcommon.c           |  131 ++++++++++++++++++
 src/polkitbackend/polkitbackendcommon.h           |    4 
 src/polkitbackend/polkitbackendduktapeauthority.c |   88 +++++++++---
 src/polkitbackend/polkitbackendjsauthority.cpp    |   88 +++++++++---
 9 files changed, 574 insertions(+), 65 deletions(-)

New commits:
commit a23d9ce375dcbc64aade92f3e082182b993c1169
Author: Luca Boccassi <bluca at debian.org>
Date:   Thu Apr 6 01:39:46 2023 +0100

    jsauthority: add 'system_unit' and 'no_new_privileges' subject attributes
    
    When building with libsystemd support, query the systemd unit name
    that the process if part of (if any) and add it as a subject attribute.
    Allows allow-listing actions based on the systemd unit:
    
     polkit.addRule(function(action, subject) {
         if (action.id.indexOf("org.foo.bar") == 0) {
             if (subject.system_unit == "test.service" && subject.no_new_privileges) {
                 return polkit.Result.YES;
             }
         }
     });
    
    We call it system_unit instead of just unit to make it extra clear that
    this is about system units, rather than user units.
    If we ran as root we could also query for the user unit, but we are
    running as the polkitd user in most cases which means we cannot connect
    to other D-Bus sessions to perform the query.
    
    We only do this if we can pin the subject process by PIDFD, as that's
    safer PIDs can be recycled. Skip if not possible because the D-Bus
    daemon and/or systemd are too old and do not support the functionality.
    Also we check for the NoNewPrivileges= being set, so that we can ensure
    that the unit cannot alter its uid via a setuid binary. But let this last
    part be decided by policy, as a system builder might simply ensure that
    no setuid binaries are shipped at all, which is equivalent.
    
    This requires dbus-broker v34 or dbus-daemon v15.7 and systemd v253 and
    kernel v6.5.

diff --git a/docs/man/polkit.xml b/docs/man/polkit.xml
index b67c4a6..081f72a 100644
--- a/docs/man/polkit.xml
+++ b/docs/man/polkit.xml
@@ -797,6 +797,19 @@ May 24 14:28:50 thinkpad polkitd[32217]: /etc/polkit-1/rules.d/10-test.rules:4:
           </listitem>
         </varlistentry>
 
+        <varlistentry>
+          <term><type>string</type> system_unit</term>
+          <listitem>
+            <para>
+              The systemd unit that the subject's process is part of (if any). Note that
+              this can only match on system units, as user units can be created with any
+              name without privileges (unlike system units which require root to create).
+              A process running in a user unit will return the user session unit in this
+              attribute (e.g.: <literal>user-1000.service</literal>).
+            </para>
+          </listitem>
+        </varlistentry>
+
         <varlistentry>
           <term><type>boolean</type> local</term>
           <listitem>
@@ -806,6 +819,19 @@ May 24 14:28:50 thinkpad polkitd[32217]: /etc/polkit-1/rules.d/10-test.rules:4:
           </listitem>
         </varlistentry>
 
+        <varlistentry>
+          <term><type>boolean</type> no_new_privileges</term>
+          <listitem>
+            <para>
+              Set only if <parameter>system_unit</parameter> is not empty, and set to
+              <constant>true</constant> only if the referenced systemd service unit
+              has the <parameter>NoNewPrivileges=</parameter> setting enabled. This
+              ensures that the process cannot gain any new privileges via executing
+              setuid binaries.
+            </para>
+          </listitem>
+        </varlistentry>
+
         <varlistentry>
           <term><type>boolean</type> active</term>
           <listitem>
@@ -945,6 +971,21 @@ polkit.addRule(function(action, subject) {
         }
     }
 });
+]]></programlisting>
+
+      <para>
+        Allow all processes running as part of the <literal>admin.service</literal>
+        systemd system unit to perform user administration, as long as they cannot
+        gain new privileges:
+      </para>
+      <programlisting><![CDATA[
+polkit.addRule(function(action, subject) {
+    if (action.id == "org.freedesktop.accounts.user-administration" &&
+        subject.system_unit == "admin.service" &&
+        subject.no_new_privileges) {
+        return polkit.Result.YES;
+    }
+});
 ]]></programlisting>
     </refsect2>
   </refsect1>
diff --git a/src/polkit/meson.build b/src/polkit/meson.build
index 63dc1e8..9699fea 100644
--- a/src/polkit/meson.build
+++ b/src/polkit/meson.build
@@ -37,6 +37,7 @@ install_headers(
 
 common_deps = [
   gio_dep,
+  gio_unix_dep,
   glib_dep,
 ]
 
diff --git a/src/polkit/polkitsystembusname.c b/src/polkit/polkitsystembusname.c
index 9c96747..d198783 100644
--- a/src/polkit/polkitsystembusname.c
+++ b/src/polkit/polkitsystembusname.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <string.h>
+#include <gio/gunixfdlist.h>
 #include "polkitsystembusname.h"
 #include "polkitunixuser.h"
 #include "polkitsubject.h"
@@ -483,6 +484,7 @@ polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus
 				       guint32                       *out_uid,
 				       GArray                       **out_gids,
 				       guint32                       *out_pid,
+				       gint                          *out_pidfd,
 				       GCancellable                  *cancellable,
 				       GError                       **error)
 {
@@ -491,9 +493,11 @@ polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus
   GMainContext *tmp_context = NULL;
   GVariantIter *iter;
   GVariant *result, *value;
+  GUnixFDList *fd_list;
   GError *dbus_error = NULL;
   const gchar *key;
   guint32 uid = G_MAXUINT32, pid = 0;
+  gint pidfd = -1;
   GArray *gids = NULL;
 
   connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, cancellable, error);
@@ -505,8 +509,10 @@ polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus
 
   /* If the new unified API is available (since dbus-daemon 1.10.4) use it,
    * or fallback to the old separate calls.
+   * Since dbus-daemon 1.15.7 and dbus-broker 34, the new API will return
+   * a ProcessFD that we can use to pin the caller against PID reuse.
    */
-  result = g_dbus_connection_call_sync (connection,
+  result = g_dbus_connection_call_with_unix_fd_list_sync (connection,
 			  "org.freedesktop.DBus",       /* name */
 			  "/org/freedesktop/DBus",      /* object path */
 			  "org.freedesktop.DBus",       /* interface name */
@@ -515,6 +521,8 @@ polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus
 			  G_VARIANT_TYPE ("(a{sv})"),
 			  G_DBUS_CALL_FLAGS_NONE,
 			  -1,
+			  NULL,
+			  &fd_list,
 			  cancellable,
 			  &dbus_error);
 
@@ -554,6 +562,11 @@ polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus
             g_array_append_val (gids, gid);
           g_variant_iter_free (group_iter);
         }
+      else if (g_strcmp0 (key, "ProcessFD") == 0)
+        {
+          gint32 index = g_variant_get_handle (value);
+          pidfd = g_unix_fd_list_get (fd_list, index, error);
+        }
     }
 
   g_variant_unref (result);
@@ -564,6 +577,10 @@ polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus
     *out_gids = g_array_ref(gids);
   if (out_pid)
     *out_pid = pid;
+  if (out_pidfd)
+    *out_pidfd = pidfd;
+  else if (pidfd >= 0)
+    close (pidfd);
   ret = TRUE;
  out:
   if (tmp_context)
@@ -573,6 +590,8 @@ polkit_system_bus_name_get_creds_sync (PolkitSystemBusName           *system_bus
     }
   if (connection != NULL)
     g_object_unref (connection);
+  if (!ret && pidfd >= 0)
+    close (pidfd);
   if (dbus_error && error)
     g_propagate_error (error, dbus_error);
   else if (dbus_error)
@@ -600,6 +619,7 @@ polkit_system_bus_name_get_process_sync (PolkitSystemBusName  *system_bus_name,
                                          GError              **error)
 {
   PolkitSubject *ret = NULL;
+  gint pidfd = -1;
   guint32 pid;
   guint32 uid;
   GArray *gids = NULL;
@@ -608,11 +628,15 @@ polkit_system_bus_name_get_process_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, &gids, &pid,
+  if (!polkit_system_bus_name_get_creds_sync (system_bus_name, &uid, &gids, &pid, &pidfd,
 					      cancellable, error))
     goto out;
 
-  ret = polkit_unix_process_new_for_owner (pid, 0, uid);
+  if (pidfd >= 0)
+    ret = polkit_unix_process_new_pidfd (pidfd, uid, gids);
+  else
+    ret = polkit_unix_process_new_for_owner (pid, 0, uid);
+
   polkit_unix_process_set_gids (POLKIT_UNIX_PROCESS (ret), gids);
 
  out:
@@ -644,7 +668,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, NULL,
+  if (!polkit_system_bus_name_get_creds_sync (system_bus_name, &uid, NULL, NULL, NULL,
 					      cancellable, error))
     goto out;
 
diff --git a/src/polkit/polkitunixprocess.c b/src/polkit/polkitunixprocess.c
index be5be2b..e05e581 100644
--- a/src/polkit/polkitunixprocess.c
+++ b/src/polkit/polkitunixprocess.c
@@ -157,6 +157,7 @@ struct _PolkitUnixProcess
   guint64 start_time;
   gint uid;
   gint pidfd;
+  gboolean pidfd_is_safe;
   GArray *gids;
 };
 
@@ -172,6 +173,7 @@ enum
   PROP_START_TIME,
   PROP_UID,
   PROP_PIDFD,
+  PROP_PIDFD_IS_SAFE,
   PROP_GIDS,
 };
 
@@ -226,6 +228,10 @@ polkit_unix_process_get_property (GObject    *object,
       g_value_set_int (value, unix_process->pidfd);
       break;
 
+    case PROP_PIDFD_IS_SAFE:
+      g_value_set_boolean (value, unix_process->pidfd_is_safe);
+      break;
+
     case PROP_START_TIME:
       g_value_set_uint64 (value, unix_process->start_time);
       break;
@@ -334,6 +340,11 @@ polkit_unix_process_constructed (GObject *object)
 
   /* sets pidfd, start_time and uid in case they are unset */
 
+  /* We didn't open it ourselves here, so we must have got it
+   * from D-Bus, mark it as safe to use */
+  if (process->pidfd >= 0)
+    process->pidfd_is_safe = TRUE;
+
 #ifdef HAVE_PIDFD_OPEN
   if (process->pid > 0 && process->pidfd < 0)
     {
@@ -471,6 +482,23 @@ polkit_unix_process_class_init (PolkitUnixProcessClass *klass)
                                                      G_PARAM_STATIC_BLURB |
                                                      G_PARAM_STATIC_NICK));
 
+  /**
+   * PolkitUnixProcess:pidfd_is_safe:
+   *
+   * Whether the UNIX process id file descriptor is safe end-to-end
+   * or it was opened locally.
+   */
+  g_object_class_install_property (gobject_class,
+                                   PROP_PIDFD_IS_SAFE,
+                                   g_param_spec_boolean ("pidfd-is-safe",
+                                                         "Process ID FD",
+                                                         "Whether the UNIX process ID file descriptor is safe",
+                                                         FALSE,
+                                                         G_PARAM_READABLE |
+                                                         G_PARAM_STATIC_NAME |
+                                                         G_PARAM_STATIC_BLURB |
+                                                         G_PARAM_STATIC_NICK));
+
   /**
    * PolkitUnixProcess:gids:
    *
@@ -640,12 +668,14 @@ polkit_unix_process_set_pid (PolkitUnixProcess *process,
     {
       close (process->pidfd);
       process->pidfd = -1;
+      process->pidfd_is_safe = FALSE;
     }
   if (pid > 0)
     {
       gint pidfd = (int) syscall (SYS_pidfd_open, process->pid, 0);
       if (pidfd >= 0)
         {
+          process->pidfd_is_safe = FALSE;
           process->pidfd = pidfd;
           process->pid = 0;
           return;
@@ -671,6 +701,22 @@ polkit_unix_process_get_pidfd (PolkitUnixProcess *process)
   return process->pidfd;
 }
 
+/**
+ * polkit_unix_process_get_pidfd_is_safe:
+ * @process: A #PolkitUnixProcess.
+ *
+ * Checks if the process id file descriptor for @process is safe
+ * or if it was opened locally and thus vulnerable to reuse.
+ *
+ * Returns: TRUE or FALSE.
+ */
+gboolean
+polkit_unix_process_get_pidfd_is_safe (PolkitUnixProcess *process)
+{
+  g_return_val_if_fail (POLKIT_IS_UNIX_PROCESS (process), FALSE);
+  return process->pidfd_is_safe;
+}
+
 /**
  * polkit_unix_process_set_pidfd:
  * @process: A #PolkitUnixProcess.
@@ -684,7 +730,10 @@ polkit_unix_process_set_pidfd (PolkitUnixProcess *process,
 {
   g_return_if_fail (POLKIT_IS_UNIX_PROCESS (process));
   if (process->pidfd >= 0)
-    close (process->pidfd);
+    {
+      close (process->pidfd);
+      process->pidfd_is_safe = FALSE;
+    }
   process->pidfd = pidfd;
 }
 
diff --git a/src/polkit/polkitunixprocess.h b/src/polkit/polkitunixprocess.h
index cc99cf8..7c9adde 100644
--- a/src/polkit/polkitunixprocess.h
+++ b/src/polkit/polkitunixprocess.h
@@ -76,6 +76,7 @@ gint            polkit_unix_process_get_owner      (PolkitUnixProcess  *process,
 gint            polkit_unix_process_get_pidfd      (PolkitUnixProcess  *process);
 void            polkit_unix_process_set_pidfd      (PolkitUnixProcess  *process,
                                                     gint                pidfd);
+gboolean        polkit_unix_process_get_pidfd_is_safe (PolkitUnixProcess *process);
 
 G_END_DECLS
 
diff --git a/src/polkitbackend/polkitbackendcommon.c b/src/polkitbackend/polkitbackendcommon.c
index 6783dff..f38147e 100644
--- a/src/polkitbackend/polkitbackendcommon.c
+++ b/src/polkitbackend/polkitbackendcommon.c
@@ -528,3 +528,134 @@ polkit_backend_common_spawn_cb (GObject       *source_object,
   data->res = (GAsyncResult*)g_object_ref (res);
   g_main_loop_quit (data->loop);
 }
+
+void
+polkit_backend_common_pidfd_to_systemd_unit (gint      pidfd,
+                                             gchar   **ret_unit,
+                                             gboolean *ret_no_new_privs)
+{
+  static int cached_has_pidfd_support = -1;
+  GError *error = NULL;
+  GDBusConnection *connection = NULL;
+  GMainContext *tmp_context = NULL;
+  GVariant *result = NULL, *no_new_privs_result = NULL, *no_new_privis_value;
+  GUnixFDList *fd_list = NULL;
+  const char *unit_path, *unit;
+  int fd_id;
+
+  /* Try to lookup using a PIDFD, so that we do not have issues with PIDs being
+   * recycled under our nose. For that we need both a kernel that supports the
+   * PIDFD syscalls (no wrapper from glibc, so need to call it directly) and a
+   * version of systemd with the new GetUnitByPIDFD method. If either are not
+   * available, then return nothing, as we don't want to be open to PID recycle
+   * attacks.
+   */
+
+  g_assert (ret_unit != NULL);
+  g_assert (ret_no_new_privs != NULL);
+
+  if (pidfd < 0 || cached_has_pidfd_support == 0)
+    return;
+
+  connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error);
+  if (connection == NULL)
+    {
+      g_warning ("Error getting system bus: %s", error->message);
+      goto out;
+    }
+
+  tmp_context = g_main_context_new ();
+  g_main_context_push_thread_default (tmp_context);
+
+  fd_list = g_unix_fd_list_new ();
+  if (fd_list == NULL)
+    goto out;
+
+  fd_id = g_unix_fd_list_append (fd_list, pidfd, &error);
+  if (fd_id < 0)
+    {
+      g_warning ("Error appending PID FD to fd list: %s", error->message);
+      goto out;
+    }
+
+  result = g_dbus_connection_call_with_unix_fd_list_sync (connection,
+        "org.freedesktop.systemd1",         /* name */
+        "/org/freedesktop/systemd1",        /* object path */
+        "org.freedesktop.systemd1.Manager", /* interface name */
+        "GetUnitByPIDFD",                   /* method */
+        g_variant_new ("(h)", fd_id),
+        G_VARIANT_TYPE ("(osay)"),
+        G_DBUS_CALL_FLAGS_NONE,
+        -1,
+        fd_list,
+        NULL,
+        NULL,
+        &error);
+
+  if (result == NULL)
+    {
+      if (g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_METHOD))
+        cached_has_pidfd_support = 0;
+
+      g_warning ("Error calling GetUnitByPIDFD: %s", error->message);
+      goto out;
+    }
+
+  g_variant_get (result, "(&o&say)", &unit_path, &unit, NULL);
+  if (unit == NULL)
+    goto out;
+
+  /* Check for NoNewPrivileges property being set on the unit via D-Bus, and
+   * return if it is not. This protects against PID changes, as if unset the
+   * unit could use a setuid binary. */
+  no_new_privs_result = g_dbus_connection_call_sync (connection,
+        "org.freedesktop.systemd1",         /* name */
+        unit_path,                          /* object path */
+        "org.freedesktop.DBus.Properties",  /* interface name */
+        "Get",                              /* method */
+        g_variant_new ("(ss)", "org.freedesktop.systemd1.Service", "NoNewPrivileges"),
+        G_VARIANT_TYPE ("(v)"),
+        G_DBUS_CALL_FLAGS_NONE,
+        -1,
+        NULL,
+        &error);
+
+  if (no_new_privs_result == NULL)
+    {
+      g_warning ("Error calling Get on NoNewPrivileges property for unit %s: %s", unit, error->message);
+      goto out;
+    }
+
+  g_variant_get (no_new_privs_result, "(v)", &no_new_privis_value);
+  if (no_new_privis_value == NULL)
+    {
+      g_warning ("Error getting value for NoNewPrivileges property for unit %s", unit);
+      goto out;
+    }
+
+  *ret_unit = strdup (unit);
+  if (!*ret_unit)
+    {
+      g_warning ("Failed to allocate memory for systemd unit ID");
+      goto out;
+    }
+
+  *ret_no_new_privs = g_variant_get_boolean (no_new_privis_value);
+
+ 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 (result != NULL)
+    g_variant_unref (result);
+  if (no_new_privs_result != NULL)
+    g_variant_unref (no_new_privs_result);
+  if (fd_list != NULL)
+    g_object_unref (fd_list);
+  if (error)
+    g_error_free (error);
+}
diff --git a/src/polkitbackend/polkitbackendcommon.h b/src/polkitbackend/polkitbackendcommon.h
index dd700fc..28342af 100644
--- a/src/polkitbackend/polkitbackendcommon.h
+++ b/src/polkitbackend/polkitbackendcommon.h
@@ -37,6 +37,7 @@
 #include <netdb.h>
 #endif
 #include <string.h>
+#include <gio/gunixfdlist.h>
 #include <glib/gstdio.h>
 #include <locale.h>
 #include <glib/gi18n-lib.h> //here, all things glib via glib.h (including -> gspawn.h)
@@ -150,6 +151,9 @@ PolkitImplicitAuthorization polkit_backend_common_js_authority_check_authorizati
                                                                                          const gchar                       *action_id,
                                                                                          PolkitDetails                     *details,
                                                                                          PolkitImplicitAuthorization        implicit);
+void polkit_backend_common_pidfd_to_systemd_unit (gint      pid,
+                                                  gchar   **ret_unit,
+                                                  gboolean *ret_no_new_privs);
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/polkitbackend/polkitbackendduktapeauthority.c b/src/polkitbackend/polkitbackendduktapeauthority.c
index f4d8174..b5eea39 100644
--- a/src/polkitbackend/polkitbackendduktapeauthority.c
+++ b/src/polkitbackend/polkitbackendduktapeauthority.c
@@ -366,6 +366,7 @@ push_subject (duk_context               *cx,
               GError                   **error)
 {
   gboolean ret = FALSE;
+  gboolean no_new_privs = FALSE;
   gint pidfd = -1;
   pid_t pid_early, pid_late;
   uid_t uid;
@@ -376,6 +377,7 @@ push_subject (duk_context               *cx,
   struct passwd *passwd;
   char *seat_str = NULL;
   char *session_str = NULL;
+  char *system_unit = NULL;
 
   if (!duk_get_global_string (cx, "Subject")) {
     return FALSE;
@@ -479,6 +481,13 @@ push_subject (duk_context               *cx,
         }
     }
 
+  /* Query the unit, will work only if we got the pidfd from dbus-daemon/broker.
+   * Best-effort operation, will log on failure, but we don't bail here. But
+   * only do so if the pidfd was marked as safe, i.e.: we got it from D-Bus so
+   * it can be trusted end-to-end, with no reuse attack window.  */
+  if (polkit_unix_process_get_pidfd_is_safe (POLKIT_UNIX_PROCESS (process)))
+    polkit_backend_common_pidfd_to_systemd_unit (pidfd, &system_unit, &no_new_privs);
+
   /* In case we are using PIDFDs, check that the PID still matches to avoid race
    * conditions and PID recycle attacks.
    */
@@ -509,6 +518,10 @@ push_subject (duk_context               *cx,
   set_property_strv (cx, "groups", groups);
   set_property_str (cx, "seat", seat_str);
   set_property_str (cx, "session", session_str);
+  set_property_str (cx, "system_unit", system_unit);
+  /* If we have a unit, also record if it has the NoNewPrivileges setting enabled */
+  if (system_unit)
+    set_property_bool (cx, "no_new_privileges", no_new_privs);
   set_property_bool (cx, "local", subject_is_local);
   set_property_bool (cx, "active", subject_is_active);
 
@@ -519,6 +532,7 @@ push_subject (duk_context               *cx,
     g_object_unref (process);
   free (session_str);
   free (seat_str);
+  free (system_unit);
   g_free (user_name);
   if (groups != NULL)
     g_ptr_array_unref (groups);
diff --git a/src/polkitbackend/polkitbackendjsauthority.cpp b/src/polkitbackend/polkitbackendjsauthority.cpp
index 7920d4d..8263de3 100644
--- a/src/polkitbackend/polkitbackendjsauthority.cpp
+++ b/src/polkitbackend/polkitbackendjsauthority.cpp
@@ -571,6 +571,7 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
                   GError                   **error)
 {
   gboolean ret = FALSE;
+  gboolean no_new_privs = FALSE;
   JS::CompileOptions options(authority->priv->cx);
   const char *src;
   JS::RootedObject obj(authority->priv->cx);
@@ -584,6 +585,7 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
   struct passwd *passwd;
   char *seat_str = NULL;
   char *session_str = NULL;
+  char *system_unit = NULL;
   JS::RootedObject global(authority->priv->cx, authority->priv->js_global->get ());
 
   src = "new Subject();";
@@ -699,6 +701,13 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
         }
     }
 
+  /* Query the unit, will work only if we got the pidfd from dbus-daemon/broker.
+   * Best-effort operation, will log on failure, but we don't bail here. But
+   * only do so if the pidfd was marked as safe, i.e.: we got it from D-Bus so
+   * it can be trusted end-to-end, with no reuse attack window.  */
+  if (polkit_unix_process_get_pidfd_is_safe (POLKIT_UNIX_PROCESS (process)))
+    polkit_backend_common_pidfd_to_systemd_unit (pidfd, &system_unit, &no_new_privs);
+
   /* In case we are using PIDFDs, check that the PID still matches to avoid race
    * conditions and PID recycle attacks.
    */
@@ -714,6 +723,10 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
   set_property_strv (authority, obj, "groups", groups);
   set_property_str (authority, obj, "seat", seat_str);
   set_property_str (authority, obj, "session", session_str);
+  set_property_str (authority, obj, "system_unit", system_unit);
+  /* If we have a unit, also record if it has the NoNewPrivileges setting enabled */
+  if (system_unit)
+    set_property_bool (authority, obj, "no_new_privileges", no_new_privs);
   set_property_bool (authority, obj, "local", subject_is_local);
   set_property_bool (authority, obj, "active", subject_is_active);
 
@@ -724,6 +737,7 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
     g_object_unref (process);
   free (session_str);
   free (seat_str);
+  free (system_unit);
   g_free (user_name);
   if (groups != NULL)
     g_ptr_array_unref (groups);
commit 8cabb1183aea59ccff125d0e2367fe5c8ac50b62
Author: Luca Boccassi <bluca at debian.org>
Date:   Thu Apr 6 01:25:16 2023 +0100

    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

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));
 }
 
 /**
@@ -496,6 +526,44 @@ polkit_unix_process_set_uid (PolkitUnixProcess *process,
   process->uid = uid;
 }
 
+/**
+ * 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;
 }


More information about the hal-commit mailing list