Skip to content

Commit

Permalink
Draft: Rip out qubes-rpc-multiplexer
Browse files Browse the repository at this point in the history
Instead, directly execute the command from C.

Marked as draft for five reasons:

1. MSG_JUST_EXEC is now unable to invoke services.  This means that
   wait=False qrexec calls from the Admin API made in dom0 do not work.

2. There is no logging of the service's stderr anymore.

3. libqrexec-utils has an ABI break, meaning that a new library cannot
   work with old programs and visa versa.

4. This PR is based on another PR (QubesOS#139), not main.

5. All variables with names beginning with QREXEC_ are stripped from the
   environment.  This is a change in behavior compared to the current
   code.

1, 2, 3, and 4 must be fixed before this can be merged.  5 is a design
decision that could go either way.

Fixes: QubesOS/qubes-issues#9062
  • Loading branch information
DemiMarie committed Apr 10, 2024
1 parent 2be9adc commit 12fb8e5
Show file tree
Hide file tree
Showing 10 changed files with 105 additions and 99 deletions.
4 changes: 3 additions & 1 deletion agent/qrexec-agent-data.c
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,16 @@ static int handle_just_exec(struct qrexec_parsed_command *cmd)

if (cmd == NULL)
return 127;
if (strncmp(cmd->command, RPC_REQUEST_COMMAND, RPC_REQUEST_COMMAND_LEN) == 0)
return 127;
switch (pid = fork()) {
case -1:
PERROR("fork");
return 127;
case 0:
fdn = open("/dev/null", O_RDWR);
fix_fds(fdn, fdn, fdn);
do_exec(cmd->command, cmd->username);
do_exec(NULL, cmd->command, cmd->username);
default:;
}
LOG(INFO, "executed (nowait): %s (pid %d)", cmd->command, pid);
Expand Down
6 changes: 3 additions & 3 deletions agent/qrexec-agent.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 *path, const char *cmd, const char *user)
{
#ifdef HAVE_PAM
int retval, status;
Expand Down Expand Up @@ -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(path, cmd, environ);

/* otherwise exec shell */
execl("/bin/sh", "sh", "-c", cmd, NULL);
Expand Down Expand Up @@ -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(path, cmd, env);

/* otherwise exec shell */
execle(pw->pw_shell, arg0, "-c", cmd, (char*)NULL, env);
Expand Down
2 changes: 1 addition & 1 deletion agent/qrexec-agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *path, const char *cmd, const char *user);
/* call before fork() for service handling process (either end) */
void prepare_child_env(void);

Expand Down
2 changes: 1 addition & 1 deletion agent/qrexec-client-vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
4 changes: 2 additions & 2 deletions agent/qrexec-fork-server.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@
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;

signal(SIGCHLD, SIG_DFL);
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");
Expand Down
6 changes: 4 additions & 2 deletions daemon/qrexec-client.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,12 @@ 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 *path,
const char *prog,
const char *username __attribute__((unused)))
{
/* avoid calling qubes-rpc-multiplexer through shell */
exec_qubes_rpc_if_requested(prog, environ);
exec_qubes_rpc_if_requested(path, prog, environ);

/* if above haven't executed qubes-rpc-multiplexer, pass it to shell */
execl("/bin/bash", "bash", "-c", prog, NULL);
Expand Down
64 changes: 0 additions & 64 deletions lib/qubes-rpc-multiplexer

This file was deleted.

2 changes: 1 addition & 1 deletion libqrexec/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ override QUBES_CFLAGS := -I. -I../libqrexec -g -O2 -Wall -Wextra -Werror \

override LDFLAGS += -pie -Wl,-z,relro,-z,now -shared

override SO_VER=3
override SO_VER=4
override VCHANLIBS := $(shell pkg-config --libs vchan)
LIBDIR ?= /usr/lib
INCLUDEDIR ?= /usr/include
Expand Down
110 changes: 88 additions & 22 deletions libqrexec/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,36 +45,106 @@ 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);
}

if (program == NULL) {
LOG(ERROR, "cannot call QUBESRPC with MSG_JUST_EXEC");
_exit(126);
}

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);
}
}

Expand All @@ -96,7 +166,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,
Expand Down Expand Up @@ -133,7 +204,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;
Expand Down Expand Up @@ -465,7 +536,7 @@ int execute_parsed_qubes_rpc_command(
cmd, pid, stdin_fd, stdout_fd, stderr_fd, stdin_buffer);
} else {
// Legacy qrexec behavior: spawn shell directly
return do_fork_exec(cmd->username, cmd->command,
return do_fork_exec(NULL, cmd->username, cmd->command,
pid, stdin_fd, stdout_fd, stderr_fd);
}
}
Expand Down Expand Up @@ -525,13 +596,8 @@ static int execute_qrexec_service(
}

if (euidaccess(service_full_path, X_OK) == 0) {
/*
Executable-based service.
Note that this delegates to qubes-rpc-multiplexer, which, for the
moment, searches for the right file again.
*/
return do_fork_exec(cmd->username, cmd->command,
/* Executable-based service. */
return do_fork_exec(service_full_path, cmd->username, cmd->command,
pid, stdin_fd, stdout_fd, stderr_fd);
}

Expand Down
4 changes: 2 additions & 2 deletions libqrexec/libqrexec-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *path, const char *cmdline, 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 *path, const char *prog, char *const envp[]);

int exec_wait_for_session(const char *source_domain);

Expand Down

0 comments on commit 12fb8e5

Please sign in to comment.