From dfdfb338abd3a537468c181d43b78a4ba37eca68 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sun, 14 Apr 2024 12:40:50 -0400 Subject: [PATCH] Support socket services with MSG_JUST_EXEC This separates _finding_ a qrexec service (and, if necessary, connecting to a socket) from actually _executing_ the service and processing I/O. This allows qrexec-agent to handle MSG_JUST_EXEC correctly, because it can connect to a socket-based service without having to be prepared for an executable service to be run immediately. Instead, qrexec-agent spawns an executable service using its own spawning routines that do support MSG_JUST_EXEC. This also adds tests for RPC services (both executable and socket) with MSG_JUST_EXEC. Previously, these were not tested, despite being essential to a running system. For instance, the Qubes app menu uses MSG_JUST_EXEC to invoke the qubes.StartApp service every time it launches an application in a non-disposable VM from dom0. --- agent/qrexec-agent-data.c | 10 ++++++ libqrexec/exec.c | 70 ++++++++++++++++-------------------- libqrexec/libqrexec-utils.h | 20 ++++++++++- qrexec/tests/socket/agent.py | 66 ++++++++++++++++++++++++++++------ 4 files changed, 115 insertions(+), 51 deletions(-) diff --git a/agent/qrexec-agent-data.c b/agent/qrexec-agent-data.c index f84cff61..bafeaff2 100644 --- a/agent/qrexec-agent-data.c +++ b/agent/qrexec-agent-data.c @@ -136,6 +136,16 @@ static int handle_just_exec(struct qrexec_parsed_command *cmd) if (cmd == NULL) return 127; + + if (cmd->service_descriptor) { + int socket_fd; + struct buffer stdin_buffer; + buffer_init(&stdin_buffer); + if (!find_qrexec_service(cmd, &socket_fd, &stdin_buffer)) + return 127; + if (socket_fd != -1) + return write_all(socket_fd, stdin_buffer.data, stdin_buffer.buflen) ? 0 : 127; + } switch (pid = fork()) { case -1: PERROR("fork"); diff --git a/libqrexec/exec.c b/libqrexec/exec.c index 5d57fad5..10b0ff5a 100644 --- a/libqrexec/exec.c +++ b/libqrexec/exec.c @@ -162,8 +162,6 @@ static int do_fork_exec(const char *user, return retval; } -#define QUBES_SOCKADDR_UN_MAX_PATH_LEN 1024 - static int qubes_connect(int s, const char *connect_path, const size_t total_path_length) { // Avoiding an extra copy is NOT worth it! #define QUBES_TMP_DIRECTORY "/tmp/qrexec-XXXXXX" @@ -205,11 +203,6 @@ static int qubes_connect(int s, const char *connect_path, const size_t total_pat return result; } -static int execute_qrexec_service( - const struct qrexec_parsed_command *cmd, - int *pid, int *stdin_fd, int *stdout_fd, int *stderr_fd, - struct buffer *stdin_buffer); - /* Find a file in the ':'-delimited list of paths given in path_list. Returns 0 on success, -1 if the file is definitely absent in all of the @@ -461,8 +454,17 @@ int execute_parsed_qubes_rpc_command( int *stdout_fd, int *stderr_fd, struct buffer *stdin_buffer) { if (cmd->service_descriptor) { // Proper Qubes RPC call - return execute_qrexec_service( - cmd, pid, stdin_fd, stdout_fd, stderr_fd, stdin_buffer); + if (!find_qrexec_service(cmd, stdin_fd, stdin_buffer)) + return -1; + if (*stdin_fd > -1) { + *stdout_fd = *stdin_fd; + if (stderr_fd) + *stderr_fd = -1; + *pid = 0; + return 0; + } + return do_fork_exec(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, @@ -470,31 +472,31 @@ int execute_parsed_qubes_rpc_command( } } -static int execute_qrexec_service( +bool find_qrexec_service( const struct qrexec_parsed_command *cmd, - int *pid, int *stdin_fd, int *stdout_fd, int *stderr_fd, - struct buffer *stdin_buffer) { - + int *socket_fd, struct buffer *stdin_buffer) { assert(cmd->service_descriptor); + char file_path[QUBES_SOCKADDR_UN_MAX_PATH_LEN]; + struct buffer path_buffer = { .data = file_path, .buflen = (int)sizeof(file_path) }; const char *qrexec_service_path = getenv("QREXEC_SERVICE_PATH"); if (!qrexec_service_path) qrexec_service_path = QREXEC_SERVICE_PATH; + *socket_fd = -1; - char service_full_path[QUBES_SOCKADDR_UN_MAX_PATH_LEN]; struct stat statbuf; int ret = find_file(qrexec_service_path, cmd->service_descriptor, - service_full_path, sizeof(service_full_path), + path_buffer.data, (size_t)path_buffer.buflen, &statbuf); if (ret == -1) ret = find_file(qrexec_service_path, cmd->service_name, - service_full_path, sizeof(service_full_path), + path_buffer.data, (size_t)path_buffer.buflen, &statbuf); if (ret < 0) { LOG(ERROR, "Service not found: %s", cmd->service_descriptor); - return -1; + return false; } if (S_ISSOCK(statbuf.st_mode)) { @@ -502,42 +504,32 @@ static int execute_qrexec_service( int s; if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) { PERROR("socket"); - return -1; + return false; } - if (qubes_connect(s, service_full_path, strlen(service_full_path))) { + if (qubes_connect(s, path_buffer.data, strlen(path_buffer.data))) { PERROR("qubes_connect"); close(s); - return -1; + return false; } - *stdout_fd = *stdin_fd = s; - if (stderr_fd) - *stderr_fd = -1; - *pid = 0; - set_nonblock(s); - if (cmd->send_service_descriptor) { /* send part after "QUBESRPC ", including trailing NUL */ const char *desc = cmd->command + RPC_REQUEST_COMMAND_LEN + 1; buffer_append(stdin_buffer, desc, strlen(desc) + 1); } - return 0; - } - if (euidaccess(service_full_path, X_OK) == 0) { - /* - Executable-based service. + *socket_fd = s; + return true; + } - 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, - pid, stdin_fd, stdout_fd, stderr_fd); + if (euidaccess(path_buffer.data, X_OK) == 0) { + /* Executable-based service. */ + return true; } - LOG(ERROR, "Unknown service type (not executable, not a socket): %s", - service_full_path); - return -1; + LOG(ERROR, "Unknown service type (not executable, not a socket): %.*s", + path_buffer.buflen, path_buffer.data); + return false; } int exec_wait_for_session(const char *source_domain) { diff --git a/libqrexec/libqrexec-utils.h b/libqrexec/libqrexec-utils.h index a042577c..dcad6462 100644 --- a/libqrexec/libqrexec-utils.h +++ b/libqrexec/libqrexec-utils.h @@ -127,7 +127,7 @@ int write_stdin(int fd, const char *data, int len, struct buffer *buffer); /** * @brief Execute an already-parsed Qubes RPC command. - * @param cmdline Null-terminated command to execute. + * @param cmd Already-parsed command to execute. * @param pid On return, holds the PID of the child process. * @param stdin_fd On return, holds a file descriptor connected to the child's * stdin. @@ -144,6 +144,24 @@ int execute_parsed_qubes_rpc_command( const struct qrexec_parsed_command *cmd, int *pid, int *stdin_fd, int *stdout_fd, int *stderr_fd, struct buffer *stdin_buffer); +/** + * @brief Find the implementation of a Qubes RPC command. If it is a socket, + * connect to it. + * @param[in] cmdline Null-terminated command to execute. + * @param[out] socket_fd On return, holds a file descriptor connected to the socket, + * or -1 for executable services. + * @param stdin_buffer This buffer will need to be prepended to the child process’s + * stdin. + * @return true if the implementation is found (and, for sockets, connected to) + * successfully, false on failure. + */ +bool find_qrexec_service( + const struct qrexec_parsed_command *cmd, + int *socket_fd, struct buffer *stdin_buffer); + +/** Suggested buffer size for the path buffer of find_qrexec_service. */ +#define QUBES_SOCKADDR_UN_MAX_PATH_LEN 1024 + /** * @brief Execute a Qubes RPC command. * @param cmdline Null-terminated command to execute. diff --git a/qrexec/tests/socket/agent.py b/qrexec/tests/socket/agent.py index 9c56dfe6..b4705e15 100644 --- a/qrexec/tests/socket/agent.py +++ b/qrexec/tests/socket/agent.py @@ -27,6 +27,7 @@ import getpass import itertools import asyncio +import shlex import psutil import pytest @@ -71,6 +72,9 @@ def assertExpectedStdout(self, target, expected_stdout: bytes, *, exit_code=0): self.assertEqual(msg_type, qrexec.MSG_DATA_STDOUT) stdout_entries.append(msg_body) + def make_executable_service(self, *args): + util.make_executable_service(self.tempdir, *args) + def setUp(self): self.tempdir = tempfile.mkdtemp() os.mkdir(os.path.join(self.tempdir, "local-rpc")) @@ -139,7 +143,6 @@ def connect_client(self): self.addCleanup(client.close) return client - @unittest.skipIf(os.environ.get("SKIP_SOCKET_TESTS"), "socket tests not set up") class TestAgent(TestAgentBase): def test_handshake(self): @@ -148,7 +151,7 @@ def test_handshake(self): dom0 = self.connect_dom0() dom0.handshake() - def test_just_exec(self): + def _test_just_exec(self, cmd): self.start_agent() dom0 = self.connect_dom0() @@ -156,9 +159,6 @@ def test_just_exec(self): user = getpass.getuser().encode("ascii") - cmd = ("touch " + os.path.join(self.tempdir, "new_file")).encode( - "ascii" - ) dom0.send_message( qrexec.MSG_JUST_EXEC, struct.pack("> " + shlex.quote(fifo)).encode("ascii", "strict") + target, dom0 = self._test_just_exec(cmd) self.assertListEqual( target.recv_all_messages(), [ (qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"), ], ) + with open(fifo, "rb") as f: + self.assertEqual(f.read(), b"a\n") + self.check_dom0(dom0) - util.wait_until( - lambda: os.path.exists(os.path.join(self.tempdir, "new_file")), - "file created", + def test_just_exec_rpc(self): + fifo = os.path.join(self.tempdir, "new_file") + os.mkfifo(fifo, mode=0o600) + util.make_executable_service( + self.tempdir, + "rpc", + "qubes.Service", + fr"""#!/bin/bash -eu +printf %s\\n "$QREXEC_SERVICE_FULL_NAME" >> {shlex.quote(fifo)} +""", ) + cmd = b"QUBESRPC qubes.Service+ domX" + target, dom0 = self._test_just_exec(cmd) + self.assertListEqual( + target.recv_all_messages(), + [ + (qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"), + ], + ) + + with open(fifo, "rb") as f: + self.assertEqual(f.read(), b"qubes.Service+\n") self.check_dom0(dom0) def test_exec_cmdline(self): @@ -305,9 +352,6 @@ def execute_qubesrpc(self, service: str, src_domain_name: str): target.handshake() return target, dom0 - def make_executable_service(self, *args): - util.make_executable_service(self.tempdir, *args) - def test_exec_service(self): util.make_executable_service( self.tempdir,