From c05b26763ae4868f9b8524eca6f8a260de8d3d38 Mon Sep 17 00:00:00 2001 From: Rafal Wojtczuk Date: Mon, 4 Jul 2011 18:56:56 +0200 Subject: [PATCH] qrexec: new communication scheme, agent<->server part 1) Instead of a set of predefined commands, we send MSG_AGENT_TO_SERVER_TRIGGER_CONNECT_EXISTING msg with a parameter (e.g. "org.qubes-os.vm.Filecopy") defining required action 2) qrexec_daemon just forks qrexec_policy, that will take care of actually allowing and executing required action 3) after MSG_AGENT_TO_SERVER_TRIGGER_CONNECT_EXISTING, qrexec_agent does not execute a command - it justs uses already established file descriptors to send data to/from. Thus, there is no need to use ~/.xxxxxspool - a command line tool can have direct access to remote fds. --- qrexec/Makefile | 4 +-- qrexec/qrexec.h | 20 +++++++---- qrexec/qrexec_agent.c | 78 +++++++++++++++++++++++++++++------------- qrexec/qrexec_daemon.c | 53 ++++++++++------------------ 4 files changed, 89 insertions(+), 66 deletions(-) diff --git a/qrexec/Makefile b/qrexec/Makefile index fc2c8fdf..16a61116 100644 --- a/qrexec/Makefile +++ b/qrexec/Makefile @@ -6,8 +6,8 @@ COMMONIOALL=../common/ioall.o all: qrexec_daemon qrexec_agent qrexec_client qrexec_daemon: qrexec_daemon.o unix_server.o $(COMMONIOALL) txrx-vchan.o buffer.o write_stdin.o $(CC) -pie -L../vchan -L../u2mfn -g -o qrexec_daemon qrexec_daemon.o unix_server.o $(COMMONIOALL) txrx-vchan.o write_stdin.o buffer.o $(XENLIBS) -qrexec_agent: qrexec_agent.o exec.o txrx-vchan.o write_stdin.o buffer.o $(COMMONIOALL) - $(CC) -pie -L../vchan -L../u2mfn -g -o qrexec_agent qrexec_agent.o exec.o txrx-vchan.o write_stdin.o buffer.o $(COMMONIOALL) $(XENLIBS) +qrexec_agent: qrexec_agent.o unix_server.o exec.o txrx-vchan.o write_stdin.o buffer.o $(COMMONIOALL) + $(CC) -pie -L../vchan -L../u2mfn -g -o qrexec_agent qrexec_agent.o unix_server.o exec.o txrx-vchan.o write_stdin.o buffer.o $(COMMONIOALL) $(XENLIBS) qrexec_client: qrexec_client.o $(COMMONIOALL) exec.o $(CC) -pie -g -o qrexec_client qrexec_client.o $(COMMONIOALL) exec.o clean: diff --git a/qrexec/qrexec.h b/qrexec/qrexec.h index bd996c48..09a25254 100644 --- a/qrexec/qrexec.h +++ b/qrexec/qrexec.h @@ -26,11 +26,13 @@ #define REXEC_PORT 512 #define QREXEC_AGENT_TRIGGER_PATH "/var/run/qubes/qrexec_agent" +#define QREXEC_AGENT_FDPASS_PATH "/var/run/qubes/qrexec_agent_fdpass" enum { MSG_CLIENT_TO_SERVER_EXEC_CMDLINE = 0x100, MSG_CLIENT_TO_SERVER_JUST_EXEC, + MSG_SERVER_TO_AGENT_CONNECT_EXISTING, MSG_SERVER_TO_AGENT_EXEC_CMDLINE, MSG_SERVER_TO_AGENT_JUST_EXEC, MSG_SERVER_TO_AGENT_INPUT, @@ -42,19 +44,13 @@ enum { MSG_AGENT_TO_SERVER_STDOUT, MSG_AGENT_TO_SERVER_STDERR, MSG_AGENT_TO_SERVER_EXIT_CODE, - MSG_AGENT_TO_SERVER_TRIGGER_EXEC, + MSG_AGENT_TO_SERVER_TRIGGER_CONNECT_EXISTING, MSG_SERVER_TO_CLIENT_STDOUT, MSG_SERVER_TO_CLIENT_STDERR, MSG_SERVER_TO_CLIENT_EXIT_CODE }; -enum { - QREXEC_EXECUTE_FILE_COPY=0x700, - QREXEC_EXECUTE_FILE_COPY_FOR_DISPVM, - QREXEC_EXECUTE_APPMENUS_SYNC -}; - struct server_header { unsigned int type; unsigned int client_id; @@ -65,3 +61,13 @@ struct client_header { unsigned int type; unsigned int len; }; + +struct connect_existing_params { + char ident[32]; +}; + +struct trigger_connect_params { + char exec_index[64]; + char target_vmname[32]; + struct connect_existing_params process_fds; +}; diff --git a/qrexec/qrexec_agent.c b/qrexec/qrexec_agent.c index 06a89103..e31e4c40 100644 --- a/qrexec/qrexec_agent.c +++ b/qrexec/qrexec_agent.c @@ -68,12 +68,14 @@ struct _process_fd process_fd[MAX_FDS]; struct _client_info client_info[MAX_FDS]; int trigger_fd; +int passfd_socket; void init() { peer_server_init(REXEC_PORT); umask(0); mkfifo(QREXEC_AGENT_TRIGGER_PATH, 0666); + passfd_socket = get_server_socket(QREXEC_AGENT_FDPASS_PATH); umask(077); trigger_fd = open(QREXEC_AGENT_TRIGGER_PATH, O_RDONLY | O_NONBLOCK); @@ -145,15 +147,9 @@ void handle_just_exec(int client_id, int len) fprintf(stderr, "executed (nowait) %s pid %d\n", buf, pid); } -void handle_exec(int client_id, int len) +void create_info_about_client(int client_id, int pid, int stdin_fd, + int stdout_fd, int stderr_fd) { - char buf[len]; - int pid, stdin_fd, stdout_fd, stderr_fd; - - read_all_vchan_ext(buf, len); - - do_fork_exec(buf, &pid, &stdin_fd, &stdout_fd, &stderr_fd); - process_fd[stdout_fd].client_id = client_id; process_fd[stdout_fd].type = FDTYPE_STDOUT; process_fd[stdout_fd].is_blocked = 0; @@ -177,11 +173,35 @@ void handle_exec(int client_id, int len) client_info[client_id].is_blocked = 0; client_info[client_id].is_close_after_flush_needed = 0; buffer_init(&client_info[client_id].buffer); +} + +void handle_exec(int client_id, int len) +{ + char buf[len]; + int pid, stdin_fd, stdout_fd, stderr_fd; + + read_all_vchan_ext(buf, len); + + do_fork_exec(buf, &pid, &stdin_fd, &stdout_fd, &stderr_fd); + + create_info_about_client(client_id, pid, stdin_fd, stdout_fd, + stderr_fd); fprintf(stderr, "executed %s pid %d\n", buf, pid); } +void handle_connect_existing(int client_id) +{ + int stdin_fd, stdout_fd, stderr_fd; + struct connect_existing_params params; + read_all_vchan_ext(¶ms, sizeof(params)); + sscanf(params.ident, "%d %d %d", &stdin_fd, &stdout_fd, + &stderr_fd); + create_info_about_client(client_id, 0, stdin_fd, stdout_fd, + stderr_fd); + client_info[client_id].is_exited = 1; //do not wait for SIGCHLD +} void update_max_process_fd() { @@ -307,6 +327,9 @@ void handle_server_data() case MSG_XOFF: set_blocked_outerr(s_hdr.client_id, 1); break; + case MSG_SERVER_TO_AGENT_CONNECT_EXISTING: + handle_connect_existing(s_hdr.client_id); + break; case MSG_SERVER_TO_AGENT_EXEC_CMDLINE: handle_exec(s_hdr.client_id, s_hdr.len); break; @@ -432,6 +455,9 @@ 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 (client_info[i].pid > 0 && client_info[i].is_blocked) { @@ -467,28 +493,31 @@ void flush_client_data_agent(int client_id) } } +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 + write(fd, &fd, sizeof(fd)); +} + + void handle_trigger_io() { struct server_header s_hdr; - char buf[5]; + struct trigger_connect_params params; int ret; s_hdr.client_id = 0; s_hdr.len = 0; - if ((ret = read(trigger_fd, buf, 4)) == 4) { - buf[4] = 0; - if (!strcmp(buf, "FCPR")) - s_hdr.client_id = QREXEC_EXECUTE_FILE_COPY; - else if (!strcmp(buf, "DVMR")) - s_hdr.client_id = - QREXEC_EXECUTE_FILE_COPY_FOR_DISPVM; - else if (!strcmp(buf, "SYNC")) - s_hdr.client_id = - QREXEC_EXECUTE_APPMENUS_SYNC; - if (s_hdr.client_id) { - s_hdr.type = MSG_AGENT_TO_SERVER_TRIGGER_EXEC; - write_all_vchan_ext(&s_hdr, sizeof s_hdr); - } + ret = read(trigger_fd, ¶ms, sizeof(params)); + if (ret == sizeof(params)) { + s_hdr.type = MSG_AGENT_TO_SERVER_TRIGGER_CONNECT_EXISTING; + write_all_vchan_ext(&s_hdr, sizeof s_hdr); + write_all_vchan_ext(¶ms, sizeof params); } // trigger_fd is nonblock - so no need to reopen // not really, need to reopen at EOF @@ -518,6 +547,9 @@ int main() wait_for_vchan_or_argfd(max, &rdset, &wrset); + if (FD_ISSET(passfd_socket, &rdset)) + handle_new_passfd(); + while (read_ready_vchan_ext()) handle_server_data(); diff --git a/qrexec/qrexec_daemon.c b/qrexec/qrexec_daemon.c index 44025158..142369ec 100644 --- a/qrexec/qrexec_daemon.c +++ b/qrexec/qrexec_daemon.c @@ -356,33 +356,27 @@ void check_children_count_and_wait_if_too_many() } } +#define ENSURE_NULL_TERMINATED(x) x[sizeof(x)-1] = 0 + /* Called when agent sends a message asking to execute a predefined command. */ -void handle_execute_predefined_command(int req) +void handle_execute_predefined_command() { - char *rcmd = NULL, *lcmd = NULL; int i; + struct trigger_connect_params untrusted_params, params; check_children_count_and_wait_if_too_many(); - switch (req) { - case QREXEC_EXECUTE_FILE_COPY: - rcmd = "directly:user:/usr/lib/qubes/qfile-agent"; - lcmd = "/usr/lib/qubes/qfile-daemon"; - break; - case QREXEC_EXECUTE_FILE_COPY_FOR_DISPVM: - rcmd = "directly:user:/usr/lib/qubes/qfile-agent-dvm"; - lcmd = "/usr/lib/qubes/qfile-daemon-dvm"; - break; - case QREXEC_EXECUTE_APPMENUS_SYNC: - rcmd = "user:grep -H = /usr/share/applications/*.desktop"; - lcmd = "/usr/bin/qvm-sync-appmenus"; - break; - default: /* cannot happen, already sanitized */ - fprintf(stderr, "got trigger exec no %d\n", req); - exit(1); - } + read_all_vchan_ext(&untrusted_params, sizeof(params)); + + /* sanitize start */ + ENSURE_NULL_TERMINATED(untrusted_params.exec_index); + ENSURE_NULL_TERMINATED(untrusted_params.target_vmname); + ENSURE_NULL_TERMINATED(untrusted_params.process_fds.ident); + params = untrusted_params; + /* sanitize end */ + switch (fork()) { case -1: perror("fork"); @@ -397,8 +391,9 @@ void handle_execute_predefined_command(int req) close(i); signal(SIGCHLD, SIG_DFL); signal(SIGPIPE, SIG_DFL); - execl("/usr/lib/qubes/qrexec_client", "qrexec_client", "-d", - remote_domain_name, "-l", lcmd, rcmd, NULL); + execl("/usr/lib/qubes/qrexec_policy", "qrexec_policy", + remote_domain_name, params.target_vmname, + params.exec_index, params.process_fds.ident, NULL); perror("execl"); exit(1); } @@ -415,18 +410,8 @@ void check_client_id_in_range(unsigned int untrusted_client_id) void sanitize_message_from_agent(struct server_header *untrusted_header) { - int untrusted_cmd; switch (untrusted_header->type) { - case MSG_AGENT_TO_SERVER_TRIGGER_EXEC: - untrusted_cmd = untrusted_header->client_id; - if (untrusted_cmd != QREXEC_EXECUTE_FILE_COPY && - untrusted_cmd != QREXEC_EXECUTE_FILE_COPY_FOR_DISPVM && - untrusted_cmd != QREXEC_EXECUTE_APPMENUS_SYNC) { - fprintf(stderr, - "received MSG_AGENT_TO_SERVER_TRIGGER_EXEC cmd %d ?\n", - untrusted_cmd); - exit(1); - } + case MSG_AGENT_TO_SERVER_TRIGGER_CONNECT_EXISTING: break; case MSG_AGENT_TO_SERVER_STDOUT: case MSG_AGENT_TO_SERVER_STDERR: @@ -465,8 +450,8 @@ void handle_message_from_agent() // fprintf(stderr, "got %x %x %x\n", s_hdr.type, s_hdr.client_id, // s_hdr.len); - if (s_hdr.type == MSG_AGENT_TO_SERVER_TRIGGER_EXEC) { - handle_execute_predefined_command(s_hdr.client_id); + if (s_hdr.type == MSG_AGENT_TO_SERVER_TRIGGER_CONNECT_EXISTING) { + handle_execute_predefined_command(); return; }