Cleanly terminate connections if command or config is invalid

Send exit status 127 in this case, as if the command could not be found.
This prevents the caller from hanging, and allows this situation to be
tested.

Fixes: QubesOS/qubes-issues#9073

Author: DemiMarie

agent/qrexec-agent-data.c

@@ -134,10 +134,12 @@ static int handle_just_exec(struct qrexec_parsed_command *cmd)
 {
     int fdn, pid;
 
+    if (cmd == NULL)
+        return 127;
     switch (pid = fork()) {
         case -1:
             PERROR("fork");
-            return -1;
+            return 127;
         case 0:
             fdn = open("/dev/null", O_RDWR);
             fix_fds(fdn, fdn, fdn);
@@ -271,7 +273,8 @@ static int handle_new_process_common(
             return 0;
         case MSG_EXEC_CMDLINE:
             buffer_init(&stdin_buf);
-            if (execute_parsed_qubes_rpc_command(cmd, &pid, &stdin_fd, &stdout_fd, &stderr_fd, &stdin_buf) < 0) {
+            if (cmd == NULL ||
+                    execute_parsed_qubes_rpc_command(cmd, &pid, &stdin_fd, &stdout_fd, &stderr_fd, &stdin_buf) < 0) {
                 struct msg_header hdr = {
                     .type = MSG_DATA_STDOUT,
                     .len = 0,

agent/qrexec-agent.c

@@ -85,6 +85,7 @@ static const char *fork_server_path = QREXEC_FORK_SERVER_SOCKET;
 static void handle_server_exec_request_do(int type, int connect_domain, int connect_port,
                                           struct qrexec_parsed_command *cmd,
                                           char *cmdline);
+static void terminate_connection(uint32_t domain, uint32_t port);
 
 const bool qrexec_is_fork_server = false;
 
@@ -587,14 +588,16 @@ static void handle_server_exec_request_init(struct msg_header *hdr)
         cmd = parse_qubes_rpc_command(buf, true);
         if (cmd == NULL) {
             LOG(ERROR, "Could not parse command line: %s", buf);
-            return;
+            goto doit;
         }
 
         /* load service config only for service requests */
         if (cmd->service_descriptor) {
             if (load_service_config_v2(cmd) < 0) {
                 LOG(ERROR, "Could not load config for command %s", buf);
-                return;
+                destroy_qrexec_parsed_command(cmd);
+                cmd = NULL;
+                goto doit;
             }
 
             /* "nogui:" prefix has priority */
@@ -620,6 +623,7 @@ static void handle_server_exec_request_init(struct msg_header *hdr)
         }
     }
 
+doit:
     handle_server_exec_request_do(hdr->type, params.connect_domain, params.connect_port, cmd, buf);
     destroy_qrexec_parsed_command(cmd);
     free(buf);
@@ -663,7 +667,7 @@ static void handle_server_exec_request_do(int type,
         return;
     }
 
-    if (!cmd->nogui) {
+    if (cmd != NULL && !cmd->nogui) {
         /* try fork server */
         int child_socket = try_fork_server(type,
                 params.connect_domain, params.connect_port,
@@ -757,20 +761,29 @@ static int find_connection(int pid)
 }
 
 static void release_connection(int id) {
-    struct msg_header hdr;
-    struct exec_params params;
-
-    hdr.type = MSG_CONNECTION_TERMINATED;
-    hdr.len = sizeof(struct exec_params);
-    params.connect_domain = connection_info[id].connect_domain;
-    params.connect_port = connection_info[id].connect_port;
-    if (libvchan_send(ctrl_vchan, &hdr, sizeof(hdr)) != sizeof(hdr))
-        handle_vchan_error("send (MSG_CONNECTION_TERMINATED hdr)");
-    if (libvchan_send(ctrl_vchan, &params, sizeof(params)) != sizeof(params))
-        handle_vchan_error("send (MSG_CONNECTION_TERMINATED data)");
+    terminate_connection(connection_info[id].connect_domain,
+                         connection_info[id].connect_port);
     connection_info[id].pid = 0;
 }
 
+static void terminate_connection(uint32_t domain, uint32_t port) {
+    struct {
+        struct msg_header hdr;
+        struct exec_params params;
+    } data = {
+        .hdr = {
+            .type = MSG_CONNECTION_TERMINATED,
+            .len = sizeof(struct exec_params),
+        },
+        .params = {
+            .connect_domain = domain,
+            .connect_port = port,
+        },
+    };
+    if (libvchan_send(ctrl_vchan, &data, sizeof(data)) != sizeof(data))
+        handle_vchan_error("send (MSG_CONNECTION_TERMINATED)");
+}
+
 static void reap_children(void)
 {
     int status;

qrexec/tests/socket/agent.py

@@ -453,6 +453,47 @@ def test_exec_service_fail(self):
         )
         self.check_dom0(dom0)
 
+    def exec_service_with_invalid_config(self, invalid_config):
+        util.make_executable_service(
+            self.tempdir,
+            "rpc",
+            "qubes.Service",
+            """\
+#!/bin/sh
+echo "arg: $1, remote domain: $QREXEC_REMOTE_DOMAIN"
+""",
+        )
+        with open(
+            os.path.join(self.tempdir, "rpc-config", "qubes.Service+arg"), "w"
+        ) as f:
+            f.write(invalid_config)
+        target, dom0 = self.execute_qubesrpc("qubes.Service+arg", "domX")
+        messages = target.recv_all_messages()
+        self.assertListEqual(
+            util.sort_messages(messages),
+            [
+                (qrexec.MSG_DATA_STDOUT, b""),
+                (qrexec.MSG_DATA_STDERR, b""),
+                (qrexec.MSG_DATA_EXIT_CODE, b"\177\0\0\0"),
+            ],
+        )
+        self.check_dom0(dom0)
+
+    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_arg(self):
         self.make_executable_service(
             "local-rpc",