Skip to content

Commit 0a78ebf

Browse files
marmarekmarmarek
committed
Merge remote-tracking branch 'origin/pr/246'
* origin/pr/246: qubes-gui-runner: get environment from systemd user instance and pass it through to the session
2 parents f03d884 + 6291d8b commit 0a78ebf

5 files changed

Lines changed: 278 additions & 24 deletions

File tree

‎appvm-scripts/usrbin/qubes-session‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,6 @@
2727

2828
loginctl activate "$XDG_SESSION_ID"
2929

30-
# Now import the environment from the systemd user session.
31-
# This is necessary to enable users to configure their
32-
# Qubes environment using the standard environment.d
33-
# facility. Documentation for the facility is at:
34-
# https://www.freedesktop.org/software/systemd/man/environment.d.html
35-
set -a # export all variables
36-
env=$(systemctl --user show-environment) && eval "$env" || exit
37-
set +a
38-
unset env
39-
40-
4130
if qsvc guivm-gui-agent; then
4231
if [ -e "$HOME/.xinitrc" ]; then
4332
. "$HOME/.xinitrc"

‎debian/control‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Build-Depends:
2626
qubesdb-dev,
2727
libltdl-dev,
2828
libunistring-dev,
29+
libdbus-1-dev,
2930
Standards-Version: 4.4.0.1
3031
Homepage: http://www.qubes-os.org/
3132
#Vcs-Git: git://git.debian.org/collab-maint/qubes-gui-agent.git

‎gui-agent/Makefile‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
#
2121

2222
CC ?= gcc
23-
CFLAGS += -I../include/ `pkg-config --cflags vchan` -g -Wall -Wextra -Werror -fPIC \
23+
CFLAGS += -I../include/ `pkg-config --cflags vchan` \
24+
`pkg-config --cflags dbus-1` -g -Wall -Wextra -Werror -fPIC \
2425
-Wmissing-prototypes -Wstrict-prototypes -Wold-style-declaration \
2526
-Wold-style-definition
2627
OBJS = vmside.o txrx-vchan.o error.o list.o encoding.o
@@ -33,7 +34,7 @@ qubes-gui: $(OBJS)
3334
$(CC) $(LDFLAGS) -pie -g -o qubes-gui $(OBJS) \
3435
$(LIBS)
3536
qubes-gui-runuser: CFLAGS += -g -Wall -Wextra -Werror -pie -fPIC
36-
qubes-gui-runuser: LDLIBS += -lpam -lqubesdb
37+
qubes-gui-runuser: LDLIBS += -lpam -lqubesdb -ldbus-1
3738
qubes-gui-runuser: qubes-gui-runuser.c
3839
clean:
3940
rm -f qubes-gui qubes-gui-runuser ./*.o ./*~

‎gui-agent/qubes-gui-runuser.c‎

Lines changed: 273 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,257 @@
3131
#include <grp.h>
3232
#include <pwd.h>
3333
#include <err.h>
34+
#include <stdbool.h>
3435
#include <qubesdb-client.h>
36+
#include <dbus/dbus.h>
3537

3638
#ifdef HAVE_PAM
3739
#include <security/pam_appl.h>
3840
#endif
3941

40-
pid_t child_pid = 0;
42+
pid_t fork_pid = 0;
43+
44+
/* Note: augment_pam_env_with_systemd_env expects out_env_ref to be pointer to
45+
* a NULL-terminated array of strings consisting of equals-sign-separated
46+
* key-value pairs. All items in out_env_ref MUST be heap-allocated, as this
47+
* function is liable to free() any item in the passed-in array in order to
48+
* replace it with an item obtained from systemd's environment.
49+
*
50+
* Note also, this function talks with the systemd user instance, not the
51+
* system instance (pid 1).
52+
*/
53+
static void augment_pam_env_with_systemd_env(char ***out_env_ref)
54+
{
55+
DBusConnection *dbus_conn = NULL;
56+
DBusError error_data = { 0 };
57+
DBusMessage *env_request = NULL;
58+
const char *systemd_manager_str = "org.freedesktop.systemd1.Manager";
59+
const char *environment_str = "Environment";
60+
dbus_bool_t ret = FALSE;
61+
DBusMessage *env_reply = NULL;
62+
int reply_type = 0;
63+
DBusMessageIter reply_iter = { 0 };
64+
DBusMessageIter reply_inner_iter = { 0 };
65+
char *inner_iter_typesig = NULL;
66+
DBusMessageIter reply_arr_iter = { 0 };
67+
const char *env_val = NULL;
68+
char **env_arr = NULL;
69+
char **out_env_arr = NULL;
70+
size_t env_arr_len = 0;
71+
size_t out_env_arr_len = 0;
72+
size_t out_env_idx = 0;
73+
size_t env_idx = 0;
74+
char *out_env_eq_ptr = NULL;
75+
char *env_eq_ptr = NULL;
76+
size_t out_env_pre_eq_len = 0;
77+
size_t env_pre_eq_len = 0;
78+
bool did_override_env_var = false;
79+
int current_type = 0;
80+
81+
if (out_env_ref == NULL)
82+
errx(1, "augment_pam_env_with_systemd_env: NULL out_env_ref argument is unsupported!\n");
83+
out_env_arr = *out_env_ref;
84+
if (out_env_arr == NULL)
85+
errx(1, "augment_pam_env_with_systemd_env: NULL array in out_env_ref argument is unsupported!\n");
86+
for (out_env_idx = 0; out_env_arr[out_env_idx] != NULL; out_env_idx++) {
87+
out_env_arr_len++;
88+
}
89+
/* Increment 1 to include the NULL element at the end of the array. */
90+
out_env_arr_len++;
91+
92+
/* Initialize D-Bus. */
93+
dbus_error_init(&error_data);
94+
dbus_conn = dbus_bus_get(DBUS_BUS_SESSION, &error_data);
95+
if (dbus_conn == NULL) {
96+
warnx("augment_pam_env_with_systemd_env: Failed to initialize D-Bus, error name: '%s', error contents: '%s'\n",
97+
error_data.name,
98+
error_data.message);
99+
goto dbus_cleanup;
100+
}
101+
102+
/* dbus_bus_get sets up our process to be killed if the D-Bus connection
103+
* closes. We don't want that, turn that off.
104+
*/
105+
dbus_connection_set_exit_on_disconnect(dbus_conn, FALSE);
106+
107+
/* Create a D-Bus method call message for getting the "Environment"
108+
* property of org.freedesktop.systemd1.Manager.
109+
*/
110+
env_request = dbus_message_new_method_call("org.freedesktop.systemd1",
111+
"/org/freedesktop/systemd1",
112+
"org.freedesktop.DBus.Properties",
113+
"Get");
114+
if (env_request == NULL) {
115+
warnx("augment_pam_env_with_systemd_env: Failed to create D-Bus method call object!\n");
116+
goto dbus_cleanup;
117+
}
118+
119+
ret = dbus_message_append_args(env_request,
120+
DBUS_TYPE_STRING, &systemd_manager_str,
121+
DBUS_TYPE_STRING, &environment_str,
122+
DBUS_TYPE_INVALID);
123+
if (ret == FALSE) {
124+
warnx("augment_pam_env_with_systemd_env: Failed to append arguments to D-Bus method call object!\n");
125+
goto dbus_cleanup;
126+
}
127+
128+
/* Send the method call to systemd, waiting a maximum of 500 milliseconds
129+
* for a response.
130+
*/
131+
env_reply = dbus_connection_send_with_reply_and_block(dbus_conn,
132+
env_request,
133+
500,
134+
&error_data);
135+
if (env_reply == NULL) {
136+
warnx("augment_pam_env_with_systemd_env: Failed to request environment data from systemd, error name: '%s', error contents: '%s'\n",
137+
error_data.name,
138+
error_data.message);
139+
goto dbus_cleanup;
140+
}
141+
142+
/* Ensure the reply is a method call return value. */
143+
reply_type = dbus_message_get_type(env_reply);
144+
if (reply_type != DBUS_MESSAGE_TYPE_METHOD_RETURN) {
145+
warnx("augment_pam_env_with_systemd_env: Did not get method call return object from systemd!\n");
146+
goto dbus_cleanup;
147+
}
148+
149+
/* D-Bus property get methods return a Variant, which is not a basic type,
150+
* thus we have to iterate through the method return value to get to the
151+
* contents.
152+
*/
153+
ret = dbus_message_iter_init(env_reply, &reply_iter);
154+
if (ret == FALSE) {
155+
warnx("augment_pam_env_with_systemd_env: systemd returned an empty method call return object!\n");
156+
goto dbus_cleanup;
157+
}
158+
159+
/* Make sure we actually got a Variant in reply. If so, recurse into it so
160+
* we can look at its contents.
161+
*/
162+
if (dbus_message_iter_get_arg_type(&reply_iter) != DBUS_TYPE_VARIANT) {
163+
warnx("augment_pam_env_with_systemd_env: systemd did not return a variant object!\n");
164+
goto dbus_cleanup;
165+
}
166+
dbus_message_iter_recurse(&reply_iter, &reply_inner_iter);
167+
168+
/* Ensure the returned Variant contains a string array. The type signature
169+
* for this data type in D-Bus is "as". If we do have a string array,
170+
* recurse into it so we can iterate through it.
171+
*/
172+
inner_iter_typesig = dbus_message_iter_get_signature(&reply_inner_iter);
173+
if (strcmp(inner_iter_typesig, "as") != 0) {
174+
warnx("augment_pam_env_with_systemd_env: Variant object from systemd does not contain a string array!\n");
175+
goto dbus_cleanup;
176+
}
177+
if (dbus_message_iter_get_arg_type(&reply_inner_iter)
178+
!= DBUS_TYPE_ARRAY) {
179+
warnx("augment_pam_env_with_systemd_env: Variant object from systemd reported itself as a string array, but is not an array!\n");
180+
goto dbus_cleanup;
181+
}
182+
dbus_message_iter_recurse(&reply_inner_iter, &reply_arr_iter);
183+
184+
/* Walk through the elements of the string array, appending them to our
185+
* internal environment array "env_arr".
186+
*/
187+
while ((current_type = dbus_message_iter_get_arg_type(&reply_arr_iter))
188+
!= DBUS_TYPE_INVALID) {
189+
if (current_type != DBUS_TYPE_STRING) {
190+
warnx("augment_pam_env_with_systemd_env: Non-string item found in string array!\n");
191+
goto dbus_cleanup;
192+
}
193+
dbus_message_iter_get_basic(&reply_arr_iter, &env_val);
194+
env_arr_len++;
195+
env_arr = reallocarray(env_arr, env_arr_len, sizeof(char *));
196+
if (env_arr == NULL)
197+
err(1, "augment_pam_env_with_systemd_env: Failed to allocate memory for environment array");
198+
env_arr[env_arr_len - 1] = strdup(env_val);
199+
if (env_arr[env_arr_len - 1] == NULL)
200+
err(1, "augment_pam_env_with_systemd_env: Failed to allocate memory for environment item");
201+
202+
dbus_message_iter_next(&reply_arr_iter);
203+
}
204+
205+
/* Merge the environment from systemd with the environment from PAM.
206+
* Prefer variables from systemd over variables from PAM.
207+
*/
208+
for (env_idx = 0; env_idx < env_arr_len; env_idx++) {
209+
env_eq_ptr = strstr(env_arr[env_idx], "=");
210+
if (env_eq_ptr == NULL)
211+
errx(1, "augment_pam_env_with_systemd_env: Environment variable without equals sign encountered in systemd environment!\n");
212+
env_pre_eq_len = env_eq_ptr - env_arr[env_idx];
213+
did_override_env_var = false;
214+
215+
for (out_env_idx = 0; out_env_arr[out_env_idx] != NULL;
216+
out_env_idx++) {
217+
out_env_eq_ptr = strstr(out_env_arr[out_env_idx], "=");
218+
if (out_env_eq_ptr == NULL)
219+
errx(1, "augment_pam_env_with_systemd_env: Environment variable without equals sign encountered in PAM environment!\n");
220+
out_env_pre_eq_len = out_env_eq_ptr - out_env_arr[out_env_idx];
221+
222+
if (out_env_pre_eq_len != env_pre_eq_len)
223+
continue;
224+
225+
if (strncmp(out_env_arr[out_env_idx], env_arr[env_idx],
226+
env_pre_eq_len) == 0) {
227+
/* According to `man pam_getenvlist`, "it is the
228+
* responsibility of the calling application to free() [the
229+
* memory allocated by pam_getenvlist()]". out_env_arr will be
230+
* a char ** created by pam_getenvlist(), thus we can safely
231+
* free items in out_env_arr.
232+
*/
233+
free(out_env_arr[out_env_idx]);
234+
235+
/* We intentionally are NOT copying the string here. Every
236+
* item in env_arr will eventually end up in out_env_arr, so
237+
* rather than copying strings from env_arr and then freeing
238+
* env_arr, we simply merge all of the pointers from env_arr
239+
* into out_env_arr, freeing anything in out_env_arr that is
240+
* overridden by something from systemd. This wastes no
241+
* memory, and is quite a bit more efficient.
242+
*/
243+
out_env_arr[out_env_idx] = env_arr[env_idx];
244+
245+
/* Signal to the outer loop that we don't need to append an
246+
* item to the environment list.
247+
*/
248+
did_override_env_var = true;
249+
break;
250+
}
251+
}
252+
253+
if (!did_override_env_var) {
254+
/* Append the variable to the list. */
255+
out_env_arr_len++;
256+
out_env_arr = reallocarray(out_env_arr, out_env_arr_len,
257+
sizeof(char *));
258+
if (out_env_arr == NULL)
259+
err(1, "augment_pam_env_with_systemd_env: Failed to allocate memory for environment item");
260+
261+
out_env_arr[out_env_arr_len - 1] = NULL;
262+
/* See above for rationale behind using assignment rather than
263+
* copying here.
264+
*/
265+
out_env_arr[out_env_arr_len - 2] = env_arr[env_idx];
266+
}
267+
}
268+
269+
dbus_cleanup:
270+
/* Don't free the elements of env_arr, they have been placed into
271+
* out_env_arr.
272+
*/
273+
if (env_arr != NULL)
274+
free(env_arr);
275+
if (inner_iter_typesig != NULL)
276+
dbus_free(inner_iter_typesig);
277+
if (env_reply != NULL)
278+
dbus_message_unref(env_reply);
279+
if (env_request != NULL)
280+
dbus_message_unref(env_request);
281+
if (dbus_conn != NULL)
282+
dbus_connection_unref(dbus_conn);
283+
*out_env_ref = out_env_arr;
284+
}
41285

42286
#ifdef HAVE_PAM
43287
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)
79323
int retval=0, status;
80324
char **env;
81325
char env_buf[256];
326+
size_t env_idx;
82327
pam_handle_t *pamh=NULL;
83-
pid_t pid;
84328

85329
if (!user)
86330
goto error;
@@ -223,9 +467,9 @@ static pid_t do_execute(char *user, char *path, char **argv)
223467
if (retval != PAM_SUCCESS)
224468
goto error;
225469

226-
pid = fork();
470+
fork_pid = fork();
227471

228-
switch (pid) {
472+
switch (fork_pid) {
229473
case -1:
230474
perror("fork xorg");
231475
goto error;
@@ -241,6 +485,24 @@ static pid_t do_execute(char *user, char *path, char **argv)
241485
/* This is a copy but don't care to free as we exec later anyway. */
242486
env = pam_getenvlist (pamh);
243487

488+
/* Get the DBUS_SESSION_BUS_ADDRESS from the PAM environment and
489+
* place it into our own environment, so that we can talk to
490+
* systemd via D-Bus to get environment variables from it.
491+
*/
492+
for (env_idx = 0; env[env_idx] != NULL; env_idx++) {
493+
if (strncmp(env[env_idx], "DBUS_SESSION_BUS_ADDRESS=",
494+
strlen("DBUS_SESSION_BUS_ADDRESS=")) == 0) {
495+
putenv(strdup(env[env_idx]));
496+
break;
497+
}
498+
}
499+
500+
/* Try to augment the environment list from PAM with the
501+
* environment from the systemd user instance for the current
502+
* user.
503+
*/
504+
augment_pam_env_with_systemd_env(&env);
505+
244506
/* try to enter home dir, but don't abort if it fails */
245507
retval = chdir(pw->pw_dir);
246508
if (retval == -1)
@@ -252,21 +514,19 @@ static pid_t do_execute(char *user, char *path, char **argv)
252514
default:;
253515
}
254516

255-
child_pid = pid;
256-
257517
for (;;) {
258-
pid_t wait_pid = waitpid(pid, &status, 0);
518+
pid_t wait_pid = waitpid(fork_pid, &status, 0);
259519
if (wait_pid == (pid_t)-1) {
260520
if (errno == EINTR)
261521
continue;
262522
perror("waitpid");
263523
goto error;
264524
}
265-
if (wait_pid == pid)
525+
if (wait_pid == fork_pid)
266526
break;
267527
}
268528

269-
child_pid = 0;
529+
fork_pid = 0;
270530

271531
retval = pam_close_session(pamh, 0);
272532
retval = pam_setcred(pamh, PAM_DELETE_CRED | PAM_SILENT);
@@ -332,8 +592,10 @@ static pid_t do_execute(char *user, char *path, char **argv)
332592
#endif
333593

334594
static void propagate_signal(int signal) {
335-
if (child_pid)
336-
kill(child_pid, signal);
595+
if (!fork_pid) {
596+
exit(0);
597+
}
598+
kill(fork_pid, signal);
337599
}
338600

339601
static void usage(char *argv0) {

‎rpm_spec/gui-agent.spec.in‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ BuildRequires: qubes-db-devel
7676
BuildRequires: xen-devel
7777
BuildRequires: systemd-rpm-macros
7878
BuildRequires: libunistring-devel
79+
BuildRequires: dbus-devel
7980
%if 0%{?is_opensuse}
8081
# for directory ownership
8182
BuildRequires: xinit

0 commit comments

Comments
 (0)