Enforce that calling CheckAuthorization() with pidfd also is passed a uid

When unix-process is used and a uid is not passed, it will be derived from
the running process. But this is racy, as the identity can change over
time (e.g.: setuid binaries can be exec'ed). If the caller can securely
fetch the pidfd of a process (e.g.: via SO_PEERPIDFD), then it can also
securely fetch the uid (e.g.: via SO_PEERCRED), so enforce that they are
passed together to avoid possible races, in case a polkit rule performs
authorization based on the uid.

Follow-up for 374a628038474a45f

3 files changed, 11 insertions, 2 deletions

diff --git a/data/org.freedesktop.PolicyKit1.Authority.xml b/data/org.freedesktop.PolicyKit1.Authority.xml
index f5b67fb..453ffc8 100644
--- a/data/org.freedesktop.PolicyKit1.Authority.xml
+++ b/data/org.freedesktop.PolicyKit1.Authority.xml
@@ -12,7 +12,7 @@
     <annotation name="org.gtk.EggDBus.DeclareStruct" value="Subject">
       <annotation name="org.gtk.EggDBus.DocString.Summary" value="Subjects"/>
       <annotation name="org.gtk.EggDBus.DocString" value="<para>This struct describes subjects such as UNIX processes. It is typically used to check if a given process is authorized for an action.</para><para>The following kinds of subjects are known:</para>
-                  <formalpara><title>Unix Process</title><para><literal>subject_kind</literal> should be set to <literal>unix-process</literal> with key <literal>pidfd</literal> (of type <literal>int32</literal>) (if the operating system supports ProcessID File Descriptors), or alternatively with keys <literal>pid</literal> (of type <literal>uint32</literal>) and <literal>start-time</literal> (of type <literal>uint64</literal>).</para></formalpara>
+                  <formalpara><title>Unix Process</title><para><literal>subject_kind</literal> should be set to <literal>unix-process</literal> with keys <literal>pidfd</literal> (of type <literal>int32</literal>) and <literal>uid</literal> (of type <literal>int32</literal>) (if the operating system supports ProcessID File Descriptors), or alternatively with keys <literal>pid</literal> (of type <literal>uint32</literal>), <literal>uid</literal> (of type <literal>int32</literal>) and <literal>start-time</literal> (of type <literal>uint64</literal>).</para></formalpara>
                   <formalpara><title>Unix Session</title><para><literal>subject_kind</literal> should be set to <literal>unix-session</literal> with the key <literal>session-id</literal> (of type <literal>string</literal>).</para></formalpara>
                   <formalpara><title>System Bus Name</title><para><literal>subject_kind</literal> should be set to <literal>system-bus-name</literal> with the key <literal>name</literal> (of type <literal>string</literal>).</para></formalpara>"/>
 
diff --git a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
index b5cc195..2dd01a0 100644
--- a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
+++ b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
@@ -275,7 +275,7 @@ The authority supports temporary authorizations that can be obtained through aut
 }
           </programlisting>
           <para>
-<para>This struct describes subjects such as UNIX processes. It is typically used to check if a given process is authorized for an action.</para><para>The following kinds of subjects are known:</para>                   <formalpara><title>Unix Process</title><para><literal>subject_kind</literal> should be set to <literal>unix-process</literal> with key <literal>pidfd</literal> (of type <literal>int32</literal>) (if the operating system supports ProcessID File Descriptors), or alternatively with keys <literal>pid</literal> (of type <literal>uint32</literal>) and <literal>start-time</literal> (of type <literal>uint64</literal>).</para></formalpara>                   <formalpara><title>Unix Session</title><para><literal>subject_kind</literal> should be set to <literal>unix-session</literal> with the key <literal>session-id</literal> (of type <literal>string</literal>).</para></formalpara>                   <formalpara><title>System Bus Name</title><para><literal>subject_kind</literal> should be set to <literal>system-bus-name</literal> with the key <literal>name</literal> (of type <literal>string</literal>).</para></formalpara>
+<para>This struct describes subjects such as UNIX processes. It is typically used to check if a given process is authorized for an action.</para><para>The following kinds of subjects are known:</para>                   <formalpara><title>Unix Process</title><para><literal>subject_kind</literal> should be set to <literal>unix-process</literal> with keys <literal>pidfd</literal> (of type <literal>int32</literal>) and <literal>uid</literal> (of type <literal>int32</literal>) (if the operating system supports ProcessID File Descriptors), or alternatively with keys <literal>pid</literal> (of type <literal>uint32</literal>), <literal>uid</literal> (of type <literal>int32</literal>) and <literal>start-time</literal> (of type <literal>uint64</literal>).</para></formalpara>                   <formalpara><title>Unix Session</title><para><literal>subject_kind</literal> should be set to <literal>unix-session</literal> with the key <literal>session-id</literal> (of type <literal>string</literal>).</para></formalpara>                   <formalpara><title>System Bus Name</title><para><literal>subject_kind</literal> should be set to <literal>system-bus-name</literal> with the key <literal>name</literal> (of type <literal>string</literal>).</para></formalpara>
           </para>
           <variablelist role="struct">
   <varlistentry>
diff --git a/src/polkit/polkitsubject.c b/src/polkit/polkitsubject.c
index e18b3e9..05c73c2 100644
--- a/src/polkit/polkitsubject.c
+++ b/src/polkit/polkitsubject.c
@@ -442,6 +442,15 @@ polkit_subject_new_for_gvariant_invocation (GVariant              *variant,
           v = lookup_asv (details_gvariant, "pidfd", G_VARIANT_TYPE_HANDLE, NULL);
           if (v != NULL)
             {
+              if (uid == -1)
+                {
+                  g_set_error (error,
+                               POLKIT_ERROR,
+                               POLKIT_ERROR_FAILED,
+                               "Error parsing unix-process subject: 'pidfd' specified withtout 'uid'");
+                  goto out;
+                }
+
               index = g_variant_get_handle (v);
               pidfd = g_unix_fd_list_get (fd_list, index, NULL);
               g_variant_unref (v);