Avoid qubes-rpc-multiplexer for dom0 -> dom0 calls

Instead, just use qrexec-client, as with any other service call.

Author: DemiMarie

daemon/qrexec-client.c

@@ -201,6 +201,10 @@ int main(int argc, char **argv)
                     usage(argv[0], 1);
                 }
                 parse_connect(optarg, &request_id, &src_domain_name, &src_domain_id);
+                if (target_refers_to_dom0(src_domain_name) || src_domain_id == 0) {
+                    warnx("ERROR: -c cannot be used for requests to dom0");
+                    usage(argv[0], 1);
+                }
                 break;
             case 't':
                 replace_chars_stdout = true;
@@ -261,22 +265,77 @@ int main(int argc, char **argv)
     }
 
     if (target_refers_to_dom0(domname)) {
-        if (request_id == NULL) {
-            fprintf(stderr, "ERROR: when target domain is 'dom0', -c must be specified\n");
-            usage(argv[0], 1);
-        }
-        strncpy(svc_params.ident, request_id, sizeof(svc_params.ident) - 1);
-        svc_params.ident[sizeof(svc_params.ident) - 1] = '\0';
-        if (src_domain_name == NULL) {
-            LOG(ERROR, "internal error: src_domain_name should not be NULL here");
-            abort();
+        if (request_id != NULL) {
+            if (request_id[0] == '\0') {
+                warnx("ERROR: request ID cannot be empty");
+                usage(argv[0], 1);
+            }
+            strncpy(svc_params.ident, request_id, sizeof(svc_params.ident) - 1);
+            svc_params.ident[sizeof(svc_params.ident) - 1] = '\0';
+            if (src_domain_name == NULL) {
+                LOG(ERROR, "internal error: src_domain_name should not be NULL here");
+                abort();
+            }
+            rc = run_qrexec_to_dom0(&svc_params,
+                                    src_domain_id,
+                                    src_domain_name,
+                                    remote_cmdline,
+                                    connection_timeout,
+                                    exit_with_code);
+        } else {
+            /* dom0 -> dom0 fake service call */
+            assert(src_domain_id == 0);
+            if (local_cmdline != NULL) {
+                warnx("dom0 -> dom0 qrexec calls with LOCAL_COMMAND not yet implemented");
+                errx(QREXEC_EXIT_PROBLEM, "please file an issue if you need this");
+            }
+            /*
+             * Normally calls to dom0 omit the username, but in this case
+             * that would require the caller to pass the user if and only if the target is _not_
+             * dom0, and that's annoying.  In the past, qrexec-client was called by qrexec-daemon
+             * which got it right, but now the main caller of qrexec-client is Python scripts
+             * that don't have access to the C target_refers_to_dom0() function.
+             * Therefore, parse the username and fail if it is not "DEFAULT".
+             */
+#define DEFAULT_USER "DEFAULT"
+            if (strncmp(remote_cmdline, DEFAULT_USER ":", sizeof(DEFAULT_USER)) != 0) {
+                errx(QREXEC_EXIT_PROBLEM, "dom0 -> dom0 commands must be prefixed with " DEFAULT_USER ":");
+            }
+            remote_cmdline += sizeof(DEFAULT_USER);
+            struct qrexec_parsed_command *command = parse_qubes_rpc_command(remote_cmdline, false);
+            int prepare_ret;
+            char file_path[QUBES_SOCKADDR_UN_MAX_PATH_LEN];
+            struct buffer buf = { .data = file_path, .buflen = (int)sizeof(file_path) };
+            if (command == NULL) {
+                prepare_ret = -2;
+            } else if (command->service_descriptor == NULL) {
+                LOG(ERROR, "For dom0 -> dom0 commands, only proper qrexec calls are allowed");
+                prepare_ret = -2;
+            } else if (!wait_for_session_maybe(command)) {
+                LOG(ERROR, "Cannot load service configuration, or forking process failed");
+                prepare_ret = -2;
+            } else {
+                prepare_ret = find_qrexec_service(command, NULL, NULL, &buf);
+            }
+            switch (prepare_ret) {
+            case -2:
+                rc = QREXEC_EXIT_PROBLEM;
+                break;
+            case -1:
+                rc = QREXEC_EXIT_SERVICE_NOT_FOUND;
+                break;
+            case 0:
+                assert(command->username == NULL);
+                assert(command->command);
+                /* qrexec-client is always in a login session. */
+                exec_qubes_rpc2(buf.data, command->command, environ, false);
+                /* not reached */
+            default:
+                assert(false);
+                rc = QREXEC_EXIT_PROBLEM;
+                break;
+            }
         }
-        rc = run_qrexec_to_dom0(&svc_params,
-                           src_domain_id,
-                           src_domain_name,
-                           remote_cmdline,
-                           connection_timeout,
-                           exit_with_code);
     } else {
         if (request_id) {
             bool const use_uuid = strncmp(domname, "uuid:", 5) == 0;

daemon/qrexec-daemon-common.c

@@ -364,7 +364,7 @@ static void sigchld_handler(int x __attribute__((__unused__)))
 }
 
 /* See also qrexec-agent.c:wait_for_session_maybe() */
-static bool wait_for_session_maybe(struct qrexec_parsed_command *cmd)
+bool wait_for_session_maybe(struct qrexec_parsed_command *cmd)
 {
     pid_t pid;
     int status;

daemon/qrexec-daemon-common.h

@@ -147,3 +147,7 @@ bool qrexec_execute_vm(const char *target, bool autostart, int remote_domain_id,
 extern int local_stdin_fd;
 __attribute__((warn_unused_result))
 bool target_refers_to_dom0(const char *target);
+
+/** Wait for a session if needed. */
+__attribute__((warn_unused_result))
+bool wait_for_session_maybe(struct qrexec_parsed_command *cmd);

libqrexec/exec.c

@@ -62,6 +62,7 @@ static bool should_strip_env_var(const char *var)
 
 void exec_qubes_rpc2(const char *program, const char *cmd, char *const envp[],
                      bool use_shell) {
+    assert(cmd);
     /* avoid calling RPC service through shell */
     char *prog_copy;
     char *tok, *savetok;
@@ -743,7 +744,9 @@ int find_qrexec_service(
     const char *qrexec_service_path = getenv("QREXEC_SERVICE_PATH");
     if (!qrexec_service_path)
         qrexec_service_path = QREXEC_SERVICE_PATH;
-    *socket_fd = -1;
+    if (socket_fd != NULL) {
+        *socket_fd = -1;
+    }
 
     struct stat statbuf;
 
@@ -764,6 +767,9 @@ int find_qrexec_service(
 
     if (S_ISSOCK(statbuf.st_mode)) {
         /* Socket-based service. */
+        if (socket_fd == NULL) {
+            goto bad_socket_service;
+        }
         int s;
         if ((s = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0)) == -1) {
             PERROR("socket");
@@ -784,6 +790,9 @@ int find_qrexec_service(
         *socket_fd = s;
         return 0;
     } else if (S_ISLNK(statbuf.st_mode)) {
+        if (socket_fd == NULL) {
+            goto bad_socket_service;
+        }
         /* TCP-based service */
         assert(path_buffer->buflen >= (int)sizeof("/dev/tcp") - 1);
         assert(memcmp(path_buffer->data, "/dev/tcp", sizeof("/dev/tcp") - 1) == 0);
@@ -873,6 +882,11 @@ int find_qrexec_service(
     LOG(ERROR, "Unknown service type (not executable, not a socket): %.*s",
         path_buffer->buflen, path_buffer->data);
     return -2;
+bad_socket_service:
+    LOG(ERROR, "Service %.*s is a socket or /dev/tcp symlink, but this is not "
+               "supported for dom0 -> dom0 calls",
+        path_buffer->buflen, path_buffer->data);
+    return -2;
 }
 
 int exec_wait_for_session(const char *source_domain) {

qrexec/client.py

@@ -24,7 +24,6 @@
 
 QREXEC_CLIENT_DOM0 = "/usr/bin/qrexec-client"
 QREXEC_CLIENT_VM = "/usr/bin/qrexec-client-vm"
-RPC_MULTIPLEXER = "/usr/lib/qubes/qubes-rpc-multiplexer"
 
 VERSION = None
 
@@ -105,11 +104,6 @@ def make_command(dest, rpcname, arg):
         assert " " not in arg
         rpcname = f"{rpcname}+{arg}"
 
-    if VERSION == "dom0" and dest == "dom0":
-        # Invoke qubes-rpc-multiplexer directly. This will work for non-socket
-        # services only.
-        return [RPC_MULTIPLEXER, rpcname, "dom0"]
-
     if VERSION == "dom0":
         return [
             QREXEC_CLIENT_DOM0,

qrexec/tests/socket/daemon.py

@@ -641,7 +641,7 @@ def setUp(self):
     def make_executable_service(self, *args):
         util.make_executable_service(self.tempdir, *args)
 
-    def start_client(self, args):
+    def start_client(self, args, stderr=None):
         env = os.environ.copy()
         env["LD_LIBRARY_PATH"] = os.path.join(ROOT_PATH, "libqrexec")
         env["VCHAN_DOMAIN"] = "0"
@@ -664,6 +664,7 @@ def start_client(self, args):
             env=env,
             stdin=subprocess.PIPE,
             stdout=subprocess.PIPE,
+            stderr=stderr,
         )
         self.addCleanup(self.stop_client)
 
@@ -1020,6 +1021,64 @@ def assertExpectedStdout(
             ],
         )
 
+    def _test_run_service_in_dom0(self, requested_target):
+        util.make_executable_service(
+            self.tempdir,
+            "rpc",
+            "qubes.Service",
+            """\
+#!/bin/sh -eu
+read input
+echo "arg: $1, remote domain: $QREXEC_REMOTE_DOMAIN, input: $input, service path: $QREXEC_SERVICE_PATH"
+case $QREXEC_REQUESTED_TARGET_TYPE in
+(name) echo "requested target name: $QREXEC_REQUESTED_TARGET";;
+(keyword) echo "requested target keyword: $QREXEC_REQUESTED_TARGET_KEYWORD";;
+esac
+""",
+        )
+        requested_target_type = "name"
+        if requested_target.startswith("@"):
+            requested_target = requested_target[1:]
+            requested_target_type = "keyword"
+
+        cmd = f"DEFAULT:QUBESRPC qubes.Service+arg src_domain {requested_target_type} {requested_target}"
+        self.start_client(["-d", "dom0", cmd], stderr=subprocess.PIPE)
+        stdout, stderr = self.client.communicate(b"stdin data\n")
+        self.client.wait()
+        self.assertFalse(stderr, stderr)
+        self.assertEqual(stderr, b"")
+        self.assertEqual(self.client.returncode, 0)
+        service_path = (
+            ":".join(
+                [
+                    os.path.join(self.tempdir, "local-rpc"),
+                    os.path.join(self.tempdir, "rpc"),
+                ]
+            )
+        ).encode("ascii", "strict")
+        self.assertEqual(
+            stdout,
+            b"arg: arg, remote domain: src_domain, "
+            b"input: stdin data, service path: %s\n"
+            b"requested target %s: %s\n"
+            % (
+                service_path,
+                requested_target_type.encode("ascii", "strict"),
+                requested_target.encode("ascii", "strict"),
+            ),
+        )
+
+    def test_dom0_to_dom0(self):
+        self._test_run_service_in_dom0("@adminvm")
+
+    def test_dom0_to_dom0_v1(self):
+        self._test_run_service_in_dom0("dom0")
+
+    def test_dom0_to_dom0_v2(self):
+        self._test_run_service_in_dom0(
+            "uuid:00000000-0000-0000-0000-000000000000"
+        )
+
     def _test_run_dom0_service_exec(self, nogui, requested_target="src_domain"):
         util.make_executable_service(
             self.tempdir,