From 0adc9e6a7e3a67ce827a6059b183b2425ba619e7 Mon Sep 17 00:00:00 2001 From: Martin Pitt 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 pkexec. As a result, pkexec will not allow you to run - e.g. X11 applications as another user since - the $DISPLAY environment variable is not set. + X11 applications as another user since + the $DISPLAY and $XAUTHORITY + environment variables are not set. These two variables will be retained + if the org.freedesktop.policykit.exec.allow_gui annotation + on an action is set to a nonempty value; this is discouraged, though, and + should only be used for legacy programs. 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