CVE-2015-4625: Bind use of cookies to specific uids

http://lists.freedesktop.org/archives/polkit-devel/2015-June/000425.html

The "cookie" value that Polkit hands out is global to all polkit
users.  And when `AuthenticationAgentResponse` is invoked, we
previously only received the cookie and *target* identity, and
attempted to find an agent from that.

The problem is that the current cookie is just an integer
counter, and if it overflowed, it would be possible for
an successful authorization in one session to trigger a response
in another session.

The overflow and ability to guess the cookie were fixed by the
previous patch.

This patch is conceptually further hardening on top of that.  Polkit
currently treats uids as equivalent from a security domain
perspective; there is no support for
SELinux/AppArmor/etc. differentiation.

We can retrieve the uid from `getuid()` in the setuid helper, which
allows us to ensure the uid invoking `AuthenticationAgentResponse2`
matches that of the agent.

Then the authority only looks at authentication sessions matching the
cookie that were created by a matching uid, thus removing the ability
for different uids to interfere with each other entirely.

Several fixes to this patch were contributed by:
Miloslav Trmač <mitr@redhat.com>

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90837
CVE: CVE-2015-4625
Reported-by: Tavis Ormandy <taviso@google.com>
Reviewed-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Colin Walters <walters@redhat.com>

2 files changed, 55 insertions, 9 deletions

diff --git a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
index 6525e25..e66bf53 100644
--- a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
+++ b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
@@ -42,6 +42,8 @@ Structure    <link linkend="eggdbus-struct-TemporaryAuthorization">TemporaryAuth
                                   IN  String                         object_path)
 <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse</link>      (IN  String                         cookie,
                                   IN  <link linkend="eggdbus-struct-Identity">Identity</link>                       identity)
+<link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse2</link>      (IN uint32 uid, IN  String                         cookie,
+                                  IN  <link linkend="eggdbus-struct-Identity">Identity</link>                       identity)
 <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.EnumerateTemporaryAuthorizations">EnumerateTemporaryAuthorizations</link> (IN  <link linkend="eggdbus-struct-Subject">Subject</link>                        subject,
                                   OUT Array&lt;<link linkend="eggdbus-struct-TemporaryAuthorization">TemporaryAuthorization</link>&gt;  temporary_authorizations)
 <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.RevokeTemporaryAuthorizations">RevokeTemporaryAuthorizations</link>    (IN  <link linkend="eggdbus-struct-Subject">Subject</link>                        subject)
@@ -777,10 +779,52 @@ AuthenticationAgentResponse (IN  String    cookie,
                              IN  <link linkend="eggdbus-struct-Identity">Identity</link>  identity)
     </programlisting>
     <para>
-Method for authentication agents to invoke on successful authentication. This method will fail unless a sufficiently privileged caller invokes it.
+Method for authentication agents to invoke on successful
+authentication, intended only for use by a privileged helper process
+internal to polkit.  Deprecated in favor of AuthenticationAgentResponse2.
+    </para>
+<variablelist role="params">
+  <varlistentry>
+    <term><literal>IN  String <parameter>cookie</parameter></literal>:</term>
+    <listitem>
+      <para>
+The cookie identifying the authentication request that was passed to the authentication agent.
+      </para>
+    </listitem>
+  </varlistentry>
+  <varlistentry>
+    <term><literal>IN  <link linkend="eggdbus-struct-Identity">Identity</link> <parameter>identity</parameter></literal>:</term>
+    <listitem>
+      <para>
+A <link linkend="eggdbus-struct-Identity">Identity</link> struct describing what identity was authenticated.
+      </para>
+    </listitem>
+  </varlistentry>
+</variablelist>
+    </refsect2>
+    <refsect2 role="function" id="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2">
+      <title>AuthenticationAgentResponse2 ()</title>
+    <programlisting>
+AuthenticationAgentResponse2 (IN  uint32 uid,
+                              IN  String cookie,
+                              IN  <link linkend="eggdbus-struct-Identity">Identity</link>  identity)
+    </programlisting>
+    <para>
+Method for authentication agents to invoke on successful
+authentication, intended only for use by a privileged helper process
+internal to polkit.  Note this method was introduced in 0.114 to fix a security issue.
     </para>
 <variablelist role="params">
   <varlistentry>
+    <term><literal>IN  uint32 <parameter>uid</parameter></literal>:</term>
+    <listitem>
+      <para>
+The user id of the agent; normally this is the owner of the parent pid
+of the process that invoked the internal setuid helper.
+      </para>
+    </listitem>
+  </varlistentry>
+  <varlistentry>
     <term><literal>IN  String <parameter>cookie</parameter></literal>:</term>
     <listitem>
       <para>
diff --git a/docs/polkit/overview.xml b/docs/polkit/overview.xml
index 150a7bc..176d2ea 100644
--- a/docs/polkit/overview.xml
+++ b/docs/polkit/overview.xml
@@ -314,16 +314,18 @@
     <para>
       Authentication agents are provided by desktop environments. When
       an user session starts, the agent registers with the polkit
-      Authority using
-      the <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.RegisterAuthenticationAgent">RegisterAuthenticationAgent()</link>
+      Authority using the <link
+      linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.RegisterAuthenticationAgent">RegisterAuthenticationAgent()</link>
       method. When services are needed, the authority will invoke
-      methods on
-      the <link linkend="eggdbus-interface-org.freedesktop.PolicyKit1.AuthenticationAgent">org.freedesktop.PolicyKit1.AuthenticationAgent</link>
+      methods on the <link
+      linkend="eggdbus-interface-org.freedesktop.PolicyKit1.AuthenticationAgent">org.freedesktop.PolicyKit1.AuthenticationAgent</link>
       D-Bus interface. Once the user is authenticated, (a privileged
-      part of) the agent invokes
-      the <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse()</link>
-      method.  Note that the polkit Authority itself does not care
-      how the agent authenticates the user.
+      part of) the agent invokes the <link
+      linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse()</link>
+      method.  This method should be treated as an internal
+      implementation detail, and callers should use the public shared
+      library API to invoke it, which currently uses a setuid helper
+      program.
     </para>
     <para>
       The <link linkend="ref-authentication-agent-api">libpolkit-agent-1</link>