From 80002db2018bc6ee86d60358c5b30a4a41cb3dd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marczewski?= Date: Fri, 6 Mar 2020 12:32:01 +0100 Subject: [PATCH] qrexec-client: use common code for process_io --- daemon/qrexec-client.c | 258 ++++++------------------------------ libqrexec/libqrexec-utils.h | 1 + libqrexec/process_io.c | 2 +- 3 files changed, 42 insertions(+), 219 deletions(-) diff --git a/daemon/qrexec-client.c b/daemon/qrexec-client.c index 7667e776..247cc7b5 100644 --- a/daemon/qrexec-client.c +++ b/daemon/qrexec-client.c @@ -50,7 +50,8 @@ static pid_t local_pid = 0; /* flag if this is "remote" end of service call. In this case swap STDIN/STDOUT * msg types and send exit code at the end */ static int is_service = 0; -static int child_exited = 0; + +static volatile sig_atomic_t sigchld = 0; static const char *socket_dir = QREXEC_DAEMON_SOCKET_DIR; @@ -72,41 +73,6 @@ static void set_remote_domain(const char *src_domain_name) { } } -static void close_stdin_fd(void) { - if (local_stdin_fd < 0) - return; - if (shutdown(local_stdin_fd, SHUT_WR) && errno != ENOTSOCK) { - fputs("Cannot shutdown socket\n", stderr); - abort(); - } - if (local_stdin_fd != local_stdout_fd) { - /* restore flags, as we may have not the only copy of this file descriptor */ - set_block(local_stdin_fd); - if (close(local_stdin_fd)) { - fputs("Cannot close socket\n", stderr); - abort(); - } - } - local_stdin_fd = -1; -} - -static void close_stdout_fd(void) { - if (local_stdout_fd < 0) - return; - if (shutdown(local_stdout_fd, SHUT_RD) && errno != ENOTSOCK) { - fputs("Cannot shutdown socket\n", stderr); - abort(); - } - if (local_stdout_fd != local_stdin_fd) { - set_block(local_stdout_fd); - if (close(local_stdout_fd)) { - fputs("Cannot close socket\n", stderr); - abort(); - } - } - local_stdout_fd = -1; -} - /* initialize data_protocol_version */ static int handle_agent_handshake(libvchan_t *vchan, int remote_send_first) { @@ -223,7 +189,7 @@ static int connect_unix_socket(const char *domname) static void sigchld_handler(int x __attribute__((__unused__))) { - child_exited = 1; + sigchld = 1; signal(SIGCHLD, sigchld_handler); } @@ -239,21 +205,6 @@ static _Noreturn void do_exec(char *prog, const char *username __attribute__((un exit(1); } -static _Noreturn void do_exit(int code) -{ - int status; - close_stdin_fd(); - close_stdout_fd(); - // sever communication lines; wait for child, if any - // so that qrexec-daemon can count (recursively) spawned processes correctly - waitpid(-1, &status, 0); - exit(code); -} - -static _Noreturn void close_vchan_and_exit(int code, libvchan_t *vchan) { - libvchan_close(vchan); - do_exit(code); -} static void prepare_local_fds(char *cmdline, struct buffer *stdin_buffer) { @@ -284,22 +235,22 @@ static void negotiate_connection_params(int s, int other_domid, unsigned type, || !write_all(s, ¶ms, sizeof(params)) || !write_all(s, cmdline_param, cmdline_size)) { perror("write daemon"); - do_exit(1); + exit(1); } /* the daemon will respond with the same message with connect_port filled * and empty cmdline */ if (!read_all(s, &hdr, sizeof(hdr))) { perror("read daemon"); - do_exit(1); + exit(1); } assert(hdr.type == type); if (hdr.len != sizeof(params)) { fprintf(stderr, "Invalid response for 0x%x\n", type); - do_exit(1); + exit(1); } if (!read_all(s, ¶ms, sizeof(params))) { perror("read daemon"); - do_exit(1); + exit(1); } *data_port = params.connect_port; *data_domain = params.connect_domain; @@ -324,163 +275,34 @@ static void send_service_connect(int s, char *conn_ident, || !write_all(s, &exec_params, sizeof(exec_params)) || !write_all(s, &srv_params, sizeof(srv_params))) { perror("write daemon"); - do_exit(1); - } -} - -static void check_child_status(libvchan_t *vchan) -{ - int status = 0; - if (local_pid >= 0) { - /* this is not a socket-based service */ - pid_t pid = waitpid(local_pid, &status, WNOHANG); - if (pid < 0) { - perror("waitpid"); - close_vchan_and_exit(1, vchan); - } - if (pid == 0 || !WIFEXITED(status)) - return; - status = WEXITSTATUS(status); - } - if (is_service && send_exit_code(vchan, status) < 0) { - close_vchan_and_exit(1, vchan); + exit(1); } - close_vchan_and_exit(status, vchan); } static void select_loop(libvchan_t *vchan, int data_protocol_version, struct buffer *stdin_buf) { - fd_set select_set; - fd_set wr_set; - int max_fd; - int ret; - int vchan_fd; - int status; - sigset_t selectmask; - struct timespec zero_timeout = { 0, 0 }; - struct timespec select_timeout = { 10, 0 }; - - sigemptyset(&selectmask); - sigaddset(&selectmask, SIGCHLD); - sigprocmask(SIG_BLOCK, &selectmask, NULL); - sigemptyset(&selectmask); - /* remember to set back to blocking mode before closing the FD - this may - * be not the only copy and some processes may misbehave when get - * nonblocking FD for input/output - */ - set_nonblock(local_stdin_fd); - - if (local_stdout_fd != local_stdin_fd) - set_nonblock(local_stdout_fd); - else if ((local_stdout_fd = fcntl(local_stdin_fd, F_DUPFD_CLOEXEC, 3)) < 0) - abort(); // not worth handling running out of file descriptors - - for (;;) { - vchan_fd = libvchan_fd_for_select(vchan); - FD_ZERO(&select_set); - FD_ZERO(&wr_set); - FD_SET(vchan_fd, &select_set); - max_fd = vchan_fd; - if (local_stdout_fd != -1 && - (size_t)libvchan_buffer_space(vchan) > sizeof(struct msg_header)) { - FD_SET(local_stdout_fd, &select_set); - if (local_stdout_fd > max_fd) - max_fd = local_stdout_fd; - } - if (local_stdout_fd == -1 && - (child_exited || (local_stdin_fd == -1 && local_pid == -1))) - check_child_status(vchan); - if (local_stdin_fd != -1 && buffer_len(stdin_buf)) { - FD_SET(local_stdin_fd, &wr_set); - if (local_stdin_fd > max_fd) - max_fd = local_stdin_fd; - } - if ((local_stdin_fd == -1 || buffer_len(stdin_buf) == 0) && - libvchan_data_ready(vchan) > 0) { - /* check for other FDs, but exit immediately */ - ret = pselect(max_fd + 1, &select_set, &wr_set, NULL, - &zero_timeout, &selectmask); - } else - ret = pselect(max_fd + 1, &select_set, &wr_set, NULL, - &select_timeout, &selectmask); - if (ret < 0) { - if (errno == EINTR && local_pid > 0) { - continue; - } else { - perror("select"); - close_vchan_and_exit(1, vchan); - } - } - if (ret == 0) { - if (!libvchan_is_open(vchan)) { - /* remote disconnected witout a proper signaling */ - do_exit(1); - } - } - if (FD_ISSET(vchan_fd, &select_set)) - libvchan_wait(vchan); - if (buffer_len(stdin_buf) && - local_stdin_fd != -1 && - FD_ISSET(local_stdin_fd, &wr_set)) { - if (flush_client_data(local_stdin_fd, stdin_buf) == WRITE_STDIN_ERROR) { - perror("write stdin"); - close_stdin_fd(); - } - } - - switch (handle_remote_data( - vchan, - local_stdin_fd, - &status, - stdin_buf, - data_protocol_version, - /* try_shutdown - TODO should it always be true? */ - true, - replace_chars_stdout > 0, - replace_chars_stderr > 0)) { - case REMOTE_ERROR: - close_vchan_and_exit(1, vchan); - break; - case REMOTE_EOF: - local_stdin_fd = -1; - break; - case REMOTE_EXITED: - flush_client_data(local_stdin_fd, stdin_buf); - close_vchan_and_exit(status, vchan); - break; - } - - if (local_stdout_fd != -1 - && FD_ISSET(local_stdout_fd, &select_set)) - switch (handle_input(vchan, - local_stdout_fd, - is_service ? MSG_DATA_STDOUT : MSG_DATA_STDIN, - data_protocol_version, - /* set_block_on_close */ - true)) { - case REMOTE_ERROR: - close_vchan_and_exit(1, vchan); - break; - case REMOTE_EOF: - local_stdout_fd = -1; - // if not a remote end of service call, wait for exit status - if (is_service) { - // if pipe in opposite direction already closed, no need to stay alive - if (local_pid <= 0) { - /* if this is "remote" service end and no real local process - * exists (using own stdin/out) send also fake exit code */ - if (send_exit_code(vchan, 0) < 0) - close_vchan_and_exit(1, vchan); - close_vchan_and_exit(0, vchan); - } - } else if (local_pid < 0) { - // socket-based service, so we will never get a SIGCHLD - close_vchan_and_exit(0, vchan); - } - break; - } - } + struct process_io_request req; + int exit_code; + + req.vchan = vchan; + req.stdin_buf = stdin_buf; + req.stdin_fd = local_stdin_fd; + req.stdout_fd = local_stdout_fd; + req.stderr_fd = -1; + req.local_pid = local_pid; + req.is_service = is_service; + req.replace_chars_stdout = replace_chars_stdout; + req.replace_chars_stderr = replace_chars_stderr; + req.data_protocol_version = data_protocol_version; + req.sigchld = &sigchld; + req.sigusr1 = NULL; + + fprintf(stderr, "calling process_io\n"); + exit_code = process_io(&req); + libvchan_close(vchan); + exit(exit_code); } + static struct option longopts[] = { { "help", no_argument, 0, 'h' }, { "socket-dir", required_argument, 0, 'd'+128 }, @@ -548,7 +370,7 @@ static void parse_connect(char *str, char **request_id, static void sigalrm_handler(int x __attribute__((__unused__))) { fprintf(stderr, "vchan connection timeout\n"); - do_exit(1); + exit(1); } static void wait_for_vchan_client_with_timeout(libvchan_t *conn, int timeout) { @@ -556,7 +378,7 @@ static void wait_for_vchan_client_with_timeout(libvchan_t *conn, int timeout) { if (timeout && gettimeofday(&start_tv, NULL) == -1) { perror("gettimeofday"); - do_exit(1); + exit(1); } while (conn && libvchan_is_open(conn) == VCHAN_WAITING) { if (timeout) { @@ -566,13 +388,13 @@ static void wait_for_vchan_client_with_timeout(libvchan_t *conn, int timeout) { /* calculate how much time left until connection timeout expire */ if (gettimeofday(&now_tv, NULL) == -1) { perror("gettimeofday"); - close_vchan_and_exit(1, conn); + exit(1); } timersub(&start_tv, &now_tv, &timeout_tv); timeout_tv.tv_sec += timeout; if (timeout_tv.tv_sec < 0) { fprintf(stderr, "vchan connection timeout\n"); - close_vchan_and_exit(1, conn); + exit(1); } FD_ZERO(&rdset); FD_SET(fd, &rdset); @@ -582,10 +404,10 @@ static void wait_for_vchan_client_with_timeout(libvchan_t *conn, int timeout) { break; } fprintf(stderr, "vchan connection error\n"); - close_vchan_and_exit(1, conn); + exit(1); case 0: fprintf(stderr, "vchan connection timeout\n"); - close_vchan_and_exit(1, conn); + exit(1); } } libvchan_wait(conn); @@ -715,11 +537,11 @@ int main(int argc, char **argv) } if (!data_vchan || !libvchan_is_open(data_vchan)) { fprintf(stderr, "Failed to open data vchan connection\n"); - do_exit(1); + exit(1); } data_protocol_version = handle_agent_handshake(data_vchan, connect_existing); if (data_protocol_version < 0) - close_vchan_and_exit(1, data_vchan); + exit(1); select_loop(data_vchan, data_protocol_version, &stdin_buffer); } else { msg_type = just_exec ? MSG_JUST_EXEC : MSG_EXEC_CMDLINE; @@ -757,16 +579,16 @@ int main(int argc, char **argv) VCHAN_BUFFER_SIZE, VCHAN_BUFFER_SIZE); if (!data_vchan) { fprintf(stderr, "Failed to start data vchan server\n"); - do_exit(1); + exit(1); } wait_for_vchan_client_with_timeout(data_vchan, connection_timeout); if (!libvchan_is_open(data_vchan)) { fprintf(stderr, "Failed to open data vchan connection\n"); - do_exit(1); + exit(1); } data_protocol_version = handle_agent_handshake(data_vchan, 0); if (data_protocol_version < 0) - close_vchan_and_exit(1, data_vchan); + exit(1); select_loop(data_vchan, data_protocol_version, &stdin_buffer); } } diff --git a/libqrexec/libqrexec-utils.h b/libqrexec/libqrexec-utils.h index 7100467c..0c6f0593 100644 --- a/libqrexec/libqrexec-utils.h +++ b/libqrexec/libqrexec-utils.h @@ -186,6 +186,7 @@ struct process_io_request { int data_protocol_version; volatile sig_atomic_t *sigchld; + // can be NULL volatile sig_atomic_t *sigusr1; }; diff --git a/libqrexec/process_io.c b/libqrexec/process_io.c index ae4c14c9..bb00d831 100644 --- a/libqrexec/process_io.c +++ b/libqrexec/process_io.c @@ -126,7 +126,7 @@ int process_io(const struct process_io_request *req) { } /* child signaled desire to use single socket for both stdin and stdout */ - if (*sigusr1) { + if (sigusr1 && *sigusr1) { if (stdout_fd != -1) { do errno = 0;