From 9840d38aaa6d663c507c8aa56e19f106dfe4efe7 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sun, 14 Jan 2024 16:34:49 -0500 Subject: [PATCH] Implement UUID support in qrexec This allows using UUIDs in qrexec policy, using the syntax uuid:VM_UUID. This works anywhere a VM name is expected. Since ':' is not allowed in VM names, there is no ambiguity. This requires the corresponding change to qubes-core-admin so that qubesd supports UUIDs in the admin and internal APIs. Fixes: QubesOS/qubes-issues#8510 --- daemon/qrexec-client.c | 2 +- daemon/qrexec-daemon-common.c | 14 +++ daemon/qrexec-daemon-common.h | 1 + daemon/qrexec-daemon.c | 155 +++++++++++++++++++---------- libqrexec/ioall.c | 32 ++++-- libqrexec/libqrexec-utils.h | 24 ++++- qrexec/policy/parser.py | 71 ++++++++++--- qrexec/tests/cli.py | 124 +++++++++++++++-------- qrexec/tests/policy_parser.py | 51 ++++++++-- qrexec/tests/socket/daemon.py | 18 +++- qrexec/tests/socket/qrexec.py | 13 +-- qrexec/tools/qrexec_policy_exec.py | 4 +- qrexec/utils.py | 11 +- 13 files changed, 369 insertions(+), 151 deletions(-) diff --git a/daemon/qrexec-client.c b/daemon/qrexec-client.c index 78e953e5..7eb1c067 100644 --- a/daemon/qrexec-client.c +++ b/daemon/qrexec-client.c @@ -488,7 +488,7 @@ int main(int argc, char **argv) usage(argv[0]); } - if (strcmp(domname, "dom0") == 0 || strcmp(domname, "@adminvm") == 0) { + if (target_refers_to_dom0(domname)) { if (request_id == NULL) { fprintf(stderr, "ERROR: when target domain is 'dom0', -c must be specified\n"); usage(argv[0]); diff --git a/daemon/qrexec-daemon-common.c b/daemon/qrexec-daemon-common.c index 3215e570..288e73e5 100644 --- a/daemon/qrexec-daemon-common.c +++ b/daemon/qrexec-daemon-common.c @@ -47,6 +47,20 @@ void negotiate_connection_params(int s, int other_domid, unsigned type, *data_domain = params.connect_domain; } +bool target_refers_to_dom0(const char *target) +{ + switch (target[0]) { + case '@': + return strcmp(target + 1, "adminvm") == 0; + case 'd': + return strcmp(target + 1, "om0") == 0; + case 'u': + return strcmp(target + 1, "uid:00000000-0000-0000-0000-000000000000") == 0; + default: + return false; + } +} + int handle_daemon_handshake(int fd) { struct msg_header hdr; diff --git a/daemon/qrexec-daemon-common.h b/daemon/qrexec-daemon-common.h index ae483a99..8ae80dd3 100644 --- a/daemon/qrexec-daemon-common.h +++ b/daemon/qrexec-daemon-common.h @@ -7,3 +7,4 @@ void negotiate_connection_params(int s, int other_domid, unsigned type, int *data_domain, int *data_port); void send_service_connect(int s, const char *conn_ident, int connect_domain, int connect_port); +bool target_refers_to_dom0(const char *target); diff --git a/daemon/qrexec-daemon.c b/daemon/qrexec-daemon.c index 8ed2c656..b16a9e42 100644 --- a/daemon/qrexec-daemon.c +++ b/daemon/qrexec-daemon.c @@ -19,6 +19,7 @@ * */ +#include #include #include #include @@ -40,6 +41,8 @@ #define QREXEC_MIN_VERSION QREXEC_PROTOCOL_V2 #define QREXEC_SOCKET_PATH "/run/qubes/policy.sock" +#define QREXEC_DISPVM_PREFIX "@dispvm:" +#define QREXEC_DISPVM_PREFIX_SIZE (sizeof QREXEC_DISPVM_PREFIX - 1) #ifdef COVERAGE void __gcov_dump(void); @@ -147,28 +150,44 @@ static void sigchld_parent_handler(int UNUSED(x)) } -static char *remote_domain_name; // guess what +static const char *remote_domain_name; // guess what +static const char *remote_domain_uuid; static int remote_domain_id; -static void unlink_qrexec_socket(void) +static void unlink_or_exit(const char *path) { - char socket_address[40]; - char link_to_socket_name[strlen(remote_domain_name) + sizeof(socket_address)]; + int v = unlink(path); + if (v != 0 && !(v == -1 && errno == ENOENT)) + err(1, "unlink(%s)", path); +} - int v = snprintf(socket_address, sizeof(socket_address), - "qrexec.%d", remote_domain_id); - if (v < (int)sizeof("qrexec.1") - 1 || v >= (int)sizeof(socket_address)) +static char __attribute__((format(printf, 1, 2))) *xasprintf(const char *fmt, ...) +{ + va_list x; + char *res; + va_start(x, fmt); + int r = vasprintf(&res, fmt, x); + va_end(x); + if (r < 0) abort(); - v = snprintf(link_to_socket_name, sizeof(link_to_socket_name), - "qrexec.%s", remote_domain_name); - if (v < (int)sizeof("qrexec.") - 1 || v >= (int)sizeof(link_to_socket_name)) + return res; +} + +static void unlink_qrexec_socket(void) +{ + char *socket_name; + const char *p[2] = {remote_domain_name, remote_domain_uuid}; + int i; + + for (i = 0; i < 2; ++i) { + char *link_to_socket_name = xasprintf("qrexec.%s%s", i > 0 ? "uuid:" : "", p[i]); + unlink_or_exit(link_to_socket_name); + free(link_to_socket_name); + } + if (asprintf(&socket_name, "qrexec.%d", remote_domain_id) < 0) abort(); - v = unlink(socket_address); - if (v != 0 && !(v == -1 && errno == ENOENT)) - err(1, "unlink(%s)", socket_address); - v = unlink(link_to_socket_name); - if (v != 0 && !(v == -1 && errno == ENOENT)) - err(1, "unlink(%s)", link_to_socket_name); + unlink_or_exit(socket_name); + free(socket_name); } static void handle_vchan_error(const char *op) @@ -177,27 +196,22 @@ static void handle_vchan_error(const char *op) exit(1); } - -static int create_qrexec_socket(int domid, const char *domname) +static int create_qrexec_socket(int domid, const char *domname, const char *domuuid) { - char socket_address[40]; - char link_to_socket_name[strlen(domname) + sizeof(socket_address)]; - int res; - - if ((unsigned)snprintf(socket_address, sizeof(socket_address), - "qrexec.%d", domid) >= sizeof(socket_address)) - errx(1, "socket name too long"); - if ((unsigned)snprintf(link_to_socket_name, sizeof link_to_socket_name, - "qrexec.%s", domname) >= sizeof link_to_socket_name) - errx(1, "socket link name too long"); - res = unlink(link_to_socket_name); - if (res != 0 && !(res == -1 && errno == ENOENT)) - err(1, "unlink(%s)", link_to_socket_name); - /* When running as root, make the socket accessible; perms on /var/run/qubes still apply */ umask(0); - if (symlink(socket_address, link_to_socket_name)) { - PERROR("symlink(%s,%s)", socket_address, link_to_socket_name); + + const char *p[2] = { domuuid, domname }; + char *socket_address = xasprintf("qrexec.%d", domid); + for (int i = 0; i < 2; ++i) { + if (p[i] == NULL) + continue; + char *link_to_socket_name = xasprintf("qrexec.%s%s", i ? "" : "uuid:", p[i]); + unlink_or_exit(link_to_socket_name); + if (symlink(socket_address, link_to_socket_name)) { + PERROR("symlink(%s,%s)", socket_address, link_to_socket_name); + } + free(link_to_socket_name); } int fd = get_server_socket(socket_address); umask(0077); @@ -397,7 +411,7 @@ static void init(int xid) atexit(unlink_qrexec_socket); qrexec_daemon_unix_socket_fd = - create_qrexec_socket(xid, remote_domain_name); + create_qrexec_socket(xid, remote_domain_name, remote_domain_uuid); struct sigaction sigchld_action = { .sa_handler = signal_handler, @@ -801,11 +815,12 @@ static int parse_policy_response( size_t result_bytes, bool daemon, char **user, + char **target_uuid, char **target, char **requested_target, int *autostart ) { - *user = *target = *requested_target = NULL; + *user = *target_uuid = *target = *requested_target = NULL; int result = *autostart = -1; const char *const msg = daemon ? "qrexec-policy-daemon" : "qrexec-policy-exec"; // At least one byte must be returned @@ -850,6 +865,12 @@ static int parse_policy_response( *target = strdup(current_response + (sizeof("target=") - 1)); if (*target == NULL) abort(); + } else if (!strncmp(current_response, "target_uuid=", sizeof("target_uuid=") - 1)) { + if (*target_uuid != NULL) + goto bad_response; + *target_uuid = strdup(current_response + 12); + if (*target_uuid == NULL) + abort(); } else if (!strncmp(current_response, "autostart=", sizeof("autostart=") - 1)) { current_response += sizeof("autostart=") - 1; if (*autostart != -1) @@ -938,6 +959,7 @@ static enum policy_response connect_daemon_socket( const char *target_domain, const char *service_name, char **user, + char **target_uuid, char **target, char **requested_target, int *autostart @@ -965,7 +987,7 @@ static enum policy_response connect_daemon_socket( size_t result_bytes; // this closes the socket char *result = qubes_read_all_to_malloc(daemon_socket, 64, 4096, &result_bytes); - int policy_result = parse_policy_response(result, result_bytes, true, user, target, requested_target, autostart); + int policy_result = parse_policy_response(result, result_bytes, true, user, target_uuid, target, requested_target, autostart); if (policy_result != RESPONSE_MALFORMED) { // This leaks 'result', but as the code execs later anyway this isn't a problem. // 'result' cannot be freed as 'user', 'target', and 'requested_target' point into @@ -1036,7 +1058,7 @@ static enum policy_response connect_daemon_socket( // This leaks 'result', but as the code execs later anyway this isn't a problem. // 'result' cannot be freed as 'user', 'target', and 'requested_target' point into // the same buffer. - return parse_policy_response(result, result_bytes, true, user, target, requested_target, autostart); + return parse_policy_response(result, result_bytes, true, user, target_uuid, target, requested_target, autostart); } } @@ -1084,11 +1106,11 @@ static void handle_execute_service( for (i = 3; i < MAX_FDS; i++) close(i); - char *user, *target, *requested_target; - int autostart; + char *user = NULL, *target = NULL, *requested_target = NULL, *target_uuid = NULL; + int autostart = -1; int policy_response = connect_daemon_socket(remote_domain_name, target_domain, service_name, - &user, &target, &requested_target, &autostart); + &user, &target_uuid, &target, &requested_target, &autostart); if (policy_response != RESPONSE_ALLOW) _exit(126); @@ -1096,7 +1118,6 @@ static void handle_execute_service( /* 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; /* @@ -1106,8 +1127,7 @@ static void handle_execute_service( const char *const trailer = strchr(service_name, '+') ? "" : "+"; /* Check if the target is dom0, which requires special handling. */ - bool target_is_dom0 = strcmp(target, "@adminvm") == 0 || - strcmp(target, "dom0") == 0; + bool target_is_dom0 = target_refers_to_dom0(target); if (target_is_dom0) { char *type; bool target_is_keyword = target_domain[0] == '@'; @@ -1125,7 +1145,8 @@ static void handle_execute_service( target_domain) <= 0) _exit(126); char *cid; - if (asprintf(&cid, "%s,%s,%d", request_id->ident, remote_domain_name, remote_domain_id) <= 0) + if (asprintf(&cid, "%s,%s,%d", request_id->ident, remote_domain_name, + remote_domain_id) <= 0) _exit(126); const char *to_exec[] = { @@ -1141,15 +1162,37 @@ static void handle_execute_service( LOG(ERROR, "execve() failed: %m"); _exit(126); } else { - char *buf; - if (strncmp("@dispvm:", target, sizeof("@dispvm:") - 1) == 0) { - disposable = true; - buf = qubesd_call(target + 8, "admin.vm.CreateDisposable", "", &resp_len); + bool const use_uuid = target_uuid != NULL; + const char *const selected_target = use_uuid ? target_uuid : target; + bool const disposable = strncmp(QREXEC_DISPVM_PREFIX, selected_target, QREXEC_DISPVM_PREFIX_SIZE) == 0; + if (disposable) { + char *buf = qubesd_call2(selected_target + QREXEC_DISPVM_PREFIX_SIZE, + "admin.vm.CreateDisposable", "", + "uuid", use_uuid ? 4 : 0, &resp_len); if (!buf) // error already printed by qubesd_call _exit(126); if (memcmp(buf, "0", 2) == 0) { - /* we exec later so memory leaks do not matter */ - target = buf + 2; + if (strlen(buf + 2) != resp_len - 2) { + LOG(ERROR, "NUL byte in qubesd response"); + _exit(126); + } + if (use_uuid) { + if (resp_len != sizeof("00000000-0000-0000-0000-000000000000") + 1) { + LOG(ERROR, "invalid UUID length"); + _exit(126); + } + target = malloc(resp_len + (sizeof("uuid:") - 2)); + if (target == NULL) { + LOG(ERROR, "out of memory"); + _exit(126); + } + memcpy(target, "uuid:", 5); + memcpy(target + 5, buf + 2, resp_len - 1); + free(buf); + buf = NULL; + } else { + target = buf + 2; + } } else { if (memcmp(buf, "2", 2) == 0) { LOG(ERROR, "qubesd could not create disposable VM: %s", buf + 2); @@ -1166,7 +1209,7 @@ static void handle_execute_service( remote_domain_name) <= 0) _exit(126); if (autostart) { - buf = qubesd_call(target, "admin.vm.Start", "", &resp_len); + char *buf = qubesd_call(target, "admin.vm.Start", "", &resp_len); if (!buf) // error already printed by qubesd_call _exit(126); if (!((memcmp(buf, "0", 2) == 0) || @@ -1491,7 +1534,7 @@ static int handle_agent_restart(int xid) { err(1, "sigaction"); qrexec_daemon_unix_socket_fd = - create_qrexec_socket(xid, remote_domain_name); + create_qrexec_socket(xid, remote_domain_name, remote_domain_uuid); return 0; } @@ -1501,6 +1544,7 @@ static struct option longopts[] = { { "socket-dir", required_argument, 0, 'd' + 128 }, { "policy-program", required_argument, 0, 'p' }, { "direct", no_argument, 0, 'D' }, + { "uuid", required_argument, 0, 'u' }, { NULL, 0, 0, 0 }, }; @@ -1525,7 +1569,7 @@ int main(int argc, char **argv) setup_logging("qrexec-daemon"); - while ((opt=getopt_long(argc, argv, "hqp:D", longopts, NULL)) != -1) { + while ((opt=getopt_long(argc, argv, "hqp:Du:", longopts, NULL)) != -1) { switch (opt) { case 'q': opt_quiet = 1; @@ -1541,6 +1585,9 @@ int main(int argc, char **argv) case 'D': opt_direct = 1; break; + case 'u': + remote_domain_uuid = optarg; + break; case 'h': default: /* '?' */ usage(argv[0]); diff --git a/libqrexec/ioall.c b/libqrexec/ioall.c index 9530e9df..5e48efbc 100644 --- a/libqrexec/ioall.c +++ b/libqrexec/ioall.c @@ -127,6 +127,7 @@ void *qubes_read_all_to_malloc(int fd, size_t initial_buffer_size, size_t max_by } *len = 0; for (;;) { + assert(buf_size > offset); size_t to_read = buf_size - offset; ssize_t res = read(fd, buf + offset, to_read); if (res < 0) { @@ -249,7 +250,12 @@ bool qubes_sendmsg_all(struct msghdr *const msg, int const sock) return true; } -char *qubesd_call(const char *dest, char *method, char *arg, size_t *len) +char *qubesd_call(const char *dest, char *method, char *arg, size_t *out_len) +{ + return qubesd_call2(dest, method, arg, "", 0, out_len); +} + +char *qubesd_call2(const char *dest, char *method, char *arg, const char *payload, size_t len, size_t *out_len) { char *buf = NULL; char *word; @@ -273,6 +279,7 @@ char *qubesd_call(const char *dest, char *method, char *arg, size_t *len) { .iov_base = arg, .iov_len = arg ? strlen(arg) : 0 }, { .iov_base = word, .iov_len = wordlen }, { .iov_base = (void *)dest, .iov_len = strlen(dest) + 1 }, + { .iov_base = (void *)payload, .iov_len = len }, }; struct sockaddr_un qubesd_sock = { @@ -285,14 +292,14 @@ char *qubesd_call(const char *dest, char *method, char *arg, size_t *len) int i = errno; PERROR("socket"); errno = i; - goto out; + goto fail; } if (connect(sock, (struct sockaddr *)&qubesd_sock, offsetof(struct sockaddr_un, sun_path) + sizeof(QUBESD_SOCK))) { LOG(ERROR, "connect(): %m"); - goto out; + goto fail; } struct msghdr msg = { @@ -306,27 +313,30 @@ char *qubesd_call(const char *dest, char *method, char *arg, size_t *len) }; if (!qubes_sendmsg_all(&msg, sock)) - goto out; + goto fail; if (shutdown(sock, SHUT_WR)) { PERROR("shutdown()"); - goto out; + goto fail; } #define BUF_SIZE 35 #define BUF_MAX 65535 - buf = qubes_read_all_to_malloc(sock, BUF_SIZE, BUF_MAX, len); - if (buf && (*len < 2 || strlen(buf) >= *len)) { + buf = qubes_read_all_to_malloc(sock, BUF_SIZE, BUF_MAX, out_len); + if (buf && (*out_len < 2 || strlen(buf) >= *out_len)) { LOG(ERROR, "Truncated response to %s: got %zu bytes", method, - *len); - *len = 0; - free(buf); - buf = NULL; + *out_len); + goto fail; } out: if (sock != -1) close(sock); return buf; +fail: + *out_len = 0; + free(buf); + buf = 0; + goto out; } diff --git a/libqrexec/libqrexec-utils.h b/libqrexec/libqrexec-utils.h index f2066204..b16cc1a2 100644 --- a/libqrexec/libqrexec-utils.h +++ b/libqrexec/libqrexec-utils.h @@ -305,17 +305,31 @@ void qrexec_log(int level, int errnoval, const char *file, int line, void setup_logging(const char *program_name); int qubes_toml_config_parse(const char *config_full_path, int *wait_for_session, char **user); +/** + * Make an Admin API call to qubesd with no payload. The returned buffer must be released by + * the caller using free(). + * + * @param[in] dest The destination VM name. + * @param[in] method The method name. + * @param[in] arg The service argument + * @param[in] payload The payload of the API call. + * @return The value on success. On failure returns NULL and sets errno. + */ +char *qubesd_call(const char *dest, char *method, char *arg, size_t *out_len); + /** * Make an Admin API call to qubesd. The returned buffer must be released by * the caller using free(). * - * @param dest The destination VM name. - * @param method The method name. - * @param arg The service argument - * @param len The length of the data returned + * @param[in] dest The destination VM name. + * @param[in] method The method name. + * @param[in] arg The service argument + * @param[in] payload The payload of the API call. + * @param[in] len The length of the payload. + * @param[out] len The length of the data returned. * @return The value on success. On failure returns NULL and sets errno. */ -char *qubesd_call(const char *dest, char *method, char *arg, size_t *len); +char *qubesd_call2(const char *dest, char *method, char *arg, const char *payload, size_t len, size_t *out_len); /** * Read all data from the file descriptor until EOF, then close it. diff --git a/qrexec/policy/parser.py b/qrexec/policy/parser.py index 8013edb2..936a2456 100644 --- a/qrexec/policy/parser.py +++ b/qrexec/policy/parser.py @@ -30,6 +30,7 @@ import itertools import logging import pathlib +import re import string from typing import ( @@ -235,11 +236,11 @@ def __new__(cls, token: str, *, filepath: Optional[pathlib.Path]=None, orig_token = token # first, adjust some aliases - if token == "dom0": + if token in ("dom0", "uuid:00000000-0000-0000-0000-000000000000"): # TODO: log a warning in Qubes 4.1 token = "@adminvm" - # if user specified just qube name, use it directly + # if user specified just qube name or UUID, use it directly if not (token.startswith("@") or token == "*"): return super().__new__(cls, token) @@ -300,13 +301,28 @@ def __init__(self, token: str, *, filepath: Optional[pathlib.Path]=None, # This replaces is_match() and is_match_single(). def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional["VMToken"]=None ) -> bool: """Check if this token matches opposite token""" - # pylint: disable=unused-argument + # pylint: disable=unused-argument,too-many-return-statements + if self == "@adminvm": + return other == "@adminvm" + info = system_info["domains"] + if self.startswith("uuid:"): + if other.startswith("uuid:"): + return self == other + try: + return self[5:] == info[str(other)]["uuid"] + except KeyError: + return False + if other.startswith("uuid:"): + try: + return other[5:] == info[str(self)]["uuid"] + except KeyError: + return False return self == other def is_special_value(self) -> bool: @@ -339,10 +355,10 @@ def expand(self, *, system_info: FullSystemInfo) -> Iterable[VMToken]: This is used as part of :py:meth:`Policy.collect_targets_for_ask()`. """ - if self in system_info["domains"]: + info = system_info["domains"] + if self in info: yield IntendedTarget(self) - class Target(_BaseTarget): # pylint: disable=missing-docstring pass @@ -362,10 +378,12 @@ def __new__( return super().__new__(cls, value, filepath=filepath, lineno=lineno) # type: ignore +_uuid_regex = re.compile(r"\A[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}\Z") + # this method (with overloads in subclasses) was verify_target_value class IntendedTarget(VMToken): # pylint: disable=missing-docstring - def verify(self, *, system_info: FullSystemInfo) -> VMToken: + def verify(self, *, system_info: FullSystemInfo) -> Optional[VMToken]: """Check if given value names valid target This function check if given value is not only syntactically correct, @@ -410,7 +428,7 @@ class WildcardVM(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -443,7 +461,7 @@ class AnyVM(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -476,7 +494,7 @@ class TypeVM(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -499,7 +517,7 @@ class TagVM(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -522,7 +540,7 @@ class DispVM(Target, Redirect, IntendedTarget): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -556,7 +574,7 @@ class DispVMTemplate(Source, Target, Redirect, IntendedTarget): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -590,7 +608,7 @@ class DispVMTag(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -698,12 +716,33 @@ async def execute(self) -> str: if request.source == target: raise AccessDenied("loopback qrexec connection not supported") - return f"""\ + if target in ("@adminvm", "dom0", "uuid:00000000-0000-0000-0000-000000000000"): + return f"""\ +user={self.user or 'DEFAULT'} +result=allow +target=@adminvm +autostart={self.autostart} +requested_target={request.target}""" + if target.startswith("@dispvm:"): + target_info = request.system_info["domains"][target[8:]] + return f"""\ user={self.user or 'DEFAULT'} result=allow target={self.target} +target_uuid=@dispvm:uuid:{target_info['uuid']} autostart={self.autostart} -requested_target={self.request.target}""" +requested_target={request.target}""" + prefix = "@dispvm:" + else: + target_info = request.system_info["domains"][target] + return f"""\ +user={self.user or 'DEFAULT'} +result=allow +target={self.target} +target_uuid=uuid:{target_info['uuid']} +autostart={self.autostart} +requested_target={request.target}""" + class AskResolution(AbstractResolution): """Resolution returned for :py:class:`Rule` with :py:class:`Ask`. diff --git a/qrexec/tests/cli.py b/qrexec/tests/cli.py index b45d27d4..ae57de49 100644 --- a/qrexec/tests/cli.py +++ b/qrexec/tests/cli.py @@ -103,39 +103,46 @@ def policy(): @pytest.fixture(autouse=True) def system_info(): system_info = { - "domains": { - "dom0": { - "icon": "black", - "template_for_dispvms": False, - "guivm": None, - }, - "source": { - "icon": "red", - "template_for_dispvms": False, - "guivm": "gui", - }, - "test-vm1": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - }, - "test-vm2": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - }, - "test-vm3": { - "icon": "green", - "template_for_dispvms": True, - "guivm": None, - }, - "gui": { - "icon": "orange", - "template_for_dispvms": False, - "guivm": None, - }, + "dom0": { + "icon": "black", + "template_for_dispvms": False, + "guivm": None, + "uuid": "00000000-0000-0000-0000-000000000000", + }, + "source": { + "icon": "red", + "template_for_dispvms": False, + "guivm": "gui", + "uuid": "33e31fb2-31a3-4486-abef-e141416221d9", + }, + "test-vm1": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "uuid": "42d488d0-1168-44eb-9829-81bde8f43065", + }, + "test-vm2": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "uuid": "5b92449a-e783-4566-a1bc-67b6389be5cc", + }, + "test-vm3": { + "icon": "green", + "template_for_dispvms": True, + "guivm": None, + "uuid": "5f3989a7-44e3-4154-bd14-7688f802ec21", + }, + "gui": { + "icon": "orange", + "template_for_dispvms": False, + "guivm": None, + "uuid": "f5c8b32b-8ade-4cdf-8b28-180e07b30ace", }, } + for i, j in system_info.items(): + j["name"] = i + system_info = {"domains": system_info} with mock.patch("qrexec.utils.get_system_info") as mock_system_info: mock_system_info.return_value = system_info yield system_info @@ -209,7 +216,12 @@ def test_000_allow(policy, agent_service): retval = qrexec_policy_exec.get_result( ["source", "test-vm1", "service+arg"] ) - assert retval == "user=user\nresult=allow\ntarget=test-vm1\nautostart=True\nrequested_target=test-vm1" + assert retval == """user=user +result=allow +target=test-vm1 +target_uuid=uuid:42d488d0-1168-44eb-9829-81bde8f43065 +autostart=True +requested_target=test-vm1""" assert agent_service.mock_calls == [] @@ -218,7 +230,12 @@ def test_001_allow_notify(policy, agent_service): retval = qrexec_policy_exec.get_result( ["source", "test-vm1", "service+arg"] ) - assert retval == "user=user\nresult=allow\ntarget=test-vm1\nautostart=True\nrequested_target=test-vm1" + assert retval == """user=user +result=allow +target=test-vm1 +target_uuid=uuid:42d488d0-1168-44eb-9829-81bde8f43065 +autostart=True +requested_target=test-vm1""" assert agent_service.mock_calls == [ notify_call("allow"), ] @@ -230,7 +247,12 @@ def test_002_allow_notify_failed(policy, agent_service): retval = qrexec_policy_exec.get_result( ["source", "test-vm1", "service+arg"] ) - assert retval == "user=user\nresult=allow\ntarget=test-vm1\nautostart=True\nrequested_target=test-vm1" + assert retval == """user=user +result=allow +target=test-vm1 +target_uuid=uuid:42d488d0-1168-44eb-9829-81bde8f43065 +autostart=True +requested_target=test-vm1""" assert agent_service.mock_calls == [ notify_call("allow"), ] @@ -243,7 +265,12 @@ def test_004_allow_no_guivm(policy, system_info, agent_service): retval = qrexec_policy_exec.get_result( ["source", "test-vm1", "service+arg"] ) - assert retval == "user=user\nresult=allow\ntarget=test-vm1\nautostart=True\nrequested_target=test-vm1" + assert retval == """user=user +result=allow +target=test-vm1 +target_uuid=uuid:42d488d0-1168-44eb-9829-81bde8f43065 +autostart=True +requested_target=test-vm1""" assert agent_service.mock_calls == [] @@ -253,7 +280,12 @@ def test_010_ask_allow(policy, agent_service): retval = qrexec_policy_exec.get_result( ["source", "test-vm1", "service+arg"] ) - assert retval == "user=user\nresult=allow\ntarget=test-vm1\nautostart=True\nrequested_target=test-vm1" + assert retval == """user=user +result=allow +target=test-vm1 +target_uuid=uuid:42d488d0-1168-44eb-9829-81bde8f43065 +autostart=True +requested_target=test-vm1""" assert agent_service.mock_calls == [ ask_call(), ] @@ -265,7 +297,12 @@ def test_011_ask_allow_notify(policy, agent_service): retval = qrexec_policy_exec.get_result( ["source", "test-vm1", "service+arg"] ) - assert retval == "user=user\nresult=allow\ntarget=test-vm1\nautostart=True\nrequested_target=test-vm1" + assert retval == """user=user +result=allow +target=test-vm1 +target_uuid=uuid:42d488d0-1168-44eb-9829-81bde8f43065 +autostart=True +requested_target=test-vm1""" assert agent_service.mock_calls == [ ask_call(), notify_call("allow"), @@ -278,7 +315,7 @@ def test_012_ask_allow_notify_no_argument(policy, agent_service): retval = qrexec_policy_exec.get_result( ["source", "test-vm1", "service"] ) - assert retval == "user=user\nresult=allow\ntarget=test-vm1\nautostart=True\nrequested_target=test-vm1" + assert retval == "user=user\nresult=allow\ntarget=test-vm1\ntarget_uuid=uuid:42d488d0-1168-44eb-9829-81bde8f43065\nautostart=True\nrequested_target=test-vm1" assert agent_service.mock_calls == [ ask_call(argument="+"), notify_call("allow", argument="+"), @@ -316,7 +353,12 @@ def test_017_ask_default_target(policy, agent_service): retval = qrexec_policy_exec.get_result( ["source", "test-vm1", "service+arg"] ) - assert retval == "user=user\nresult=allow\ntarget=test-vm1\nautostart=True\nrequested_target=test-vm1" + assert retval == """user=user +result=allow +target=test-vm1 +target_uuid=uuid:42d488d0-1168-44eb-9829-81bde8f43065 +autostart=True +requested_target=test-vm1""" assert agent_service.mock_calls == [ ask_call(default_target="test-vm1"), ] @@ -440,14 +482,14 @@ def test_034_allow_policy_exec(policy, agent_service): ["source-id", "source", "test-vm1", "service+arg", "process_ident"] ) - assert c.mock_calls == [mock.call("test-vm1", "admin.vm.Start")] + assert c.mock_calls == [mock.call("uuid:42d488d0-1168-44eb-9829-81bde8f43065", "admin.vm.Start")] assert agent_service.mock_calls == [] assert retval == 0 assert m.mock_calls == [ mock.call(( QREXEC_CLIENT, "-Ed", - "test-vm1", + "uuid:42d488d0-1168-44eb-9829-81bde8f43065", "-c", "process_ident,source,source-id", "--", diff --git a/qrexec/tests/policy_parser.py b/qrexec/tests/policy_parser.py index e0e9a112..feeefdbc 100644 --- a/qrexec/tests/policy_parser.py +++ b/qrexec/tests/policy_parser.py @@ -38,6 +38,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Running", + "uuid": "00000000-0000-0000-0000-000000000000", }, "test-vm1": { "tags": ["tag1", "tag2"], @@ -45,6 +46,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Running", + "uuid": "c9024a97-9b15-46cc-8341-38d75d5d421b", }, "test-vm2": { "tags": ["tag2"], @@ -52,6 +54,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Running", + "uuid": "b3eb69d0-f9d9-4c3c-ad5c-454500303ea4", }, "test-vm3": { "tags": ["tag3"], @@ -59,6 +62,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": True, "power_state": "Halted", + "uuid": "fa6d56e8-a89d-4106-aa62-22e172a43c8b", }, "default-dvm": { "tags": [], @@ -66,6 +70,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": True, "power_state": "Halted", + "uuid": "f3e538bd-4427-4697-bed7-45ef3270df21", }, "test-invalid-dvm": { "tags": ["tag1", "tag2"], @@ -73,6 +78,7 @@ "default_dispvm": "test-vm1", "template_for_dispvms": False, "power_state": "Halted", + "uuid": "c4fa3586-a6b6-4dc4-bdda-c9e7375a12b5", }, "test-no-dvm": { "tags": ["tag1", "tag2"], @@ -80,6 +86,7 @@ "default_dispvm": None, "template_for_dispvms": False, "power_state": "Halted", + "uuid": "53a450b9-a454-4416-8adb-46812257ad29", }, "test-template": { "tags": ["tag1", "tag2"], @@ -87,6 +94,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Halted", + "uuid": "a9fe2b04-9fd5-4e95-be20-162433d64de0", }, "test-standalone": { "tags": ["tag1", "tag2"], @@ -94,6 +102,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Halted", + "uuid": "6d7a02b5-532b-467f-b9fb-6596bae03c33", }, }, } @@ -110,6 +119,11 @@ async def __call__(self, *args, **kwargs): class TC_00_VMToken(unittest.TestCase): + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + for i, j in SYSTEM_INFO["domains"].items(): + j["name"] = i + def test_010_Source(self): # with self.assertRaises(exc.PolicySyntaxError): # parser.Source(None) @@ -120,6 +134,11 @@ def test_010_Source(self): parser.Source("*") with self.assertRaises(exc.PolicySyntaxError): parser.Source("@default") + parser.Source("uuid:d8a249f1-b02b-4944-a9e5-437def2fbe2c") + with self.assertRaises(exc.PolicySyntaxError): + parser.Source("@uuid:") + with self.assertRaises(exc.PolicySyntaxError): + parser.Source("@uuid:d8a249f1-b02b-4944-a9e5-437def2fbe2c") parser.Source("@type:AppVM") parser.Source("@tag:tag1") with self.assertRaises(exc.PolicySyntaxError): @@ -150,6 +169,7 @@ def test_020_Target(self): parser.Target("@dispvm") parser.Target("@dispvm:default-dvm") parser.Target("@dispvm:@tag:tag3") + parser.Target("uuid:d8a249f1-b02b-4944-a9e5-437def2fbe2c") with self.assertRaises(exc.PolicySyntaxError): parser.Target("@invalid") @@ -163,19 +183,22 @@ def test_020_Target(self): parser.Target("@type:") def test_021_Target_expand(self): - self.assertCountEqual( - parser.Target("test-vm1").expand(system_info=SYSTEM_INFO), + self.assertEqual( + list(parser.Target("test-vm1").expand(system_info=SYSTEM_INFO)), ["test-vm1"], ) - self.assertCountEqual( - parser.Target("@adminvm").expand(system_info=SYSTEM_INFO), + self.assertEqual( + list(parser.Target("@adminvm").expand(system_info=SYSTEM_INFO)), ["@adminvm"], ) - self.assertCountEqual( - parser.Target("dom0").expand(system_info=SYSTEM_INFO), ["@adminvm"] + self.assertEqual( + list(parser.Target("dom0").expand(system_info=SYSTEM_INFO)), ["@adminvm"] ) - self.assertCountEqual( - parser.Target("@anyvm").expand(system_info=SYSTEM_INFO), + self.assertEqual( + list(parser.Target("uuid:00000000-0000-0000-0000-000000000000").expand(system_info=SYSTEM_INFO)), ["@adminvm"] + ) + self.assertEqual( + list(parser.Target("@anyvm").expand(system_info=SYSTEM_INFO)), [ "test-vm1", "test-vm2", @@ -286,6 +309,7 @@ def test_030_Redirect(self): parser.Redirect("test-vm1") parser.Redirect("@adminvm") parser.Redirect("dom0") + parser.Redirect("uuid:00000000-0000-0000-0000-000000000000") with self.assertRaises(exc.PolicySyntaxError): parser.Redirect("@anyvm") with self.assertRaises(exc.PolicySyntaxError): @@ -313,6 +337,7 @@ def test_030_Redirect(self): parser.Redirect("@type:") def test_040_IntendedTarget(self): + parser.IntendedTarget("uuid:00000000-0000-0000-0000-000000000000") parser.IntendedTarget("test-vm1") parser.IntendedTarget("@adminvm") parser.IntendedTarget("dom0") @@ -344,6 +369,10 @@ def test_040_IntendedTarget(self): def test_100_match_single(self): # pytest: disable=no-self-use cases = [ + ("uuid:00000000-0000-0000-0000-000000000000", "@adminvm", True), + ("uuid:00000000-0000-0000-0000-000000000000", "dom0", True), + ("uuid:00000000-0000-0000-0000-000000000000", "@dispvm:default-dvm", False), + ("uuid:00000000-0000-0000-0000-000000000000", "test-vm1", False), ("@anyvm", "test-vm1", True), ("@anyvm", "@default", True), ("@default", "@default", True), @@ -1795,6 +1824,7 @@ async def _test_120_execute(self): user=DEFAULT result=allow target=test-vm2 +target_uuid=uuid:b3eb69d0-f9d9-4c3c-ad5c-454500303ea4 autostart=True requested_target=test-vm2\ """, @@ -1817,7 +1847,7 @@ async def _test_121_execute_dom0(self): """\ user=DEFAULT result=allow -target=dom0 +target=@adminvm autostart=True requested_target=@adminvm\ """ @@ -1871,6 +1901,7 @@ async def _test_122_execute_dispvm(self): user=DEFAULT result=allow target=@dispvm:default-dvm +target_uuid=@dispvm:uuid:f3e538bd-4427-4697-bed7-45ef3270df21 autostart=True requested_target=@dispvm:default-dvm\ """) @@ -1887,12 +1918,14 @@ async def _test_123_execute_already_running(self): rule, request, user=None, target="test-vm2", autostart=True, ) result = await resolution.execute() + self.assertTrue(result.index("uuid:") != 0) self.assertEqual( result, """\ user=DEFAULT result=allow target=test-vm2 +target_uuid=uuid:b3eb69d0-f9d9-4c3c-ad5c-454500303ea4 autostart=True requested_target=test-vm2\ """) diff --git a/qrexec/tests/socket/daemon.py b/qrexec/tests/socket/daemon.py index 5e936b8c..110c8f90 100644 --- a/qrexec/tests/socket/daemon.py +++ b/qrexec/tests/socket/daemon.py @@ -44,6 +44,7 @@ class TestDaemon(unittest.TestCase): daemon = None domain = 42 domain_name = "domain_name" + domain_uuid = "f5086f02-e322-4876-a670-14973aed21b5" # Stub qrexec-policy-exec program. # Strictly speaking, the program should also run qrexec-client in case the @@ -77,6 +78,7 @@ def start_daemon(self): env["VCHAN_SOCKET_DIR"] = self.tempdir cmd = [ os.path.join(ROOT_PATH, "daemon", "qrexec-daemon"), + "--uuid=" + self.domain_uuid, "--socket-dir=" + self.tempdir, "--policy-program=" + policy_program_path, "--direct", @@ -493,12 +495,16 @@ def stop_client(self): self.client.communicate() self.client = None - def connect_daemon(self, domain_id, domain_name): + def connect_daemon(self, domain_id, domain_name, domain_uuid = ""): assert isinstance(domain_id, int), "domain ID is first" assert isinstance(domain_name, str), "domain name is second" + assert isinstance(domain_uuid, str), "domain UUID is third" daemon = qrexec.socket_server( os.path.join(self.tempdir, "qrexec.{}".format(domain_id)), - os.path.join(self.tempdir, "qrexec.{}".format(domain_name)), + ( + os.path.join(self.tempdir, "qrexec.{}".format(domain_name)), + os.path.join(self.tempdir, "qrexec.uuid:{}".format(domain_uuid)), + ) if domain_uuid else (os.path.join(self.tempdir, "qrexec.{}".format(domain_name)),), ) self.addCleanup(daemon.close) return daemon @@ -516,10 +522,11 @@ def connect_source(self, domain, port): def test_run_vm_command_from_dom0(self): cmd = "user:command" target_domain_name = "target_domain" + target_domain_uuid = "d95e1147-2d82-4595-90bb-5a7500cc3196" target_domain = 42 target_port = 513 - target_daemon = self.connect_daemon(target_domain, target_domain_name) + target_daemon = self.connect_daemon(target_domain, target_domain_name, target_domain_uuid) self.start_client(["-d", target_domain_name, cmd]) target_daemon.accept() target_daemon.handshake() @@ -552,11 +559,12 @@ def test_run_vm_command_from_dom0_with_local_command(self): cmd = "user:command" local_cmd = "while read x; do echo input: $x; done; exit 44" target_domain_name = "target_domain" + target_domain_uuid = "d95e1147-2d82-4595-90bb-5a7500cc3196" target_domain = 42 target_port = 513 - target_daemon = self.connect_daemon(target_domain, target_domain_name) - self.start_client(["-d", target_domain_name, "-l", local_cmd, cmd]) + target_daemon = self.connect_daemon(target_domain, target_domain_name, target_domain_uuid) + self.start_client(["-d", "uuid:" + target_domain_uuid, "-l", local_cmd, cmd]) target_daemon.accept() target_daemon.handshake() diff --git a/qrexec/tests/socket/qrexec.py b/qrexec/tests/socket/qrexec.py index a6a63ce1..7d0bcdda 100644 --- a/qrexec/tests/socket/qrexec.py +++ b/qrexec/tests/socket/qrexec.py @@ -131,21 +131,22 @@ def vchan_server(socket_dir, domain, remote_domain, port): return socket_server(vchan_socket_path) -def socket_server(socket_path, socket_path_alt=None): +def socket_server(socket_path, socket_path_alt=()): + assert isinstance(socket_path_alt, tuple), "did you forget the tuple?" try: os.unlink(socket_path) except FileNotFoundError: pass - if socket_path_alt is not None: - assert socket_path_alt[0] == '/', "path not absolute" + for i in socket_path_alt: + assert i[0] == '/', "path not absolute" try: - os.unlink(socket_path_alt) + os.unlink(i) except FileNotFoundError: pass server = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) server.bind(socket_path) - if socket_path_alt is not None: - os.symlink(socket_path, socket_path_alt) + for i in socket_path_alt: + os.symlink(socket_path, i) server.listen(1) return QrexecServer(server) diff --git a/qrexec/tools/qrexec_policy_exec.py b/qrexec/tools/qrexec_policy_exec.py index cbd6dcf0..5eb5ce60 100644 --- a/qrexec/tools/qrexec_policy_exec.py +++ b/qrexec/tools/qrexec_policy_exec.py @@ -287,10 +287,10 @@ def get_result(args: Optional[List[str]]) -> Union[str, int]: intended_target = intended_target[1:] cmd += f" {target_type} {intended_target}" else: + target = result["target_uuid"] cmd = f"{result['user'] or 'DEFAULT'}:" + cmd if dispvm: - target = (utils.qubesd_call(target[8:], - "admin.vm.CreateDisposable") + target = (utils.qubesd_call(target, "admin.vm.CreateDisposable", payload=b"uuid") .decode("ascii", "strict")) utils.qubesd_call(target, "admin.vm.Start") return subprocess.call(( diff --git a/qrexec/utils.py b/qrexec/utils.py index 348b36d0..603c7228 100644 --- a/qrexec/utils.py +++ b/qrexec/utils.py @@ -115,6 +115,7 @@ class SystemInfoEntry(TypedDict): power_state: str icon: str guivm: Optional[str] + uuid: Optional[str] SystemInfo: 'TypeAlias' = Dict[str, SystemInfoEntry] @@ -136,7 +137,15 @@ def get_system_info() -> FullSystemInfo: """ system_info = qubesd_call("dom0", "internal.GetSystemInfo") - return cast(SystemInfo, json.loads(system_info.decode("utf-8"))) + system_info_decoded = cast(FullSystemInfo, json.loads(system_info.decode("utf-8"))) + inner = system_info_decoded["domains"] + for i, j in list(inner.items()): + j["name"] = i + try: + inner["uuid:" + j["uuid"]] = j + except KeyError: + pass + return system_info_decoded def prepare_subprocess_kwds(input: object) -> Dict[str, object]: