qrexec: handle data vchan directly from qrexec-client-vm

This way qrexec-client-vm will have much more information, at least:
 - will know whether the service call was accepted or refused
 - potentially will know remote process exit code
This commit implements the first point - the local process will not be
started if service call was refused.

qrexec/Makefile

@@ -6,7 +6,7 @@ LDLIBS=`pkg-config --libs vchan-$(BACKEND_VMM)` -lqrexec-utils
 all: qrexec-agent qrexec-client-vm qrexec-fork-server
 qrexec-agent: qrexec-agent.o qrexec-agent-data.o
 qrexec-fork-server: qrexec-fork-server.o qrexec-agent-data.o
-qrexec-client-vm: qrexec-client-vm.o
+qrexec-client-vm: qrexec-client-vm.o qrexec-agent-data.o
 clean:
 	rm -f *.o *~ qrexec-agent qrexec-client-vm
 

qrexec/qrexec-agent-data.c

@@ -32,6 +32,7 @@
 #include <sys/socket.h>
 #include <fcntl.h>
 #include <libvchan.h>
+#include <assert.h>
 #include "qrexec.h"
 #include "libqrexec-utils.h"
 #include "qrexec-agent.h"
@@ -42,6 +43,7 @@ static volatile int child_exited;
 static volatile int stdio_socket_requested;
 int stdout_msg_type = MSG_DATA_STDOUT;
 pid_t child_process_pid;
+int remote_process_status = 0;
 
 static void sigchld_handler(int __attribute__((__unused__))x)
 {
@@ -386,45 +388,37 @@ void process_child_io(libvchan_t *data_vchan,
                 stdout_fd = -1;
                 close(stderr_fd);
                 stderr_fd = -1;
+                /* we do not care for any local process */
+                return;
                 break;
         }
     }
 }
 
-pid_t handle_new_process(int type, int connect_domain, int connect_port,
-        char *cmdline, int cmdline_len)
+/* Behaviour depends on type parameter:
+ *  MSG_SERVICE_CONNECT - create vchan server, pass the data to/from given FDs
+ *    (stdin_fd, stdout_fd, stderr_fd), then return 0
+ *  MSG_JUST_EXEC - connect to vchan server, fork+exec process given by cmdline
+ *    parameter, send artificial exit code "0" (local process can still be
+ *    running), then return 0
+ *  MSG_EXEC_CMDLINE - connect to vchan server, fork+exec process given by
+ *    cmdline parameter, pass the data to/from that process, then return local
+ *    process exit code
+ */
+int handle_new_process_common(int type, int connect_domain, int connect_port,
+                char *cmdline, int cmdline_len, /* MSG_JUST_EXEC and MSG_EXEC_CMDLINE */
+                int stdin_fd, int stdout_fd, int stderr_fd /* MSG_SERVICE_CONNECT */)
 {
-    struct service_params *svc_params = (struct service_params*)cmdline;
     libvchan_t *data_vchan;
+    int exit_code = 0;
     pid_t pid;
-    int stdin_fd, stdout_fd, stderr_fd;
     char pid_s[10];
 
-    if (type == MSG_SERVICE_CONNECT) {
-        if (cmdline_len != sizeof(*svc_params)) {
-            fprintf(stderr, "Invalid MSG_SERVICE_CONNECT packet (cmdline len %d)\n", cmdline_len);
-            return -1;
-        }
-        sscanf(cmdline, "%d %d %d", &stdin_fd, &stdout_fd, &stderr_fd);
+    if (type != MSG_SERVICE_CONNECT) {
+        assert(cmdline != NULL);
+        cmdline[cmdline_len-1] = 0;
     }
 
-    switch (pid=fork()){
-        case -1:
-            perror("fork");
-            return -1;
-        case 0:
-            break;
-        default:
-            if (type == MSG_SERVICE_CONNECT) {
-                /* no longer needed in parent process */
-                close(stdin_fd);
-                close(stdout_fd);
-                close(stderr_fd);
-            }
-            return pid;
-    }
-
-    /* child process */
     if (type == MSG_SERVICE_CONNECT) {
         data_vchan = libvchan_server_init(connect_domain, connect_port,
                 VCHAN_BUFFER_SIZE, VCHAN_BUFFER_SIZE);
@@ -448,13 +442,13 @@ pid_t handle_new_process(int type, int connect_domain, int connect_port,
     switch (type) {
         case MSG_JUST_EXEC:
             send_exit_code(data_vchan, handle_just_exec(cmdline));
-            libvchan_close(data_vchan);
             break;
         case MSG_EXEC_CMDLINE:
             do_fork_exec(cmdline, &pid, &stdin_fd, &stdout_fd, &stderr_fd);
             fprintf(stderr, "executed %s pid %d\n", cmdline, pid);
             child_process_pid = pid;
-            process_child_io(data_vchan, stdin_fd, stdout_fd, stderr_fd);
+            exit_code = process_child_io(data_vchan, stdin_fd, stdout_fd, stderr_fd);
+            fprintf(stderr, "pid %d exited with %d\n", pid, exit_code);
             break;
         case MSG_SERVICE_CONNECT:
             child_process_pid = 0;
@@ -462,7 +456,44 @@ pid_t handle_new_process(int type, int connect_domain, int connect_port,
             process_child_io(data_vchan, stdin_fd, stdout_fd, stderr_fd);
             break;
     }
-    exit(0);
+    libvchan_close(data_vchan);
+    return exit_code;
+}
+
+/* Returns PID of data processing process */
+pid_t handle_new_process(int type, int connect_domain, int connect_port,
+        char *cmdline, int cmdline_len)
+{
+    int exit_code;
+    pid_t pid;
+    assert(type != MSG_SERVICE_CONNECT);
+
+    switch (pid=fork()){
+        case -1:
+            perror("fork");
+            return -1;
+        case 0:
+            break;
+        default:
+            return pid;
+    }
+
+    /* child process */
+    exit_code = handle_new_process_common(type, connect_domain, connect_port,
+            cmdline, cmdline_len,
+            -1, -1, -1);
+
+    exit(exit_code);
     /* suppress warning */
     return 0;
 }
+
+void handle_data_client(int type, int connect_domain, int connect_port,
+                int stdin_fd, int stdout_fd, int stderr_fd)
+{
+
+    assert(type == MSG_SERVICE_CONNECT);
+
+    handle_new_process_common(type, connect_domain, connect_port,
+            NULL, 0, stdin_fd, stdout_fd, stderr_fd);
+}

qrexec/qrexec-agent.c

@@ -55,7 +55,6 @@ struct _connection_info connection_info[MAX_FDS];
 libvchan_t *ctrl_vchan;
 
 int trigger_fd;
-int passfd_socket;
 
 int meminfo_write_started = 0;
 
@@ -107,11 +106,8 @@ void init()
     if (handle_handshake(ctrl_vchan) < 0)
         exit(1);
     umask(0);
-    mkfifo(QREXEC_AGENT_TRIGGER_PATH, 0666);
-    passfd_socket = get_server_socket(QREXEC_AGENT_FDPASS_PATH);
+    trigger_fd = get_server_socket(QREXEC_AGENT_TRIGGER_PATH);
     umask(077);
-    trigger_fd =
-        open(QREXEC_AGENT_TRIGGER_PATH, O_RDONLY | O_NONBLOCK);
     register_exec_func(do_exec);
 
     /* wait for qrexec daemon */
@@ -229,6 +225,7 @@ void handle_server_exec_request(struct msg_header *hdr)
     struct exec_params params;
     char buf[hdr->len-sizeof(params)];
     pid_t child_agent;
+    int client_fd;
 
     assert(hdr->len >= sizeof(params));
 
@@ -248,7 +245,24 @@ void handle_server_exec_request(struct msg_header *hdr)
             return;
         }
     }
+    if (hdr->type == MSG_SERVICE_CONNECT && sscanf(buf, "SOCKET%d", &client_fd)) {
+        /* FIXME: Maybe add some check if client_fd is really FD to some
+         * qrexec-client-vm process; but this data comes from qrexec-daemon
+         * (which sends back what it got from us earlier), so it isn't critical.
+         */
+        write(client_fd, &params, sizeof(params));
+        /* No need to send request_id (buf) - the client don't need it, there
+         * is only meaningless (for the client) socket FD */
+        /* Register connection even if there was an error sending params to
+         * qrexec-client-vm. This way the mainloop will clean the things up
+         * (close socket, send MSG_CONNECTION_TERMINATED) when qrexec-client-vm
+         * will close the socket (terminate itself). */
+        register_vchan_connection(-1, client_fd,
+                params.connect_domain, params.connect_port);
+        return;
+    }
 
+    /* No fork server case */
     child_agent = handle_new_process(hdr->type,
             params.connect_domain, params.connect_port,
             buf, hdr->len-sizeof(params));
@@ -260,7 +274,7 @@ void handle_server_exec_request(struct msg_header *hdr)
 void handle_service_refused(struct msg_header *hdr)
 {
     struct service_params params;
-    int stdin_fd, stdout_fd, stderr_fd;
+    int socket_fd;
 
     if (hdr->len != sizeof(params)) {
         fprintf(stderr, "Invalid msg 0x%x length (%d)\n", MSG_SERVICE_REFUSED, hdr->len);
@@ -270,11 +284,10 @@ void handle_service_refused(struct msg_header *hdr)
     if (libvchan_recv(ctrl_vchan, &params, sizeof(params)) < 0)
         handle_vchan_error("read exec params");
 
-    sscanf(params.ident, "%d %d %d", &stdin_fd, &stdout_fd, &stderr_fd);
-    /* TODO: send some signal? some response? */
-    close(stdin_fd);
-    close(stdout_fd);
-    close(stderr_fd);
+    if (sscanf(params.ident, "SOCKET%d", &socket_fd))
+        close(socket_fd);
+    else
+        fprintf(stderr, "Received REFUSED for unknown service request '%s'\n", params.ident);
 }
 
 void handle_server_cmd()
@@ -360,9 +373,6 @@ int fill_fds_for_select(fd_set * rdset, fd_set * wrset)
     FD_SET(trigger_fd, rdset);
     if (trigger_fd > max)
         max = trigger_fd;
-    FD_SET(passfd_socket, rdset);
-    if (passfd_socket > max)
-        max = passfd_socket;
 
     for (i = 0; i < MAX_FDS; i++) {
         if (connection_info[i].pid != 0 && connection_info[i].fd != -1) {
@@ -374,41 +384,31 @@ int fill_fds_for_select(fd_set * rdset, fd_set * wrset)
     return max;
 }
 
-void handle_new_passfd()
-{
-    int fd = do_accept(passfd_socket);
-    if (fd >= MAX_FDS) {
-        fprintf(stderr, "too many clients ?\n");
-        exit(1);
-    }
-    // let client know what fd has been allocated
-    if (write(fd, &fd, sizeof(fd)) != sizeof(fd)) {
-        perror("write to client");
-    }
-}
-
 void handle_trigger_io()
 {
     struct msg_header hdr;
     struct trigger_service_params params;
     int ret;
+    int client_fd;
 
+    client_fd = do_accept(trigger_fd);
+    if (client_fd < 0)
+        return;
     hdr.len = sizeof(params);
-    ret = read(trigger_fd, &params, sizeof(params));
+    ret = read(client_fd, &params, sizeof(params));
     if (ret == sizeof(params)) {
         hdr.type = MSG_TRIGGER_SERVICE;
+        snprintf(params.request_id.ident, sizeof(params.request_id), "SOCKET%d", client_fd);
         if (libvchan_send(ctrl_vchan, &hdr, sizeof(hdr)) < 0)
             handle_vchan_error("write hdr");
         if (libvchan_send(ctrl_vchan, &params, sizeof(params)) < 0)
             handle_vchan_error("write params");
     }
-    // trigger_fd is nonblock - so no need to reopen
-    // not really, need to reopen at EOF
     if (ret <= 0) {
-        close(trigger_fd);
-        trigger_fd =
-            open(QREXEC_AGENT_TRIGGER_PATH, O_RDONLY | O_NONBLOCK);
+        close(client_fd);
     }
+    /* do not close client_fd - we'll need it to send the connection details
+     * later (when dom0 accepts the request) */
 }
 
 void handle_terminated_fork_client(fd_set *rdset) {
@@ -454,9 +454,6 @@ int main()
         wait_for_vchan_or_argfd(ctrl_vchan, max, &rdset, &wrset);
         sigprocmask(SIG_UNBLOCK, &chld_set, NULL);
 
-        if (FD_ISSET(passfd_socket, &rdset))
-            handle_new_passfd();
-
         while (libvchan_data_ready(ctrl_vchan))
             handle_server_cmd();
 

qrexec/qrexec-agent.h

@@ -28,6 +28,10 @@ void do_exec(const char *cmd);
 pid_t handle_new_process(int type,
         int connect_domain, int connect_port,
         char *cmdline, int cmdline_len);
+void handle_data_client(int type,
+        int connect_domain, int connect_port,
+        int stdin_fd, int stdout_fd, int stderr_fd);
+
 
 struct qrexec_cmd_info {
 	int type;

qrexec/qrexec-client-vm.c

@@ -20,14 +20,29 @@
  */
 #define _GNU_SOURCE
 #include <sys/socket.h>
+#include <sys/wait.h>
 #include <sys/un.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <string.h>
+#include "libqrexec-utils.h"
 #include "qrexec.h"
-int connect_unix_socket()
+#include "qrexec-agent.h"
+
+void handle_vchan_error(const char *op)
+{
+    fprintf(stderr, "Error while vchan %s, exiting\n", op);
+    exit(1);
+}
+
+void do_exec(const char *cmd __attribute__((__unused__))) {
+    fprintf(stderr, "BUG: do_exec function shouldn't be called!\n");
+    exit(1);
+}
+
+int connect_unix_socket(char *path)
 {
     int s, len;
     struct sockaddr_un remote;
@@ -38,7 +53,7 @@ int connect_unix_socket()
     }
 
     remote.sun_family = AF_UNIX;
-    strncpy(remote.sun_path, QREXEC_AGENT_FDPASS_PATH,
+    strncpy(remote.sun_path, path,
             sizeof(remote.sun_path));
     len = strlen(remote.sun_path) + sizeof(remote.sun_family);
     if (connect(s, (struct sockaddr *) &remote, len) == -1) {
@@ -61,9 +76,12 @@ int main(int argc, char **argv)
 {
     int trigger_fd;
     struct trigger_service_params params;
-    int local_fd[3], remote_fd[3];
-    int i;
+    struct exec_params exec_params;
+    int ret, i;
     char *abs_exec_path;
+    pid_t child_pid;
+    int inpipe[2], outpipe[2];
+    char pid_s[10];
 
     if (argc < 4) {
         fprintf(stderr,
@@ -72,50 +90,76 @@ int main(int argc, char **argv)
         exit(1);
     }
 
-    trigger_fd = open(QREXEC_AGENT_TRIGGER_PATH, O_WRONLY);
-    if (trigger_fd < 0) {
-        perror("open " QREXEC_AGENT_TRIGGER_PATH);
-        exit(1);
-    }
-
-    for (i = 0; i < 3; i++) {
-        local_fd[i] = connect_unix_socket();
-        if (read(local_fd[i], &remote_fd[i], sizeof(remote_fd[i])) != sizeof(remote_fd[i])) {
-            perror("read client fd");
-            exit(1);
-        }
-        if (i != 2 || getenv("PASS_LOCAL_STDERR")) {
-            char *env;
-            if (asprintf(&env, "SAVED_FD_%d=%d", i, dup(i)) < 0) {
-                perror("prepare SAVED_FD_");
-                exit(1);
-            }
-            putenv(env);
-            dup2(local_fd[i], i);
-            close(local_fd[i]);
-        } else
-            close(local_fd[i]);
-    }
+    trigger_fd = connect_unix_socket(QREXEC_AGENT_TRIGGER_PATH);
 
     memset(&params, 0, sizeof(params));
     strncpy(params.service_name, argv[2], sizeof(params.service_name));
     strncpy(params.target_domain, argv[1],
             sizeof(params.target_domain));
     snprintf(params.request_id.ident,
-            sizeof(params.request_id.ident), "%d %d %d",
-            remote_fd[0], remote_fd[1], remote_fd[2]);
+            sizeof(params.request_id.ident), "SOCKET");
 
     if (write(trigger_fd, &params, sizeof(params)) < 0) {
-        if (!getenv("PASS_LOCAL_STDERR"))
-            perror("write to agent");
+        perror("write to agent");
+        exit(1);
+    }
+    ret = read(trigger_fd, &exec_params, sizeof(exec_params));
+    if (ret == 0) {
+        fprintf(stderr, "Request refused\n");
+        exit(1);
+    }
+    if (ret < 0 || ret != sizeof(exec_params)) {
+        perror("read");
         exit(1);
     }
 
+    if (socketpair(AF_UNIX, SOCK_STREAM, 0, inpipe) ||
+            socketpair(AF_UNIX, SOCK_STREAM, 0, outpipe)) {
+        perror("socketpair");
+        exit(1);
+    }
+    snprintf(pid_s, sizeof(pid_s), "%d", getpid());
+    setenv("QREXEC_AGENT_PID", pid_s, 1);
+
+    switch (child_pid = fork()) {
+    case -1:
+        perror("fork");
+        exit(-1);
+    case 0:
+        close(inpipe[1]);
+        close(outpipe[0]);
+        close(trigger_fd);
+        for (i = 0; i < 3; i++) {
+            if (i != 2 || getenv("PASS_LOCAL_STDERR")) {
+                char *env;
+                if (asprintf(&env, "SAVED_FD_%d=%d", i, dup(i)) < 0) {
+                    perror("prepare SAVED_FD_");
+                    exit(1);
+                }
+                putenv(env);
+            }
+        }
+
+        dup2(inpipe[0], 0);
+        dup2(outpipe[1], 1);
+        close(inpipe[0]);
+        close(outpipe[1]);
+
+        abs_exec_path = strdup(argv[3]);
+        argv[3] = get_program_name(argv[3]);
+        execv(abs_exec_path, argv + 3);
+        perror("execv");
+        exit(-1);
+    }
+    close(inpipe[0]);
+    close(outpipe[1]);
+
+    handle_data_client(MSG_SERVICE_CONNECT,
+            exec_params.connect_domain, exec_params.connect_port,
+            inpipe[1], outpipe[0], -1);
+
     close(trigger_fd);
+    waitpid(child_pid, &i, 0);
 
-    abs_exec_path = strdup(argv[3]);
-    argv[3] = get_program_name(argv[3]);
-    execv(abs_exec_path, argv + 3);
-    perror("execv");
-    return 1;
+    return 0;
 }