PolicyKit: Branch 'master'

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Aug 17 13:52:50 UTC 2023


 meson.build                                             |    4 
 src/polkit/polkitunixprocess.c                          |  274 ++++++++++++++--
 src/polkit/polkitunixprocess.h                          |    5 
 src/polkitbackend/polkitbackendduktapeauthority.c       |   56 ++-
 src/polkitbackend/polkitbackendinteractiveauthority.c   |   19 +
 src/polkitbackend/polkitbackendjsauthority.cpp          |   41 +-
 src/polkitbackend/polkitbackendsessionmonitor-systemd.c |   32 +
 7 files changed, 386 insertions(+), 45 deletions(-)

New commits:
commit 7f0d792323cdca70b3d581cc9ed54df3d844a637
Author: Luca Boccassi <bluca at debian.org>
Date:   Thu Jan 19 01:49:57 2023 +0000

    polkitd: use PIDFDs if available to track processes
    
    PIDs can be recycled, so when possible it is best to try and use PIDFDs,
    which are pinned. Change polkitd's unixprocess class so that, if the
    PIDFD syscall is available, it does not store a PID but only the PIDFD,
    and gets the PID when required on the fly (which will intentionally fail
    if the process has disappeared, so that we avoid recycling races).
    
    In the future we will be able to get the PIDFD directly from D-Bus'
    GetConnectionCredentials() call, but for now get the FD from the PID.
    It does not completely close the window, but makes things significantly
    better already.

diff --git a/meson.build b/meson.build
index 40f8fa8..50d19c0 100644
--- a/meson.build
+++ b/meson.build
@@ -206,6 +206,8 @@ if enable_logind
 
   func = 'sd_uid_get_display'
   config_h.set10('HAVE_' + func.to_upper(), cc.has_function(func, dependencies: logind_dep))
+  func = 'sd_pidfd_get_session'
+  config_h.set10('HAVE_' + func.to_upper(), cc.has_function(func, dependencies: logind_dep))
 
   # systemd unit / service files
   systemd_systemdsystemunitdir = get_option('systemdsystemunitdir')
@@ -217,6 +219,8 @@ if enable_logind
 endif
 config_h.set('HAVE_LIBSYSTEMD', enable_logind)
 
+config_h.set('HAVE_PIDFD_OPEN', cc.get_define('SYS_pidfd_open', prefix: '#include <sys/syscall.h>') != '')
+
 # User for running polkitd
 polkitd_user = get_option('polkitd_user')
 config_h.set_quoted('POLKITD_USER', polkitd_user)
diff --git a/src/polkit/polkitunixprocess.c b/src/polkit/polkitunixprocess.c
index 289a82e..151085f 100644
--- a/src/polkit/polkitunixprocess.c
+++ b/src/polkit/polkitunixprocess.c
@@ -36,6 +36,9 @@
 #ifdef HAVE_OPENBSD
 #include <sys/sysctl.h>
 #endif
+#ifdef HAVE_PIDFD_OPEN
+#include <sys/syscall.h>
+#endif /* HAVE_PIDFD_OPEN */
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
@@ -51,9 +54,15 @@
  * @title: PolkitUnixProcess
  * @short_description: Unix processs
  *
- * An object for representing a UNIX process.  NOTE: This object as
- * designed is now known broken; a mechanism to exploit a delay in
- * start time in the Linux kernel was identified.  Avoid
+ * An object for representing a UNIX process. In order to be reliable and
+ * race-free, this requires support for PID File Descriptors in the kernel,
+ * dbus-daemon/broker and systemd. With this functionality, we can reliably
+ * track processes without risking PID reuse and race conditions, and compare
+ * them.
+ *
+ * NOTE: If PID FDs are not available, this object will fall back to using
+ * PIDs, and this designed is now known broken; a mechanism to exploit a delay
+ * in start time in the Linux kernel was identified.  Avoid
  * calling polkit_subject_equal() to compare two processes.
  *
  * To uniquely identify processes, both the process id and the start
@@ -147,6 +156,7 @@ struct _PolkitUnixProcess
   gint pid;
   guint64 start_time;
   gint uid;
+  gint pidfd;
 };
 
 struct _PolkitUnixProcessClass
@@ -159,7 +169,8 @@ enum
   PROP_0,
   PROP_PID,
   PROP_START_TIME,
-  PROP_UID
+  PROP_UID,
+  PROP_PIDFD,
 };
 
 static void subject_iface_init (PolkitSubjectIface *subject_iface);
@@ -184,6 +195,7 @@ static void
 polkit_unix_process_init (PolkitUnixProcess *unix_process)
 {
   unix_process->uid = -1;
+  unix_process->pidfd = -1;
 }
 
 static void
@@ -197,13 +209,17 @@ polkit_unix_process_get_property (GObject    *object,
   switch (prop_id)
     {
     case PROP_PID:
-      g_value_set_int (value, unix_process->pid);
+      g_value_set_int (value, polkit_unix_process_get_pid (unix_process));
       break;
 
     case PROP_UID:
       g_value_set_int (value, unix_process->uid);
       break;
 
+    case PROP_PIDFD:
+      g_value_set_int (value, unix_process->pidfd);
+      break;
+
     case PROP_START_TIME:
       g_value_set_uint64 (value, unix_process->start_time);
       break;
@@ -232,6 +248,10 @@ polkit_unix_process_set_property (GObject      *object,
       polkit_unix_process_set_uid (unix_process, g_value_get_int (value));
       break;
 
+    case PROP_PIDFD:
+      polkit_unix_process_set_pidfd (unix_process, g_value_get_int (value));
+      break;
+
     case PROP_START_TIME:
       polkit_unix_process_set_start_time (unix_process, g_value_get_uint64 (value));
       break;
@@ -242,15 +262,82 @@ polkit_unix_process_set_property (GObject      *object,
     }
 }
 
+static gint
+polkit_unix_process_get_pid_from_pidfd (PolkitUnixProcess  *process,
+                                        GError            **error)
+{
+  gint result;
+  gchar *contents;
+  gchar **lines;
+  gchar filename[64];
+  guint n;
+
+  g_return_val_if_fail (POLKIT_IS_UNIX_PROCESS (process), -1);
+  g_return_val_if_fail (error == NULL || *error == NULL, -1);
+  g_return_val_if_fail (process->pidfd >= 0, -1);
+
+  result = -1;
+  lines = NULL;
+  contents = NULL;
+
+  g_snprintf (filename, sizeof filename, "/proc/self/fdinfo/%d", process->pidfd);
+  if (!g_file_get_contents (filename,
+                            &contents,
+                            NULL,
+                            error))
+    goto out;
+
+  lines = g_strsplit (contents, "\n", -1);
+  for (n = 0; lines != NULL && lines[n] != NULL; n++)
+    {
+      gint pid;
+      if (!g_str_has_prefix (lines[n], "Pid:"))
+        continue;
+      if (sscanf (lines[n] + 4, "%d", &pid) != 1)
+        g_set_error (error,
+                      POLKIT_ERROR,
+                      POLKIT_ERROR_FAILED,
+                      "Unexpected line `%s' in file %s",
+                      lines[n],
+                      filename);
+      else
+        result = pid;
+      goto out;
+    }
+
+  g_set_error (error,
+               POLKIT_ERROR,
+               POLKIT_ERROR_FAILED,
+               "Didn't find any line starting with `Pid:' in file %s",
+               filename);
+
+out:
+  g_strfreev (lines);
+  g_free (contents);
+  return result;
+}
+
 static void
 polkit_unix_process_constructed (GObject *object)
 {
   PolkitUnixProcess *process = POLKIT_UNIX_PROCESS (object);
 
-  /* sets start_time and uid in case they are unset */
+  /* sets pidfd, start_time and uid in case they are unset */
+
+#ifdef HAVE_PIDFD_OPEN
+  if (process->pid > 0 && process->pidfd < 0)
+    {
+      gint pidfd = (int) syscall (SYS_pidfd_open, process->pid, 0);
+      if (pidfd >= 0)
+        {
+          process->pidfd = pidfd;
+          process->pid = 0;
+        }
+    }
+#endif /* HAVE_PIDFD_OPEN */
 
   if (process->start_time == 0)
-    process->start_time = get_start_time_for_pid (process->pid, NULL);
+    process->start_time = get_start_time_for_pid (polkit_unix_process_get_pid (process), NULL);
 
   if (process->uid == -1)
     {
@@ -268,6 +355,21 @@ polkit_unix_process_constructed (GObject *object)
     G_OBJECT_CLASS (polkit_unix_process_parent_class)->constructed (object);
 }
 
+static void
+polkit_unix_process_finalize (GObject *object)
+{
+  PolkitUnixProcess *process = POLKIT_UNIX_PROCESS (object);
+
+  if (process->pidfd >= 0)
+    {
+      close (process->pidfd);
+      process->pidfd = -1;
+    }
+
+  if (G_OBJECT_CLASS (polkit_unix_process_parent_class)->finalize != NULL)
+    G_OBJECT_CLASS (polkit_unix_process_parent_class)->finalize (object);
+}
+
 static void
 polkit_unix_process_class_init (PolkitUnixProcessClass *klass)
 {
@@ -276,6 +378,7 @@ polkit_unix_process_class_init (PolkitUnixProcessClass *klass)
   gobject_class->get_property = polkit_unix_process_get_property;
   gobject_class->set_property = polkit_unix_process_set_property;
   gobject_class->constructed =  polkit_unix_process_constructed;
+  gobject_class->finalize =     polkit_unix_process_finalize;
 
   /**
    * PolkitUnixProcess:pid:
@@ -336,6 +439,24 @@ polkit_unix_process_class_init (PolkitUnixProcessClass *klass)
                                                         G_PARAM_STATIC_BLURB |
                                                         G_PARAM_STATIC_NICK));
 
+  /**
+   * PolkitUnixProcess:pidfd:
+   *
+   * The UNIX process id file descriptor.
+   */
+  g_object_class_install_property (gobject_class,
+                                   PROP_PIDFD,
+                                   g_param_spec_int ("pidfd",
+                                                     "Process ID FD",
+                                                     "The UNIX process ID file descriptor",
+                                                     -1,
+                                                     G_MAXINT,
+                                                     -1,
+                                                     G_PARAM_CONSTRUCT |
+                                                     G_PARAM_READWRITE |
+                                                     G_PARAM_STATIC_NAME |
+                                                     G_PARAM_STATIC_BLURB |
+                                                     G_PARAM_STATIC_NICK));
 }
 
 /**
@@ -387,6 +508,19 @@ gint
 polkit_unix_process_get_pid (PolkitUnixProcess *process)
 {
   g_return_val_if_fail (POLKIT_IS_UNIX_PROCESS (process), 0);
+
+  if (process->pidfd >= 0)
+    {
+      GError *error = NULL;
+      gint pid = polkit_unix_process_get_pid_from_pidfd(process, &error);
+
+      if (pid > 0)
+        return pid;
+
+      g_error_free (error);
+      return -1;
+    }
+
   return process->pid;
 }
 
@@ -432,9 +566,60 @@ polkit_unix_process_set_pid (PolkitUnixProcess *process,
                              gint              pid)
 {
   g_return_if_fail (POLKIT_IS_UNIX_PROCESS (process));
+
+#ifdef HAVE_PIDFD_OPEN
+  if (process->pidfd >= 0)
+    {
+      close (process->pidfd);
+      process->pidfd = -1;
+    }
+  if (pid > 0)
+    {
+      gint pidfd = (int) syscall (SYS_pidfd_open, process->pid, 0);
+      if (pidfd >= 0)
+        {
+          process->pidfd = pidfd;
+          process->pid = 0;
+          return;
+        }
+    }
+#endif /* HAVE_PIDFD_OPEN */
+
   process->pid = pid;
 }
 
+/**
+ * polkit_unix_process_get_pidfd:
+ * @process: A #PolkitUnixProcess.
+ *
+ * Gets the process id file descriptor for @process.
+ *
+ * Returns: The process id file descriptor for @process.
+ */
+gint
+polkit_unix_process_get_pidfd (PolkitUnixProcess *process)
+{
+  g_return_val_if_fail (POLKIT_IS_UNIX_PROCESS (process), -1);
+  return process->pidfd;
+}
+
+/**
+ * polkit_unix_process_set_pidfd:
+ * @process: A #PolkitUnixProcess.
+ * @pidfd: A process id file descriptor.
+ *
+ * Sets @pidfd for @process.
+ */
+void
+polkit_unix_process_set_pidfd (PolkitUnixProcess *process,
+                               gint               pidfd)
+{
+  g_return_if_fail (POLKIT_IS_UNIX_PROCESS (process));
+  if (process->pidfd >= 0)
+    close (process->pidfd);
+  process->pidfd = pidfd;
+}
+
 /**
  * polkit_unix_process_new:
  * @pid: The process id.
@@ -500,12 +685,31 @@ polkit_unix_process_new_for_owner (gint    pid,
                                        NULL));
 }
 
+/**
+ * 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>.
+ *
+ * Creates a new #PolkitUnixProcess object for @pidfd and @uid.
+ *
+ * Returns: (transfer full): A #PolkitSubject. Free with g_object_unref().
+ */
+PolkitSubject *
+polkit_unix_process_new_pidfd (gint    pidfd,
+                               gint    uid)
+{
+  return POLKIT_SUBJECT (g_object_new (POLKIT_TYPE_UNIX_PROCESS,
+                                       "pidfd", pidfd,
+                                       "uid", uid,
+                                       NULL));
+}
+
 static guint
 polkit_unix_process_hash (PolkitSubject *subject)
 {
   PolkitUnixProcess *process = POLKIT_UNIX_PROCESS (subject);
 
-  return g_direct_hash (GSIZE_TO_POINTER ((process->pid + process->start_time))) ;
+  return g_direct_hash (GSIZE_TO_POINTER ((polkit_unix_process_get_pid(process) + process->start_time))) ;
 }
 
 static gboolean
@@ -514,21 +718,35 @@ polkit_unix_process_equal (PolkitSubject *a,
 {
   PolkitUnixProcess *process_a;
   PolkitUnixProcess *process_b;
+  gint pid_a, pid_b;
+  gint pidfd_a, pidfd_b;
 
   process_a = POLKIT_UNIX_PROCESS (a);
   process_b = POLKIT_UNIX_PROCESS (b);
 
+  pid_a = polkit_unix_process_get_pid(process_a);
+  pid_b = polkit_unix_process_get_pid(process_b);
+
+  pidfd_a = polkit_unix_process_get_pidfd(process_a);
+  pidfd_b = polkit_unix_process_get_pidfd(process_b);
+
   return
-    (process_a->pid == process_b->pid) &&
-    (process_a->start_time == process_b->start_time);
+    (pid_a > 0) &&
+    (pid_b > 0) &&
+    (pid_a == pid_b) &&
+    ((pidfd_a >= 0 && pidfd_b >= 0) ||
+     (process_a->start_time == process_b->start_time));
 }
 
 static gchar *
 polkit_unix_process_to_string (PolkitSubject *subject)
 {
   PolkitUnixProcess *process = POLKIT_UNIX_PROCESS (subject);
+  gint pid = polkit_unix_process_get_pid(process);
+  if (pid <= 0)
+    return g_strdup_printf ("unix-process:unknown");
 
-  return g_strdup_printf ("unix-process:%d:%" G_GUINT64_FORMAT, process->pid, process->start_time);
+  return g_strdup_printf ("unix-process:%d:%" G_GUINT64_FORMAT, pid, process->start_time);
 }
 
 static gboolean
@@ -540,11 +758,21 @@ polkit_unix_process_exists_sync (PolkitSubject   *subject,
   GError *local_error;
   guint64 start_time;
   gboolean ret;
+  gint pid;
 
   ret = TRUE;
 
+  pid = polkit_unix_process_get_pid(process);
+  if (pid <= 0)
+    return FALSE;
+
+  /* If we have both a valid PID and a PID FD then we know the process is still the
+   * same and it hasn't exited. */
+  if (polkit_unix_process_get_pidfd(process) >= 0)
+    return TRUE;
+
   local_error = NULL;
-  start_time = get_start_time_for_pid (process->pid, &local_error);
+  start_time = get_start_time_for_pid (pid, &local_error);
   if (local_error != NULL)
     {
       /* Don't propagate the error - it just means there is no process with this pid */
@@ -797,7 +1025,7 @@ gint
 polkit_unix_process_get_racy_uid__ (PolkitUnixProcess  *process,
                                     GError            **error)
 {
-  gint result;
+  gint result, pid;
   gchar *contents;
   gchar **lines;
   guint64 start_time;
@@ -818,14 +1046,24 @@ polkit_unix_process_get_racy_uid__ (PolkitUnixProcess  *process,
   lines = NULL;
   contents = NULL;
 
+  pid = polkit_unix_process_get_pid(process);
+  if (pid <= 0)
+    {
+      g_set_error (error,
+                   POLKIT_ERROR,
+                   POLKIT_ERROR_FAILED,
+                   "Process not found");
+      goto out;
+    }
+
 #if defined(HAVE_FREEBSD) || defined(HAVE_NETBSD) || defined(HAVE_OPENBSD)
-  if (get_kinfo_proc (process->pid, &p) == 0)
+  if (get_kinfo_proc (pid, &p) == 0)
     {
       g_set_error (error,
                    POLKIT_ERROR,
                    POLKIT_ERROR_FAILED,
                    "get_kinfo_proc() failed for pid %d: %s",
-                   process->pid,
+                   pid,
                    g_strerror (errno));
       goto out;
     }
@@ -843,7 +1081,7 @@ polkit_unix_process_get_racy_uid__ (PolkitUnixProcess  *process,
    *
    * Uid, Gid: Real, effective, saved set,  and  file  system  UIDs (GIDs).
    */
-  g_snprintf (filename, sizeof filename, "/proc/%d/status", process->pid);
+  g_snprintf (filename, sizeof filename, "/proc/%d/status", pid);
   if (!g_file_get_contents (filename,
                             &contents,
                             NULL,
@@ -886,7 +1124,7 @@ found:
    * before and after reading the UID, it couldn't have changed.
    */
   local_error = NULL;
-  start_time = get_start_time_for_pid (process->pid, &local_error);
+  start_time = get_start_time_for_pid (pid, &local_error);
   if (local_error != NULL)
     {
       g_propagate_error (error, local_error);
@@ -897,7 +1135,7 @@ found:
   if (process->start_time != start_time)
     {
       g_set_error (error, POLKIT_ERROR, POLKIT_ERROR_FAILED,
-		   "process with PID %d has been replaced", process->pid);
+		   "process with PID %d has been replaced", pid);
       goto out;
     }
 
diff --git a/src/polkit/polkitunixprocess.h b/src/polkit/polkitunixprocess.h
index f5ed1a7..9ac5bc9 100644
--- a/src/polkit/polkitunixprocess.h
+++ b/src/polkit/polkitunixprocess.h
@@ -55,6 +55,8 @@ PolkitSubject  *polkit_unix_process_new_full       (gint               pid,
 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            polkit_unix_process_get_pid        (PolkitUnixProcess *process);
 guint64         polkit_unix_process_get_start_time (PolkitUnixProcess *process);
 gint            polkit_unix_process_get_uid        (PolkitUnixProcess *process);
@@ -67,6 +69,9 @@ void            polkit_unix_process_set_start_time (PolkitUnixProcess *process,
 
 gint            polkit_unix_process_get_owner      (PolkitUnixProcess  *process,
                                                     GError            **error) G_GNUC_DEPRECATED_FOR (polkit_unix_process_get_uid);
+gint            polkit_unix_process_get_pidfd      (PolkitUnixProcess  *process);
+void            polkit_unix_process_set_pidfd      (PolkitUnixProcess  *process,
+                                                    gint                pidfd);
 
 G_END_DECLS
 
diff --git a/src/polkitbackend/polkitbackendduktapeauthority.c b/src/polkitbackend/polkitbackendduktapeauthority.c
index dc13971..d6baf11 100644
--- a/src/polkitbackend/polkitbackendduktapeauthority.c
+++ b/src/polkitbackend/polkitbackendduktapeauthority.c
@@ -366,8 +366,10 @@ push_subject (duk_context               *cx,
               GError                   **error)
 {
   gboolean ret = FALSE;
-  pid_t pid;
+  gint pidfd = -1;
+  pid_t pid_early, pid_late;
   uid_t uid;
+  PolkitSubject *process = NULL;
   gchar *user_name = NULL;
   GPtrArray *groups = NULL;
   struct passwd *passwd;
@@ -382,30 +384,31 @@ push_subject (duk_context               *cx,
 
   if (POLKIT_IS_UNIX_PROCESS (subject))
     {
-      pid = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (subject));
+      process = subject;
     }
   else if (POLKIT_IS_SYSTEM_BUS_NAME (subject))
     {
-      PolkitSubject *process;
       process = polkit_system_bus_name_get_process_sync (POLKIT_SYSTEM_BUS_NAME (subject), NULL, error);
       if (process == NULL)
         goto out;
-      pid = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (process));
-      g_object_unref (process);
     }
   else
     {
       g_assert_not_reached ();
     }
 
+  pid_early = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (process));
+  pidfd = polkit_unix_process_get_pidfd (POLKIT_UNIX_PROCESS (process));
+
 #ifdef HAVE_LIBSYSTEMD
-  if (sd_pid_get_session (pid, &session_str) == 0)
-    {
-      if (sd_session_get_seat (session_str, &seat_str) == 0)
-        {
-          /* do nothing */
-        }
-    }
+#if HAVE_SD_PIDFD_GET_SESSION
+  if (pidfd >= 0)
+    sd_pidfd_get_session (pidfd, &session_str);
+  else
+#endif /* HAVE_SD_PIDFD_GET_SESSION */
+    sd_pid_get_session (pid_early, &session_str);
+  if (session_str)
+    sd_session_get_seat (session_str, &seat_str);
 #endif /* HAVE_LIBSYSTEMD */
 
   g_assert (POLKIT_IS_UNIX_USER (user_for_subject));
@@ -452,7 +455,32 @@ push_subject (duk_context               *cx,
         }
     }
 
-  set_property_int32 (cx, "pid", pid);
+  /* In case we are using PIDFDs, check that the PID still matches to avoid race
+   * conditions and PID recycle attacks.
+   */
+  pid_late = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (process));
+  if (pid_late != pid_early)
+    {
+      if (pid_late == -1)
+        {
+          g_warning ("Process %d terminated", (gint) pid_early);
+          g_set_error (error,
+                       POLKIT_ERROR,
+                       POLKIT_ERROR_FAILED,
+                       "Process %d terminated", (gint) pid_early);
+        }
+      else
+      {
+        g_warning ("Process changed pid from %d to %d", (gint) pid_early, (gint) pid_late);
+        g_set_error (error,
+                     POLKIT_ERROR,
+                     POLKIT_ERROR_FAILED,
+                     "Process changed pid from %d to %d", (gint) pid_early, (gint) pid_late);
+      }
+      goto out;
+    }
+
+  set_property_int32 (cx, "pid", pid_early);
   set_property_str (cx, "user", user_name);
   set_property_strv (cx, "groups", groups);
   set_property_str (cx, "seat", seat_str);
@@ -463,6 +491,8 @@ push_subject (duk_context               *cx,
   ret = TRUE;
 
  out:
+  if (POLKIT_IS_SYSTEM_BUS_NAME (subject))
+    g_object_unref (process);
   free (session_str);
   free (seat_str);
   g_free (user_name);
diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c
index 0baab54..f4be303 100644
--- a/src/polkitbackend/polkitbackendinteractiveauthority.c
+++ b/src/polkitbackend/polkitbackendinteractiveauthority.c
@@ -513,6 +513,11 @@ _polkit_subject_get_cmdline (PolkitSubject *subject)
     }
 
   pid = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (process));
+  if (pid <= 0)
+    {
+      g_debug ("Process is no longer active, cannot fetch cmdline.");
+      goto out;
+    }
 
   filename = g_strdup_printf ("/proc/%d/cmdline", pid);
 
@@ -3090,9 +3095,21 @@ subject_equal_for_authz (PolkitSubject *a,
     return FALSE;
 
   /* Now special case unix processes, as we want to protect against
-   * pid reuse by including the UID.
+   * pid reuse by including the PID FDs or UIDs as a fallback.
    */
   if (POLKIT_IS_UNIX_PROCESS (a) && POLKIT_IS_UNIX_PROCESS (b)) {
+    /* If both objects are tracking via PID FD then we can rely on that,
+     * as the PID is resolved on-the-fly via the pinned file descriptor,
+     * and it will be -1 if the process exited in the meanwhile. */
+    if (polkit_unix_process_get_pidfd ((PolkitUnixProcess*)a) >= 0 &&
+        polkit_unix_process_get_pidfd ((PolkitUnixProcess*)b) >= 0)
+      {
+        int pid_a = polkit_unix_process_get_pid ((PolkitUnixProcess*)a);
+        int pid_b = polkit_unix_process_get_pid ((PolkitUnixProcess*)b);
+
+        return pid_a > 0 && pid_b > 0 && pid_a == pid_b;
+      }
+
     int uid_a = polkit_unix_process_get_uid ((PolkitUnixProcess*)a);
     int uid_b = polkit_unix_process_get_uid ((PolkitUnixProcess*)b);
 
diff --git a/src/polkitbackend/polkitbackendjsauthority.cpp b/src/polkitbackend/polkitbackendjsauthority.cpp
index e265234..a577c1c 100644
--- a/src/polkitbackend/polkitbackendjsauthority.cpp
+++ b/src/polkitbackend/polkitbackendjsauthority.cpp
@@ -574,8 +574,10 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
   JS::CompileOptions options(authority->priv->cx);
   const char *src;
   JS::RootedObject obj(authority->priv->cx);
-  pid_t pid;
+  gint pidfd = -1;
+  pid_t pid_early, pid_late;
   uid_t uid;
+  PolkitSubject *process = NULL;
   gchar *user_name = NULL;
   GPtrArray *groups = NULL;
   struct passwd *passwd;
@@ -602,30 +604,31 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
 
   if (POLKIT_IS_UNIX_PROCESS (subject))
     {
-      pid = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (subject));
+      process = subject;
     }
   else if (POLKIT_IS_SYSTEM_BUS_NAME (subject))
     {
-      PolkitSubject *process;
       process = polkit_system_bus_name_get_process_sync (POLKIT_SYSTEM_BUS_NAME (subject), NULL, error);
       if (process == NULL)
         goto out;
-      pid = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (process));
-      g_object_unref (process);
     }
   else
     {
       g_assert_not_reached ();
     }
 
+  pid_early = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (process));
+  pidfd = polkit_unix_process_get_pidfd (POLKIT_UNIX_PROCESS (process));
+
 #ifdef HAVE_LIBSYSTEMD
-  if (sd_pid_get_session (pid, &session_str) == 0)
-    {
-      if (sd_session_get_seat (session_str, &seat_str) == 0)
-        {
-          /* do nothing */
-        }
-    }
+#if HAVE_SD_PIDFD_GET_SESSION
+  if (pidfd >= 0)
+    sd_pidfd_get_session (pidfd, &session_str);
+  else
+#endif /* HAVE_SD_PIDFD_GET_SESSION */
+    sd_pid_get_session (pid_early, &session_str);
+  if (session_str)
+    sd_session_get_seat (session_str, &seat_str);
 #endif /* HAVE_LIBSYSTEMD */
 
   g_assert (POLKIT_IS_UNIX_USER (user_for_subject));
@@ -672,7 +675,17 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
         }
     }
 
-  set_property_int32 (authority, obj, "pid", pid);
+  /* In case we are using PIDFDs, check that the PID still matches to avoid race
+   * conditions and PID recycle attacks.
+   */
+  pid_late = polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (process));
+  if (pid_late != pid_early)
+    {
+      g_warning ("pid changed from %d to %d, ignoring", (gint) pid_early, (gint) pid_late);
+      pid_early = -1;
+    }
+
+  set_property_int32 (authority, obj, "pid", pid_early);
   set_property_str (authority, obj, "user", user_name);
   set_property_strv (authority, obj, "groups", groups);
   set_property_str (authority, obj, "seat", seat_str);
@@ -683,6 +696,8 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
   ret = TRUE;
 
  out:
+  if (POLKIT_IS_SYSTEM_BUS_NAME (subject))
+    g_object_unref (process);
   free (session_str);
   free (seat_str);
   g_free (user_name);
diff --git a/src/polkitbackend/polkitbackendsessionmonitor-systemd.c b/src/polkitbackend/polkitbackendsessionmonitor-systemd.c
index b00cdbd..1a6107a 100644
--- a/src/polkitbackend/polkitbackendsessionmonitor-systemd.c
+++ b/src/polkitbackend/polkitbackendsessionmonitor-systemd.c
@@ -351,6 +351,9 @@ polkit_backend_session_monitor_get_session_for_subject (PolkitBackendSessionMoni
 #if HAVE_SD_UID_GET_DISPLAY
   uid_t uid;
 #endif
+#if HAVE_SD_PIDFD_GET_SESSION
+  int pidfd;
+#endif
 
   if (POLKIT_IS_UNIX_PROCESS (subject))
     process = POLKIT_UNIX_PROCESS (subject); /* We already have a process */
@@ -371,6 +374,19 @@ polkit_backend_session_monitor_get_session_for_subject (PolkitBackendSessionMoni
                    g_type_name (G_TYPE_FROM_INSTANCE (subject)));
     }
 
+#if HAVE_SD_PIDFD_GET_SESSION
+  /* First try to get the session from the pidfd (systemd version 253) */
+  pidfd = polkit_unix_process_get_pidfd (process);
+  if (pidfd >= 0)
+    {
+      if (sd_pidfd_get_session (pidfd, &session_id) >= 0)
+        {
+          session = polkit_unix_session_new (session_id);
+          goto out;
+        }
+    }
+#endif
+
   /* Now do process -> pid -> same session */
   g_assert (process != NULL);
   pid = polkit_unix_process_get_pid (process);
@@ -381,6 +397,22 @@ polkit_backend_session_monitor_get_session_for_subject (PolkitBackendSessionMoni
       goto out;
     }
 
+#if HAVE_SD_PIDFD_GET_SESSION
+  /* Now do process fd -> uid -> graphical session (systemd version 253) */
+  pidfd = polkit_unix_process_get_pidfd (process);
+  if (pidfd >= 0)
+    {
+      if (sd_pidfd_get_owner_uid (pidfd, &uid) < 0)
+        goto out;
+
+      if (sd_uid_get_display (uid, &session_id) >= 0)
+        {
+          session = polkit_unix_session_new (session_id);
+          goto out;
+        }
+    }
+#endif
+
 #if HAVE_SD_UID_GET_DISPLAY
   /* Now do process -> uid -> graphical session (systemd version 213)*/
   if (sd_pid_get_owner_uid (pid, &uid) < 0)


More information about the hal-commit mailing list