From fa332f1fa288ca5686fa845d912f5a03680c75dc Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Wed, 10 Apr 2024 14:25:05 -0400 Subject: [PATCH] Draft: Rip out qubes-rpc-multiplexer Instead, directly execute the command from C. Marked as draft for four reasons: 1. There is no logging of the service's stderr anymore. 2. This PR is based on another PR (#139), not main. 3. All variables with names beginning with QREXEC_ are stripped from the environment. This is a change in behavior compared to the current code. 1 and 2 must be fixed before this can be merged. 3 is a design decision that could go either way. Fixes: QubesOS/qubes-issues#9062 --- agent/qrexec-agent-data.c | 4 +- agent/qrexec-agent.c | 6 +-- agent/qrexec-agent.h | 2 +- agent/qrexec-client-vm.c | 2 +- agent/qrexec-fork-server.c | 4 +- daemon/qrexec-client.c | 8 +-- lib/qubes-rpc-multiplexer | 64 ---------------------- libqrexec/exec.c | 102 ++++++++++++++++++++++++++++++------ libqrexec/libqrexec-utils.h | 4 +- 9 files changed, 102 insertions(+), 94 deletions(-) delete mode 100755 lib/qubes-rpc-multiplexer diff --git a/agent/qrexec-agent-data.c b/agent/qrexec-agent-data.c index 4c364350..ac4fa0ef 100644 --- a/agent/qrexec-agent-data.c +++ b/agent/qrexec-agent-data.c @@ -156,8 +156,10 @@ static int handle_just_exec(struct qrexec_parsed_command *cmd) return 127; case 0: fdn = open("/dev/null", O_RDWR); + if (fdn < 0) + _exit(127); fix_fds(fdn, fdn, fdn); - do_exec(cmd->command, cmd->username); + do_exec(buf.data, cmd->command, cmd->username); default:; } LOG(INFO, "executed (nowait): %s (pid %d)", cmd->command, pid); diff --git a/agent/qrexec-agent.c b/agent/qrexec-agent.c index 7195f1ac..11a9f860 100644 --- a/agent/qrexec-agent.c +++ b/agent/qrexec-agent.c @@ -139,7 +139,7 @@ static struct pam_conv conv = { * If dom0 sends overly long cmd, it will probably crash qrexec-agent (unless * process can allocate up to 4GB on both stack and heap), sorry. */ -_Noreturn void do_exec(const char *cmd, const char *user) +_Noreturn void do_exec(const char *prog, const char *cmd, const char *user) { #ifdef HAVE_PAM int retval, status; @@ -178,7 +178,7 @@ _Noreturn void do_exec(const char *cmd, const char *user) exit(1); } /* call QUBESRPC if requested */ - exec_qubes_rpc_if_requested(cmd, environ); + exec_qubes_rpc_if_requested(prog, cmd, environ); /* otherwise exec shell */ execl("/bin/sh", "sh", "-c", cmd, NULL); @@ -285,7 +285,7 @@ _Noreturn void do_exec(const char *cmd, const char *user) warn("chdir(%s)", pw->pw_dir); /* call QUBESRPC if requested */ - exec_qubes_rpc_if_requested(cmd, env); + exec_qubes_rpc_if_requested(prog, cmd, env); /* otherwise exec shell */ execle(pw->pw_shell, arg0, "-c", cmd, (char*)NULL, env); diff --git a/agent/qrexec-agent.h b/agent/qrexec-agent.h index 10d1ff7e..99a197d7 100644 --- a/agent/qrexec-agent.h +++ b/agent/qrexec-agent.h @@ -29,7 +29,7 @@ int handle_handshake(libvchan_t *ctrl); void handle_vchan_error(const char *op); -_Noreturn void do_exec(const char *cmd, const char *user); +_Noreturn void do_exec(const char *prog, const char *cmd, const char *user); /* call before fork() for service handling process (either end) */ void prepare_child_env(void); diff --git a/agent/qrexec-client-vm.c b/agent/qrexec-client-vm.c index 97ca9884..0cdd25ce 100644 --- a/agent/qrexec-client-vm.c +++ b/agent/qrexec-client-vm.c @@ -42,7 +42,7 @@ _Noreturn void handle_vchan_error(const char *op) exit(1); } -_Noreturn void do_exec(const char *cmd __attribute__((unused)), char const* user __attribute__((__unused__))) { +_Noreturn void do_exec(const char *prog __attribute__((unused)), const char *cmd __attribute__((unused)), char const* user __attribute__((__unused__))) { LOG(ERROR, "BUG: do_exec function shouldn't be called!"); abort(); } diff --git a/agent/qrexec-fork-server.c b/agent/qrexec-fork-server.c index 8a8ecd6e..a9e6beb4 100644 --- a/agent/qrexec-fork-server.c +++ b/agent/qrexec-fork-server.c @@ -37,7 +37,7 @@ extern char **environ; const bool qrexec_is_fork_server = true; -void do_exec(const char *cmd, const char *user __attribute__((unused))) +void do_exec(const char *prog, const char *cmd, const char *user __attribute__((unused))) { char *shell; @@ -45,7 +45,7 @@ void do_exec(const char *cmd, const char *user __attribute__((unused))) signal(SIGPIPE, SIG_DFL); /* call QUBESRPC if requested */ - exec_qubes_rpc_if_requested(cmd, environ); + exec_qubes_rpc_if_requested(prog, cmd, environ); /* otherwise, pass it to shell */ shell = getenv("SHELL"); diff --git a/daemon/qrexec-client.c b/daemon/qrexec-client.c index 8517bf5d..b42c31d8 100644 --- a/daemon/qrexec-client.c +++ b/daemon/qrexec-client.c @@ -198,13 +198,15 @@ static void sigchld_handler(int x __attribute__((__unused__))) } /* called from do_fork_exec */ -static _Noreturn void do_exec(const char *prog, const char *username __attribute__((unused))) +static _Noreturn void do_exec(const char *prog, + const char *cmdline, + const char *username __attribute__((unused))) { /* avoid calling qubes-rpc-multiplexer through shell */ - exec_qubes_rpc_if_requested(prog, environ); + exec_qubes_rpc_if_requested(prog, cmdline, environ); /* if above haven't executed qubes-rpc-multiplexer, pass it to shell */ - execl("/bin/bash", "bash", "-c", prog, NULL); + execl("/bin/bash", "bash", "-c", cmdline, NULL); PERROR("exec bash"); exit(1); } diff --git a/lib/qubes-rpc-multiplexer b/lib/qubes-rpc-multiplexer deleted file mode 100755 index 0c34d740..00000000 --- a/lib/qubes-rpc-multiplexer +++ /dev/null @@ -1,64 +0,0 @@ -#!/bin/sh -l -# we don't use globbing, disable it -set -f - -if [ -z "${QREXEC_SERVICE_PATH+x}" ]; then - QREXEC_SERVICE_PATH=/usr/local/etc/qubes-rpc:/etc/qubes-rpc -fi -tmpdir=${XDG_RUNTIME_DIR-/tmp} - -# write stderr to both calling party and local log; be very careful about -# closing file descriptors here - if either stdout or stderr will not be closed -# when service process does the same - service call will hang (waiting for EOF -# on stdout/stderr) -stderr_pipe=$tmpdir/qrexec-rpc-stderr.$$ -mkfifo -- "$stderr_pipe" -# tee can't write to file descriptor, nor /proc/self/fd/2 (EXIO on open) -return_stderr_pipe=$tmpdir/qrexec-rpc-stderr-return.$$ -mkfifo -- "$return_stderr_pipe" -{ cat <"$return_stderr_pipe" >&2 2>/dev/null; rm -f -- "$return_stderr_pipe"; } /dev/null & -{ tee -- "$return_stderr_pipe" <"$stderr_pipe" | - logger -t "$1-$2"; rm -f -- "$stderr_pipe"; } /dev/null 2>&1 & -exec 2>"$stderr_pipe" - -if ! [ $# = 2 -o $# = 4 ] ; then - echo "$0: bad argument count, usage: $0 SERVICE-NAME REMOTE-DOMAIN-NAME [REQUESTED_TARGET_TYPE REQUESTED_TARGET]" >&2 - exit 1 -fi -# Avoid inheriting these from the environment -unset QREXEC_REQUESTED_TARGET QREXEC_REQUESTED_TARGET_KEYWORD QREXEC_SERVICE_ARGUMENT -export QREXEC_REQUESTED_TARGET_TYPE="$3" -if [ "$QREXEC_REQUESTED_TARGET_TYPE" = "name" ]; then - export QREXEC_REQUESTED_TARGET="$4" -elif [ "$QREXEC_REQUESTED_TARGET_TYPE" = "keyword" ]; then - export QREXEC_REQUESTED_TARGET_KEYWORD="$4" -fi -# else: requested target type unknown or not given, ignore -export QREXEC_REMOTE_DOMAIN="$2" -export QREXEC_SERVICE_FULL_NAME="$1" -SERVICE_WITHOUT_ARGUMENT="${1%%+*}" -if [ "${QREXEC_SERVICE_FULL_NAME}" != "${SERVICE_WITHOUT_ARGUMENT}" ]; then - export QREXEC_SERVICE_ARGUMENT="${QREXEC_SERVICE_FULL_NAME#*+}" -else - # Search for qubes.Service+ if given qubes.Service - set -- "$1+" "$2" -fi - - -ifs=$IFS -IFS=: -for DIR in $QREXEC_SERVICE_PATH; do - CFG_FILE="$DIR/$1" - if [ -s "$CFG_FILE" ]; then - break - fi - CFG_FILE="$DIR/$SERVICE_WITHOUT_ARGUMENT" - if [ -s "$CFG_FILE" ]; then - break - fi -done -IFS=$ifs - -exec "$CFG_FILE" ${QREXEC_SERVICE_ARGUMENT:+"$QREXEC_SERVICE_ARGUMENT"} -echo "$0: failed to execute handler for $1" >&2 -exit 1 diff --git a/libqrexec/exec.c b/libqrexec/exec.c index 29ab93d8..9912f248 100644 --- a/libqrexec/exec.c +++ b/libqrexec/exec.c @@ -45,36 +45,103 @@ void register_exec_func(do_exec_t *func) { exec_func = func; } -void exec_qubes_rpc_if_requested(const char *prog, char *const envp[]) { - /* avoid calling qubes-rpc-multiplexer through shell */ - if (strncmp(prog, RPC_REQUEST_COMMAND, RPC_REQUEST_COMMAND_LEN) == 0) { +void exec_qubes_rpc_if_requested(const char *program, const char *cmd, char *const envp[]) { + /* avoid calling RPC service through shell */ + if (strncmp(cmd, RPC_REQUEST_COMMAND, RPC_REQUEST_COMMAND_LEN) == 0) { char *prog_copy; char *tok, *savetok; - char *argv[16]; // right now 6 are used, but allow future extensions + const char *argv[6]; size_t i = 0; + size_t const extra_env_vars = 5; + size_t env_amount = extra_env_vars; - prog_copy = strdup(prog); + if (cmd[RPC_REQUEST_COMMAND_LEN] != ' ') { + LOG(ERROR, "\"" RPC_REQUEST_COMMAND "\" not followed by space"); + _exit(126); + } + + assert(program); + + for (char *const *env = envp; *env; ++env) { + if (strncmp(*env, "QREXEC_", sizeof "QREXEC") != 0) + env_amount++; + } +#define EXTEND(...) \ + do { \ + if (iterator >= env_amount) \ + abort(); \ + if (asprintf(&buf[iterator++], __VA_ARGS__) < 0) \ + goto bad_asprintf; \ + } while (0) +#define EXTEND_RAW(arg) \ + do { \ + if (iterator >= env_amount) \ + abort(); \ + buf[iterator++] = (arg); \ + } while (0) + + char **buf = calloc(env_amount + 1, sizeof(char *)); + if (buf == NULL) { + LOG(ERROR, "calloc(%zu, %zu) failed: %m", env_amount, sizeof(char *)); + _exit(126); + } + size_t iterator = 0; + for (char *const *env = envp; *env; ++env) { + if (strncmp(*env, "QREXEC_", sizeof "QREXEC") != 0) { + EXTEND_RAW(*env); + } + } + + prog_copy = strdup(cmd + RPC_REQUEST_COMMAND_LEN + 1); if (!prog_copy) { PERROR("strdup"); - _exit(1); + _exit(126); } + argv[i++] = (char *)program; tok=strtok_r(prog_copy, " ", &savetok); - do { + while (tok != NULL) { if (i >= sizeof(argv)/sizeof(argv[0])-1) { LOG(ERROR, "To many arguments to %s", RPC_REQUEST_COMMAND); exit(1); } argv[i++] = tok; - } while ((tok=strtok_r(NULL, " ", &savetok))); + tok = strtok_r(NULL, " ", &savetok); + } argv[i] = NULL; - argv[0] = getenv("QREXEC_MULTIPLEXER_PATH"); - if (!argv[0]) - argv[0] = QUBES_RPC_MULTIPLEXER_PATH; - execve(argv[0], argv, envp); + if (i == 5) { + EXTEND("QREXEC_REQUESTED_TARGET_TYPE=%s", argv[3]); + if (strcmp(argv[3], "name") == 0) { + EXTEND("QREXEC_REQUESTED_TARGET_TYPE=%s", argv[4]); + } else if (strcmp(argv[3], "keyword") == 0) { + EXTEND("QREXEC_REQUESTED_TARGET_KEYWORD=%s", argv[4]); + } else { + // requested target type unknown or not given, ignore + } + } else if (i == 3) { + EXTEND_RAW("QREXEC_REQUESTED_TARGET_TYPE="); + } else { + LOG(ERROR, "invalid number of arguments: %zu", i); + _exit(126); + } + EXTEND("QREXEC_SERVICE_FULL_NAME=%s", argv[1]); + EXTEND("QREXEC_REMOTE_DOMAIN=%s", argv[2]); + const char *p = strchr(argv[1], '+'); + argv[1] = NULL; + argv[2] = NULL; + if (p != NULL) { + EXTEND("QREXEC_SERVICE_ARGUMENT=%s", p + 1); + if (p[1]) + argv[1] = p + 1; + } + buf[iterator] = NULL; + execve(argv[0], (char *const *)argv, buf); PERROR("exec qubes-rpc-multiplexer"); _exit(126); +bad_asprintf: + PERROR("asprintf"); + _exit(126); } } @@ -96,7 +163,8 @@ void fix_fds(int fdin, int fdout, int fderr) } } -static int do_fork_exec(const char *user, +static int do_fork_exec(const char *prog, + const char *user, const char *cmdline, int *pid, int *stdin_fd, @@ -133,7 +201,7 @@ static int do_fork_exec(const char *user, fcntl(statuspipe[1], F_SETFD, status | FD_CLOEXEC); } if (exec_func != NULL) - exec_func(cmdline, user); + exec_func(prog, cmdline, user); else abort(); status = errno; @@ -465,12 +533,12 @@ int execute_parsed_qubes_rpc_command( *pid = 0; return 0; } - return do_fork_exec(cmd->username, cmd->command, + return do_fork_exec(buf.data, cmd->username, cmd->command, pid, stdin_fd, stdout_fd, stderr_fd); } else { // Legacy qrexec behavior: spawn shell directly - return do_fork_exec(cmd->username, cmd->command, - pid, stdin_fd, stdout_fd, stderr_fd); + return do_fork_exec(NULL, cmd->username, cmd->command, + pid, stdin_fd, stdout_fd, stderr_fd); } } diff --git a/libqrexec/libqrexec-utils.h b/libqrexec/libqrexec-utils.h index 02d0eaef..91d456fb 100644 --- a/libqrexec/libqrexec-utils.h +++ b/libqrexec/libqrexec-utils.h @@ -104,14 +104,14 @@ int load_service_config(struct qrexec_parsed_command *cmd_name, __attribute__((deprecated("use load_service_config_v2() instead"))); int load_service_config_v2(struct qrexec_parsed_command *cmd); -typedef void (do_exec_t)(const char *cmdline, const char *user); +typedef void (do_exec_t)(const char *program, const char *cmd, const char *user); void register_exec_func(do_exec_t *func); /* * exec() qubes-rpc-multiplexer if *prog* starts with magic "QUBESRPC" keyword, * do not return in that case; pass *envp* to execve() as en environment * otherwise, return false without any action */ -void exec_qubes_rpc_if_requested(const char *prog, char *const envp[]); +void exec_qubes_rpc_if_requested(const char *program, const char *cmd, char *const envp[]); int exec_wait_for_session(const char *source_domain);