qrexec-client: fail if service configuration loading fails

Previously all such errors were ignored.

Fixes: QubesOS/qubes-issues#9101

Author: DemiMarie

daemon/qrexec-client.c

@@ -211,36 +211,46 @@ static _Noreturn void do_exec(const char *prog, const char *username __attribute
 
 
 /* See also qrexec-agent.c:wait_for_session_maybe() */
-static void wait_for_session_maybe(char *cmdline)
+static bool wait_for_session_maybe(char *cmdline)
 {
     struct qrexec_parsed_command *cmd;
     pid_t pid;
     int status;
+    bool rc = false;
 
     cmd = parse_qubes_rpc_command(cmdline, false);
     if (!cmd)
         goto out;
 
-    if (cmd->nogui)
+    if (cmd->nogui) {
+        rc = true;
         goto out;
+    }
 
-    if (!cmd->service_descriptor)
+    if (!cmd->service_descriptor) {
+        rc = true;
         goto out;
+    }
 
-    load_service_config_v2(cmd);
-    if (!cmd->wait_for_session)
+    if (load_service_config_v2(cmd) < 0)
         goto out;
+    if (!cmd->wait_for_session) {
+        rc = true;
+        goto out;
+    }
 
     pid = fork();
     switch (pid) {
         case 0:
             close(0);
             exec_wait_for_session(cmd->source_domain);
             PERROR("exec");
-            exit(1);
+            _exit(1);
         case -1:
             PERROR("fork");
             goto out;
+        default:
+            rc = true;
     }
 
     if (waitpid(local_pid, &status, 0) > 0) {
@@ -251,6 +261,7 @@ static void wait_for_session_maybe(char *cmdline)
 
 out:
     destroy_qrexec_parsed_command(cmd);
+    return rc;
 }
 
 static int prepare_local_fds(char *cmdline, struct buffer *stdin_buffer)
@@ -617,8 +628,12 @@ int main(int argc, char **argv)
 
         struct buffer stdin_buffer;
         buffer_init(&stdin_buffer);
-        wait_for_session_maybe(remote_cmdline);
-        prepare_ret = prepare_local_fds(remote_cmdline, &stdin_buffer);
+        if (!wait_for_session_maybe(remote_cmdline)) {
+            LOG(ERROR, "Cannot load service configuration, or forking process failed");
+            prepare_ret = -1;
+        } else {
+            prepare_ret = prepare_local_fds(remote_cmdline, &stdin_buffer);
+        }
         if (request_id) {
             void (*old_handler)(int);
 

qrexec/tests/socket/daemon.py

@@ -782,19 +782,56 @@ def test_run_dom0_command_and_connect_vm(self):
         self.client.wait()
         self.assertEqual(self.client.returncode, 0)
 
-    def test_run_dom0_service_exec(self):
+    def exec_service_with_invalid_config(self, invalid_config):
+        config_path = os.path.join(self.tempdir, "rpc-config", "qubes.Service+arg")
+        if invalid_config is not None:
+            with open(config_path, "w") as f:
+                f.write(invalid_config)
+        else:
+            os.symlink("/dev/null/doesnotexist", config_path)
         util.make_executable_service(
             self.tempdir,
             "rpc",
             "qubes.Service",
             """\
-        #!/bin/sh
-        read input
-        echo "arg: $1, remote domain: $QREXEC_REMOTE_DOMAIN, input: $input"
-        """,
+#!/bin/sh
+read input
+echo "arg: $1, remote domain: $QREXEC_REMOTE_DOMAIN, input: $input"
+""",
         )
+        self.test_run_dom0_service_failed()
 
-        cmd = "QUBESRPC qubes.Service+arg src_domain name src_domain"
+    def test_exec_service_with_invalid_config_1(self):
+        self.exec_service_with_invalid_config("wait-for-session = 00\n")
+
+    def test_exec_service_with_invalid_config_2(self):
+        self.exec_service_with_invalid_config("wait-for-session = 01\n")
+
+    def test_exec_service_with_invalid_config_3(self):
+        self.exec_service_with_invalid_config("wait-for-session = \n")
+
+    def test_exec_service_with_invalid_config_4(self):
+        self.exec_service_with_invalid_config("wait-for-session = \"a\"\n")
+
+    def test_exec_service_with_invalid_config_5(self):
+        self.exec_service_with_invalid_config("wait-for-session\n")
+
+    def test_exec_service_with_invalid_config_6(self):
+        self.exec_service_with_invalid_config(None)
+
+    def _test_run_dom0_service_exec(self, nogui):
+        util.make_executable_service(
+            self.tempdir,
+            "rpc",
+            "qubes.Service",
+            """\
+#!/bin/sh
+read input
+echo "arg: $1, remote domain: $QREXEC_REMOTE_DOMAIN, input: $input"
+""",
+        )
+
+        cmd = ("nogui:" if nogui else "") + "QUBESRPC qubes.Service+arg src_domain name src_domain"
         source = self.connect_service_request(cmd)
 
         source.send_message(qrexec.MSG_DATA_STDIN, b"stdin data\n")
@@ -814,6 +851,12 @@ def test_run_dom0_service_exec(self):
         self.client.wait()
         self.assertEqual(self.client.returncode, 0)
 
+    def test_run_dom0_service_exec(self):
+        self._test_run_dom0_service_exec(False)
+
+    def test_run_dom0_service_exec_nogui(self):
+        self._test_run_dom0_service_exec(True)
+
     def test_run_dom0_service_failed(self):
         # qubes.Service does not exist
         cmd = "QUBESRPC qubes.Service+arg src_domain name src_domain"