Attachment 36055 Details for Bug 28377 – patch

[patch] patch

0001-Only-set-sessions-to-be-is-local-true-if-set-by-a-tr.patch (text/plain), 8.51 KB, created by william.jon.mccann on 2010-06-03 17:55:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: william.jon.mccann

Created: 2010-06-03 17:55:03 UTC

Size: 8.51 KB

patch

obsolete

>From 3f7b27f2acf715e43e2b07947d90e2a1612aae66 Mon Sep 17 00:00:00 2001
>From: William Jon McCann <jmccann@redhat.com>
>Date: Thu, 3 Jun 2010 20:02:35 -0400
>Subject: [PATCH] Only set sessions to be is-local=true if set by a trusted party
>
>https://bugs.freedesktop.org/show_bug.cgi?id=28377
>---
> src/ck-manager.c                |  178 ++++++++++++++++++++++++++++++++++++++-
> tools/ck-collect-session-info.c |   10 +--
> 2 files changed, 177 insertions(+), 11 deletions(-)
>
>diff --git a/src/ck-manager.c b/src/ck-manager.c
>index 1dfc789..b8dea2c 100644
>--- a/src/ck-manager.c
>+++ b/src/ck-manager.c
>@@ -1654,13 +1654,187 @@ open_session_for_leader (CkManager             *manager,
>         dbus_g_method_return (context, cookie);
> }
> 
>+enum {
>+        P_STRING,
>+        P_BOOLEAN,
>+};
>+
>+#define CK_TYPE_PARAMETER_STRUCT (dbus_g_type_get_struct ("GValueArray", \
>+                                                          G_TYPE_STRING,  \
>+                                                          G_TYPE_VALUE, \
>+                                                          G_TYPE_INVALID))
>+
>+static gboolean
>+_get_parameter (GPtrArray  *parameters,
>+                const char *name,
>+                int         prop_type,
>+                gpointer   *value)
>+{
>+        gboolean ret;
>+        int      i;
>+
>+        if (parameters == NULL) {
>+                return FALSE;
>+        }
>+
>+        ret = FALSE;
>+
>+        for (i = 0; i < parameters->len; i++) {
>+                gboolean    res;
>+                GValue      val_struct = { 0, };
>+                char       *prop_name;
>+                GValue     *prop_val;
>+
>+                g_value_init (&val_struct, CK_TYPE_PARAMETER_STRUCT);
>+                g_value_set_static_boxed (&val_struct, g_ptr_array_index (parameters, i));
>+
>+                res = dbus_g_type_struct_get (&val_struct,
>+                                              0, &prop_name,
>+                                              1, &prop_val,
>+                                              G_MAXUINT);
>+                if (! res) {
>+                        g_debug ("Unable to extract parameter input");
>+                        goto cont;
>+                }
>+
>+                if (prop_name == NULL) {
>+                        g_debug ("Skipping NULL parameter");
>+                        goto cont;
>+                }
>+
>+                if (strcmp (prop_name, name) != 0) {
>+                        goto cont;
>+                }
>+
>+                switch (prop_type) {
>+                case P_STRING:
>+                        {
>+                                const char *val;
>+                                val = g_value_get_string (prop_val);
>+                                if (value) {
>+                                        *value = g_strdup (val);
>+                                }
>+                        }
>+                        break;
>+                case P_BOOLEAN:
>+                        {
>+                                gboolean val;
>+                                val = g_value_get_boolean (prop_val);
>+                                if (value) {
>+                                        *(gboolean *)value = val;
>+                                }
>+                        }
>+                        break;
>+                default:
>+                        g_assert_not_reached ();
>+                        break;
>+                }
>+
>+                g_free (prop_name);
>+                g_value_unset (prop_val);
>+                g_free (prop_val);
>+                ret = TRUE;
>+                break;
>+
>+        cont:
>+                g_free (prop_name);
>+                if (prop_val != NULL) {
>+                        g_value_unset (prop_val);
>+                        g_free (prop_val);
>+                }
>+        }
>+
>+        return ret;
>+}
>+
>+static gboolean
>+_verify_login_session_id_is_local (CkManager  *manager,
>+                                   const char *login_session_id)
>+{
>+        GHashTableIter iter;
>+        const char    *id;
>+        CkSession     *session;
>+
>+        g_return_val_if_fail (CK_IS_MANAGER (manager), FALSE);
>+
>+        /* If any local session exists for the given login session id
>+           then that means a trusted party has vouched for the
>+           original login */
>+
>+        g_debug ("Looking for local sessions for login-session-id=%s", login_session_id);
>+
>+        session = NULL;
>+        g_hash_table_iter_init (&iter, manager->priv->sessions);
>+        while (g_hash_table_iter_next (&iter, (gpointer *)&id, (gpointer *)&session)) {
>+                if (session != NULL) {
>+                        gboolean is_local;
>+                        char    *sessid;
>+
>+                        sessid = NULL;
>+                        g_object_get (session,
>+                                      "login-session-id", &sessid,
>+                                      "is-local", &is_local,
>+                                      NULL);
>+                        if (sessid != NULL && strcmp (sessid, login_session_id) == 0 && is_local) {
>+                                g_debug ("CkManager: found is-local=true on %s", id);
>+                                return TRUE;
>+                        }
>+                }
>+        }
>+
>+        return FALSE;
>+}
>+
>+static void
>+add_param_boolean (GPtrArray  *parameters,
>+                   const char *key,
>+                   gboolean    value)
>+{
>+        GValue   val = { 0, };
>+        GValue   param_val = { 0, };
>+
>+        g_value_init (&val, G_TYPE_BOOLEAN);
>+        g_value_set_boolean (&val, value);
>+        g_value_init (&param_val, CK_TYPE_PARAMETER_STRUCT);
>+        g_value_take_boxed (&param_val,
>+                            dbus_g_type_specialized_construct (CK_TYPE_PARAMETER_STRUCT));
>+        dbus_g_type_struct_set (&param_val,
>+                                0, key,
>+                                1, &val,
>+                                G_MAXUINT);
>+        g_value_unset (&val);
>+
>+        g_ptr_array_add (parameters, g_value_get_boxed (&param_val));
>+}
>+
> static void
> verify_and_open_session_for_leader (CkManager             *manager,
>                                     CkSessionLeader       *leader,
>-                                    const GPtrArray       *parameters,
>+                                    GPtrArray             *parameters,
>                                     DBusGMethodInvocation *context)
> {
>-        /* for now don't bother verifying since we protect OpenSessionWithParameters */
>+        /* Only allow a local session if originating from an existing
>+           local session.  Effectively this means that only trusted
>+           parties can create local sessions. */
>+
>+        g_debug ("CkManager: verifying session for leader");
>+
>+        if (parameters != NULL && ! _get_parameter (parameters, "is-local", P_BOOLEAN, NULL)) {
>+                gboolean is_local;
>+                char    *login_session_id;
>+
>+                g_debug ("CkManager: is-local has not been set, will inherit from existing login-session-id if available");
>+
>+                is_local = FALSE;
>+
>+                if (_get_parameter (parameters, "login-session-id", P_STRING, (gpointer *) &login_session_id)) {
>+                        is_local = _verify_login_session_id_is_local (manager, login_session_id);
>+                        g_debug ("CkManager: found is-local=%s", is_local ? "true" : "false");
>+                }
>+
>+                add_param_boolean (parameters, "is-local", is_local);
>+        }
>+
>         open_session_for_leader (manager,
>                                  leader,
>                                  parameters,
>diff --git a/tools/ck-collect-session-info.c b/tools/ck-collect-session-info.c
>index 75d8d45..5c10dfe 100644
>--- a/tools/ck-collect-session-info.c
>+++ b/tools/ck-collect-session-info.c
>@@ -274,8 +274,7 @@ fill_x11_info (SessionInfo *si)
>         si->x11_display_device = ck_process_stat_get_tty (xorg_stat);
>         ck_process_stat_free (xorg_stat);
> 
>-        si->is_local = TRUE;
>-        si->is_local_is_set = TRUE;
>+        /* don't set is-local here - let the daemon do that */
> 
>         g_free (si->remote_host_name);
>         si->remote_host_name = NULL;
>@@ -304,13 +303,6 @@ fill_session_info (SessionInfo *si)
> 
>         fill_x11_info (si);
> 
>-        if (! si->is_local_is_set) {
>-                /* FIXME: how should we set this? */
>-                /* non x11 sessions must be local I guess */
>-                si->is_local = TRUE;
>-                si->is_local_is_set = TRUE;
>-        }
>-
>         res = ck_unix_pid_get_login_session_id (si->pid, &si->login_session_id);
>         if (! res) {
>                 si->login_session_id = NULL;
>-- 
>1.7.0.1
>

Actions: View | Splinter Review

Attachments on bug 28377: 36055 | 36637

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.