diff --git a/agent/qrexec-agent-data.c b/agent/qrexec-agent-data.c index bafeaff2..684f4e81 100644 --- a/agent/qrexec-agent-data.c +++ b/agent/qrexec-agent-data.c @@ -74,8 +74,14 @@ static void sigusr1_handler(int __attribute__((__unused__))x) void prepare_child_env(void) { char pid_s[10]; - signal(SIGCHLD, sigchld_handler); - signal(SIGUSR1, sigusr1_handler); + struct sigaction action = { + .sa_handler = sigchld_handler, + .sa_flags = 0, + }; + sigemptyset(&action.sa_mask); + if (sigaction(SIGCHLD, &action, NULL)) abort(); + action.sa_handler = sigusr1_handler; + if (sigaction(SIGUSR1, &action, NULL)) abort(); int res = snprintf(pid_s, sizeof(pid_s), "%d", getpid()); if (res < 0) abort(); if (res >= (int)sizeof(pid_s)) abort(); @@ -160,69 +166,6 @@ static int handle_just_exec(struct qrexec_parsed_command *cmd) return 0; } -static const long BILLION_NANOSECONDS = 1000000000L; - -static int wait_for_vchan_connection_with_timeout( - libvchan_t *conn, int wait_fd, bool is_server, time_t timeout) { - struct timespec end_tp, now_tp, timeout_tp; - - if (timeout && clock_gettime(CLOCK_MONOTONIC, &end_tp)) { - PERROR("clock_gettime"); - return -1; - } - assert(end_tp.tv_nsec >= 0 && end_tp.tv_nsec < BILLION_NANOSECONDS); - end_tp.tv_sec += timeout; - while (true) { - bool did_timeout = true; - struct pollfd fds = { .fd = wait_fd, .events = POLLIN | POLLHUP, .revents = 0 }; - - /* calculate how much time left until connection timeout expire */ - if (clock_gettime(CLOCK_MONOTONIC, &now_tp)) { - PERROR("clock_gettime"); - return -1; - } - assert(now_tp.tv_nsec >= 0 && now_tp.tv_nsec < BILLION_NANOSECONDS); - if (now_tp.tv_sec <= end_tp.tv_sec) { - timeout_tp.tv_sec = end_tp.tv_sec - now_tp.tv_sec; - timeout_tp.tv_nsec = end_tp.tv_nsec - now_tp.tv_nsec; - if (timeout_tp.tv_nsec < 0) { - timeout_tp.tv_nsec += BILLION_NANOSECONDS; - timeout_tp.tv_sec--; - } - did_timeout = timeout_tp.tv_sec < 0; - } - switch (did_timeout ? 0 : ppoll(&fds, 1, &timeout_tp, NULL)) { - case -1: - if (errno == EINTR) - break; - LOG(ERROR, "vchan connection error"); - return -1; - case 0: - LOG(ERROR, "vchan connection timeout"); - return -1; - case 1: - break; - default: - abort(); - } - if (fds.revents & POLLIN) { - if (is_server) { - libvchan_wait(conn); - return 0; - } else { - int connect_ret = libvchan_client_init_async_finish(conn, true); - - if (connect_ret < 0) { - LOG(ERROR, "vchan connection error"); - return -1; - } else if (connect_ret == 0) { - return 0; - } - } - } - } -} - /* Behaviour depends on type parameter: * MSG_JUST_EXEC - connect to vchan server, fork+exec process given by cmdline @@ -263,7 +206,8 @@ static int handle_new_process_common( LOG(ERROR, "Data vchan connection failed"); exit(1); } - if (wait_for_vchan_connection_with_timeout(data_vchan, wait_fd, false, connection_timeout) < 0) { + if (qubes_wait_for_vchan_connection_with_timeout( + data_vchan, wait_fd, false, connection_timeout) < 0) { LOG(ERROR, "Data vchan connection failed"); exit(1); } @@ -391,7 +335,7 @@ int handle_data_client( LOG(ERROR, "Data vchan connection failed"); exit(1); } - if (wait_for_vchan_connection_with_timeout( + if (qubes_wait_for_vchan_connection_with_timeout( data_vchan, libvchan_fd_for_select(data_vchan), true, connection_timeout) < 0) { LOG(ERROR, "Data vchan connection failed"); exit(1); diff --git a/agent/qrexec-agent.c b/agent/qrexec-agent.c index 4e10f3ef..e50f2f3e 100644 --- a/agent/qrexec-agent.c +++ b/agent/qrexec-agent.c @@ -928,8 +928,14 @@ int main(int argc, char **argv) } init(); - signal(SIGCHLD, sigchld_handler); - signal(SIGTERM, sigterm_handler); + struct sigaction action = { + .sa_handler = sigchld_handler, + .sa_flags = SA_RESTART, + }; + sigemptyset(&action.sa_mask); + sigaction(SIGCHLD, &action, NULL); + action.sa_handler = sigterm_handler; + sigaction(SIGTERM, &action, NULL); signal(SIGPIPE, SIG_IGN); sigemptyset(&selectmask); diff --git a/agent/qrexec-fork-server.c b/agent/qrexec-fork-server.c index 8a8ecd6e..9edba38e 100644 --- a/agent/qrexec-fork-server.c +++ b/agent/qrexec-fork-server.c @@ -124,10 +124,6 @@ int main(int argc, char **argv) { } s = get_server_socket(socket_path); - if (fcntl(s, F_SETFD, O_CLOEXEC) < 0) { - PERROR("fcntl"); - exit(1); - } /* fork into background */ switch (fork()) { case -1: diff --git a/daemon/qrexec-client.c b/daemon/qrexec-client.c index fe84005e..848a270a 100644 --- a/daemon/qrexec-client.c +++ b/daemon/qrexec-client.c @@ -143,57 +143,6 @@ static void parse_connect(char *str, char **request_id, exit(1); } -static const long BILLION_NANOSECONDS = 1000000000L; - -static void wait_for_vchan_client_with_timeout(libvchan_t *conn, time_t timeout) { - struct timespec end_tp, now_tp, timeout_tp; - - if (timeout && clock_gettime(CLOCK_MONOTONIC, &end_tp)) { - PERROR("clock_gettime"); - exit(1); - } - assert(end_tp.tv_nsec >= 0 && end_tp.tv_nsec < BILLION_NANOSECONDS); - end_tp.tv_sec += timeout; - int const fd = libvchan_fd_for_select(conn); - while (conn && libvchan_is_open(conn) == VCHAN_WAITING) { - if (timeout) { - bool did_timeout = true; - struct pollfd fds = { .fd = fd, .events = POLLIN | POLLHUP, .revents = 0 }; - - /* calculate how much time left until connection timeout expire */ - if (clock_gettime(CLOCK_MONOTONIC, &now_tp)) { - PERROR("clock_gettime"); - exit(1); - } - assert(now_tp.tv_nsec >= 0 && now_tp.tv_nsec < BILLION_NANOSECONDS); - if (now_tp.tv_sec <= end_tp.tv_sec) { - timeout_tp.tv_sec = end_tp.tv_sec - now_tp.tv_sec; - timeout_tp.tv_nsec = end_tp.tv_nsec - now_tp.tv_nsec; - if (timeout_tp.tv_nsec < 0) { - timeout_tp.tv_nsec += BILLION_NANOSECONDS; - timeout_tp.tv_sec--; - } - did_timeout = timeout_tp.tv_sec < 0; - } - switch (did_timeout ? 0 : ppoll(&fds, 1, &timeout_tp, NULL)) { - case -1: - if (errno == EINTR) - break; - LOG(ERROR, "vchan connection error"); - exit(1); - case 0: - LOG(ERROR, "vchan connection timeout"); - exit(1); - case 1: - break; - default: - abort(); - } - } - libvchan_wait(conn); - } -} - static size_t compute_service_length(const char *const remote_cmdline, const char *const prog_name) { const size_t service_length = strlen(remote_cmdline) + 1; if (service_length < 2 || service_length > MAX_QREXEC_CMD_LEN) { @@ -365,9 +314,16 @@ int main(int argc, char **argv) VCHAN_BUFFER_SIZE, VCHAN_BUFFER_SIZE); if (!data_vchan) { LOG(ERROR, "Failed to start data vchan server"); - exit(1); + rc = QREXEC_EXIT_PROBLEM; + goto cleanup; + } + int fd = libvchan_fd_for_select(data_vchan); + if (qubes_wait_for_vchan_connection_with_timeout( + data_vchan, fd, true, connection_timeout) < 0) { + LOG(ERROR, "qrexec connection timeout"); + rc = QREXEC_EXIT_PROBLEM; + goto cleanup; } - wait_for_vchan_client_with_timeout(data_vchan, connection_timeout); struct handshake_params params = { .data_vchan = data_vchan, .stdin_buffer = &stdin_buffer, diff --git a/daemon/qrexec-daemon-common.c b/daemon/qrexec-daemon-common.c index 6bbb7b7c..46752d5e 100644 --- a/daemon/qrexec-daemon-common.c +++ b/daemon/qrexec-daemon-common.c @@ -220,7 +220,6 @@ int handle_agent_handshake(libvchan_t *vchan, bool remote_send_first) static void sigchld_handler(int x __attribute__((__unused__))) { sigchld = 1; - signal(SIGCHLD, sigchld_handler); } /* See also qrexec-agent.c:wait_for_session_maybe() */ @@ -272,7 +271,12 @@ int prepare_local_fds(struct qrexec_parsed_command *command, struct buffer *stdi { if (stdin_buffer == NULL) abort(); - if (signal(SIGCHLD, sigchld_handler) == SIG_ERR) + struct sigaction action = { + .sa_handler = sigchld_handler, + .sa_flags = 0, + }; + sigemptyset(&action.sa_mask); + if (sigaction(SIGCHLD, &action, NULL)) return 126; return execute_parsed_qubes_rpc_command(command, &local_pid, &local_stdin_fd, &local_stdout_fd, NULL, stdin_buffer); @@ -331,12 +335,6 @@ static int select_loop(struct handshake_params *params) return (params->exit_with_code ? exit_code : 0); } -static void sigalrm_handler(int x __attribute__((__unused__))) -{ - LOG(ERROR, "vchan connection timeout"); - _exit(1); -} - int run_qrexec_to_dom0(const struct service_params *svc_params, int src_domain_id, const char *src_domain_name, @@ -375,15 +373,18 @@ int run_qrexec_to_dom0(const struct service_params *svc_params, } else { prepare_ret = prepare_local_fds(command, &stdin_buffer); } - void (*old_handler)(int); - - /* libvchan_client_init is blocking and does not support connection - * timeout, so use alarm(2) for that... */ - old_handler = signal(SIGALRM, sigalrm_handler); - alarm(connection_timeout); - data_vchan = libvchan_client_init(data_domain, data_port); - alarm(0); - signal(SIGALRM, old_handler); + int wait_fd; + data_vchan = libvchan_client_init_async(data_domain, data_port, &wait_fd); + if (!data_vchan) { + LOG(ERROR, "Cannot create data vchan connection"); + return QREXEC_EXIT_PROBLEM; + } + if (qubes_wait_for_vchan_connection_with_timeout( + data_vchan, wait_fd, false, connection_timeout) < 0) { + LOG(ERROR, "qrexec connection timeout"); + return QREXEC_EXIT_PROBLEM; + } + struct handshake_params params = { .data_vchan = data_vchan, .stdin_buffer = &stdin_buffer, diff --git a/daemon/qrexec-daemon.c b/daemon/qrexec-daemon.c index 87993071..6759361b 100644 --- a/daemon/qrexec-daemon.c +++ b/daemon/qrexec-daemon.c @@ -120,7 +120,6 @@ static const char *policy_program = QREXEC_POLICY_PROGRAM; # define UNUSED(x) UNUSED_ ## x #endif -static volatile int children_count; static volatile int child_exited; static volatile int terminate_requested; @@ -135,24 +134,6 @@ static #endif int protocol_version; -static void sigusr1_handler(int UNUSED(x)) -{ - if (!opt_quiet) - LOG(INFO, "Connected to VM"); - exit(0); -} - -static void sigchld_parent_handler(int UNUSED(x)) -{ - children_count--; - /* starting value is 0 so we see dead real qrexec-daemon as -1 */ - if (children_count < 0) { - LOG(ERROR, "Connection to the VM failed"); - exit(1); - } -} - - static char *remote_domain_name; // guess what static int remote_domain_id; @@ -308,41 +289,55 @@ static void init(int xid) startup_timeout = MAX_STARTUP_TIME_DEFAULT; } + int pipes[2]; if (!opt_direct) { - struct sigaction action = { - .sa_handler = sigusr1_handler, - .sa_flags = 0, - }; - if (sigaction(SIGUSR1, &action, NULL)) - err(1, "sigaction"); - action.sa_handler = sigchld_parent_handler; - if (sigaction(SIGCHLD, &action, NULL)) - err(1, "sigaction"); + if (pipe2(pipes, O_CLOEXEC)) + err(1, "pipe2()"); switch (pid=fork()) { case -1: PERROR("fork"); exit(1); case 0: + close(pipes[0]); break; default: if (getenv("QREXEC_STARTUP_NOWAIT")) exit(0); + close(pipes[1]); if (!opt_quiet) LOG(ERROR, "Waiting for VM's qrexec agent."); - for (i=0;i 0) { + if (dup2(null_fd, 0) != 0) + err(1, "dup2(%d, 0)", null_fd); + if (null_fd > 2 && close(null_fd) != 0) + err(1, "close(%d)", null_fd); + } + } + setup_logging("qrexec-daemon"); while ((opt=getopt_long(argc, argv, "hqp:D", longopts, NULL)) != -1) { diff --git a/fuzz/Makefile b/fuzz/Makefile index d21d5603..bc510ce7 100644 --- a/fuzz/Makefile +++ b/fuzz/Makefile @@ -16,7 +16,7 @@ CXXFLAGS += -O1 -fno-omit-frame-pointer -gline-tables-only \ -fsanitize-address-use-after-scope -fsanitize=fuzzer endif -_LIBQREXEC_OBJS = remote.o write-stdin.o ioall.o txrx-vchan.o buffer.o replace.o exec.o log.o unix-server.o toml.o process_io.o +_LIBQREXEC_OBJS = remote.o write-stdin.o ioall.o txrx-vchan.o buffer.o replace.o exec.o log.o unix-server.o toml.o process_io.o vchan_timeout.o LIBQREXEC_OBJS = $(patsubst %.o,libqrexec-%.o,$(_LIBQREXEC_OBJS)) FUZZERS = qubesrpc_parse_fuzzer qrexec_remote_fuzzer qrexec_daemon_fuzzer diff --git a/fuzz/fuzz.c b/fuzz/fuzz.c index cf2e7f55..7579b711 100644 --- a/fuzz/fuzz.c +++ b/fuzz/fuzz.c @@ -140,7 +140,18 @@ ssize_t fuzz_write(int fd, const void *buf, size_t count) { return count; } +typedef int EVTCHN; fuzz_file_t *fuzz_libvchan_client_init(int domain, int port) { /* not implemented yet */ abort(); } + +fuzz_file_t *fuzz_libvchan_client_init_async(int domain, int port, EVTCHN *watch_fd) { + /* not implemented yet */ + abort(); +} + +int fuzz_libvchan_client_init_async_finish(fuzz_file_t *ctrl, bool blocking) { + /* not implemented yet */ + abort(); +} diff --git a/fuzz/fuzz.h b/fuzz/fuzz.h index aa0d56d7..9a879002 100644 --- a/fuzz/fuzz.h +++ b/fuzz/fuzz.h @@ -31,6 +31,8 @@ int fuzz_libvchan_is_open(fuzz_file_t *file); int fuzz_libvchan_data_ready(fuzz_file_t *file); int fuzz_libvchan_buffer_space(fuzz_file_t *file); fuzz_file_t *fuzz_libvchan_client_init(int domain, int port); +fuzz_file_t *fuzz_libvchan_client_init_async(int domain, int port, int *fd); +int fuzz_libvchan_client_init_async_finish(fuzz_file_t *file, bool blocking); ssize_t fuzz_read(int fd, void *buf, size_t count); ssize_t fuzz_write(int fd, const void *buf, size_t count); diff --git a/fuzz/mock-fuzz.h b/fuzz/mock-fuzz.h index d8eda723..f8769ec0 100644 --- a/fuzz/mock-fuzz.h +++ b/fuzz/mock-fuzz.h @@ -16,6 +16,8 @@ #define libvchan_data_ready fuzz_libvchan_data_ready #define libvchan_buffer_space fuzz_libvchan_buffer_space #define libvchan_client_init fuzz_libvchan_client_init +#define libvchan_client_init_async fuzz_libvchan_client_init_async +#define libvchan_client_init_async_finish fuzz_libvchan_client_init_async_finish #define read fuzz_read #define write fuzz_write diff --git a/libqrexec/Makefile b/libqrexec/Makefile index cfe4ef2d..baecffa6 100644 --- a/libqrexec/Makefile +++ b/libqrexec/Makefile @@ -21,7 +21,7 @@ endif all: libqrexec-utils.so -libqrexec-utils.so.$(SO_VER): unix-server.o ioall.o buffer.o exec.o txrx-vchan.o write-stdin.o replace.o remote.o process_io.o log.o toml.o +libqrexec-utils.so.$(SO_VER): unix-server.o ioall.o buffer.o exec.o txrx-vchan.o write-stdin.o replace.o remote.o process_io.o log.o toml.o vchan_timeout.o $(CC) $(LDFLAGS) -Wl,-soname,$@ -o $@ $^ $(VCHANLIBS) libqrexec-utils.so: libqrexec-utils.so.$(SO_VER) diff --git a/libqrexec/libqrexec-utils.h b/libqrexec/libqrexec-utils.h index dcad6462..eadc2317 100644 --- a/libqrexec/libqrexec-utils.h +++ b/libqrexec/libqrexec-utils.h @@ -32,6 +32,7 @@ #include #include +#include #include #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION @@ -365,4 +366,15 @@ void *qubes_read_all_to_malloc(int fd, size_t initial_buffer_size, size_t max_by */ bool qubes_sendmsg_all(struct msghdr *msg, int sock); +/** + * Wait for a vchan connection with a timeout. + * + * @param conn the vchan + * @param wait_fd The FD set by libvchan_client_init_async() for clients, + * or the FD returned by libvchan_fd_for_select() for servers. + * @param is_server Is this a server or a client vchan? + * @param timeout The timeout to use. + */ +int qubes_wait_for_vchan_connection_with_timeout( + libvchan_t *conn, int wait_fd, bool is_server, time_t timeout); #endif /* LIBQREXEC_UTILS_H */ diff --git a/libqrexec/qrexec.h b/libqrexec/qrexec.h index a466c006..462f5464 100644 --- a/libqrexec/qrexec.h +++ b/libqrexec/qrexec.h @@ -173,4 +173,11 @@ enum { // support only very small configuration files, #define MAX_CONFIG_SIZE 4096 +// Exit codes + +// Service call refused +#define QREXEC_EXIT_REQUEST_REFUSED 126 +// Problem with qrexec itself +#define QREXEC_EXIT_PROBLEM 125 + #endif /* QREXEC_H */ diff --git a/libqrexec/unix-server.c b/libqrexec/unix-server.c index f5c4ff36..ba046493 100644 --- a/libqrexec/unix-server.c +++ b/libqrexec/unix-server.c @@ -36,7 +36,7 @@ int get_server_socket(const char *socket_address) unlink(socket_address); - s = socket(AF_UNIX, SOCK_STREAM, 0); + s = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0); if (s < 0) { PERROR("socket"); exit(1); diff --git a/libqrexec/vchan_timeout.c b/libqrexec/vchan_timeout.c new file mode 100644 index 00000000..e15a8a0c --- /dev/null +++ b/libqrexec/vchan_timeout.c @@ -0,0 +1,67 @@ +#include +#include +#include +#include + +static const long BILLION_NANOSECONDS = 1000000000L; + +int qubes_wait_for_vchan_connection_with_timeout( + libvchan_t *conn, int wait_fd, bool is_server, time_t timeout) { + struct timespec end_tp, now_tp, timeout_tp; + + if (clock_gettime(CLOCK_MONOTONIC, &end_tp)) { + PERROR("clock_gettime"); + return -1; + } + assert(end_tp.tv_nsec >= 0 && end_tp.tv_nsec < BILLION_NANOSECONDS); + end_tp.tv_sec += timeout; + for (;;) { + bool did_timeout = true; + struct pollfd fds = { .fd = wait_fd, .events = POLLIN | POLLHUP, .revents = 0 }; + + /* calculate how much time left until connection timeout expire */ + if (clock_gettime(CLOCK_MONOTONIC, &now_tp)) { + PERROR("clock_gettime"); + return -1; + } + assert(now_tp.tv_nsec >= 0 && now_tp.tv_nsec < BILLION_NANOSECONDS); + if (now_tp.tv_sec <= end_tp.tv_sec) { + timeout_tp.tv_sec = end_tp.tv_sec - now_tp.tv_sec; + timeout_tp.tv_nsec = end_tp.tv_nsec - now_tp.tv_nsec; + if (timeout_tp.tv_nsec < 0) { + timeout_tp.tv_nsec += BILLION_NANOSECONDS; + timeout_tp.tv_sec--; + } + did_timeout = timeout_tp.tv_sec < 0; + } + switch (did_timeout ? 0 : ppoll(&fds, 1, &timeout_tp, NULL)) { + case -1: + if (errno == EINTR) + break; + LOG(ERROR, "vchan connection error"); + return -1; + case 0: + LOG(ERROR, "vchan connection timeout"); + return -1; + case 1: + break; + default: + abort(); + } + if (fds.revents & POLLIN) { + if (is_server) { + libvchan_wait(conn); + return 0; + } else { + int connect_ret = libvchan_client_init_async_finish(conn, true); + + if (connect_ret < 0) { + LOG(ERROR, "vchan connection error"); + return -1; + } else if (connect_ret == 0) { + return 0; + } + } + } + } +}