Add support for exiting on stdin or stdout EOF

This adds two new boolean service configuration options:

- exit-on-stdout-eof: exit when the socket service shuts down its
  output stream for writing.

- exit-on-stdin-eof: exit when the client sends EOF.

To avoid compatibility problems, global variables are used to pass this
information from the configuration parser to the I/O code.  In the
future, there should be a better way to pass this information, but this
is not possible without more extensive changes.  Fortunately, the
configuration parser only runs once in the life of any process right
now, so this commit adds assertions to check this.  Qubes OS ships with
assertions enabled, so any violation of this rule will be detected.

The main use of these features is to emulate the old qubes.ConnectTCP
and qubes.UpdatesProxy services, which already had this behavior due to
the use of socat.

These features are only supported for socket-based services, as
executable services are more complicated and do not have a use case
right now.

Currently, if a service exits due to exit-on-stdin-eof, the empty
MSG_DATA_STDOUT that indicates EOF is not sent.  This is not a problem
because qrexec-client-vm interprets MSG_DATA_EXIT_CODE as also
indicating EOF on stdout and stderr.

Fixes: QubesOS/qubes-issues#9176

Author: DemiMarie

agent/qrexec-agent-data.c

@@ -281,7 +281,7 @@ static int handle_new_process_common(
     req.prefix_data.data = NULL;
     req.prefix_data.len = 0;
 
-    exit_code = process_io(&req);
+    exit_code = qrexec_process_io(&req, cmd);
 
     if (type == MSG_EXEC_CMDLINE && pid > 0)
         LOG(INFO, "pid %d exited with %d", pid, exit_code);
@@ -378,7 +378,7 @@ int handle_data_client(
         req.prefix_data.len = 0;
     }
 
-    exit_code = process_io(&req);
+    exit_code = qrexec_process_io(&req, NULL);
     libvchan_close(data_vchan);
     return exit_code;
 }

agent/qrexec-agent.c

@@ -648,9 +648,8 @@ static void handle_server_exec_request_do(int type,
         return;
     }
 
-    /* Fork server does not load configuration, so if sending a service
-     * descriptor is not enabled, do not use it. */
-    if (cmd != NULL && !cmd->nogui && cmd->send_service_descriptor) {
+    /* Ask libqrexec-utils if the fork server is safe to use */
+    if (qrexec_cmd_use_fork_server(cmd)) {
         /* try fork server */
         int child_socket = try_fork_server(type,
                 params->connect_domain, params->connect_port,

agent/qrexec-client-vm.c

@@ -149,7 +149,7 @@ int main(int argc, char **argv)
 
     setup_logging("qrexec-client-vm");
 
-    // TODO: this should be in process_io
+    // TODO: this should be in qrexec_process_io
     signal(SIGPIPE, SIG_IGN);
 
     while (1) {

daemon/qrexec-client.c

@@ -303,6 +303,8 @@ int main(int argc, char **argv)
                     prepare_ret = QREXEC_EXIT_PROBLEM;
                 else {
                     prepare_ret = prepare_local_fds(command, &stdin_buffer);
+                    /* Don't pass this to handshake_and_go() as this is not
+                     * a service call to dom0. */
                     destroy_qrexec_parsed_command(command);
                 }
             } else {
@@ -333,7 +335,7 @@ int main(int argc, char **argv)
                 .replace_chars_stdout = replace_chars_stdout,
                 .replace_chars_stderr = replace_chars_stderr,
             };
-            rc = handshake_and_go(&params);
+            rc = handshake_and_go(&params, NULL);
 cleanup:
             if (kill && domname) {
                 size_t l;

daemon/qrexec-daemon-common.c

@@ -409,7 +409,8 @@ static void handle_failed_exec(libvchan_t *data_vchan, bool is_service, int exit
     }
 }
 
-static int select_loop(struct handshake_params *params)
+static int select_loop(const struct handshake_params *params,
+                       const struct qrexec_parsed_command *cmd)
 {
     struct process_io_request req = { 0 };
     int exit_code;
@@ -429,7 +430,7 @@ static int select_loop(struct handshake_params *params)
     req.prefix_data.data = NULL;
     req.prefix_data.len = 0;
 
-    exit_code = process_io(&req);
+    exit_code = qrexec_process_io(&req, cmd);
     return (params->exit_with_code ? exit_code : 0);
 }
 
@@ -493,10 +494,11 @@ int run_qrexec_to_dom0(const struct service_params *svc_params,
         .replace_chars_stdout = false, // stdout is _from_ dom0
         .replace_chars_stderr = false, // stderr is _from_ dom0
     };
-    return handshake_and_go(&params);
+    return handshake_and_go(&params, command);
 }
 
-int handshake_and_go(struct handshake_params *params)
+int handshake_and_go(struct handshake_params *params,
+                     const struct qrexec_parsed_command *cmd)
 {
     int rc = QREXEC_EXIT_PROBLEM;
     if (params->data_vchan == NULL || !libvchan_is_open(params->data_vchan)) {
@@ -507,11 +509,12 @@ int handshake_and_go(struct handshake_params *params)
                                                        params->remote_send_first);
     if (data_protocol_version >= 0) {
         if (params->prepare_ret != 0) {
-            rc = params->prepare_ret == -1 ? QREXEC_EXIT_SERVICE_NOT_FOUND : QREXEC_EXIT_PROBLEM;
+            rc = params->prepare_ret == -1 ? QREXEC_EXIT_SERVICE_NOT_FOUND : rc;
             handle_failed_exec(params->data_vchan, params->remote_send_first, rc);
         } else {
+            assert((cmd != NULL) == params->remote_send_first);
             params->data_protocol_version = data_protocol_version;
-            rc = select_loop(params);
+            rc = select_loop(params, cmd);
         }
     }
 cleanup:

daemon/qrexec-daemon-common.h

@@ -33,7 +33,8 @@ struct handshake_params {
     bool replace_chars_stderr;
 };
 __attribute__((warn_unused_result))
-int handshake_and_go(struct handshake_params *params);
+int handshake_and_go(struct handshake_params *params,
+                     const struct qrexec_parsed_command *cmd);
 __attribute__((warn_unused_result))
 int handle_agent_handshake(libvchan_t *vchan, bool remote_send_first);
 __attribute__((warn_unused_result))

libqrexec/exec.c

@@ -328,7 +328,16 @@ static int load_service_config_raw(struct qrexec_parsed_command *cmd,
     if (ret == -1)
         return 0;
     return qubes_toml_config_parse(config_full_path, &cmd->wait_for_session, user,
-                                   &cmd->send_service_descriptor);
+                                   &cmd->send_service_descriptor,
+                                   &cmd->exit_on_stdout_eof,
+                                   &cmd->exit_on_stdin_eof);
+}
+
+bool qrexec_cmd_use_fork_server(const struct qrexec_parsed_command *cmd) {
+    if (cmd == NULL)
+        return false;
+    return !cmd->nogui && cmd->send_service_descriptor && !cmd->exit_on_stdin_eof &&
+           !cmd->exit_on_stdout_eof;
 }
 
 int load_service_config_v2(struct qrexec_parsed_command *cmd) {
@@ -731,6 +740,18 @@ int find_qrexec_service(
                 path_buffer.data);
             return -2;
         }
+        if (cmd->exit_on_stdout_eof) {
+            LOG(ERROR, "Refusing to execute executable service %s with "
+                       "exit-on-service-eof=true",
+                path_buffer.data);
+            return -2;
+        }
+        if (cmd->exit_on_stdin_eof) {
+            LOG(ERROR, "Refusing to execute executable service %s with "
+                       "exit-on-client-eof=true",
+                path_buffer.data);
+            return -2;
+        }
         return 0;
     }
 

libqrexec/libqrexec-utils.h

@@ -89,6 +89,14 @@ struct qrexec_parsed_command {
     /* Remaining fields are private to libqrexec-utils.  Do not access them
      * directly - they may change in any update. */
 
+    /* For socket-based services: Should the event loop exit on EOF from
+     * the client? */
+    bool exit_on_stdin_eof;
+
+    /* For socket-based services: Should the event loop exit on EOF from
+     * the service? */
+    bool exit_on_stdout_eof;
+
     /* Pointer to the argument, or NULL if there is no argument.
      * Same buffer as "service_descriptor". */
     char *arg;
@@ -301,75 +309,29 @@ struct process_io_request {
     volatile sig_atomic_t *sigusr1;
     struct prefix_data prefix_data;
 };
-#pragma GCC diagnostic push
-#pragma GCC diagnostic error "-Wpadded"
-
-/* Set of options for qubes_qrexec_process_io(). */
-struct qrexec_process_io_request {
-    /* Note that stdin_fd, stdout_fd, and stderr_fd are named assuming a
-     * *local* process.  For a *remote* process, stdin_fd is the standard
-     * *output*, stdout_fd is the standard *input*, and stderr_fd must be -1. */
-    int stdin_fd, stdout_fd, stderr_fd;
-    // 0 if no child process
-    pid_t local_pid;
-    /*
-      is_service true (this is a service):
-        - send local data as MSG_DATA_STDOUT
-        - send local stderr as MSG_DATA_STDERR, unless in dom0
-        - send exit code
-        - wait just for local end
-        - return local exit code
-
-      is_service false (this is a client):
-        - send local data as MSG_DATA_STDIN
-        - stderr_fd is always -1
-        - don't send exit code
-        - wait for local and remote end
-        - return remote exit code
-     */
-    uint32_t is_service: 1;
-    uint32_t replace_chars_stdout: 1;
-    uint32_t replace_chars_stderr: 1;
-    uint32_t exit_on_stdout_eof: 1;
-    uint32_t exit_on_stdin_eof: 1;
-    uint32_t data_protocol_version: 16;
-    // if the struct ever exceeds 2KiB there is a major problem
-    uint32_t size: 11; // Size of the struct.  Fill in with
-                       // sizeof(struct qrexec_process_io_request).
-    // FD to /dev/null, might be -1.
-    int null_fd;
-    libvchan_t *vchan;
-    struct buffer *stdin_buf;
-    volatile sig_atomic_t *sigchld;
-    // can be NULL
-    volatile sig_atomic_t *sigusr1;
-    struct prefix_data prefix_data;
-    // New fields can be added at the end (only) and their use will be guarded
-    // by checks on `size`.
-};
-#pragma GCC diagnostic pop
-_Static_assert(offsetof(struct qrexec_process_io_request, vchan) -
-               offsetof(struct qrexec_process_io_request, local_pid) == 8 + sizeof(pid_t),
-               "alignment bug?");
-
 
 /*
  * Pass IO between vchan and local FDs. See the comments for
  * process_io_request.
  *
  * Returns intended exit code (local or remote), but calls exit() on errors.
  *
- * Deprecated, use qubes_qrexec_process_io_v2() instead.
+ * Deprecated, use qrexec_process_io() instead.
  */
-__attribute__((visibility("default")))
+__attribute__((visibility("default"), warn_unused_result))
 int process_io(const struct process_io_request *req);
+
 /*
  * Pass IO between vchan and local FDs. See the comments for
  * process_io_request.
  *
  * Returns intended exit code (local or remote), but calls exit() on errors.
+ *
+ * cmd may be NULL to use the default behavior.
  */
-int qubes_qrexec_process_io(const struct qrexec_process_io_request *req);
+__attribute__((visibility("default"), warn_unused_result))
+int qrexec_process_io(const struct process_io_request *req,
+                      const struct qrexec_parsed_command *cmd);
 
 // Logging
 
@@ -448,4 +410,15 @@ bool qubes_sendmsg_all(struct msghdr *msg, int sock);
 __attribute__((visibility("default")))
 int qubes_wait_for_vchan_connection_with_timeout(
         libvchan_t *conn, int wait_fd, bool is_server, time_t timeout);
+
+/**
+ * Determine if the fork server should be used, even though the fork server
+ * does not load service configuration.
+ *
+ * \param cmd The command to check.
+ * \return true if the command should be executed using the fork server,
+ *         false otherwise.
+ */
+__attribute__((visibility("default")))
+bool qrexec_cmd_use_fork_server(const struct qrexec_parsed_command *cmd);
 #endif /* LIBQREXEC_UTILS_H */

libqrexec/private.h

@@ -1,2 +1,7 @@
+#pragma once
 #include <stdbool.h>
-int qubes_toml_config_parse(const char *config_full_path, bool *wait_for_session, char **user, bool *skip_service_descriptor);
+int qubes_toml_config_parse(const char *config_full_path, bool *wait_for_session,
+                            char **user,
+                            bool *send_service_descriptor,
+                            bool *exit_on_stdout_eof,
+                            bool *exit_on_stdin_eof);

libqrexec/process_io.c

@@ -32,6 +32,7 @@
 
 #include "libqrexec-utils.h"
 #include "remote.h"
+#include "private.h"
 
 static _Noreturn void handle_vchan_error(const char *op)
 {
@@ -78,41 +79,24 @@ enum {
     FD_NUM
 };
 
-int process_io(const struct process_io_request *old_req) {
-    if (old_req->data_protocol_version < 0 || old_req->data_protocol_version > 65535) {
-        LOG(ERROR, "Bad data protocol version %d", old_req->data_protocol_version);
-        return 125;
-    }
-    struct qrexec_process_io_request new_req = {
-        .stdin_fd = old_req->stdin_fd,
-        .stdout_fd = old_req->stdout_fd,
-        .stderr_fd = old_req->stderr_fd,
-        .local_pid = old_req->local_pid,
-        .is_service = old_req->is_service,
-        .replace_chars_stdout = old_req->replace_chars_stdout,
-        .replace_chars_stderr = old_req->replace_chars_stderr,
-        .data_protocol_version = (uint16_t)old_req->data_protocol_version,
-        .size = sizeof(new_req),
-        .null_fd = -1,
-        .vchan = old_req->vchan,
-        .stdin_buf = old_req->stdin_buf,
-        .sigchld = old_req->sigchld,
-        .sigusr1 = old_req->sigusr1,
-        .prefix_data = old_req->prefix_data,
-    };
-    return qubes_qrexec_process_io(&new_req);
+int process_io(const struct process_io_request *req) {
+    return qrexec_process_io(req, NULL);
 }
 
-int qubes_qrexec_process_io(const struct qrexec_process_io_request *req) {
+int qrexec_process_io(const struct process_io_request *req,
+                      const struct qrexec_parsed_command *cmd) {
     libvchan_t *vchan = req->vchan;
     int stdin_fd = req->stdin_fd;
     int stdout_fd = req->stdout_fd;
     int stderr_fd = req->stderr_fd;
     struct buffer *stdin_buf = req->stdin_buf;
 
-    bool is_service = req->is_service;
+    bool const is_service = req->is_service;
+    assert(is_service == (cmd != NULL));
     bool replace_chars_stdout = req->replace_chars_stdout;
     bool replace_chars_stderr = req->replace_chars_stderr;
+    bool const exit_on_stdin_eof = cmd != NULL && cmd->exit_on_stdin_eof;
+    bool const exit_on_stdout_eof = cmd != NULL && cmd->exit_on_stdout_eof;
     const int data_protocol_version = req->data_protocol_version;
     const size_t max_chunk_size = max_data_chunk_size(data_protocol_version);
     pid_t local_pid = req->local_pid;
@@ -144,19 +128,41 @@ int qubes_qrexec_process_io(const struct qrexec_process_io_request *req) {
     set_nonblock(stdin_fd);
     if (stdout_fd != stdin_fd)
         set_nonblock(stdout_fd);
+    if (is_service && local_pid == 0) {
+        assert(stdin_fd == stdout_fd);
+        assert(stderr_fd == -1);
+    }
     if (stderr_fd >= 0) {
         assert(is_service); // if this is a client, stderr_fd is *always* -1
         set_nonblock(stderr_fd);
     }
+    if (exit_on_stdin_eof || exit_on_stdout_eof) {
+        assert(is_service); // only valid for socket services
+        assert(local_pid == 0); // ditto
+    }
 
     /* Convenience macros that eliminate a ton of error-prone boilerplate */
-#define close_stdin() do {                      \
-    close_stdio(stdin_fd, stdout_fd, SHUT_WR);  \
-    stdin_fd = -1;                              \
+#define close_stdin() do {                          \
+    if (exit_on_stdin_eof) {                        \
+        /* Set stdin_fd and stdout_fd to -1.        \
+         * No need to close them as the process     \
+         * will soon exit. */                       \
+        stdin_fd = stdout_fd = -1;                  \
+    } else {                                        \
+        close_stdio(stdin_fd, stdout_fd, SHUT_WR);  \
+        stdin_fd = -1;                              \
+    }                                               \
 } while (0)
-#define close_stdout() do {                     \
-    close_stdio(stdout_fd, stdin_fd, SHUT_RD);  \
-    stdout_fd = -1;                             \
+#define close_stdout() do {                         \
+    if (exit_on_stdout_eof) {                       \
+        /* Set stdin_fd and stdout_fd to -1.        \
+         * No need to close them as the process     \
+         * will soon exit. */                       \
+        stdin_fd = stdout_fd = -1;                  \
+    } else {                                        \
+        close_stdio(stdout_fd, stdin_fd, SHUT_RD);  \
+        stdout_fd = -1;                             \
+    }                                               \
 } while (0)
 #pragma GCC poison close_stdio