Share qrexec-daemon VM -> VM call code with qrexec-client

DemiMarie · DemiMarie · commit e06514b4692b · 2024-04-27T21:46:50.000-04:00
This allows the code to be unit-tested, and also reduces duplication.
diff --git a/daemon/qrexec-client.c b/daemon/qrexec-client.c
@@ -143,16 +143,6 @@ static void parse_connect(char *str, char **request_id,
     exit(1);
 }
 
-static size_t compute_service_length(const char *const remote_cmdline, const char *const prog_name) {
-    const size_t service_length = strlen(remote_cmdline) + 1;
-    if (service_length < 2 || service_length > MAX_QREXEC_CMD_LEN) {
-        /* This is arbitrary, but it helps reduce the risk of overflows in other code */
-        fprintf(stderr, "Bad command: command line too long or empty: length %zu\n", service_length);
-        usage(prog_name);
-    }
-    return service_length;
-}
-
 int main(int argc, char **argv)
 {
     int opt;
@@ -162,7 +152,6 @@ int main(int argc, char **argv)
     int data_domain;
     int s;
     bool just_exec = false;
-    int wait_connection_end = -1;
     char *local_cmdline = NULL;
     char *remote_cmdline = NULL;
     char *request_id = NULL;
@@ -174,6 +163,7 @@ int main(int argc, char **argv)
     bool kill = false;
     bool replace_chars_stdout = false;
     bool replace_chars_stderr = false;
+    bool wait_connection_end = false;
     bool exit_with_code = true;
     int rc = QREXEC_EXIT_PROBLEM;
 
@@ -215,7 +205,7 @@ int main(int argc, char **argv)
                 connection_timeout = parse_int(optarg, "connection timeout");
                 break;
             case 'W':
-                wait_connection_end = 1;
+                wait_connection_end = true;
                 break;
             case 'd' + 128:
                 socket_dir = strdup(optarg);
@@ -259,41 +249,22 @@ int main(int argc, char **argv)
                            connection_timeout,
                            exit_with_code);
     } else {
-        s = connect_unix_socket(domname);
-        if (!negotiate_connection_params(s,
-                src_domain_id,
-                just_exec ? MSG_JUST_EXEC : MSG_EXEC_CMDLINE,
-                remote_cmdline,
-                compute_service_length(remote_cmdline, argv[0]),
-                &data_domain,
-                &data_port)) {
-            goto cleanup;
-        }
         if (request_id) {
-            if (wait_connection_end != -1) {
-                /* save socket fd, 's' will be reused for the other qrexec-daemon
-                 * connection */
-                wait_connection_end = s;
-            } else {
-                close(s);
-            }
-            s = connect_unix_socket_by_id(src_domain_id);
-            if (s == -1) {
-                goto cleanup;
-            }
-            if (!send_service_connect(s, request_id, data_domain, data_port)) {
+            rc = qrexec_execute_vm(domname, false, src_domain_id,
+                                   remote_cmdline, strlen(remote_cmdline) + 1,
+                                   request_id, just_exec,
+                                   wait_connection_end) ? 0 : 137;
+        } else {
+            s = connect_unix_socket(domname);
+            if (!negotiate_connection_params(s,
+                        src_domain_id,
+                        just_exec ? MSG_JUST_EXEC : MSG_EXEC_CMDLINE,
+                        remote_cmdline,
+                        strlen(remote_cmdline) + 1,
+                        &data_domain,
+                        &data_port)) {
                 goto cleanup;
             }
-            close(s);
-            if (wait_connection_end != -1) {
-                /* wait for EOF */
-                struct pollfd fds[1] = {
-                    { .fd = wait_connection_end, .events = POLLIN | POLLHUP, .revents = 0 },
-                };
-                poll(fds, 1, -1);
-            }
-            rc = 0;
-        } else {
             set_remote_domain(domname);
             struct buffer stdin_buffer;
             buffer_init(&stdin_buffer);
@@ -334,13 +305,12 @@ int main(int argc, char **argv)
                 .replace_chars_stderr = replace_chars_stderr,
             };
             rc = handshake_and_go(&params);
-        }
-    }
-
 cleanup:
-    if (kill && domname) {
-        size_t l;
-        qubesd_call(domname, "admin.vm.Kill", "", &l);
+            if (kill && domname) {
+                size_t l;
+                qubesd_call(domname, "admin.vm.Kill", "", &l);
+            }
+        }
     }
 
     return rc;
diff --git a/daemon/qrexec-daemon-common.c b/daemon/qrexec-daemon-common.c
@@ -90,6 +90,106 @@ int handle_daemon_handshake(int fd)
     return 0;
 }
 
+bool qrexec_execute_vm(const char *target, bool autostart, int remote_domain_id,
+                       const char *cmd, size_t const service_length,
+                       const char *request_id, bool just_exec,
+                       bool wait_connection_end)
+{
+    if (service_length < 2 || (size_t)service_length > MAX_QREXEC_CMD_LEN) {
+        /* This is arbitrary, but it helps reduce the risk of overflows in other code */
+        LOG(ERROR, "Bad command: command line too long or empty: length %zu\n",
+            service_length);
+        return false;
+    }
+    bool rc = false;
+    bool disposable = strncmp("@dispvm:", target, sizeof("@dispvm:") - 1) == 0;
+    char *buf = NULL;
+    size_t resp_len;
+    if (disposable) {
+        if (!autostart) {
+            // Otherwise, we create a disposable VM, do not start it, and
+            // time out connecting to it.
+            LOG(ERROR, "Target %s with autostart=False makes no sense", target);
+            return false;
+        }
+        if (just_exec) {
+            // Otherwise, we kill the VM immediately after starting it.
+            LOG(ERROR, "Target %s with MSG_JUST_EXEC makes no sense", target);
+            return false;
+        }
+        // Otherwise, we kill the VM immediately after starting it.
+        wait_connection_end = true;
+        buf = qubesd_call(target + 8, "admin.vm.CreateDisposable", "", &resp_len);
+        if (buf == NULL) // error already printed by qubesd_call
+            return false;
+        if (memcmp(buf, "0", 2) == 0) {
+            /* we exit later so memory leaks do not matter */
+            target = buf + 2;
+        } else {
+            if (memcmp(buf, "2", 2) == 0) {
+                LOG(ERROR, "qubesd could not create disposable VM: %s", buf + 2);
+            } else {
+                LOG(ERROR, "invalid response to admin.vm.CreateDisposable");
+            }
+            free(buf);
+            return false;
+        }
+    }
+    if (autostart) {
+        buf = qubesd_call(target, "admin.vm.Start", "", &resp_len);
+        if (buf == NULL) // error already printed by qubesd_call
+            return false;
+        if (!((memcmp(buf, "0", 2) == 0) ||
+                    (resp_len >= 24 && memcmp(buf, "2\0QubesVMNotHaltedError", 24) == 0))) {
+            if (memcmp(buf, "2", 2) == 0) {
+                LOG(ERROR, "qubesd could not start VM %s: %s", target, buf + 2);
+            } else {
+                LOG(ERROR, "invalid response to admin.vm.Start");
+            }
+            free(buf);
+            return false;
+        }
+        free(buf);
+    }
+    int s = connect_unix_socket(target);
+    if (s == -1)
+        goto kill;
+    int data_domain;
+    int data_port;
+    if (!negotiate_connection_params(s,
+                remote_domain_id,
+                just_exec ? MSG_JUST_EXEC : MSG_EXEC_CMDLINE,
+                cmd,
+                service_length,
+                &data_domain,
+                &data_port))
+        goto kill;
+    int wait_connection_fd = -1;
+    if (wait_connection_end) {
+        /* save socket fd, 's' will be reused for the other qrexec-daemon
+         * connection */
+        wait_connection_fd = s;
+    } else {
+        close(s);
+    }
+
+    s = connect_unix_socket_by_id((unsigned)remote_domain_id);
+    rc = send_service_connect(s, request_id, data_domain, data_port);
+    close(s);
+    if (wait_connection_end) {
+        /* wait for EOF */
+        struct pollfd fds[1] = {
+            { .fd = wait_connection_fd, .events = POLLIN | POLLHUP, .revents = 0 },
+        };
+        poll(fds, 1, -1);
+        size_t l;
+kill:
+        if (disposable)
+            qubesd_call(target, "admin.vm.Kill", "", &l);
+    }
+    return rc;
+}
+
 int connect_unix_socket_by_id(unsigned int domid)
 {
     char id_str[11];
diff --git a/daemon/qrexec-daemon-common.h b/daemon/qrexec-daemon-common.h
@@ -38,3 +38,7 @@ __attribute__((warn_unused_result))
 int handle_agent_handshake(libvchan_t *vchan, bool remote_send_first);
 __attribute__((warn_unused_result))
 int prepare_local_fds(struct qrexec_parsed_command *command, struct buffer *stdin_buffer);
+__attribute__((warn_unused_result))
+bool qrexec_execute_vm(const char *target, bool autostart, int remote_domain_id,
+                       const char *cmd, size_t service_length, const char *request_id,
+                       bool just_exec, bool wait_connection_end);
diff --git a/daemon/qrexec-daemon.c b/daemon/qrexec-daemon.c
@@ -1095,15 +1095,6 @@ static enum policy_response connect_daemon_socket(
     }
 }
 
-static size_t compute_service_length(const char *const remote_cmdline) {
-    const size_t service_length = strlen(remote_cmdline) + 1;
-    if (service_length < 2 || service_length > MAX_QREXEC_CMD_LEN) {
-        /* This is arbitrary, but it helps reduce the risk of overflows in other code */
-        errx(1, "Bad command: command line too long or empty: length %zu\n", service_length);
-    }
-    return service_length;
-}
-
 /* called from do_fork_exec */
 static _Noreturn void do_exec(const char *prog, const char *username __attribute__((unused)))
 {
@@ -1139,8 +1130,6 @@ _Noreturn static void handle_execute_service_child(
     /* Replace the target domain with the version normalized by the policy engine */
     target_domain = requested_target;
     char *cmd = NULL;
-    bool disposable = false;
-    size_t resp_len;
 
     /*
      * If there was no service argument, pretend that an empty argument was
@@ -1175,78 +1164,18 @@ _Noreturn static void handle_execute_service_child(
                            5 /* 5 second timeout */,
                            false /* return 0 not remote status code */));
     } else {
-        char *buf;
-        if (strncmp("@dispvm:", target, sizeof("@dispvm:") - 1) == 0) {
-            disposable = true;
-            buf = qubesd_call(target + 8, "admin.vm.CreateDisposable", "", &resp_len);
-            if (!buf) // error already printed by qubesd_call
-                daemon__exit(QREXEC_EXIT_PROBLEM);
-            if (memcmp(buf, "0", 2) == 0) {
-                /* we exec later so memory leaks do not matter */
-                target = buf + 2;
-            } else {
-                if (memcmp(buf, "2", 2) == 0) {
-                    LOG(ERROR, "qubesd could not create disposable VM: %s", buf + 2);
-                } else {
-                    LOG(ERROR, "invalid response to admin.vm.CreateDisposable");
-                }
-                daemon__exit(QREXEC_EXIT_PROBLEM);
-            }
-        }
-        if (asprintf(&cmd, "%s:QUBESRPC %s%s %s",
-                    user,
-                    service_name,
-                    trailer,
-                    remote_domain_name) <= 0)
+        int service_length = asprintf(&cmd, "%s:QUBESRPC %s%s %s",
+                                      user,
+                                      service_name,
+                                      trailer,
+                                      remote_domain_name);
+        if (service_length < 0)
             daemon__exit(QREXEC_EXIT_PROBLEM);
-        if (autostart) {
-            buf = qubesd_call(target, "admin.vm.Start", "", &resp_len);
-            if (!buf) // error already printed by qubesd_call
-                daemon__exit(QREXEC_EXIT_PROBLEM);
-            if (!((memcmp(buf, "0", 2) == 0) ||
-                  (resp_len >= 24 && memcmp(buf, "2\0QubesVMNotHaltedError", 24) == 0))) {
-                if (memcmp(buf, "2", 2) == 0) {
-                    LOG(ERROR, "qubesd could not start VM %s: %s", target, buf + 2);
-                } else {
-                    LOG(ERROR, "invalid response to admin.vm.Start");
-                }
-                daemon__exit(QREXEC_EXIT_PROBLEM);
-            }
-            free(buf);
-        }
-        int s = connect_unix_socket(target);
-        int data_domain;
-        int data_port;
-        int rc = QREXEC_EXIT_PROBLEM;
-        if (!negotiate_connection_params(s,
-                remote_domain_id,
-                MSG_EXEC_CMDLINE,
-                cmd,
-                compute_service_length(cmd),
-                &data_domain,
-                &data_port))
-            daemon__exit(QREXEC_EXIT_PROBLEM);
-        int wait_connection_end = -1;
-        if (disposable) {
-            wait_connection_end = s;
-        } else {
-            close(s);
-        }
-
-        s = connect_unix_socket_by_id((unsigned)remote_domain_id);
-        if (send_service_connect(s, request_id->ident, data_domain, data_port))
-            rc = 0;
-        close(s);
-        if (wait_connection_end != -1) {
-            /* wait for EOF */
-            struct pollfd fds[1] = {
-                { .fd = wait_connection_end, .events = POLLIN | POLLHUP, .revents = 0 },
-            };
-            poll(fds, 1, -1);
-            size_t l;
-            qubesd_call(target, "admin.vm.Kill", "", &l);
-        }
-        daemon__exit(rc);
+        daemon__exit(qrexec_execute_vm(target, autostart, remote_domain_id,
+                                       cmd,
+                                       (size_t)service_length + 1,
+                                       request_id->ident, false, false)
+                ? 0 : QREXEC_EXIT_PROBLEM);
     }
 }
 
