This patch gets gdm-session-worker to build the target environment using the pam_env* functions instead of in a g_hash_table. This was suggested in a comment in the gdm_session_worker_set_environment /* FIXME: maybe we should use use pam_putenv instead of our * own hash table, so pam can override our choices if it knows * better? */ As comment says, the advantage is it gives PAM access to the environment. Note that this isn't currently as useful as it could be because gdm-simple- slave sends nearly all of the environment variables right at the very end. It would be nice if it would send them as soon as their value was established. For environment variables set after (at least some of) PAM has run, it might also be nice to have gdm-simple-slave pass along a default flag to tell gdm- session-worker whether it should override a PAM setting or not. Cheers! -Tyson PS: Please CC me in any replies as I am not a regular subscriber to the list.
From ded823ae0f093b1a7b48f07fd5d7030383f6d273 Mon Sep 17 00:00:00 2001
From: Tyson Whitehead <twhitehead gmail com>
Date: Sat, 13 Mar 2010 23:59:52 -0500
Subject: [PATCH] Maintain the target environment solely in PAM
---
daemon/gdm-session-worker.c | 129 +++++++++++++++++++-----------------------
1 files changed, 58 insertions(+), 71 deletions(-)
diff --git a/daemon/gdm-session-worker.c b/daemon/gdm-session-worker.c
index 5e34fb9..c3187ad 100644
--- a/daemon/gdm-session-worker.c
+++ b/daemon/gdm-session-worker.c
@@ -128,7 +128,6 @@ struct GdmSessionWorkerPrivate
int cred_flags;
char **arguments;
- GHashTable *environment;
guint32 cancelled : 1;
guint32 timed_out : 1;
guint state_change_idle_id;
@@ -1495,13 +1494,45 @@ gdm_session_worker_set_environment_variable (GdmSessionWorker *worker,
const char *key,
const char *value)
{
- /* FIXME: maybe we should use use pam_putenv instead of our
- * own hash table, so pam can override our choices if it knows
- * better?
- */
- g_hash_table_replace (worker->priv->environment,
- g_strdup (key),
- g_strdup (value));
+ int error_code;
+ char *keyvalue;
+
+ if (value != NULL) {
+ keyvalue = g_strdup_printf("%s=%s",key,value);
+ }
+ else {
+ keyvalue = g_strdup(key);
+ }
+
+ error_code = pam_putenv (worker->priv->pam_handle,
+ keyvalue);
+
+ g_free(keyvalue);
+
+ if (error_code != PAM_SUCCESS) {
+ if (value) {
+ g_warning ("cannot set environment variable %s=%s,\n"
+ "%s\n", key, value, pam_strerror (worker->priv->pam_handle, error_code) );
+ }
+ else {
+ g_warning ("cannot unset environment variable %s,\n"
+ "%s\n", key, pam_strerror (worker->priv->pam_handle, error_code) );
+ }
+ }
+}
+
+static gchar *
+gdm_session_worker_get_environment_variable (GdmSessionWorker *worker,
+ const char *key)
+{
+ char *value;
+ const char *ret;
+
+ ret = pam_getenv (worker->priv->pam_handle, value);
+ value = g_strdup(ret);
+ free(ret);
+
+ return value;
}
static void
@@ -1522,7 +1553,11 @@ static gboolean
gdm_session_worker_environment_variable_is_set (GdmSessionWorker *worker,
const char *name)
{
- return g_hash_table_lookup (worker->priv->environment, name) != NULL;
+ const char *ret;
+ ret = pam_getenv (worker->priv->pam_handle, name);
+ free (ret);
+
+ return (ret != NULL);
}
static gboolean
@@ -1734,58 +1769,23 @@ gdm_session_worker_accredit_user (GdmSessionWorker *worker,
return ret;
}
-static void
-gdm_session_worker_update_environment_from_pam (GdmSessionWorker *worker)
+static gchar **
+gdm_session_worker_get_environment (GdmSessionWorker *worker)
{
- char **environment;
- gsize i;
-
- environment = pam_getenvlist (worker->priv->pam_handle);
-
- for (i = 0; environment[i] != NULL; i++) {
- char **key_and_value;
-
- key_and_value = g_strsplit (environment[i], "=", 2);
+ char **res;
+ char **ret;
+ int i;
- gdm_session_worker_set_environment_variable (worker, key_and_value[0], key_and_value[1]);
+ res = pam_getenvlist (worker->priv->pam_handle);
+ ret = g_strdupv(res);
- g_strfreev (key_and_value);
- }
-
- for (i = 0; environment[i]; i++) {
- free (environment[i]);
+ if (res) {
+ for (i = 0; res[i] != NULL; i++)
+ free (res[i]);
+ free (res);
}
- free (environment);
-}
-
-static void
-gdm_session_worker_fill_environment_array (const char *key,
- const char *value,
- GPtrArray *environment)
-{
- char *variable;
-
- if (value == NULL)
- return;
-
- variable = g_strdup_printf ("%s=%s", key, value);
-
- g_ptr_array_add (environment, variable);
-}
-
-static char **
-gdm_session_worker_get_environment (GdmSessionWorker *worker)
-{
- GPtrArray *environment;
-
- environment = g_ptr_array_new ();
- g_hash_table_foreach (worker->priv->environment,
- (GHFunc) gdm_session_worker_fill_environment_array,
- environment);
- g_ptr_array_add (environment, NULL);
-
- return (char **) g_ptr_array_free (environment, FALSE);
+ return ret;
}
static void
@@ -1963,9 +1963,6 @@ gdm_session_worker_start_user_session (GdmSessionWorker *worker,
pid_t session_pid;
int error_code;
- g_debug ("GdmSessionWorker: querying pam for user environment");
- gdm_session_worker_update_environment_from_pam (worker);
-
register_ck_session (worker);
passwd_entry = getpwnam (worker->priv->username);
@@ -2029,12 +2026,11 @@ gdm_session_worker_start_user_session (GdmSessionWorker *worker,
g_assert (geteuid () == getuid ());
- home_dir = g_hash_table_lookup (worker->priv->environment,
- "HOME");
-
+ home_dir = gdm_session_worker_get_environment_variable (worker, "HOME");
if ((home_dir == NULL) || g_chdir (home_dir) < 0) {
g_chdir ("/");
}
+ g_free (home_dir);
fd = open ("/dev/null", O_RDWR);
dup2 (fd, STDIN_FILENO);
@@ -2939,10 +2935,6 @@ gdm_session_worker_init (GdmSessionWorker *worker)
{
worker->priv = GDM_SESSION_WORKER_GET_PRIVATE (worker);
- worker->priv->environment = g_hash_table_new_full (g_str_hash,
- g_str_equal,
- (GDestroyNotify) g_free,
- (GDestroyNotify) g_free);
}
static void
@@ -2980,11 +2972,6 @@ gdm_session_worker_finalize (GObject *object)
worker->priv->arguments = NULL;
}
- if (worker->priv->environment != NULL) {
- g_hash_table_destroy (worker->priv->environment);
- worker->priv->environment = NULL;
- }
-
G_OBJECT_CLASS (gdm_session_worker_parent_class)->finalize (object);
}
--
1.6.3.3
Attachment:
signature.asc
Description: This is a digitally signed message part.