Attachment 116290 Details for Bug 90837 – 0001-CVE-XXX-XXXX-Bind-use-of-cookies-to-specific-uids.patch

[patch] 0001-CVE-XXX-XXXX-Bind-use-of-cookies-to-specific-uids.patch

0001-CVE-XXX-XXXX-Bind-use-of-cookies-to-specific-uids.patch (text/plain), 19.61 KB, created by Colin Walters on 2015-06-04 13:53:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Walters

Created: 2015-06-04 13:53:48 UTC

Size: 19.61 KB

patch

obsolete

>From 476538e8ae905e2ebf247f380b2da75feedab2e5 Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>Date: Wed, 3 Jun 2015 18:50:40 -0400
>Subject: [PATCH] CVE-XXX-XXXX: Bind use of cookies to specific uids
>
>Note: CVE not assigned yet
>
>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.
>
>One way to fix this would be to make the cookie unforgeable, but this
>approach passes through the uid of the caller from the setuid binary,
>ensuring that we only look up `AuthenticationAgent`s that were created
>by a matching uid.
>
>Signed-off-by: Colin Walters <walters@verbum.org>
>---
> ...g.freedesktop.PolicyKit1.AuthenticationAgent.xml |  2 +-
> data/org.freedesktop.PolicyKit1.Authority.xml       |  9 ++++++++-
> ...terface-org.freedesktop.PolicyKit1.Authority.xml | 10 +++++++---
> docs/polkit/overview.xml                            | 18 ++++++++++--------
> src/polkit/polkitauthority.c                        | 11 ++++++++++-
> src/polkitbackend/polkitbackendauthority.c          |  9 +++++++--
> src/polkitbackend/polkitbackendauthority.h          |  2 ++
> .../polkitbackendinteractiveauthority.c             | 21 ++++++++++++++++++---
> 8 files changed, 63 insertions(+), 19 deletions(-)
>
>diff --git a/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml b/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
>index 3b519c2..d9b7fcd 100644
>--- a/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
>+++ b/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
>@@ -8,7 +8,7 @@
>     <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>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.  Note this method is normally only accessible to root, and called via a setuid helper program.</para><para>If the user dismisses the 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 fbfb9cd..ce71401 100644
>--- a/data/org.freedesktop.PolicyKit1.Authority.xml
>+++ b/data/org.freedesktop.PolicyKit1.Authority.xml
>@@ -313,7 +313,14 @@
>     </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.  Note the signature for this method changed in
>+0.114."/>
>+
>+      <arg name="uid" direction="in" type="i">
>+        <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..90ff6cd 100644
>--- a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
>+++ b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
>@@ -40,7 +40,7 @@ Structure    <link linkend="eggdbus-struct-TemporaryAuthorization">TemporaryAuth
>                                   IN  Dict&lt;String,Variant&gt;     options)
> <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.UnregisterAuthenticationAgent">UnregisterAuthenticationAgent</link>    (IN  <link linkend="eggdbus-struct-Subject">Subject</link>                        subject,
>                                   IN  String                         object_path)
>-<link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse</link>      (IN  String                         cookie,
>+<link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse</link>      (IN int 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)
>@@ -773,11 +773,15 @@ The <parameter>object_path</parameter> passed to <link linkend="eggdbus-method-o
>     <refsect2 role="function" id="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">
>       <title>AuthenticationAgentResponse ()</title>
>     <programlisting>
>-AuthenticationAgentResponse (IN  String    cookie,
>+AuthenticationAgentResponse (IN  int32 uid,
>+                             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.  Note the signature for this method changed in
>+0.114.
>     </para>
> <variablelist role="params">
>   <varlistentry>
>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..0f04bcf 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.
>+   */
>+  int uid = (int) getuid ();
> 
>   g_return_if_fail (POLKIT_IS_AUTHORITY (authority));
>   g_return_if_fail (cookie != NULL);
>@@ -1502,7 +1510,8 @@ polkit_authority_authentication_agent_response (PolkitAuthority      *authority,
>   g_variant_ref_sink (identity_value);
>   g_dbus_proxy_call (authority->proxy,
>                      "AuthenticationAgentResponse",
>-                     g_variant_new ("(s@(sa{sv}))",
>+                     g_variant_new ("(is@(sa{sv}))",
>+                                    uid,
>                                     cookie,
>                                     identity_value),
>                      G_DBUS_CALL_FLAGS_NONE,
>diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c
>index 601a974..db6cec8 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,
>+                                                        gint                       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);
>     }
> }
> 
>@@ -584,6 +585,7 @@ static const gchar *server_introspection_data =
>   "      <arg type='s' name='object_path' direction='in'/>"
>   "    </method>"
>   "    <method name='AuthenticationAgentResponse'>"
>+  "      <arg type='i' name='uid' direction='in'/>"
>   "      <arg type='s' name='cookie' direction='in'/>"
>   "      <arg type='(sa{sv})' name='identity' direction='in'/>"
>   "    </method>"
>@@ -1014,11 +1016,13 @@ server_handle_authentication_agent_response (Server                 *server,
>   GVariant *identity_gvariant;
>   PolkitIdentity *identity;
>   GError *error;
>+  int uid;
> 
>   identity = NULL;
> 
>   g_variant_get (parameters,
>-                 "(&s@(sa{sv}))",
>+                 "(i&s@(sa{sv}))",
>+                 &uid,
>                  &cookie,
>                  &identity_gvariant);
> 
>@@ -1035,6 +1039,7 @@ server_handle_authentication_agent_response (Server                 *server,
>   error = NULL;
>   if (!polkit_backend_authority_authentication_agent_response (server->authority,
>                                                                caller,
>+                                                               uid,
>                                                                cookie,
>                                                                identity,
>                                                                &error))
>diff --git a/src/polkitbackend/polkitbackendauthority.h b/src/polkitbackend/polkitbackendauthority.h
>index f9f7385..4a30446 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,
>+                                             gint                      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,
>+                                                                 gint                       uid,
>                                                                  const gchar               *cookie,
>                                                                  PolkitIdentity            *identity,
>                                                                  GError                   **error);
>diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c
>index aae8cfb..2e29520 100644
>--- a/src/polkitbackend/polkitbackendinteractiveauthority.c
>+++ b/src/polkitbackend/polkitbackendinteractiveauthority.c
>@@ -169,6 +169,7 @@ static gboolean polkit_backend_interactive_authority_unregister_authentication_a
> 
> static gboolean polkit_backend_interactive_authority_authentication_agent_response (PolkitBackendAuthority   *authority,
>                                                                               PolkitSubject            *caller,
>+                                                                              gint                      uid,
>                                                                               const gchar              *cookie,
>                                                                               PolkitIdentity           *identity,
>                                                                               GError                  **error);
>@@ -431,6 +432,7 @@ struct AuthenticationAgent
> {
>   volatile gint ref_count;
> 
>+  gint creator_uid;
>   PolkitSubject *scope;
> 
>   gchar *locale;
>@@ -1548,6 +1550,7 @@ authentication_agent_unref (AuthenticationAgent *agent)
> 
> static AuthenticationAgent *
> authentication_agent_new (PolkitSubject *scope,
>+                          gint           creator_uid,
>                           const gchar *unique_system_bus_name,
>                           const gchar *locale,
>                           const gchar *object_path,
>@@ -1582,6 +1585,7 @@ authentication_agent_new (PolkitSubject *scope,
>   agent = g_new0 (AuthenticationAgent, 1);
>   agent->ref_count = 1;
>   agent->scope = g_object_ref (scope);
>+  agent->creator_uid = creator_uid;
>   agent->object_path = g_strdup (object_path);
>   agent->unique_system_bus_name = g_strdup (unique_system_bus_name);
>   agent->locale = g_strdup (locale);
>@@ -1661,8 +1665,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,
>+                                               guint32 uid,
>+                                               const gchar *cookie)
> {
>   PolkitBackendInteractiveAuthorityPrivate *priv;
>   GHashTableIter hash_iter;
>@@ -1680,6 +1685,14 @@ 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 AuthenticationAgentResponse.  See
>+       * http://lists.freedesktop.org/archives/polkit-devel/2015-June/000425.html
>+       */
>+      if (agent->creator_uid != uid)
>+        continue;
>+
>       for (l = agent->active_sessions; l != NULL; l = l->next)
>         {
>           AuthenticationSession *session = l->data;
>@@ -2479,6 +2492,7 @@ polkit_backend_interactive_authority_register_authentication_agent (PolkitBacken
>     }
> 
>   agent = authentication_agent_new (subject,
>+                                    caller_uid,
>                                     polkit_system_bus_name_get_name (POLKIT_SYSTEM_BUS_NAME (caller)),
>                                     locale,
>                                     object_path,
>@@ -2692,6 +2706,7 @@ polkit_backend_interactive_authority_unregister_authentication_agent (PolkitBack
> static gboolean
> polkit_backend_interactive_authority_authentication_agent_response (PolkitBackendAuthority   *authority,
>                                                               PolkitSubject            *caller,
>+                                                              gint                    uid,
>                                                               const gchar              *cookie,
>                                                               PolkitIdentity           *identity,
>                                                               GError                  **error)
>@@ -2734,7 +2749,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,
>-- 
>1.8.3.1
>

Actions: View | Splinter Review

Attachments on bug 90837: 116273 | 116289 | 116290 | 116337 | 116338 | 116542 | 116543

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.