From 3f7b27f2acf715e43e2b07947d90e2a1612aae66 Mon Sep 17 00:00:00 2001 From: William Jon McCann 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 (¶m_val, CK_TYPE_PARAMETER_STRUCT); + g_value_take_boxed (¶m_val, + dbus_g_type_specialized_construct (CK_TYPE_PARAMETER_STRUCT)); + dbus_g_type_struct_set (¶m_val, + 0, key, + 1, &val, + G_MAXUINT); + g_value_unset (&val); + + g_ptr_array_add (parameters, g_value_get_boxed (¶m_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