Skip to content

Commit

Permalink
Avoid qrexec-client for VM -> VM calls
Browse files Browse the repository at this point in the history
This saves an execve().  It also allows qrexec-daemon to change how it
performs a VM -> VM service call without having to change qrexec-client.
During an upgrade, it is possible that qrexec-daemon is newer than
qrexec-client, causing the new qrexec-daemon to try to use a calling
convention that the old qrexec-client doesn't support.  Doing VM -> VM
calls without calling execve() means this cannot happen.  VM -> dom0
and dom0 -> VM calls still use qrexec-client, but VM -> dom0 calls are
safe from domain name reuse races as of [1], and the latter do not
involve qrexec-daemon at all.

[1]: 100fbb9 ("qrexec-client: Use XID to connect to qrexec daemon when possible")
  • Loading branch information
DemiMarie committed Feb 14, 2024
1 parent 5f41e36 commit 122466c
Show file tree
Hide file tree
Showing 6 changed files with 227 additions and 166 deletions.
7 changes: 4 additions & 3 deletions daemon/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ override QUBES_CFLAGS:=-I../libqrexec -g -O2 -Wall -Wextra -Werror -fPIC \
$(shell pkg-config --cflags $(VCHAN_PKG)) -fstack-protector \
-D_FORTIFY_SOURCE=2 -fstack-protector-strong -std=gnu11 -D_POSIX_C_SOURCE=200809L \
-D_GNU_SOURCE $(CFLAGS) \
-Wstrict-prototypes -Wold-style-definition -Wmissing-declarations
-Wstrict-prototypes -Wold-style-definition -Wmissing-declarations \
-fvisibility=hidden
override LDFLAGS += -pie -Wl,-z,relro,-z,now -L../libqrexec
override LDLIBS += $(shell pkg-config --libs $(VCHAN_PKG)) -lqrexec-utils

Expand All @@ -22,8 +23,8 @@ install: all
ln -sf ../../bin/qrexec-client $(DESTDIR)/usr/lib/qubes/qrexec-client
.PHONY: all clean install

qrexec-daemon qrexec-client: %: %.o
$(CC) $(LDFLAGS) -pie -g -o $@ $< $(LDLIBS)
qrexec-daemon qrexec-client: %: %.o qrexec-daemon-common.o
$(CC) $(LDFLAGS) -pie -g -o $@ $^ $(LDLIBS)

%.o: %.c
$(CC) $< -c -o $@ $(QUBES_CFLAGS) -MD -MP -MF $@.dep
Expand Down
156 changes: 12 additions & 144 deletions daemon/qrexec-client.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <time.h>

#include "libqrexec-utils.h"
#include "qrexec-daemon-common.h"

// whether qrexec-client should replace problematic bytes with _ before printing the output
static bool replace_chars_stdout = false;
Expand All @@ -58,8 +59,6 @@ static bool is_service = false;

static volatile sig_atomic_t sigchld = 0;

static const char *socket_dir = QREXEC_DAEMON_SOCKET_DIR;

extern char **environ;

static char *xstrdup(const char *arg) {
Expand Down Expand Up @@ -128,71 +127,6 @@ static int handle_agent_handshake(libvchan_t *vchan, bool remote_send_first)
return data_protocol_version;
}

static int handle_daemon_handshake(int fd)
{
struct msg_header hdr;
struct peer_info info;

/* daemon send MSG_HELLO first */
if (!read_all(fd, &hdr, sizeof(hdr))) {
PERROR("daemon handshake");
return -1;
}
if (hdr.type != MSG_HELLO || hdr.len != sizeof(info)) {
LOG(ERROR, "Invalid daemon MSG_HELLO");
return -1;
}
if (!read_all(fd, &info, sizeof(info))) {
PERROR("daemon handshake");
return -1;
}

if (info.version != QREXEC_PROTOCOL_VERSION) {
LOG(ERROR, "Incompatible daemon protocol version "
"(daemon %d, client %d)",
info.version, QREXEC_PROTOCOL_VERSION);
return -1;
}

hdr.type = MSG_HELLO;
hdr.len = sizeof(info);
info.version = QREXEC_PROTOCOL_VERSION;

if (!write_all(fd, &hdr, sizeof(hdr))) {
LOG(ERROR, "Failed to send MSG_HELLO hdr to daemon");
return -1;
}
if (!write_all(fd, &info, sizeof(info))) {
LOG(ERROR, "Failed to send MSG_HELLO to daemon");
return -1;
}
return 0;
}

static int connect_unix_socket(const char *domname)
{
int s, len, res;
struct sockaddr_un remote;

if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
err(1, "socket");

remote.sun_family = AF_UNIX;
res = snprintf(remote.sun_path, sizeof remote.sun_path,
"%s/qrexec.%s", socket_dir, domname);
if (res < 0)
err(1, "snprintf");
if (res >= (int)sizeof(remote.sun_path))
errx(1, "%s/qrexec.%s is too long for AF_UNIX socket path",
socket_dir, domname);
len = (size_t)res + 1 + offsetof(struct sockaddr_un, sun_path);
if (connect(s, (struct sockaddr *) &remote, len) == -1)
err(1, "connect %s", remote.sun_path);
if (handle_daemon_handshake(s) < 0)
exit(1);
return s;
}

static void sigchld_handler(int x __attribute__((__unused__)))
{
sigchld = 1;
Expand Down Expand Up @@ -299,66 +233,6 @@ static _Noreturn void handle_failed_exec(libvchan_t *data_vchan)
}
exit(exit_code);
}

/* ask the daemon to allocate vchan port */
static void negotiate_connection_params(int s, int other_domid, unsigned type,
void *cmdline_param, int cmdline_size,
int *data_domain, int *data_port)
{
struct msg_header hdr;
struct exec_params params;
hdr.type = type;
hdr.len = sizeof(params) + cmdline_size;
params.connect_domain = other_domid;
params.connect_port = 0;
if (!write_all(s, &hdr, sizeof(hdr))
|| !write_all(s, &params, sizeof(params))
|| !write_all(s, cmdline_param, cmdline_size)) {
PERROR("write daemon");
exit(1);
}
/* the daemon will respond with the same message with connect_port filled
* and empty cmdline */
if (!read_all(s, &hdr, sizeof(hdr))) {
PERROR("read daemon");
exit(1);
}
assert(hdr.type == type);
if (hdr.len != sizeof(params)) {
LOG(ERROR, "Invalid response for 0x%x", type);
exit(1);
}
if (!read_all(s, &params, sizeof(params))) {
PERROR("read daemon");
exit(1);
}
*data_port = params.connect_port;
*data_domain = params.connect_domain;
}

static void send_service_connect(int s, char *conn_ident,
int connect_domain, int connect_port)
{
struct msg_header hdr;
struct exec_params exec_params;
struct service_params srv_params;

hdr.type = MSG_SERVICE_CONNECT;
hdr.len = sizeof(exec_params) + sizeof(srv_params);

exec_params.connect_domain = connect_domain;
exec_params.connect_port = connect_port;
strncpy(srv_params.ident, conn_ident, sizeof(srv_params.ident) - 1);
srv_params.ident[sizeof(srv_params.ident) - 1] = '\0';

if (!write_all(s, &hdr, sizeof(hdr))
|| !write_all(s, &exec_params, sizeof(exec_params))
|| !write_all(s, &srv_params, sizeof(srv_params))) {
PERROR("write daemon");
exit(1);
}
}

static void select_loop(libvchan_t *vchan, int data_protocol_version, struct buffer *stdin_buf)
{
struct process_io_request req = { 0 };
Expand Down Expand Up @@ -547,7 +421,7 @@ int main(int argc, char **argv)
int data_domain;
int s;
bool just_exec = false;
int wait_connection_end = 0;
int wait_connection_end = -1;
char *local_cmdline = NULL;
char *remote_cmdline = NULL;
char *request_id = NULL;
Expand Down Expand Up @@ -614,13 +488,6 @@ int main(int argc, char **argv)
usage(argv[0]);
}

char src_domain_id_str[11];
{
int snprintf_res = snprintf(src_domain_id_str, sizeof(src_domain_id_str), "%d", src_domain_id);
if (snprintf_res < 0 || snprintf_res >= (int)sizeof(src_domain_id_str))
err(1, "snprintf()");
}

if (strcmp(domname, "dom0") == 0 || strcmp(domname, "@adminvm") == 0) {
if (request_id == NULL) {
fprintf(stderr, "ERROR: when target domain is 'dom0', -c must be specified\n");
Expand All @@ -633,7 +500,7 @@ int main(int argc, char **argv)
abort();
}
set_remote_domain(src_domain_name);
s = connect_unix_socket(src_domain_id_str);
s = connect_unix_socket_by_id(src_domain_id);
negotiate_connection_params(s,
0, /* dom0 */
MSG_SERVICE_CONNECT,
Expand Down Expand Up @@ -665,17 +532,18 @@ int main(int argc, char **argv)
compute_service_length(remote_cmdline, argv[0]),
&data_domain,
&data_port);
if (wait_connection_end && request_id)
/* save socket fd, 's' will be reused for the other qrexec-daemon
* connection */
wait_connection_end = s;
else
close(s);
if (request_id) {
s = connect_unix_socket(src_domain_id_str);
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);
send_service_connect(s, request_id, data_domain, data_port);
close(s);
if (wait_connection_end) {
if (wait_connection_end != -1) {
/* wait for EOF */
struct pollfd fds[1] = {
{ .fd = wait_connection_end, .events = POLLIN | POLLHUP, .revents = 0 },
Expand Down
145 changes: 145 additions & 0 deletions daemon/qrexec-daemon-common.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
#include <stdlib.h>
#include <assert.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <stdio.h>
#include <err.h>

#include "qrexec.h"
#include "libqrexec-utils.h"
#include "qrexec-daemon-common.h"

const char *socket_dir = QREXEC_DAEMON_SOCKET_DIR;

/* ask the daemon to allocate vchan port */
void negotiate_connection_params(int s, int other_domid, unsigned type,
void *cmdline_param, int cmdline_size,
int *data_domain, int *data_port)
{
struct msg_header hdr;
struct exec_params params;
hdr.type = type;
hdr.len = sizeof(params) + cmdline_size;
params.connect_domain = other_domid;
params.connect_port = 0;
if (!write_all(s, &hdr, sizeof(hdr))
|| !write_all(s, &params, sizeof(params))
|| !write_all(s, cmdline_param, cmdline_size)) {
PERROR("write daemon");
exit(1);
}
/* the daemon will respond with the same message with connect_port filled
* and empty cmdline */
if (!read_all(s, &hdr, sizeof(hdr))) {
PERROR("read daemon");
exit(1);
}
assert(hdr.type == type);
if (hdr.len != sizeof(params)) {
LOG(ERROR, "Invalid response for 0x%x", type);
exit(1);
}
if (!read_all(s, &params, sizeof(params))) {
PERROR("read daemon");
exit(1);
}
*data_port = params.connect_port;
*data_domain = params.connect_domain;
}

int handle_daemon_handshake(int fd)
{
struct msg_header hdr;
struct peer_info info;

/* daemon send MSG_HELLO first */
if (!read_all(fd, &hdr, sizeof(hdr))) {
PERROR("daemon handshake");
return -1;
}
if (hdr.type != MSG_HELLO || hdr.len != sizeof(info)) {
LOG(ERROR, "Invalid daemon MSG_HELLO");
return -1;
}
if (!read_all(fd, &info, sizeof(info))) {
PERROR("daemon handshake");
return -1;
}

if (info.version != QREXEC_PROTOCOL_VERSION) {
LOG(ERROR, "Incompatible daemon protocol version "
"(daemon %d, client %d)",
info.version, QREXEC_PROTOCOL_VERSION);
return -1;
}

hdr.type = MSG_HELLO;
hdr.len = sizeof(info);
info.version = QREXEC_PROTOCOL_VERSION;

if (!write_all(fd, &hdr, sizeof(hdr))) {
LOG(ERROR, "Failed to send MSG_HELLO hdr to daemon");
return -1;
}
if (!write_all(fd, &info, sizeof(info))) {
LOG(ERROR, "Failed to send MSG_HELLO to daemon");
return -1;
}
return 0;
}

int connect_unix_socket_by_id(unsigned int domid)
{
char id_str[11];
int snprintf_res = snprintf(id_str, sizeof(id_str), "%u", domid);
if (snprintf_res < 0 || snprintf_res >= (int)sizeof(id_str))
err(1, "snprintf()");
return connect_unix_socket(id_str);
}

int connect_unix_socket(const char *domname)
{
int s, len, res;
struct sockaddr_un remote;

if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
err(1, "socket");

remote.sun_family = AF_UNIX;
res = snprintf(remote.sun_path, sizeof remote.sun_path,
"%s/qrexec.%s", socket_dir, domname);
if (res < 0)
err(1, "snprintf");
if (res >= (int)sizeof(remote.sun_path))
errx(1, "%s/qrexec.%s is too long for AF_UNIX socket path",
socket_dir, domname);
len = (size_t)res + 1 + offsetof(struct sockaddr_un, sun_path);
if (connect(s, (struct sockaddr *) &remote, len) == -1)
err(1, "connect %s", remote.sun_path);
if (handle_daemon_handshake(s) < 0)
exit(1);
return s;
}

void send_service_connect(int s, const char *conn_ident,
int connect_domain, int connect_port)
{
struct msg_header hdr;
struct exec_params exec_params;
struct service_params srv_params;

hdr.type = MSG_SERVICE_CONNECT;
hdr.len = sizeof(exec_params) + sizeof(srv_params);

exec_params.connect_domain = connect_domain;
exec_params.connect_port = connect_port;
strncpy(srv_params.ident, conn_ident, sizeof(srv_params.ident) - 1);
srv_params.ident[sizeof(srv_params.ident) - 1] = '\0';

if (!write_all(s, &hdr, sizeof(hdr))
|| !write_all(s, &exec_params, sizeof(exec_params))
|| !write_all(s, &srv_params, sizeof(srv_params))) {
PERROR("write daemon");
exit(1);
}
}
9 changes: 9 additions & 0 deletions daemon/qrexec-daemon-common.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extern const char *socket_dir;
int connect_unix_socket_by_id(unsigned int domid);
int connect_unix_socket(const char *domname);
int handle_daemon_handshake(int fd);
void negotiate_connection_params(int s, int other_domid, unsigned type,
void *cmdline_param, int cmdline_size,
int *data_domain, int *data_port);
void send_service_connect(int s, const char *conn_ident,
int connect_domain, int connect_port);
Loading

0 comments on commit 122466c

Please sign in to comment.