Skip to content

Commit 162de55

Browse files
committed
Avoid qrexec-client for VM -> dom0 calls
It isn't needed in this case either. This saves an execve() and makes it easier to change the behavior of qrexec-client without breaking existing qrexec-daemon processes. For the same reasons as the previous commit, it is critical to avoid calling exit() in the child process, either directly or indirectly. Therefore, ensure that none of the code that is now run in the child process calls exit(). There were places where this was done previously, so this commit also fixes a bug. Fixes: QubesOS/qubes-issues#9066
1 parent 1010488 commit 162de55

9 files changed

+339
-273
lines changed

daemon/qrexec-client.c

+20-240
Original file line numberDiff line numberDiff line change
@@ -41,24 +41,8 @@
4141
#include "libqrexec-utils.h"
4242
#include "qrexec-daemon-common.h"
4343

44-
// whether qrexec-client should replace problematic bytes with _ before printing the output
45-
static bool replace_chars_stdout = false;
46-
static bool replace_chars_stderr = false;
47-
48-
static int exit_with_code = 1;
49-
5044
#define VCHAN_BUFFER_SIZE 65536
5145

52-
#define QREXEC_DATA_MIN_VERSION QREXEC_PROTOCOL_V2
53-
54-
static int local_stdin_fd, local_stdout_fd;
55-
static pid_t local_pid = 0;
56-
/* flag if this is "remote" end of service call. In this case swap STDIN/STDOUT
57-
* msg types and send exit code at the end */
58-
static bool is_service = false;
59-
60-
static volatile sig_atomic_t sigchld = 0;
61-
6246
extern char **environ;
6347

6448
static char *xstrdup(const char *arg) {
@@ -77,62 +61,6 @@ static void set_remote_domain(const char *src_domain_name) {
7761
}
7862
}
7963

80-
/* initialize data_protocol_version */
81-
static int handle_agent_handshake(libvchan_t *vchan, bool remote_send_first)
82-
{
83-
struct msg_header hdr;
84-
struct peer_info info;
85-
int data_protocol_version = -1;
86-
int who = 0; // even - send to remote, odd - receive from remote
87-
88-
while (who < 2) {
89-
if ((who+remote_send_first) & 1) {
90-
if (!read_vchan_all(vchan, &hdr, sizeof(hdr))) {
91-
PERROR("daemon handshake");
92-
return -1;
93-
}
94-
if (hdr.type != MSG_HELLO || hdr.len != sizeof(info)) {
95-
LOG(ERROR, "Invalid daemon MSG_HELLO");
96-
return -1;
97-
}
98-
if (!read_vchan_all(vchan, &info, sizeof(info))) {
99-
PERROR("daemon handshake");
100-
return -1;
101-
}
102-
103-
data_protocol_version = info.version < QREXEC_PROTOCOL_VERSION ?
104-
info.version : QREXEC_PROTOCOL_VERSION;
105-
if (data_protocol_version < QREXEC_DATA_MIN_VERSION) {
106-
LOG(ERROR, "Incompatible daemon protocol version "
107-
"(daemon %d, client %d)",
108-
info.version, QREXEC_PROTOCOL_VERSION);
109-
return -1;
110-
}
111-
} else {
112-
hdr.type = MSG_HELLO;
113-
hdr.len = sizeof(info);
114-
info.version = QREXEC_PROTOCOL_VERSION;
115-
116-
if (!write_vchan_all(vchan, &hdr, sizeof(hdr))) {
117-
LOG(ERROR, "Failed to send MSG_HELLO hdr to daemon");
118-
return -1;
119-
}
120-
if (!write_vchan_all(vchan, &info, sizeof(info))) {
121-
LOG(ERROR, "Failed to send MSG_HELLO to daemon");
122-
return -1;
123-
}
124-
}
125-
who++;
126-
}
127-
return data_protocol_version;
128-
}
129-
130-
static void sigchld_handler(int x __attribute__((__unused__)))
131-
{
132-
sigchld = 1;
133-
signal(SIGCHLD, sigchld_handler);
134-
}
135-
13664
/* called from do_fork_exec */
13765
static _Noreturn void do_exec(const char *prog, const char *username __attribute__((unused)))
13866
{
@@ -145,119 +73,6 @@ static _Noreturn void do_exec(const char *prog, const char *username __attribute
14573
exit(1);
14674
}
14775

148-
149-
/* See also qrexec-agent.c:wait_for_session_maybe() */
150-
static void wait_for_session_maybe(char *cmdline)
151-
{
152-
int wait_for_session = 0;
153-
struct qrexec_parsed_command *cmd;
154-
pid_t pid;
155-
int status;
156-
157-
cmd = parse_qubes_rpc_command(cmdline, false);
158-
if (!cmd)
159-
goto out;
160-
161-
if (cmd->nogui)
162-
goto out;
163-
164-
if (!cmd->service_descriptor)
165-
goto out;
166-
167-
char *user = NULL;
168-
load_service_config(cmd, &wait_for_session, &user);
169-
if (!wait_for_session)
170-
goto out;
171-
172-
pid = fork();
173-
switch (pid) {
174-
case 0:
175-
close(0);
176-
exec_wait_for_session(cmd->source_domain);
177-
PERROR("exec");
178-
exit(1);
179-
case -1:
180-
PERROR("fork");
181-
goto out;
182-
}
183-
184-
if (waitpid(local_pid, &status, 0) > 0) {
185-
if (status != 0)
186-
LOG(ERROR, "wait-for-session exited with status %d", status);
187-
} else
188-
PERROR("waitpid");
189-
190-
out:
191-
destroy_qrexec_parsed_command(cmd);
192-
}
193-
194-
static int prepare_local_fds(char *cmdline, struct buffer *stdin_buffer)
195-
{
196-
if (stdin_buffer == NULL)
197-
abort();
198-
if (!cmdline) {
199-
local_stdin_fd = 1;
200-
local_stdout_fd = 0;
201-
return 0;
202-
}
203-
signal(SIGCHLD, sigchld_handler);
204-
return execute_qubes_rpc_command(cmdline, &local_pid, &local_stdin_fd, &local_stdout_fd,
205-
NULL, false, stdin_buffer);
206-
}
207-
208-
// See also qrexec-agent/qrexec-agent-data.c
209-
static _Noreturn void handle_failed_exec(libvchan_t *data_vchan)
210-
{
211-
int exit_code = 127;
212-
struct msg_header hdr = {
213-
.type = MSG_DATA_STDOUT,
214-
.len = 0,
215-
};
216-
217-
LOG(ERROR, "failed to spawn process, exiting");
218-
/*
219-
* TODO: In case we fail to execute a *local* process (is_service false),
220-
* we should either
221-
* - exit even before connecting to remote domain, or
222-
* - send stdin EOF and keep waiting for remote exit code.
223-
*
224-
* That will require a slightly bigger refactoring. Right now it's not
225-
* important, because this function should handle QUBESRPC command failure
226-
* only (normal commands go through fork+exec), but it will be necessary
227-
* when we support sockets as a local process.
228-
*/
229-
if (is_service) {
230-
libvchan_send(data_vchan, &hdr, sizeof(hdr));
231-
send_exit_code(data_vchan, exit_code);
232-
libvchan_close(data_vchan);
233-
}
234-
exit(exit_code);
235-
}
236-
static void select_loop(libvchan_t *vchan, int data_protocol_version, struct buffer *stdin_buf)
237-
{
238-
struct process_io_request req = { 0 };
239-
int exit_code;
240-
241-
req.vchan = vchan;
242-
req.stdin_buf = stdin_buf;
243-
req.stdin_fd = local_stdin_fd;
244-
req.stdout_fd = local_stdout_fd;
245-
req.stderr_fd = -1;
246-
req.local_pid = local_pid;
247-
req.is_service = is_service;
248-
req.replace_chars_stdout = replace_chars_stdout;
249-
req.replace_chars_stderr = replace_chars_stderr;
250-
req.data_protocol_version = data_protocol_version;
251-
req.sigchld = &sigchld;
252-
req.sigusr1 = NULL;
253-
req.prefix_data.data = NULL;
254-
req.prefix_data.len = 0;
255-
256-
exit_code = process_io(&req);
257-
libvchan_close(vchan);
258-
exit(exit_with_code ? exit_code : 0);
259-
}
260-
26176
static struct option longopts[] = {
26277
{ "help", no_argument, 0, 'h' },
26378
{ "socket-dir", required_argument, 0, 'd'+128 },
@@ -328,12 +143,6 @@ static void parse_connect(char *str, char **request_id,
328143
exit(1);
329144
}
330145

331-
static void sigalrm_handler(int x __attribute__((__unused__)))
332-
{
333-
LOG(ERROR, "vchan connection timeout");
334-
exit(1);
335-
}
336-
337146
static const long BILLION_NANOSECONDS = 1000000000L;
338147

339148
static void wait_for_vchan_client_with_timeout(libvchan_t *conn, time_t timeout) {
@@ -395,23 +204,6 @@ static size_t compute_service_length(const char *const remote_cmdline, const cha
395204
return service_length;
396205
}
397206

398-
static void handshake_and_go(libvchan_t *data_vchan,
399-
struct buffer *stdin_buffer,
400-
bool remote_send_first,
401-
int prepare_ret)
402-
{
403-
if (data_vchan == NULL || !libvchan_is_open(data_vchan)) {
404-
LOG(ERROR, "Failed to open data vchan connection");
405-
exit(1);
406-
}
407-
int data_protocol_version = handle_agent_handshake(data_vchan, remote_send_first);
408-
if (data_protocol_version < 0)
409-
exit(1);
410-
if (prepare_ret < 0)
411-
handle_failed_exec(data_vchan);
412-
select_loop(data_vchan, data_protocol_version, stdin_buffer);
413-
}
414-
415207
int main(int argc, char **argv)
416208
{
417209
int opt;
@@ -431,6 +223,9 @@ int main(int argc, char **argv)
431223
struct service_params svc_params;
432224
int prepare_ret;
433225
bool kill = false;
226+
bool replace_chars_stdout = false;
227+
bool replace_chars_stderr = false;
228+
bool exit_with_code = true;
434229
int rc = 126;
435230

436231
if (argc < 3) {
@@ -452,15 +247,14 @@ int main(int argc, char **argv)
452247
just_exec = true;
453248
break;
454249
case 'E':
455-
exit_with_code = 0;
250+
exit_with_code = false;
456251
break;
457252
case 'c':
458253
if (request_id != NULL) {
459254
warnx("ERROR: -c passed more than once");
460255
usage(argv[0]);
461256
}
462257
parse_connect(optarg, &request_id, &src_domain_name, &src_domain_id);
463-
is_service = true;
464258
break;
465259
case 't':
466260
replace_chars_stdout = true;
@@ -509,35 +303,12 @@ int main(int argc, char **argv)
509303
LOG(ERROR, "internal error: src_domain_name should not be NULL here");
510304
abort();
511305
}
512-
set_remote_domain(src_domain_name);
513-
s = connect_unix_socket_by_id(src_domain_id);
514-
if (s < 0) {
515-
goto cleanup;
516-
}
517-
if (!negotiate_connection_params(s,
518-
0, /* dom0 */
519-
MSG_SERVICE_CONNECT,
520-
(void*)&svc_params,
521-
sizeof(svc_params),
522-
&data_domain,
523-
&data_port)) {
524-
goto cleanup;
525-
}
526-
527-
struct buffer stdin_buffer;
528-
buffer_init(&stdin_buffer);
529-
wait_for_session_maybe(remote_cmdline);
530-
prepare_ret = prepare_local_fds(remote_cmdline, &stdin_buffer);
531-
void (*old_handler)(int);
532-
533-
/* libvchan_client_init is blocking and does not support connection
534-
* timeout, so use alarm(2) for that... */
535-
old_handler = signal(SIGALRM, sigalrm_handler);
536-
alarm(connection_timeout);
537-
data_vchan = libvchan_client_init(data_domain, data_port);
538-
alarm(0);
539-
signal(SIGALRM, old_handler);
540-
handshake_and_go(data_vchan, &stdin_buffer, true, prepare_ret);
306+
rc = run_qrexec_to_dom0(&svc_params,
307+
src_domain_id,
308+
src_domain_name,
309+
remote_cmdline,
310+
connection_timeout,
311+
exit_with_code);
541312
} else {
542313
s = connect_unix_socket(domname);
543314
if (!negotiate_connection_params(s,
@@ -585,7 +356,16 @@ int main(int argc, char **argv)
585356
exit(1);
586357
}
587358
wait_for_vchan_client_with_timeout(data_vchan, connection_timeout);
588-
handshake_and_go(data_vchan, &stdin_buffer, false, prepare_ret);
359+
struct handshake_params params = {
360+
.data_vchan = data_vchan,
361+
.stdin_buffer = &stdin_buffer,
362+
.remote_send_first = false,
363+
.prepare_ret = prepare_ret,
364+
.exit_with_code = exit_with_code,
365+
.replace_chars_stdout = replace_chars_stdout,
366+
.replace_chars_stderr = replace_chars_stderr,
367+
};
368+
rc = handshake_and_go(&params);
589369
}
590370
}
591371

0 commit comments

Comments
 (0)