diff --git a/appvm-scripts/usrbin/qubes-session b/appvm-scripts/usrbin/qubes-session index 4417ba7e..e5bedc2d 100755 --- 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 index 69b1cf10..47a27058 100644 --- 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 index 17f9fb6e..7d270b66 100644 --- 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 index 006efcd7..f9eddc25 100644 --- a/gui-agent/qubes-gui-runuser.c +++ b/gui-agent/qubes-gui-runuser.c @@ -31,13 +31,257 @@ #include #include #include +#include #include +#include #ifdef HAVE_PAM #include #endif -pid_t child_pid = 0; +pid_t fork_pid = 0; + +/* Note: augment_pam_env_with_systemd_env expects out_env_ref to be pointer to + * a NULL-terminated array of strings consisting of equals-sign-separated + * key-value pairs. All items in out_env_ref MUST be heap-allocated, as this + * function is liable to free() any item in the passed-in array in order to + * replace it with an item obtained from systemd's environment. + * + * Note also, this function talks with the systemd user instance, not the + * system instance (pid 1). + */ +static void augment_pam_env_with_systemd_env(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, "augment_pam_env_with_systemd_env: NULL out_env_ref argument is unsupported!\n"); + out_env_arr = *out_env_ref; + if (out_env_arr == NULL) + errx(1, "augment_pam_env_with_systemd_env: NULL array in out_env_ref argument is unsupported!\n"); + 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("augment_pam_env_with_systemd_env: Failed to initialize D-Bus, 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 D-Bus connection + * closes. We don't want that, turn that off. + */ + dbus_connection_set_exit_on_disconnect(dbus_conn, FALSE); + + /* Create a D-Bus 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("augment_pam_env_with_systemd_env: Failed to create D-Bus method call object!\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("augment_pam_env_with_systemd_env: Failed to append arguments to D-Bus method call object!\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("augment_pam_env_with_systemd_env: 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("augment_pam_env_with_systemd_env: Did not get method call return object 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("augment_pam_env_with_systemd_env: systemd returned an empty method call return object!\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("augment_pam_env_with_systemd_env: systemd did not return a variant object!\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. + */ + inner_iter_typesig = dbus_message_iter_get_signature(&reply_inner_iter); + if (strcmp(inner_iter_typesig, "as") != 0) { + warnx("augment_pam_env_with_systemd_env: Variant object from systemd does not contain a string array!\n"); + goto dbus_cleanup; + } + if (dbus_message_iter_get_arg_type(&reply_inner_iter) + != DBUS_TYPE_ARRAY) { + warnx("augment_pam_env_with_systemd_env: Variant object from systemd reported itself as a string array, but is not an 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". + */ + while ((current_type = dbus_message_iter_get_arg_type(&reply_arr_iter)) + != DBUS_TYPE_INVALID) { + if (current_type != DBUS_TYPE_STRING) { + warnx("augment_pam_env_with_systemd_env: 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, "augment_pam_env_with_systemd_env: 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, "augment_pam_env_with_systemd_env: Failed to allocate memory for environment item"); + + dbus_message_iter_next(&reply_arr_iter); + } + + /* 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, "augment_pam_env_with_systemd_env: Environment variable without equals sign encountered in systemd environment!\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, "augment_pam_env_with_systemd_env: Environment variable without equals sign encountered in PAM environment!\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, "augment_pam_env_with_systemd_env: 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: + /* Don't free the elements of env_arr, they have been placed into + * out_env_arr. + */ + if (env_arr != NULL) + free(env_arr); + if (inner_iter_typesig != NULL) + dbus_free(inner_iter_typesig); + if (env_reply != NULL) + dbus_message_unref(env_reply); + if (env_request != NULL) + dbus_message_unref(env_request); + if (dbus_conn != NULL) + dbus_connection_unref(dbus_conn); + *out_env_ref = out_env_arr; +} #ifdef HAVE_PAM static int pam_conv_callback(int num_msg, const struct pam_message **msg, @@ -79,8 +323,8 @@ static pid_t do_execute(char *user, char *path, char **argv) int retval=0, status; char **env; char env_buf[256]; + size_t env_idx; pam_handle_t *pamh=NULL; - pid_t pid; if (!user) goto error; @@ -223,9 +467,9 @@ static pid_t do_execute(char *user, char *path, char **argv) if (retval != PAM_SUCCESS) goto error; - pid = fork(); + fork_pid = fork(); - switch (pid) { + switch (fork_pid) { case -1: perror("fork xorg"); goto error; @@ -241,6 +485,24 @@ 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); + /* Get the DBUS_SESSION_BUS_ADDRESS from the PAM environment and + * place it into our own environment, so that we can talk to + * systemd via D-Bus to get environment variables from it. + */ + for (env_idx = 0; env[env_idx] != NULL; env_idx++) { + if (strncmp(env[env_idx], "DBUS_SESSION_BUS_ADDRESS=", + strlen("DBUS_SESSION_BUS_ADDRESS=")) == 0) { + putenv(strdup(env[env_idx])); + break; + } + } + + /* Try to augment the environment list from PAM with the + * environment from the systemd user instance for the current + * user. + */ + augment_pam_env_with_systemd_env(&env); + /* try to enter home dir, but don't abort if it fails */ retval = chdir(pw->pw_dir); if (retval == -1) @@ -252,21 +514,19 @@ static pid_t do_execute(char *user, char *path, char **argv) default:; } - child_pid = pid; - for (;;) { - pid_t wait_pid = waitpid(pid, &status, 0); + pid_t wait_pid = waitpid(fork_pid, &status, 0); if (wait_pid == (pid_t)-1) { if (errno == EINTR) continue; perror("waitpid"); goto error; } - if (wait_pid == pid) + if (wait_pid == fork_pid) break; } - child_pid = 0; + fork_pid = 0; retval = pam_close_session(pamh, 0); retval = pam_setcred(pamh, PAM_DELETE_CRED | PAM_SILENT); @@ -332,8 +592,10 @@ static pid_t do_execute(char *user, char *path, char **argv) #endif static void propagate_signal(int signal) { - if (child_pid) - kill(child_pid, signal); + if (!fork_pid) { + exit(0); + } + kill(fork_pid, signal); } static void usage(char *argv0) { diff --git a/rpm_spec/gui-agent.spec.in b/rpm_spec/gui-agent.spec.in index d5b3b28c..ac95d997 100644 --- 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