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.

Author: DemiMarie

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

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,83 +454,82 @@ 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,
                            pid, stdin_fd, stdout_fd, stderr_fd);
     }
 }
 
-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)) {
         /* Socket-based 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) {

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.

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,17 +151,14 @@ 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()
         dom0.handshake()
 
         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("<LL", self.target_domain, self.target_port)
@@ -170,17 +170,64 @@ def test_just_exec(self):
 
         target = self.connect_target()
         target.handshake()
+        return target, dom0
+
+    def test_just_exec_socket(self):
+        socket_path = os.path.join(
+            self.tempdir, "rpc", "qubes.SocketService+"
+        )
+        server = qrexec.socket_server(socket_path)
+
+        cmd = b"QUBESRPC qubes.SocketService a"
+        target, dom0 = self._test_just_exec(cmd)
+        server.accept()
+        self.assertEqual(server.recvall(len(cmd)), cmd[9:] + b"\0")
+        self.assertListEqual(
+            target.recv_all_messages(),
+            [
+                (qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
+            ],
+        )
+
+        self.check_dom0(dom0)
+
+    def test_just_exec(self):
+        fifo = os.path.join(self.tempdir, "new_file")
+        os.mkfifo(fifo, mode=0o600)
+        cmd = ("echo a >> " + 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,