Working socket-based qrexec

This commit includes working socket-based qrexec.  The main
unimplemented feature is passing metadata about the connection to the
remote command.  This will be implemented in a future commit.

qrexec-client needed to be changed to use shutdown() instead of close().

Author: DemiMarie

agent/qrexec-agent-data.c

@@ -19,22 +19,29 @@
  *
  */
 
+#define _POSIX_C_SOURCE 200809L
+#define _GNU_SOURCE 1
+#include <limits.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <signal.h>
-#include <unistd.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <string.h>
+#include <stddef.h>
+#include <assert.h>
+#include <stdbool.h>
+
+#include <unistd.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
 #include <sys/select.h>
 #include <sys/socket.h>
+#include <sys/un.h>
 #include <fcntl.h>
 #include <libvchan.h>
-#include <assert.h>
-#include <limits.h>
 
 #include "qrexec.h"
 #include "libqrexec-utils.h"
@@ -146,14 +153,20 @@ static int handle_just_exec(char *cmdline)
 {
     int fdn, pid;
 
+    char *end_username = strchr(cmdline, ':');
+    if (!end_username) {
+        fprintf(stderr, "No colon in command from dom0\n");
+        return -1;
+    }
+    *end_username++ = '\0';
     switch (pid = fork()) {
         case -1:
             perror("fork");
             return -1;
         case 0:
             fdn = open("/dev/null", O_RDWR);
             fix_fds(fdn, fdn, fdn);
-            do_exec(cmdline);
+            do_exec(end_username, cmdline);
         default:;
     }
     fprintf(stderr, "executed (nowait) %s pid %d\n", cmdline, pid);
@@ -354,7 +367,7 @@ static int process_child_io(libvchan_t *data_vchan,
     fd_set rdset, wrset;
     int vchan_fd;
     sigset_t selectmask;
-    int child_process_status = child_process_pid ? -1 : 0;
+    int child_process_status = child_process_pid > 0 ? -1 : 0;
     int remote_process_status = -1;
     int ret, max_fd;
     struct timespec zero_timeout = { 0, 0 };
@@ -378,7 +391,7 @@ static int process_child_io(libvchan_t *data_vchan,
     while (1) {
         if (child_exited) {
             int status;
-            if (child_process_pid &&
+            if (child_process_pid > 0 &&
                     waitpid(child_process_pid, &status, WNOHANG) > 0) {
                 if (WIFSIGNALED(status))
                     child_process_status = 128 + WTERMSIG(status);
@@ -590,11 +603,6 @@ static int handle_new_process_common(int type, int connect_domain, int connect_p
     pid_t pid;
     int data_protocol_version;
 
-    if (type != MSG_SERVICE_CONNECT) {
-        assert(cmdline != NULL);
-        cmdline[cmdline_len-1] = 0;
-    }
-
     if (buffer_size == 0)
         buffer_size = VCHAN_BUFFER_SIZE;
 
@@ -604,6 +612,18 @@ static int handle_new_process_common(int type, int connect_domain, int connect_p
         if (data_vchan)
             libvchan_wait(data_vchan);
     } else {
+        if (cmdline == NULL) {
+            fputs("internal qrexec error: NULL cmdline passed to a non-MSG_SERVICE_CONNECT call\n", stderr);
+            abort();
+        } else if (cmdline_len == 0) {
+            fputs("internal qrexec error: zero-length command line passed to a non-MSG_SERVICE_CONNECT call\n", stderr);
+            abort();
+        } else if (cmdline_len > MAX_QREXEC_CMD_LEN) {
+            /* This is arbitrary, but it helps reduce the risk of overflows in other code */
+            fprintf(stderr, "Bad command from dom0: command line too long: length %zu\n", cmdline_len);
+            abort();
+        }
+        cmdline[cmdline_len-1] = 0;
         data_vchan = libvchan_client_init(connect_domain, connect_port);
     }
     if (!data_vchan) {
@@ -620,7 +640,8 @@ static int handle_new_process_common(int type, int connect_domain, int connect_p
             send_exit_code(data_vchan, handle_just_exec(cmdline));
             break;
         case MSG_EXEC_CMDLINE:
-            do_fork_exec(cmdline, &pid, &stdin_fd, &stdout_fd, &stderr_fd);
+            if (execute_qubes_rpc_command(cmdline, &pid, &stdin_fd, &stdout_fd, &stderr_fd, !qrexec_is_fork_server) < 0)
+                fputs("failed to spawn process\n", stderr);
             fprintf(stderr, "executed %s pid %d\n", cmdline, pid);
             child_process_pid = pid;
             exit_code = process_child_io(
@@ -664,8 +685,6 @@ pid_t handle_new_process(int type, int connect_domain, int connect_port,
             -1, -1, -1, 0);
 
     exit(exit_code);
-    /* suppress warning */
-    return 0;
 }
 
 /* Returns exit code of remote process */
@@ -680,3 +699,10 @@ int handle_data_client(int type, int connect_domain, int connect_port,
             NULL, 0, stdin_fd, stdout_fd, stderr_fd, buffer_size);
     return exit_code;
 }
+
+/* Local Variables: */
+/* mode: c */
+/* indent-tabs-mode: nil */
+/* c-basic-offset: 4 */
+/* coding: utf-8-unix */
+/* End: */

agent/qrexec-agent.c

@@ -26,8 +26,10 @@
 #include <sys/un.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <signal.h>
 #include <unistd.h>
+#include <stddef.h>
 #include <errno.h>
 #include <err.h>
 #include <sys/wait.h>
@@ -78,12 +80,7 @@ static int meminfo_write_started = 0;
 
 static void handle_server_exec_request_do(int type, int connect_domain, int connect_port, char *cmdline);
 
-static _Noreturn void no_colon_in_cmd(void)
-{
-    fprintf(stderr,
-            "cmdline is supposed to be in user:command form\n");
-    exit(1);
-}
+const bool qrexec_is_fork_server = false;
 
 #ifdef HAVE_PAM
 static int pam_conv_callback(int num_msg, const struct pam_message **msg,
@@ -118,7 +115,6 @@ static struct pam_conv conv = {
     NULL
 };
 #endif
-
 /* Start program requested by dom0 in already prepared process
  * (stdin/stdout/stderr already set, etc)
  * Called in two cases:
@@ -136,9 +132,8 @@ static struct pam_conv conv = {
  * If dom0 sends overly long cmd, it will probably crash qrexec-agent (unless
  * process can allocate up to 4GB on both stack and heap), sorry.
  */
-_Noreturn void do_exec(char *cmd)
+_Noreturn void do_exec(char *cmd, const char *user)
 {
-    char *realcmd = strchr(cmd, ':'), *user;
 #ifdef HAVE_PAM
     int retval, status;
     pam_handle_t *pamh=NULL;
@@ -151,14 +146,9 @@ _Noreturn void do_exec(char *cmd)
     char *shell_basename;
 #endif
 
-    if (!realcmd)
-        no_colon_in_cmd();
-    /* mark end of username and move to command */
-    user=strndup(cmd,(size_t)(realcmd-cmd));
-    realcmd++;
     /* ignore "nogui:" prefix in linux agent */
-    if (strncmp(realcmd, NOGUI_CMD_PREFIX, NOGUI_CMD_PREFIX_LEN) == 0)
-        realcmd += NOGUI_CMD_PREFIX_LEN;
+    if (strncmp(cmd, NOGUI_CMD_PREFIX, NOGUI_CMD_PREFIX_LEN) == 0)
+        cmd += NOGUI_CMD_PREFIX_LEN;
 
     signal(SIGCHLD, SIG_DFL);
     signal(SIGPIPE, SIG_DFL);
@@ -258,10 +248,10 @@ _Noreturn void do_exec(char *cmd)
                 warn("chdir(%s)", pw->pw_dir);
 
             /* call QUBESRPC if requested */
-            exec_qubes_rpc_if_requested(realcmd, env);
+            exec_qubes_rpc_if_requested(cmd, env);
 
             /* otherwise exec shell */
-            execle(pw->pw_shell, arg0, "-c", realcmd, (char*)NULL, env);
+            execle(pw->pw_shell, arg0, "-c", cmd, (char*)NULL, env);
             exit(127);
         default:
             /* parent */
@@ -296,10 +286,10 @@ _Noreturn void do_exec(char *cmd)
     exit(1);
 #else
     /* call QUBESRPC if requested */
-    exec_qubes_rpc_if_requested(realcmd, environ);
+    exec_qubes_rpc_if_requested(cmd, environ);
 
     /* otherwise exec shell */
-    execl("/bin/su", "su", "-", user, "-c", realcmd, NULL);
+    execl("/bin/su", "su", "-", user, "-c", cmd, NULL);
     perror("execl");
     exit(1);
 #endif
@@ -380,7 +370,7 @@ static void wake_meminfo_writer(void)
         /* wake meminfo-writer only once */
         return;
 
-    f = fopen(MEMINFO_WRITER_PIDFILE, "r");
+    f = fopen(MEMINFO_WRITER_PIDFILE, "re");
     if (f == NULL) {
         /* no meminfo-writer found, ignoring */
         return;
@@ -407,10 +397,10 @@ static int try_fork_server(int type, int connect_domain, int connect_port,
         char *cmdline, size_t cmdline_len) {
     char *colon;
     char *fork_server_socket_path;
-    int s;
+    int s = -1;
     struct sockaddr_un remote;
     struct qrexec_cmd_info info;
-    if (cmdline_len > (1ULL << 20))
+    if (cmdline_len > MAX_QREXEC_CMD_LEN)
         return -1;
     char *username = malloc(cmdline_len);
     if (!username) {
@@ -420,16 +410,14 @@ static int try_fork_server(int type, int connect_domain, int connect_port,
     memcpy(username, cmdline, cmdline_len);
     colon = strchr(username, ':');
     if (!colon)
-        return -1;
+        goto fail;
     *colon = '\0';
 
     if (asprintf(&fork_server_socket_path, QREXEC_FORK_SERVER_SOCKET, username) < 0) {
         fprintf(stderr, "Memory allocation failed\n");
-        free(username);
-        return -1;
+        goto fail;
     }
 
-    _Static_assert(sizeof(remote.sun_path) > 64, "bad size of sun_path");
     remote.sun_path[sizeof(remote.sun_path) - 1] = '\0';
     remote.sun_family = AF_UNIX;
     strncpy(remote.sun_path, fork_server_socket_path,
@@ -438,42 +426,39 @@ static int try_fork_server(int type, int connect_domain, int connect_port,
 
     if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
         perror("socket");
-        free(username);
-        return -1;
+        goto fail;
     }
     size_t len = strlen(remote.sun_path) + sizeof(remote.sun_family);
     if (connect(s, (struct sockaddr *) &remote, (socklen_t)len) == -1) {
         if (errno != ECONNREFUSED && errno != ENOENT)
             perror("connect");
-        close(s);
-        free(username);
-        return -1;
+        goto fail;
     }
 
     memset(&info, 0, sizeof info);
     info.type = type;
     info.connect_domain = connect_domain;
     info.connect_port = connect_port;
     size_t username_len = strlen(username);
-    assert(username_len < SIZE_MAX);
     assert(cmdline_len <= INT_MAX);
     assert(cmdline_len > username_len);
     info.cmdline_len = (int)(cmdline_len - (username_len + 1));
     if (!write_all(s, &info, sizeof(info))) {
         perror("write");
-        close(s);
-        free(username);
-        return -1;
+        goto fail;
     }
-    free(username);
-    username = NULL;
     if (!write_all(s, colon+1, info.cmdline_len)) {
         perror("write");
-        close(s);
-        return -1;
+        goto fail;
     }
 
+    free(username);
     return s;
+fail:
+    if (s >= 0)
+        close(s);
+    free(username);
+    return -1;
 }
 
 

agent/qrexec-agent.h

@@ -29,15 +29,19 @@
 // support only very small configuration files,
 #define MAX_CONFIG_SIZE 4096
 
+#include <stdbool.h>
 int handle_handshake(libvchan_t *ctrl);
 void handle_vchan_error(const char *op);
-_Noreturn void do_exec(char *cmd);
+_Noreturn void do_exec(char *cmd, const char *user);
 /* call before fork() for service handling process (either end) */
 void prepare_child_env(void);
 
 // whether qrexec-client should replace problematic bytes with _ before printing the output
 extern int replace_chars_stdout;
 extern int replace_chars_stderr;
+#include <stdbool.h>
+/* true in qrexec-fork-server, false in qrexec-agent */
+extern const bool qrexec_is_fork_server;
 
 pid_t handle_new_process(int type,
         int connect_domain, int connect_port,

agent/qrexec-client-vm.c

@@ -31,15 +31,17 @@
 #include "qrexec.h"
 #include "qrexec-agent.h"
 
+const bool qrexec_is_fork_server = false;
+
 _Noreturn void handle_vchan_error(const char *op)
 {
     fprintf(stderr, "Error while vchan %s, exiting\n", op);
     exit(1);
 }
 
-void do_exec(char *const cmd __attribute__((__unused__))) {
+_Noreturn void do_exec(char *cmd __attribute__((unused)), char const* user __attribute__((__unused__))) {
     fprintf(stderr, "BUG: do_exec function shouldn't be called!\n");
-    exit(1);
+    abort();
 }
 
 static int connect_unix_socket(char *path)

agent/qrexec-fork-server.c

@@ -34,8 +34,9 @@
 #include "qrexec-agent.h"
 
 extern char **environ;
+const bool qrexec_is_fork_server = true;
 
-void do_exec(char *cmd)
+void do_exec(char *cmd, const char *user __attribute__((unused)))
 {
     char *shell;