Attachment 48572 Details for Bug 38769 – pkexec: Support running X11 apps

[patch] pkexec: Support running X11 apps

0001-Bug-38769-pkexec-Support-running-X11-apps.patch (text/plain), 5.70 KB, created by Martin Pitt on 2011-06-29 14:53:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Pitt

Created: 2011-06-29 14:53:25 UTC

Size: 5.70 KB

patch

obsolete

>From 0adc9e6a7e3a67ce827a6059b183b2425ba619e7 Mon Sep 17 00:00:00 2001
>From: Martin Pitt <martin.pitt@ubuntu.com>
>Date: Wed, 29 Jun 2011 22:43:48 +0100
>Subject: [PATCH] =?UTF-8?q?Bug=2038769=20=E2=80=94=20pkexec:=20Support=20run?=
> =?UTF-8?q?ning=20X11=20apps?=
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Introduce a new annotation flag "org.freedesktop.policykit.exec.allow_gui"
>which will cause pkexec to preserve $DISPLAY and $XAUTHORITY. With this, the
>remaining few legacy X11 programs which still need to run as root can finally
>be migrated away from gksu (or similar) to pkexec, with the help of some
>.polkit files. This will provide a consistent UI and also help with making the
>authentication dialogs less spoofable.
>
>Relax validate_environment_variable() to allow '/' in $XAUTHORITY, as this
>variable actually is a full path.
>---
> docs/man/pkexec.xml   |    8 ++++++--
> src/programs/pkexec.c |   35 +++++++++++++++++++++++++----------
> 2 files changed, 31 insertions(+), 12 deletions(-)
>
>diff --git a/docs/man/pkexec.xml b/docs/man/pkexec.xml
>index 2a0e721..8196511 100644
>--- a/docs/man/pkexec.xml
>+++ b/docs/man/pkexec.xml
>@@ -128,8 +128,12 @@
>       environment variable is set to the user id of the process
>       invoking <command>pkexec</command>. As a
>       result, <command>pkexec</command> will not allow you to run
>-      e.g. X11 applications as another user since
>-      the <literal>$DISPLAY</literal> environment variable is not set.
>+      X11 applications as another user since
>+      the <literal>$DISPLAY</literal> and <literal>$XAUTHORITY</literal>
>+      environment variables are not set. These two variables will be retained
>+      if the <emphasis>org.freedesktop.policykit.exec.allow_gui</emphasis> annotation
>+      on an action is set to a nonempty value; this is discouraged, though, and
>+      should only be used for legacy programs.
>     </para>
>   </refsect1>
> 
>diff --git a/src/programs/pkexec.c b/src/programs/pkexec.c
>index 3e656be..373977b 100644
>--- a/src/programs/pkexec.c
>+++ b/src/programs/pkexec.c
>@@ -229,7 +229,8 @@ fdwalk (FdCallback callback,
> 
> static gchar *
> find_action_for_path (PolkitAuthority *authority,
>-                      const gchar     *path)
>+                      const gchar     *path,
>+                      gboolean        *allow_gui)
> {
>   GList *l;
>   GList *actions;
>@@ -239,6 +240,7 @@ find_action_for_path (PolkitAuthority *authority,
>   actions = NULL;
>   action_id = NULL;
>   error = NULL;
>+  *allow_gui = FALSE;
> 
>   actions = polkit_authority_enumerate_actions_sync (authority,
>                                                      NULL,
>@@ -254,6 +256,7 @@ find_action_for_path (PolkitAuthority *authority,
>     {
>       PolkitActionDescription *action_desc = POLKIT_ACTION_DESCRIPTION (l->data);
>       const gchar *path_for_action;
>+      const gchar *allow_gui_annotation;
> 
>       path_for_action = polkit_action_description_get_annotation (action_desc, "org.freedesktop.policykit.exec.path");
>       if (path_for_action == NULL)
>@@ -262,6 +265,12 @@ find_action_for_path (PolkitAuthority *authority,
>       if (g_strcmp0 (path_for_action, path) == 0)
>         {
>           action_id = g_strdup (polkit_action_description_get_action_id (action_desc));
>+
>+          allow_gui_annotation = polkit_action_description_get_annotation (action_desc, "org.freedesktop.policykit.exec.allow_gui");
>+
>+          if (allow_gui_annotation != NULL && strlen (allow_gui_annotation) > 0)
>+            *allow_gui = TRUE;
>+
>           goto out;
>         }
>     }
>@@ -352,7 +361,7 @@ validate_environment_variable (const gchar *key,
>           goto out;
>         }
>     }
>-  else if (strstr (value, "/") != NULL ||
>+  else if ((g_strcmp0 (key, "XAUTHORITY") != 0 && strstr (value, "/") != NULL) ||
>            strstr (value, "%") != NULL ||
>            strstr (value, "..") != NULL)
>     {
>@@ -388,6 +397,7 @@ main (int argc, char *argv[])
>   PolkitDetails *details;
>   GError *error;
>   gchar *action_id;
>+  gboolean allow_gui;
>   gchar **exec_argv;
>   gchar *path;
>   struct passwd pwstruct;
>@@ -408,20 +418,20 @@ main (int argc, char *argv[])
>     "TERM",
>     "COLORTERM",
> 
>-    /* For now, avoiding pretend that running X11 apps as another user in the same session
>-     * will ever work... See
>+    /* By default we don't allow running X11 apps, as it does not work in the
>+     * general case. See
>      *
>      *  https://bugs.freedesktop.org/show_bug.cgi?id=17970#c26
>      *
>      * and surrounding comments for a lot of discussion about this.
>+     *
>+     * However, it can be enabled for some selected and tested legacy programs
>+     * which previously used e. g. gksu, by setting the
>+     * org.freedesktop.policykit.exec.allow_gui annotation to a nonempty value.
>+     * See https://bugs.freedesktop.org/show_bug.cgi?id=38769 for details.
>      */
>-#if 0
>-    "DESKTOP_STARTUP_ID",
>     "DISPLAY",
>     "XAUTHORITY",
>-    "DBUS_SESSION_BUS_ADDRESS",
>-    "ORBIT_SOCKETDIR",
>-#endif
>     NULL
>   };
>   GPtrArray *saved_env;
>@@ -654,7 +664,7 @@ main (int argc, char *argv[])
>       goto out;
>     }
> 
>-  action_id = find_action_for_path (authority, path);
>+  action_id = find_action_for_path (authority, path, &allow_gui);
>   g_assert (action_id != NULL);
> 
>   details = polkit_details_new ();
>@@ -790,6 +800,11 @@ main (int argc, char *argv[])
>       const gchar *key = saved_env->pdata[n];
>       const gchar *value = saved_env->pdata[n + 1];
> 
>+      /* Only set $DISPLAY and $XAUTHORITY when explicitly allowed in the .policy */
>+      if (!allow_gui &&
>+              (strcmp (key, "DISPLAY") == 0 || strcmp (key, "XAUTHORITY") == 0))
>+          continue;
>+
>       if (!g_setenv (key, value, TRUE))
>         {
>           g_printerr ("Error setting environment variable %s to '%s': %s\n",
>-- 
>1.7.5.4
>

Actions: View | Splinter Review

Attachments on bug 38769: 48572

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.