WIP: qubes-gui-runner: get environment from systemd and pass it through to the session

ArrayBolt3 · ArrayBolt3 · commit 046814c76540 · 2025-10-07T22:29:04.000-05:00
diff --git a/appvm-scripts/usrbin/qubes-session b/appvm-scripts/usrbin/qubes-session
@@ -27,17 +27,6 @@
 
 loginctl activate "$XDG_SESSION_ID"
 
-# Now import the environment from the systemd user session.
-# This is necessary to enable users to configure their
-# Qubes environment using the standard environment.d
-# facility.  Documentation for the facility is at:
-# https://www.freedesktop.org/software/systemd/man/environment.d.html
-set -a # export all variables
-env=$(systemctl --user show-environment) && eval "$env" || exit
-set +a
-unset env
-
-
 if qsvc guivm-gui-agent; then
     if [ -e "$HOME/.xinitrc" ]; then
         . "$HOME/.xinitrc"
diff --git a/debian/control b/debian/control
@@ -26,6 +26,7 @@ Build-Depends:
     qubesdb-dev,
     libltdl-dev,
     libunistring-dev,
+    libdbus-1-dev,
 Standards-Version: 4.4.0.1
 Homepage: http://www.qubes-os.org/
 #Vcs-Git: git://git.debian.org/collab-maint/qubes-gui-agent.git
diff --git a/gui-agent/Makefile b/gui-agent/Makefile
@@ -20,7 +20,8 @@
 #
 
 CC ?= gcc
-CFLAGS += -I../include/ `pkg-config --cflags vchan` -g -Wall -Wextra -Werror -fPIC \
+CFLAGS += -I../include/ `pkg-config --cflags vchan` \
+	  `pkg-config --cflags dbus-1` -g -Wall -Wextra -Werror -fPIC \
 	  -Wmissing-prototypes -Wstrict-prototypes -Wold-style-declaration \
 	  -Wold-style-definition
 OBJS = vmside.o txrx-vchan.o error.o list.o encoding.o
@@ -33,7 +34,7 @@ qubes-gui: $(OBJS)
 	$(CC) $(LDFLAGS) -pie -g -o qubes-gui $(OBJS) \
 		$(LIBS)
 qubes-gui-runuser: CFLAGS += -g -Wall -Wextra -Werror -pie -fPIC
-qubes-gui-runuser: LDLIBS += -lpam -lqubesdb
+qubes-gui-runuser: LDLIBS += -lpam -lqubesdb -ldbus-1
 qubes-gui-runuser: qubes-gui-runuser.c
 clean:
 	rm -f qubes-gui qubes-gui-runuser ./*.o ./*~
diff --git a/gui-agent/qubes-gui-runuser.c b/gui-agent/qubes-gui-runuser.c
@@ -31,14 +31,243 @@
 #include <grp.h>
 #include <pwd.h>
 #include <err.h>
+#include <stdbool.h>
 #include <qubesdb-client.h>
+#include <dbus/dbus.h>
 
 #ifdef HAVE_PAM
 #include <security/pam_appl.h>
 #endif
 
 pid_t child_pid = 0;
 
+static void get_systemd_env_from_dbus(char ***out_env_ref)
+{
+    DBusConnection *dbus_conn = NULL;
+    DBusError error_data = { 0 };
+    DBusMessage *env_request = NULL;
+    const char *systemd_manager_str = "org.freedesktop.systemd1.Manager";
+    const char *environment_str = "Environment";
+    dbus_bool_t ret = FALSE;
+    DBusMessage *env_reply = NULL;
+    int reply_type = 0;
+    DBusMessageIter reply_iter = { 0 };
+    DBusMessageIter reply_inner_iter = { 0 };
+    char *inner_iter_typesig = NULL;
+    DBusMessageIter reply_arr_iter = { 0 };
+    const char *env_val = NULL;
+    char **env_arr = NULL;
+    char **out_env_arr = NULL;
+    size_t env_arr_len = 0;
+    size_t out_env_arr_len = 0;
+    size_t out_env_idx = 0;
+    size_t env_idx = 0;
+    char *out_env_eq_ptr = NULL;
+    char *env_eq_ptr = NULL;
+    size_t out_env_pre_eq_len = 0;
+    size_t env_pre_eq_len = 0;
+    bool did_override_env_var = false;
+    int current_type = 0;
+
+    if (out_env_ref == NULL)
+        errx(1, "get_systemd_env_from_dbus does not support a NULL out_env_ref argument!\n");
+    out_env_arr = *out_env_ref;
+    for (out_env_idx = 0; out_env_arr[out_env_idx] != NULL; out_env_idx++) {
+        out_env_arr_len++;
+    }
+    /* Increment 1 to include the NULL element at the end of the array. */
+    out_env_arr_len++;
+
+    /* Initialize D-Bus. */
+    dbus_error_init(&error_data);
+    dbus_conn = dbus_bus_get(DBUS_BUS_SESSION, &error_data);
+    if (dbus_conn == NULL) {
+        warnx("Failed to initialize DBus, error name: '%s', error contents: '%s'\n",
+              error_data.name,
+              error_data.message);
+        goto dbus_cleanup;
+    }
+
+    /* dbus_bus_get sets up our process to be killed if the DBus connection
+     * closes. We don't want that, turn that off.
+     */
+    dbus_connection_set_exit_on_disconnect(dbus_conn, FALSE);
+
+    /* Create a DBus method call message for getting the "Environment"
+     * property of org.freedesktop.systemd1.Manager.
+     */
+    env_request = dbus_message_new_method_call("org.freedesktop.systemd1",
+                                               "/org/freedesktop/systemd1",
+                                               "org.freedesktop.DBus.Properties",
+                                               "Get");
+    if (env_request == NULL) {
+        warnx("Failed to create DBus method call!\n");
+        goto dbus_cleanup;
+    }
+
+    ret = dbus_message_append_args(env_request,
+                                   DBUS_TYPE_STRING, &systemd_manager_str,
+                                   DBUS_TYPE_STRING, &environment_str,
+                                   DBUS_TYPE_INVALID);
+    if (ret == FALSE) {
+        warnx("Failed to append arguments to DBus method call!\n");
+        goto dbus_cleanup;
+    }
+
+    /* Send the method call to systemd, waiting a maximum of 500 milliseconds
+     * for a response.
+     */
+    env_reply = dbus_connection_send_with_reply_and_block(dbus_conn,
+                                                          env_request,
+                                                          500,
+                                                          &error_data);
+    if (env_reply == NULL) {
+        warnx("Failed to request environment data from systemd, error name: '%s', error contents: '%s'\n",
+              error_data.name,
+              error_data.message);
+        goto dbus_cleanup;
+    }
+
+    /* Ensure the reply is a method call return value. */
+    reply_type = dbus_message_get_type(env_reply);
+    if (reply_type != DBUS_MESSAGE_TYPE_METHOD_RETURN) {
+        warnx("Did not get expected method reply from systemd!\n");
+        goto dbus_cleanup;
+    }
+
+    /* D-Bus property get methods return a Variant, which is not a basic type,
+     * thus we have to iterate through the method return value to get to the
+     * contents.
+     */
+    ret = dbus_message_iter_init(env_reply, &reply_iter);
+    if (ret == FALSE) {
+        warnx("systemd returned no environment variables!\n");
+        goto dbus_cleanup;
+    }
+
+    /* Make sure we actually got a Variant in reply. If so, recurse into it so
+     * we can look at its contents.
+     */
+    if (dbus_message_iter_get_arg_type(&reply_iter) != DBUS_TYPE_VARIANT) {
+        warnx("Did not get variant container from systemd!\n");
+        goto dbus_cleanup;
+    }
+    dbus_message_iter_recurse(&reply_iter, &reply_inner_iter);
+
+    /* Ensure the returned Variant contains a string array. The type signature
+     * for this data type in D-Bus is "as". If we do have a string array,
+     * recurse into it so we can iterate through it.
+     * TODO: The D-Bus API documentation warns "Be sure you have somehow
+     * checked that dbus_message_iter_get_arg_type() matches the type you are
+     * expecting to recurse into. Results of this function are undefined if
+     * there is no container to recurse into at the current iterator position.
+     * Are we sure that dbus_message_iter_get_signature() does a sufficiently
+     * equivalent job here?
+     */
+    inner_iter_typesig = dbus_message_iter_get_signature(&reply_inner_iter);
+    if (strcmp(inner_iter_typesig, "as") != 0) {
+        warnx("Variant container from systemd does not contain a string array!\n");
+        goto dbus_cleanup;
+    }
+    dbus_message_iter_recurse(&reply_inner_iter, &reply_arr_iter);
+
+    /* Walk through the elements of the string array, appending them to our
+     * internal environment array "env_arr".
+     * TODO: What is the expected behavior if the same variable is returned
+     * here more than once?
+     */
+    while ((current_type = dbus_message_iter_get_arg_type(&reply_arr_iter))
+                != DBUS_TYPE_INVALID) {
+        if (current_type != DBUS_TYPE_STRING) {
+            warnx("Non-string item found in string array!\n");
+            goto dbus_cleanup;
+        }
+        dbus_message_iter_get_basic(&reply_arr_iter, &env_val);
+        env_arr_len++;
+        env_arr = reallocarray(env_arr, env_arr_len, sizeof(char *));
+        if (env_arr == NULL)
+            err(1, "Failed to allocate memory for environment array");
+        env_arr[env_arr_len - 1] = strdup(env_val);
+        if (env_arr[env_arr_len - 1] == NULL)
+            err(1, "Failed to allocate memory for environment item");
+    }
+
+    /* Merge the environment from systemd with the environment from PAM.
+     * Prefer variables from systemd over variables from PAM.
+     */
+    for (env_idx = 0; env_idx < env_arr_len; env_idx++) {
+        env_eq_ptr = strstr(env_arr[env_idx], "=");
+        if (env_eq_ptr == NULL)
+            errx(1, "Environment variable without equals sign encountered!\n");
+        env_pre_eq_len = env_eq_ptr - env_arr[env_idx];
+        did_override_env_var = false;
+
+        for (out_env_idx = 0; out_env_arr[out_env_idx] != NULL;
+                    out_env_idx++) {
+            out_env_eq_ptr = strstr(out_env_arr[out_env_idx], "=");
+            if (out_env_eq_ptr == NULL)
+                errx(1, "Environment variable without equals sign encountered!\n");
+            out_env_pre_eq_len = out_env_eq_ptr - out_env_arr[out_env_idx];
+
+            if (out_env_pre_eq_len != env_pre_eq_len)
+                continue;
+
+            if (strncmp(out_env_arr[out_env_idx], env_arr[env_idx],
+                        env_pre_eq_len) == 0) {
+                /* According to `man pam_getenvlist`, "it is the
+                 * responsibility of the calling application to free() [the
+                 * memory allocated by pam_getenvlist()]". out_env_arr will be
+                 * a char ** created by pam_getenvlist(), thus we can safely
+                 * free items in out_env_arr.
+                 */
+                free(out_env_arr[out_env_idx]);
+
+                /* We intentionally are NOT copying the string here. Every
+                 * item in env_arr will eventually end up in out_env_arr, so
+                 * rather than copying strings from env_arr and then freeing
+                 * env_arr, we simply merge all of the pointers from env_arr
+                 * into out_env_arr, freeing anything in out_env_arr that is
+                 * overridden by something from systemd. This wastes no
+                 * memory, and is quite a bit more efficient.
+                 */
+                out_env_arr[out_env_idx] = env_arr[env_idx];
+
+                /* Signal to the outer loop that we don't need to append an
+                 * item to the environment list.
+                 */
+                did_override_env_var = true;
+                break;
+            }
+        }
+
+        if (!did_override_env_var) {
+            /* Append the variable to the list. */
+            out_env_arr_len++;
+            out_env_arr = reallocarray(out_env_arr, out_env_arr_len,
+                                       sizeof(char *));
+            if (out_env_arr == NULL)
+                err(1, "Failed to allocate memory for environment item");
+
+            out_env_arr[out_env_arr_len - 1] = NULL;
+            /* See above for rationale behind using assignment rather than
+             * copying here.
+             */
+            out_env_arr[out_env_arr_len - 2] = env_arr[env_idx];
+        }
+    }
+
+dbus_cleanup:
+    if (dbus_conn != NULL)
+        dbus_connection_unref(dbus_conn);
+    if (env_request != NULL)
+        dbus_message_unref(env_request);
+    if (env_reply != NULL)
+        dbus_message_unref(env_reply);
+    if (inner_iter_typesig != NULL)
+        dbus_free(inner_iter_typesig);
+    *out_env_ref = out_env_arr;
+}
+
 #ifdef HAVE_PAM
 static int pam_conv_callback(int num_msg, const struct pam_message **msg,
         struct pam_response **resp, void *appdata_ptr __attribute__((__unused__)))
@@ -241,6 +470,11 @@ static pid_t do_execute(char *user, char *path, char **argv)
             /* This is a copy but don't care to free as we exec later anyway.  */
             env = pam_getenvlist (pamh);
 
+            /* Augment the environment list from PAM with the environment from
+             * systemd's user manager.
+             */
+            get_systemd_env_from_dbus(&env);
+
             /* try to enter home dir, but don't abort if it fails */
             retval = chdir(pw->pw_dir);
             if (retval == -1)
diff --git a/rpm_spec/gui-agent.spec.in b/rpm_spec/gui-agent.spec.in
@@ -76,6 +76,7 @@ BuildRequires:	qubes-db-devel
 BuildRequires:	xen-devel
 BuildRequires:	systemd-rpm-macros
 BuildRequires:	libunistring-devel
+BuildRequires:  dbus-devel
 %if 0%{?is_opensuse}
 # for directory ownership
 BuildRequires:  xinit
