From 1aa05ebc366a4efc1bec5b25d0e15da09ca6702b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 16 Mar 2015 21:10:25 +0100 Subject: [PATCH] qrexec: handle data vchan directly from qrexec-client-vm This way qrexec-client-vm will have much more information, at least: - will know whether the service call was accepted or refused - potentially will know remote process exit code This commit implements the first point - the local process will not be started if service call was refused. --- qrexec/Makefile | 2 +- qrexec/qrexec-agent-data.c | 91 ++++++++++++++++++---------- qrexec/qrexec-agent.c | 69 ++++++++++----------- qrexec/qrexec-agent.h | 4 ++ qrexec/qrexec-client-vm.c | 120 +++++++++++++++++++++++++------------ 5 files changed, 181 insertions(+), 105 deletions(-) diff --git a/qrexec/Makefile b/qrexec/Makefile index 9855451..a42c244 100644 --- a/qrexec/Makefile +++ b/qrexec/Makefile @@ -6,7 +6,7 @@ LDLIBS=`pkg-config --libs vchan-$(BACKEND_VMM)` -lqrexec-utils all: qrexec-agent qrexec-client-vm qrexec-fork-server qrexec-agent: qrexec-agent.o qrexec-agent-data.o qrexec-fork-server: qrexec-fork-server.o qrexec-agent-data.o -qrexec-client-vm: qrexec-client-vm.o +qrexec-client-vm: qrexec-client-vm.o qrexec-agent-data.o clean: rm -f *.o *~ qrexec-agent qrexec-client-vm diff --git a/qrexec/qrexec-agent-data.c b/qrexec/qrexec-agent-data.c index fcedf48..7cb8a14 100644 --- a/qrexec/qrexec-agent-data.c +++ b/qrexec/qrexec-agent-data.c @@ -32,6 +32,7 @@ #include #include #include +#include #include "qrexec.h" #include "libqrexec-utils.h" #include "qrexec-agent.h" @@ -42,6 +43,7 @@ static volatile int child_exited; static volatile int stdio_socket_requested; int stdout_msg_type = MSG_DATA_STDOUT; pid_t child_process_pid; +int remote_process_status = 0; static void sigchld_handler(int __attribute__((__unused__))x) { @@ -386,45 +388,37 @@ void process_child_io(libvchan_t *data_vchan, stdout_fd = -1; close(stderr_fd); stderr_fd = -1; + /* we do not care for any local process */ + return; break; } } } -pid_t handle_new_process(int type, int connect_domain, int connect_port, - char *cmdline, int cmdline_len) +/* Behaviour depends on type parameter: + * MSG_SERVICE_CONNECT - create vchan server, pass the data to/from given FDs + * (stdin_fd, stdout_fd, stderr_fd), then return 0 + * MSG_JUST_EXEC - connect to vchan server, fork+exec process given by cmdline + * parameter, send artificial exit code "0" (local process can still be + * running), then return 0 + * MSG_EXEC_CMDLINE - connect to vchan server, fork+exec process given by + * cmdline parameter, pass the data to/from that process, then return local + * process exit code + */ +int handle_new_process_common(int type, int connect_domain, int connect_port, + char *cmdline, int cmdline_len, /* MSG_JUST_EXEC and MSG_EXEC_CMDLINE */ + int stdin_fd, int stdout_fd, int stderr_fd /* MSG_SERVICE_CONNECT */) { - struct service_params *svc_params = (struct service_params*)cmdline; libvchan_t *data_vchan; + int exit_code = 0; pid_t pid; - int stdin_fd, stdout_fd, stderr_fd; char pid_s[10]; - if (type == MSG_SERVICE_CONNECT) { - if (cmdline_len != sizeof(*svc_params)) { - fprintf(stderr, "Invalid MSG_SERVICE_CONNECT packet (cmdline len %d)\n", cmdline_len); - return -1; - } - sscanf(cmdline, "%d %d %d", &stdin_fd, &stdout_fd, &stderr_fd); + if (type != MSG_SERVICE_CONNECT) { + assert(cmdline != NULL); + cmdline[cmdline_len-1] = 0; } - switch (pid=fork()){ - case -1: - perror("fork"); - return -1; - case 0: - break; - default: - if (type == MSG_SERVICE_CONNECT) { - /* no longer needed in parent process */ - close(stdin_fd); - close(stdout_fd); - close(stderr_fd); - } - return pid; - } - - /* child process */ if (type == MSG_SERVICE_CONNECT) { data_vchan = libvchan_server_init(connect_domain, connect_port, VCHAN_BUFFER_SIZE, VCHAN_BUFFER_SIZE); @@ -448,13 +442,13 @@ pid_t handle_new_process(int type, int connect_domain, int connect_port, switch (type) { case MSG_JUST_EXEC: send_exit_code(data_vchan, handle_just_exec(cmdline)); - libvchan_close(data_vchan); break; case MSG_EXEC_CMDLINE: do_fork_exec(cmdline, &pid, &stdin_fd, &stdout_fd, &stderr_fd); fprintf(stderr, "executed %s pid %d\n", cmdline, pid); child_process_pid = pid; - process_child_io(data_vchan, stdin_fd, stdout_fd, stderr_fd); + exit_code = process_child_io(data_vchan, stdin_fd, stdout_fd, stderr_fd); + fprintf(stderr, "pid %d exited with %d\n", pid, exit_code); break; case MSG_SERVICE_CONNECT: child_process_pid = 0; @@ -462,7 +456,44 @@ pid_t handle_new_process(int type, int connect_domain, int connect_port, process_child_io(data_vchan, stdin_fd, stdout_fd, stderr_fd); break; } - exit(0); + libvchan_close(data_vchan); + return exit_code; +} + +/* Returns PID of data processing process */ +pid_t handle_new_process(int type, int connect_domain, int connect_port, + char *cmdline, int cmdline_len) +{ + int exit_code; + pid_t pid; + assert(type != MSG_SERVICE_CONNECT); + + switch (pid=fork()){ + case -1: + perror("fork"); + return -1; + case 0: + break; + default: + return pid; + } + + /* child process */ + exit_code = handle_new_process_common(type, connect_domain, connect_port, + cmdline, cmdline_len, + -1, -1, -1); + + exit(exit_code); /* suppress warning */ return 0; } + +void handle_data_client(int type, int connect_domain, int connect_port, + int stdin_fd, int stdout_fd, int stderr_fd) +{ + + assert(type == MSG_SERVICE_CONNECT); + + handle_new_process_common(type, connect_domain, connect_port, + NULL, 0, stdin_fd, stdout_fd, stderr_fd); +} diff --git a/qrexec/qrexec-agent.c b/qrexec/qrexec-agent.c index d10d59e..31d944e 100644 --- a/qrexec/qrexec-agent.c +++ b/qrexec/qrexec-agent.c @@ -55,7 +55,6 @@ struct _connection_info connection_info[MAX_FDS]; libvchan_t *ctrl_vchan; int trigger_fd; -int passfd_socket; int meminfo_write_started = 0; @@ -107,11 +106,8 @@ void init() if (handle_handshake(ctrl_vchan) < 0) exit(1); umask(0); - mkfifo(QREXEC_AGENT_TRIGGER_PATH, 0666); - passfd_socket = get_server_socket(QREXEC_AGENT_FDPASS_PATH); + trigger_fd = get_server_socket(QREXEC_AGENT_TRIGGER_PATH); umask(077); - trigger_fd = - open(QREXEC_AGENT_TRIGGER_PATH, O_RDONLY | O_NONBLOCK); register_exec_func(do_exec); /* wait for qrexec daemon */ @@ -229,6 +225,7 @@ void handle_server_exec_request(struct msg_header *hdr) struct exec_params params; char buf[hdr->len-sizeof(params)]; pid_t child_agent; + int client_fd; assert(hdr->len >= sizeof(params)); @@ -248,7 +245,24 @@ void handle_server_exec_request(struct msg_header *hdr) return; } } + if (hdr->type == MSG_SERVICE_CONNECT && sscanf(buf, "SOCKET%d", &client_fd)) { + /* FIXME: Maybe add some check if client_fd is really FD to some + * qrexec-client-vm process; but this data comes from qrexec-daemon + * (which sends back what it got from us earlier), so it isn't critical. + */ + write(client_fd, ¶ms, sizeof(params)); + /* No need to send request_id (buf) - the client don't need it, there + * is only meaningless (for the client) socket FD */ + /* Register connection even if there was an error sending params to + * qrexec-client-vm. This way the mainloop will clean the things up + * (close socket, send MSG_CONNECTION_TERMINATED) when qrexec-client-vm + * will close the socket (terminate itself). */ + register_vchan_connection(-1, client_fd, + params.connect_domain, params.connect_port); + return; + } + /* No fork server case */ child_agent = handle_new_process(hdr->type, params.connect_domain, params.connect_port, buf, hdr->len-sizeof(params)); @@ -260,7 +274,7 @@ void handle_server_exec_request(struct msg_header *hdr) void handle_service_refused(struct msg_header *hdr) { struct service_params params; - int stdin_fd, stdout_fd, stderr_fd; + int socket_fd; if (hdr->len != sizeof(params)) { fprintf(stderr, "Invalid msg 0x%x length (%d)\n", MSG_SERVICE_REFUSED, hdr->len); @@ -270,11 +284,10 @@ void handle_service_refused(struct msg_header *hdr) if (libvchan_recv(ctrl_vchan, ¶ms, sizeof(params)) < 0) handle_vchan_error("read exec params"); - sscanf(params.ident, "%d %d %d", &stdin_fd, &stdout_fd, &stderr_fd); - /* TODO: send some signal? some response? */ - close(stdin_fd); - close(stdout_fd); - close(stderr_fd); + if (sscanf(params.ident, "SOCKET%d", &socket_fd)) + close(socket_fd); + else + fprintf(stderr, "Received REFUSED for unknown service request '%s'\n", params.ident); } void handle_server_cmd() @@ -360,9 +373,6 @@ int fill_fds_for_select(fd_set * rdset, fd_set * wrset) FD_SET(trigger_fd, rdset); if (trigger_fd > max) max = trigger_fd; - FD_SET(passfd_socket, rdset); - if (passfd_socket > max) - max = passfd_socket; for (i = 0; i < MAX_FDS; i++) { if (connection_info[i].pid != 0 && connection_info[i].fd != -1) { @@ -374,41 +384,31 @@ int fill_fds_for_select(fd_set * rdset, fd_set * wrset) return max; } -void handle_new_passfd() -{ - int fd = do_accept(passfd_socket); - if (fd >= MAX_FDS) { - fprintf(stderr, "too many clients ?\n"); - exit(1); - } - // let client know what fd has been allocated - if (write(fd, &fd, sizeof(fd)) != sizeof(fd)) { - perror("write to client"); - } -} - void handle_trigger_io() { struct msg_header hdr; struct trigger_service_params params; int ret; + int client_fd; + client_fd = do_accept(trigger_fd); + if (client_fd < 0) + return; hdr.len = sizeof(params); - ret = read(trigger_fd, ¶ms, sizeof(params)); + ret = read(client_fd, ¶ms, sizeof(params)); if (ret == sizeof(params)) { hdr.type = MSG_TRIGGER_SERVICE; + snprintf(params.request_id.ident, sizeof(params.request_id), "SOCKET%d", client_fd); if (libvchan_send(ctrl_vchan, &hdr, sizeof(hdr)) < 0) handle_vchan_error("write hdr"); if (libvchan_send(ctrl_vchan, ¶ms, sizeof(params)) < 0) handle_vchan_error("write params"); } - // trigger_fd is nonblock - so no need to reopen - // not really, need to reopen at EOF if (ret <= 0) { - close(trigger_fd); - trigger_fd = - open(QREXEC_AGENT_TRIGGER_PATH, O_RDONLY | O_NONBLOCK); + close(client_fd); } + /* do not close client_fd - we'll need it to send the connection details + * later (when dom0 accepts the request) */ } void handle_terminated_fork_client(fd_set *rdset) { @@ -454,9 +454,6 @@ int main() wait_for_vchan_or_argfd(ctrl_vchan, max, &rdset, &wrset); sigprocmask(SIG_UNBLOCK, &chld_set, NULL); - if (FD_ISSET(passfd_socket, &rdset)) - handle_new_passfd(); - while (libvchan_data_ready(ctrl_vchan)) handle_server_cmd(); diff --git a/qrexec/qrexec-agent.h b/qrexec/qrexec-agent.h index d2f10e8..e1cb0e6 100644 --- a/qrexec/qrexec-agent.h +++ b/qrexec/qrexec-agent.h @@ -28,6 +28,10 @@ void do_exec(const char *cmd); pid_t handle_new_process(int type, int connect_domain, int connect_port, char *cmdline, int cmdline_len); +void handle_data_client(int type, + int connect_domain, int connect_port, + int stdin_fd, int stdout_fd, int stderr_fd); + struct qrexec_cmd_info { int type; diff --git a/qrexec/qrexec-client-vm.c b/qrexec/qrexec-client-vm.c index 737ebc4..28850af 100644 --- a/qrexec/qrexec-client-vm.c +++ b/qrexec/qrexec-client-vm.c @@ -20,14 +20,29 @@ */ #define _GNU_SOURCE #include +#include #include #include #include #include #include #include +#include "libqrexec-utils.h" #include "qrexec.h" -int connect_unix_socket() +#include "qrexec-agent.h" + +void handle_vchan_error(const char *op) +{ + fprintf(stderr, "Error while vchan %s, exiting\n", op); + exit(1); +} + +void do_exec(const char *cmd __attribute__((__unused__))) { + fprintf(stderr, "BUG: do_exec function shouldn't be called!\n"); + exit(1); +} + +int connect_unix_socket(char *path) { int s, len; struct sockaddr_un remote; @@ -38,7 +53,7 @@ int connect_unix_socket() } remote.sun_family = AF_UNIX; - strncpy(remote.sun_path, QREXEC_AGENT_FDPASS_PATH, + strncpy(remote.sun_path, path, sizeof(remote.sun_path)); len = strlen(remote.sun_path) + sizeof(remote.sun_family); if (connect(s, (struct sockaddr *) &remote, len) == -1) { @@ -61,9 +76,12 @@ int main(int argc, char **argv) { int trigger_fd; struct trigger_service_params params; - int local_fd[3], remote_fd[3]; - int i; + struct exec_params exec_params; + int ret, i; char *abs_exec_path; + pid_t child_pid; + int inpipe[2], outpipe[2]; + char pid_s[10]; if (argc < 4) { fprintf(stderr, @@ -72,50 +90,76 @@ int main(int argc, char **argv) exit(1); } - trigger_fd = open(QREXEC_AGENT_TRIGGER_PATH, O_WRONLY); - if (trigger_fd < 0) { - perror("open " QREXEC_AGENT_TRIGGER_PATH); - exit(1); - } - - for (i = 0; i < 3; i++) { - local_fd[i] = connect_unix_socket(); - if (read(local_fd[i], &remote_fd[i], sizeof(remote_fd[i])) != sizeof(remote_fd[i])) { - perror("read client fd"); - exit(1); - } - if (i != 2 || getenv("PASS_LOCAL_STDERR")) { - char *env; - if (asprintf(&env, "SAVED_FD_%d=%d", i, dup(i)) < 0) { - perror("prepare SAVED_FD_"); - exit(1); - } - putenv(env); - dup2(local_fd[i], i); - close(local_fd[i]); - } else - close(local_fd[i]); - } + trigger_fd = connect_unix_socket(QREXEC_AGENT_TRIGGER_PATH); memset(¶ms, 0, sizeof(params)); strncpy(params.service_name, argv[2], sizeof(params.service_name)); strncpy(params.target_domain, argv[1], sizeof(params.target_domain)); snprintf(params.request_id.ident, - sizeof(params.request_id.ident), "%d %d %d", - remote_fd[0], remote_fd[1], remote_fd[2]); + sizeof(params.request_id.ident), "SOCKET"); if (write(trigger_fd, ¶ms, sizeof(params)) < 0) { - if (!getenv("PASS_LOCAL_STDERR")) - perror("write to agent"); + perror("write to agent"); + exit(1); + } + ret = read(trigger_fd, &exec_params, sizeof(exec_params)); + if (ret == 0) { + fprintf(stderr, "Request refused\n"); + exit(1); + } + if (ret < 0 || ret != sizeof(exec_params)) { + perror("read"); exit(1); } - close(trigger_fd); + if (socketpair(AF_UNIX, SOCK_STREAM, 0, inpipe) || + socketpair(AF_UNIX, SOCK_STREAM, 0, outpipe)) { + perror("socketpair"); + exit(1); + } + snprintf(pid_s, sizeof(pid_s), "%d", getpid()); + setenv("QREXEC_AGENT_PID", pid_s, 1); - abs_exec_path = strdup(argv[3]); - argv[3] = get_program_name(argv[3]); - execv(abs_exec_path, argv + 3); - perror("execv"); - return 1; + switch (child_pid = fork()) { + case -1: + perror("fork"); + exit(-1); + case 0: + close(inpipe[1]); + close(outpipe[0]); + close(trigger_fd); + for (i = 0; i < 3; i++) { + if (i != 2 || getenv("PASS_LOCAL_STDERR")) { + char *env; + if (asprintf(&env, "SAVED_FD_%d=%d", i, dup(i)) < 0) { + perror("prepare SAVED_FD_"); + exit(1); + } + putenv(env); + } + } + + dup2(inpipe[0], 0); + dup2(outpipe[1], 1); + close(inpipe[0]); + close(outpipe[1]); + + abs_exec_path = strdup(argv[3]); + argv[3] = get_program_name(argv[3]); + execv(abs_exec_path, argv + 3); + perror("execv"); + exit(-1); + } + close(inpipe[0]); + close(outpipe[1]); + + handle_data_client(MSG_SERVICE_CONNECT, + exec_params.connect_domain, exec_params.connect_port, + inpipe[1], outpipe[0], -1); + + close(trigger_fd); + waitpid(child_pid, &i, 0); + + return 0; }