PolicyKit: Branch 'master' - 2 commits

Colin Walters walters at kemper.freedesktop.org
Wed Jun 17 11:04:11 PDT 2015


 data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml                          |   14 ++
 data/org.freedesktop.PolicyKit1.Authority.xml                                    |   29 ++++
 docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.AuthenticationAgent.xml |    7 -
 docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml           |   52 ++++++++
 docs/polkit/overview.xml                                                         |   18 +-
 src/polkit/polkitauthority.c                                                     |   37 +++++
 src/polkitagent/polkitagentlistener.c                                            |    5 
 src/polkitbackend/polkitbackendauthority.c                                       |   62 +++++++++-
 src/polkitbackend/polkitbackendauthority.h                                       |    2 
 src/polkitbackend/polkitbackendinteractiveauthority.c                            |   39 +++++-
 10 files changed, 236 insertions(+), 29 deletions(-)

New commits:
commit fb5076b7c05d01a532d593a4079a29cf2d63a228
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Wed Jun 17 01:01:27 2015 +0200

    docs: Update for changes to uid binding/AuthenticationAgentResponse2
    
     - Refer to PolkitAgentSession in general instead of to _response only
     - Revert to the original description of authentication cancellation, the
       agent really needs to return an error to the caller (in addition to dealing
       with the session if any).
     - Explicitly document the UID assumption; in the process fixing bug #69980.
     - Keep documenting that we need a sufficiently privileged caller.
     - Refer to the ...Response2 API in more places.
     - Also update docbook documentation.
     - Drop a paragraph suggesting non-PolkitAgentSession implementations are
       expected and commonplace.
    
    Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90837
    Reviewed-by: Colin Walters <walters at redhat.com>

diff --git a/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml b/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
index 5beef7d..482332f 100644
--- a/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
+++ b/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
@@ -13,14 +13,14 @@
       user to authenticate as one of the identities in @identities for
       the action with the identifier @action_id.</para><para>This
       authentication is normally achieved via the
-      polkit_agent_session_response() API, which invokes a private
+      PolkitAgentSession API, which invokes a private
       setuid helper process to verify the authentication.  When
       successful, it calls the
       org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2()
       method on the #org.freedesktop.PolicyKit1.Authority interface of
       the PolicyKit daemon before returning.  If the user dismisses the
-      authentication dialog, the authentication agent should call
-      polkit_agent_session_cancel().</para>"/>
+      authentication dialog, the authentication agent should return an
+      error.</para>"/>
 
       <arg name="action_id" direction="in" type="s">
         <annotation name="org.gtk.EggDBus.DocString" value="The identifier for the action that the user is authentication for."/>
diff --git a/data/org.freedesktop.PolicyKit1.Authority.xml b/data/org.freedesktop.PolicyKit1.Authority.xml
index f9021ee..88da3c0 100644
--- a/data/org.freedesktop.PolicyKit1.Authority.xml
+++ b/data/org.freedesktop.PolicyKit1.Authority.xml
@@ -283,7 +283,7 @@
     <!-- ---------------------------------------------------------------------------------------------------- -->
 
     <method name="RegisterAuthenticationAgent">
-      <annotation name="org.gtk.EggDBus.DocString" value="<para>Register an authentication agent.</para><para>Note that current versions of PolicyKit will only work if @session_id is set to the empty string. In the future it might work for non-empty strings if the caller is sufficiently privileged.</para>"/>
+      <annotation name="org.gtk.EggDBus.DocString" value="<para>Register an authentication agent.</para><para>Note that this should be called by the same effective UID which will be passed to org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2().</para>"/>
 
       <arg name="subject" direction="in" type="(sa{sv})">
         <annotation name="org.gtk.EggDBus.Type" value="Subject"/>
@@ -315,7 +315,8 @@
     <method name="AuthenticationAgentResponse">
       <annotation name="org.gtk.EggDBus.DocString" value="Method for authentication agents to invoke on successful
 authentication, intended only for use by a privileged helper process
-internal to polkit."/>
+internal to polkit. This method will fail unless a sufficiently privileged
+caller invokes it. Deprecated in favor of org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2."/>
 
       <arg name="cookie" direction="in" type="s">
         <annotation name="org.gtk.EggDBus.DocString" value="The cookie identifying the authentication request that was passed to the authentication agent."/>
@@ -330,11 +331,13 @@ internal to polkit."/>
     <method name="AuthenticationAgentResponse2">
       <annotation name="org.gtk.EggDBus.DocString" value="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 added in 0.114, and should be preferred over AuthenticationAgentResponse
+internal to polkit. This method will fail unless a sufficiently privileged
+caller invokes it. Note this method was added in 0.114, and should be preferred over org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse()
 as it fixes a security issue."/>
 
       <arg name="uid" direction="in" type="u">
-        <annotation name="org.gtk.EggDBus.DocString" value="The real uid of the agent.  Normally set by the setuid helper program."/>
+        <annotation name="org.gtk.EggDBus.DocString" value="The real uid of the agent.  Normally set by the setuid helper program.
+Must match the effective UID of the caller of org.freedesktop.PolicyKit1.Authority.RegisterAuthenticationAgent()."/>
       </arg>
 
       <arg name="cookie" direction="in" type="s">
diff --git a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.AuthenticationAgent.xml b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.AuthenticationAgent.xml
index ec59626..ab27b2f 100644
--- a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.AuthenticationAgent.xml
+++ b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.AuthenticationAgent.xml
@@ -47,10 +47,13 @@ BeginAuthentication (IN  String               action_id,
         identifier <parameter>action_id</parameter>.</para><para>Upon
         succesful authentication, the authentication agent must invoke
         the <link
-        linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse()</link>
+        linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2">AuthenticationAgentResponse2()</link>
         method on the <link
         linkend="eggdbus-interface-org.freedesktop.PolicyKit1.Authority">org.freedesktop.PolicyKit1.Authority</link>
-        interface of the PolicyKit daemon before returning.
+        interface of the PolicyKit daemon before returning. This is normally
+        achieved via the <link linkend="PolkitAgentSession">PolkitAgentSession</link>
+        API, which invokes a private setuid helper process to verify the
+        authentication.
       </para>
       <para>
         The authentication agent should not return until after authentication is complete.
diff --git a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
index e66bf53..f2eed63 100644
--- a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
+++ b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
@@ -42,7 +42,7 @@ 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,
+<link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2">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<<link linkend="eggdbus-struct-TemporaryAuthorization">TemporaryAuthorization</link>>  temporary_authorizations)
@@ -701,7 +701,7 @@ RegisterAuthenticationAgent (IN  <link linkend="eggdbus-struct-Subject">Subject<
                              IN  String   object_path)
     </programlisting>
     <para>
-<para>Register an authentication agent.</para><para>Note that current versions of PolicyKit will only work if <parameter>session_id</parameter> is set to the empty string. In the future it might work for non-empty strings if the caller is sufficiently privileged.</para>
+<para>Register an authentication agent.</para><para>Note that this should be called by same effective UID which will be passed to <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2">AuthenticationAgentResponse2()</link>.</para>
     </para>
 <variablelist role="params">
   <varlistentry>
@@ -781,7 +781,8 @@ AuthenticationAgentResponse (IN  String    cookie,
     <para>
 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.
+internal to polkit. This method will fail unless a sufficiently privileged
++caller invokes it. Deprecated in favor of <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2">AuthenticationAgentResponse2()</link>.
     </para>
 <variablelist role="params">
   <varlistentry>
@@ -812,7 +813,10 @@ AuthenticationAgentResponse2 (IN  uint32 uid,
     <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.
+internal to polkit. This method will fail unless a sufficiently privileged
+caller invokes it. Note this method was introduced in 0.114 and should be
+preferred over <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse()</link>
+as it fixes a security issue.
     </para>
 <variablelist role="params">
   <varlistentry>
diff --git a/docs/polkit/overview.xml b/docs/polkit/overview.xml
index 176d2ea..2fa55bf 100644
--- a/docs/polkit/overview.xml
+++ b/docs/polkit/overview.xml
@@ -321,11 +321,11 @@
       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>
+      linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2">AuthenticationAgentResponse2()</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.
+      implementation detail, and callers should use the
+      <link linkend="PolkitAgentSession">PolkitAgentSession</link> API to invoke
+      it, which currently uses a setuid helper program.
     </para>
     <para>
       The <link linkend="ref-authentication-agent-api">libpolkit-agent-1</link>
diff --git a/src/polkit/polkitauthority.c b/src/polkit/polkitauthority.c
index 6bd684a..7c4db7b 100644
--- a/src/polkit/polkitauthority.c
+++ b/src/polkit/polkitauthority.c
@@ -1038,6 +1038,10 @@ polkit_authority_check_authorization_sync (PolkitAuthority               *author
  *
  * Asynchronously registers an authentication agent.
  *
+ * Note that this should be called by the same effective UID which will be
+ * the real UID using the #PolkitAgentSession API or otherwise calling
+ * polkit_authority_authentication_agent_response().
+ *
  * When the operation is finished, @callback will be invoked in the
  * <link linkend="g-main-context-push-thread-default">thread-default
  * main loop</link> of the thread you are calling this method
@@ -1129,7 +1133,13 @@ polkit_authority_register_authentication_agent_finish (PolkitAuthority *authorit
  * @cancellable: (allow-none): A #GCancellable or %NULL.
  * @error: (allow-none): Return location for error or %NULL.
  *
- * Registers an authentication agent. The calling thread is blocked
+ * Registers an authentication agent.
+ *
+ * Note that this should be called by the same effective UID which will be
+ * the real UID using the #PolkitAgentSession API or otherwise calling
+ * polkit_authority_authentication_agent_response().
+ *
+ * The calling thread is blocked
  * until a reply is received. See
  * polkit_authority_register_authentication_agent() for the
  * asynchronous version.
@@ -1178,6 +1188,10 @@ polkit_authority_register_authentication_agent_sync (PolkitAuthority     *author
  *
  * Asynchronously registers an authentication agent.
  *
+ * Note that this should be called by the same effective UID which will be
+ * the real UID using the #PolkitAgentSession API or otherwise calling
+ * polkit_authority_authentication_agent_response().
+ *
  * When the operation is finished, @callback will be invoked in the
  * <link linkend="g-main-context-push-thread-default">thread-default
  * main loop</link> of the thread you are calling this method
@@ -1292,7 +1306,13 @@ polkit_authority_register_authentication_agent_with_options_finish (PolkitAuthor
  * @cancellable: (allow-none): A #GCancellable or %NULL.
  * @error: (allow-none): Return location for error or %NULL.
  *
- * Registers an authentication agent. The calling thread is blocked
+ * Registers an authentication agent.
+ *
+ * Note that this should be called by the same effective UID which will be
+ * the real UID using the #PolkitAgentSession API or otherwise calling
+ * polkit_authority_authentication_agent_response().
+ *
+ * The calling thread is blocked
  * until a reply is received. See
  * polkit_authority_register_authentication_agent_with_options() for the
  * asynchronous version.
diff --git a/src/polkitagent/polkitagentlistener.c b/src/polkitagent/polkitagentlistener.c
index 8c333af..80d1dc1 100644
--- a/src/polkitagent/polkitagentlistener.c
+++ b/src/polkitagent/polkitagentlistener.c
@@ -37,10 +37,7 @@
  *
  * Typically authentication agents use #PolkitAgentSession to
  * authenticate users (via passwords) and communicate back the
- * authentication result to the PolicyKit daemon.  This is however not
- * requirement. Depending on the system an authentication agent may
- * use other means (such as a Yes/No dialog) to obtain sufficient
- * evidence that the user is one of the requested identities.
+ * authentication result to the PolicyKit daemon.
  *
  * To register a #PolkitAgentListener with the PolicyKit daemon, use
  * polkit_agent_listener_register() or
diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c
index 03a4e84..a09d667 100644
--- a/src/polkitbackend/polkitbackendauthority.c
+++ b/src/polkitbackend/polkitbackendauthority.c
@@ -343,6 +343,7 @@ polkit_backend_authority_unregister_authentication_agent (PolkitBackendAuthority
  * polkit_backend_authority_authentication_agent_response:
  * @authority: A #PolkitBackendAuthority.
  * @caller: The system bus name that initiated the query.
+ * @uid: The real UID of the registered agent, or (uid_t)-1 if unknown.
  * @cookie: The cookie passed to the authentication agent from the authority.
  * @identity: The identity that was authenticated.
  * @error: Return location for error or %NULL.
commit 493aa5dc1d278ab9097110c1262f5229bbaf1766
Author: Colin Walters <walters at redhat.com>
Date:   Wed Jun 17 13:07:02 2015 -0400

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

diff --git a/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml b/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
index 3b519c2..5beef7d 100644
--- a/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
+++ b/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
@@ -8,7 +8,19 @@
     <annotation name="org.gtk.EggDBus.DocString" value="<para>This D-Bus interface is used for communication between the system-wide PolicyKit daemon and one or more authentication agents each running in a user session.</para><para>An authentication agent must implement this interface and register (passing the object path of the object implementing the interface) using the org.freedesktop.PolicyKit1.Authority.RegisterAuthenticationAgent() and org.freedesktop.PolicyKit1.Authority.UnregisterAuthenticationAgent() methods on the #org.freedesktop.PolicyKit1.Authority interface of the PolicyKit daemon.</para>"/>
 
     <method name="BeginAuthentication">
-      <annotation name="org.gtk.EggDBus.DocString" value="<para>Called by the PolicyKit daemon when the authentication agent needs the user to authenticate as one of the identities in @identities for the action with the identifier @action_id.</para><para>Upon succesful authentication, the authentication agent must invoke the org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse() method on the #org.freedesktop.PolicyKit1.Authority interface of the PolicyKit daemon before returning.</para><para>If the user dismisses the authentication dialog, the authentication agent should return an error.</para>"/>
+      <annotation name="org.gtk.EggDBus.DocString" value="<para>Called
+      by the PolicyKit daemon when the authentication agent needs the
+      user to authenticate as one of the identities in @identities for
+      the action with the identifier @action_id.</para><para>This
+      authentication is normally achieved via the
+      polkit_agent_session_response() API, which invokes a private
+      setuid helper process to verify the authentication.  When
+      successful, it calls the
+      org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2()
+      method on the #org.freedesktop.PolicyKit1.Authority interface of
+      the PolicyKit daemon before returning.  If the user dismisses the
+      authentication dialog, the authentication agent should call
+      polkit_agent_session_cancel().</para>"/>
 
       <arg name="action_id" direction="in" type="s">
         <annotation name="org.gtk.EggDBus.DocString" value="The identifier for the action that the user is authentication for."/>
diff --git a/data/org.freedesktop.PolicyKit1.Authority.xml b/data/org.freedesktop.PolicyKit1.Authority.xml
index fbfb9cd..f9021ee 100644
--- a/data/org.freedesktop.PolicyKit1.Authority.xml
+++ b/data/org.freedesktop.PolicyKit1.Authority.xml
@@ -313,7 +313,29 @@
     </method>
 
     <method name="AuthenticationAgentResponse">
-      <annotation name="org.gtk.EggDBus.DocString" value="Method for authentication agents to invoke on successful authentication. This method will fail unless a sufficiently privileged caller invokes it."/>
+      <annotation name="org.gtk.EggDBus.DocString" value="Method for authentication agents to invoke on successful
+authentication, intended only for use by a privileged helper process
+internal to polkit."/>
+
+      <arg name="cookie" direction="in" type="s">
+        <annotation name="org.gtk.EggDBus.DocString" value="The cookie identifying the authentication request that was passed to the authentication agent."/>
+      </arg>
+
+      <arg name="identity" direction="in" type="(sa{sv})">
+        <annotation name="org.gtk.EggDBus.Type" value="Identity"/>
+        <annotation name="org.gtk.EggDBus.DocString" value="A #Identity struct describing what identity was authenticated."/>
+      </arg>
+    </method>
+
+    <method name="AuthenticationAgentResponse2">
+      <annotation name="org.gtk.EggDBus.DocString" value="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 added in 0.114, and should be preferred over AuthenticationAgentResponse
+as it fixes a security issue."/>
+
+      <arg name="uid" direction="in" type="u">
+        <annotation name="org.gtk.EggDBus.DocString" value="The real uid of the agent.  Normally set by the setuid helper program."/>
+      </arg>
 
       <arg name="cookie" direction="in" type="s">
         <annotation name="org.gtk.EggDBus.DocString" value="The cookie identifying the authentication request that was passed to the authentication agent."/>
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<<link linkend="eggdbus-struct-TemporaryAuthorization">TemporaryAuthorization</link>>  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>
diff --git a/src/polkit/polkitauthority.c b/src/polkit/polkitauthority.c
index ab6d3cd..6bd684a 100644
--- a/src/polkit/polkitauthority.c
+++ b/src/polkit/polkitauthority.c
@@ -1492,6 +1492,14 @@ polkit_authority_authentication_agent_response (PolkitAuthority      *authority,
                                                 gpointer              user_data)
 {
   GVariant *identity_value;
+  /* Note that in reality, this API is only accessible to root, and
+   * only called from the setuid helper `polkit-agent-helper-1`.
+   *
+   * However, because this is currently public API, we avoid
+   * triggering warnings from ABI diff type programs by just grabbing
+   * the real uid of the caller here.
+   */
+  uid_t uid = getuid ();
 
   g_return_if_fail (POLKIT_IS_AUTHORITY (authority));
   g_return_if_fail (cookie != NULL);
@@ -1501,8 +1509,9 @@ polkit_authority_authentication_agent_response (PolkitAuthority      *authority,
   identity_value = polkit_identity_to_gvariant (identity);
   g_variant_ref_sink (identity_value);
   g_dbus_proxy_call (authority->proxy,
-                     "AuthenticationAgentResponse",
-                     g_variant_new ("(s@(sa{sv}))",
+                     "AuthenticationAgentResponse2",
+                     g_variant_new ("(us@(sa{sv}))",
+                                    (guint32)uid,
                                     cookie,
                                     identity_value),
                      G_DBUS_CALL_FLAGS_NONE,
diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c
index 601a974..03a4e84 100644
--- a/src/polkitbackend/polkitbackendauthority.c
+++ b/src/polkitbackend/polkitbackendauthority.c
@@ -355,6 +355,7 @@ polkit_backend_authority_unregister_authentication_agent (PolkitBackendAuthority
 gboolean
 polkit_backend_authority_authentication_agent_response (PolkitBackendAuthority    *authority,
                                                         PolkitSubject             *caller,
+                                                        uid_t                      uid,
                                                         const gchar               *cookie,
                                                         PolkitIdentity            *identity,
                                                         GError                   **error)
@@ -373,7 +374,7 @@ polkit_backend_authority_authentication_agent_response (PolkitBackendAuthority
     }
   else
     {
-      return klass->authentication_agent_response (authority, caller, cookie, identity, error);
+      return klass->authentication_agent_response (authority, caller, uid, cookie, identity, error);
     }
 }
 
@@ -587,6 +588,11 @@ static const gchar *server_introspection_data =
   "      <arg type='s' name='cookie' direction='in'/>"
   "      <arg type='(sa{sv})' name='identity' direction='in'/>"
   "    </method>"
+  "    <method name='AuthenticationAgentResponse2'>"
+  "      <arg type='u' name='uid' direction='in'/>"
+  "      <arg type='s' name='cookie' direction='in'/>"
+  "      <arg type='(sa{sv})' name='identity' direction='in'/>"
+  "    </method>"
   "    <method name='EnumerateTemporaryAuthorizations'>"
   "      <arg type='(sa{sv})' name='subject' direction='in'/>"
   "      <arg type='a(ss(sa{sv})tt)' name='temporary_authorizations' direction='out'/>"
@@ -1035,6 +1041,57 @@ server_handle_authentication_agent_response (Server                 *server,
   error = NULL;
   if (!polkit_backend_authority_authentication_agent_response (server->authority,
                                                                caller,
+                                                               (uid_t)-1,
+                                                               cookie,
+                                                               identity,
+                                                               &error))
+    {
+      g_dbus_method_invocation_return_gerror (invocation, error);
+      g_error_free (error);
+      goto out;
+    }
+
+  g_dbus_method_invocation_return_value (invocation, g_variant_new ("()"));
+
+ out:
+  if (identity != NULL)
+    g_object_unref (identity);
+}
+
+static void
+server_handle_authentication_agent_response2 (Server                 *server,
+                                              GVariant               *parameters,
+                                              PolkitSubject          *caller,
+                                              GDBusMethodInvocation  *invocation)
+{
+  const gchar *cookie;
+  GVariant *identity_gvariant;
+  PolkitIdentity *identity;
+  GError *error;
+  guint32 uid;
+
+  identity = NULL;
+
+  g_variant_get (parameters,
+                 "(u&s@(sa{sv}))",
+                 &uid,
+                 &cookie,
+                 &identity_gvariant);
+
+  error = NULL;
+  identity = polkit_identity_new_for_gvariant (identity_gvariant, &error);
+  if (identity == NULL)
+    {
+      g_prefix_error (&error, "Error getting identity: ");
+      g_dbus_method_invocation_return_gerror (invocation, error);
+      g_error_free (error);
+      goto out;
+    }
+
+  error = NULL;
+  if (!polkit_backend_authority_authentication_agent_response (server->authority,
+                                                               caller,
+                                                               (uid_t)uid,
                                                                cookie,
                                                                identity,
                                                                &error))
@@ -1222,6 +1279,8 @@ server_handle_method_call (GDBusConnection        *connection,
     server_handle_unregister_authentication_agent (server, parameters, caller, invocation);
   else if (g_strcmp0 (method_name, "AuthenticationAgentResponse") == 0)
     server_handle_authentication_agent_response (server, parameters, caller, invocation);
+  else if (g_strcmp0 (method_name, "AuthenticationAgentResponse2") == 0)
+    server_handle_authentication_agent_response2 (server, parameters, caller, invocation);
   else if (g_strcmp0 (method_name, "EnumerateTemporaryAuthorizations") == 0)
     server_handle_enumerate_temporary_authorizations (server, parameters, caller, invocation);
   else if (g_strcmp0 (method_name, "RevokeTemporaryAuthorizations") == 0)
diff --git a/src/polkitbackend/polkitbackendauthority.h b/src/polkitbackend/polkitbackendauthority.h
index f9f7385..88df82e 100644
--- a/src/polkitbackend/polkitbackendauthority.h
+++ b/src/polkitbackend/polkitbackendauthority.h
@@ -147,6 +147,7 @@ struct _PolkitBackendAuthorityClass
 
   gboolean (*authentication_agent_response) (PolkitBackendAuthority   *authority,
                                              PolkitSubject            *caller,
+                                             uid_t                     uid,
                                              const gchar              *cookie,
                                              PolkitIdentity           *identity,
                                              GError                  **error);
@@ -249,6 +250,7 @@ gboolean polkit_backend_authority_unregister_authentication_agent (PolkitBackend
 
 gboolean polkit_backend_authority_authentication_agent_response (PolkitBackendAuthority    *authority,
                                                                  PolkitSubject             *caller,
+                                                                 uid_t                      uid,
                                                                  const gchar               *cookie,
                                                                  PolkitIdentity            *identity,
                                                                  GError                   **error);
diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c
index 15adc6a..96725f7 100644
--- a/src/polkitbackend/polkitbackendinteractiveauthority.c
+++ b/src/polkitbackend/polkitbackendinteractiveauthority.c
@@ -108,8 +108,9 @@ static AuthenticationAgent *get_authentication_agent_for_subject (PolkitBackendI
                                                                   PolkitSubject *subject);
 
 
-static AuthenticationSession *get_authentication_session_for_cookie (PolkitBackendInteractiveAuthority *authority,
-                                                                     const gchar *cookie);
+static AuthenticationSession *get_authentication_session_for_uid_and_cookie (PolkitBackendInteractiveAuthority *authority,
+                                                                             uid_t                              uid,
+                                                                             const gchar                       *cookie);
 
 static GList *get_authentication_sessions_initiated_by_system_bus_unique_name (PolkitBackendInteractiveAuthority *authority,
                                                                                const gchar *system_bus_unique_name);
@@ -169,6 +170,7 @@ static gboolean polkit_backend_interactive_authority_unregister_authentication_a
 
 static gboolean polkit_backend_interactive_authority_authentication_agent_response (PolkitBackendAuthority   *authority,
                                                                               PolkitSubject            *caller,
+                                                                              uid_t                     uid,
                                                                               const gchar              *cookie,
                                                                               PolkitIdentity           *identity,
                                                                               GError                  **error);
@@ -440,6 +442,7 @@ struct AuthenticationAgent
 {
   volatile gint ref_count;
 
+  uid_t creator_uid;
   PolkitSubject *scope;
   guint64 serial;
 
@@ -1603,6 +1606,7 @@ authentication_agent_unref (AuthenticationAgent *agent)
 static AuthenticationAgent *
 authentication_agent_new (guint64      serial,
                           PolkitSubject *scope,
+                          PolkitIdentity *creator,
                           const gchar *unique_system_bus_name,
                           const gchar *locale,
                           const gchar *object_path,
@@ -1611,6 +1615,10 @@ authentication_agent_new (guint64      serial,
 {
   AuthenticationAgent *agent;
   GDBusProxy *proxy;
+  PolkitUnixUser *creator_user;
+
+  g_assert (POLKIT_IS_UNIX_USER (creator));
+  creator_user = POLKIT_UNIX_USER (creator);
 
   if (!g_variant_is_object_path (object_path))
     {
@@ -1638,6 +1646,7 @@ authentication_agent_new (guint64      serial,
   agent->ref_count = 1;
   agent->serial = serial;
   agent->scope = g_object_ref (scope);
+  agent->creator_uid = (uid_t)polkit_unix_user_get_uid (creator_user);
   agent->object_path = g_strdup (object_path);
   agent->unique_system_bus_name = g_strdup (unique_system_bus_name);
   agent->locale = g_strdup (locale);
@@ -1736,8 +1745,9 @@ get_authentication_agent_for_subject (PolkitBackendInteractiveAuthority *authori
 }
 
 static AuthenticationSession *
-get_authentication_session_for_cookie (PolkitBackendInteractiveAuthority *authority,
-                                       const gchar *cookie)
+get_authentication_session_for_uid_and_cookie (PolkitBackendInteractiveAuthority *authority,
+                                               uid_t                              uid,
+                                               const gchar                       *cookie)
 {
   PolkitBackendInteractiveAuthorityPrivate *priv;
   GHashTableIter hash_iter;
@@ -1755,6 +1765,23 @@ get_authentication_session_for_cookie (PolkitBackendInteractiveAuthority *author
     {
       GList *l;
 
+      /* We need to ensure that if somehow we have duplicate cookies
+       * due to wrapping, that the cookie used is matched to the user
+       * who called AuthenticationAgentResponse2.  See
+       * http://lists.freedesktop.org/archives/polkit-devel/2015-June/000425.html
+       * 
+       * Except if the legacy AuthenticationAgentResponse is invoked,
+       * we don't know the uid and hence use -1.  Continue to support
+       * the old behavior for backwards compatibility, although everyone
+       * who is using our own setuid helper will automatically be updated
+       * to the new API.
+       */
+      if (uid != (uid_t)-1)
+        {
+          if (agent->creator_uid != uid)
+            continue;
+        }
+
       for (l = agent->active_sessions; l != NULL; l = l->next)
         {
           AuthenticationSession *session = l->data;
@@ -2544,6 +2571,7 @@ polkit_backend_interactive_authority_register_authentication_agent (PolkitBacken
   priv->agent_serial++;
   agent = authentication_agent_new (priv->agent_serial,
                                     subject,
+                                    user_of_caller,
                                     polkit_system_bus_name_get_name (POLKIT_SYSTEM_BUS_NAME (caller)),
                                     locale,
                                     object_path,
@@ -2757,6 +2785,7 @@ polkit_backend_interactive_authority_unregister_authentication_agent (PolkitBack
 static gboolean
 polkit_backend_interactive_authority_authentication_agent_response (PolkitBackendAuthority   *authority,
                                                               PolkitSubject            *caller,
+                                                              uid_t                     uid,
                                                               const gchar              *cookie,
                                                               PolkitIdentity           *identity,
                                                               GError                  **error)
@@ -2799,7 +2828,7 @@ polkit_backend_interactive_authority_authentication_agent_response (PolkitBacken
     }
 
   /* find the authentication session */
-  session = get_authentication_session_for_cookie (interactive_authority, cookie);
+  session = get_authentication_session_for_uid_and_cookie (interactive_authority, uid, cookie);
   if (session == NULL)
     {
       g_set_error (error,


More information about the hal-commit mailing list