From 578de5903316c3e1d678eef91592e79838e90cee 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 | 5 +- daemon/qrexec-daemon-common.c | 37 ++++++- daemon/qrexec-daemon-common.h | 4 +- daemon/qrexec-daemon.c | 118 ++++++++++++-------- libqrexec/ioall.c | 32 ++++-- libqrexec/libqrexec-utils.h | 24 ++++- qrexec/policy/parser.py | 141 +++++++++++++++++------- qrexec/tests/cli.py | 124 ++++++++++++++------- qrexec/tests/policy_graph.py | 103 ++++++++++-------- qrexec/tests/policy_parser.py | 148 +++++++++++++++++++------- qrexec/tests/qrexec_legacy_convert.py | 102 ++++++++++-------- qrexec/tests/socket/daemon.py | 18 +++- qrexec/tests/socket/qrexec.py | 13 +-- qrexec/tools/qrexec_policy_exec.py | 10 +- qrexec/tools/qrexec_policy_graph.py | 8 +- qrexec/utils.py | 19 +++- 16 files changed, 615 insertions(+), 291 deletions(-) diff --git a/daemon/qrexec-client.c b/daemon/qrexec-client.c index dd891037..f0d65d7b 100644 --- a/daemon/qrexec-client.c +++ b/daemon/qrexec-client.c @@ -259,7 +259,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]); @@ -278,10 +278,11 @@ int main(int argc, char **argv) exit_with_code); } else { if (request_id) { + bool const use_uuid = strncmp(domname, "uuid:", 5) == 0; rc = qrexec_execute_vm(domname, false, src_domain_id, remote_cmdline, strlen(remote_cmdline) + 1, request_id, just_exec, - wait_connection_end) ? 0 : 137; + wait_connection_end, use_uuid) ? 0 : 137; } else { s = connect_unix_socket(domname); if (!negotiate_connection_params(s, diff --git a/daemon/qrexec-daemon-common.c b/daemon/qrexec-daemon-common.c index 9813fd8d..0a22b073 100644 --- a/daemon/qrexec-daemon-common.c +++ b/daemon/qrexec-daemon-common.c @@ -11,6 +11,8 @@ #include "qrexec-daemon-common.h" const char *socket_dir = QREXEC_DAEMON_SOCKET_DIR; +#define QREXEC_DISPVM_PREFIX "@dispvm:" +#define QREXEC_DISPVM_PREFIX_SIZE (sizeof QREXEC_DISPVM_PREFIX - 1) /* ask the daemon to allocate vchan port */ bool negotiate_connection_params(int s, int other_domid, unsigned type, @@ -49,6 +51,20 @@ bool negotiate_connection_params(int s, int other_domid, unsigned type, return true; } +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; @@ -93,7 +109,7 @@ int handle_daemon_handshake(int fd) bool qrexec_execute_vm(const char *target, bool autostart, int remote_domain_id, const char *cmd, size_t const service_length, const char *request_id, bool just_exec, - bool wait_connection_end) + bool wait_connection_end, bool use_uuid) { if (service_length < 2 || (size_t)service_length > MAX_QREXEC_CMD_LEN) { /* This is arbitrary, but it helps reduce the risk of overflows in other code */ @@ -119,11 +135,26 @@ bool qrexec_execute_vm(const char *target, bool autostart, int remote_domain_id, } // Otherwise, we kill the VM immediately after starting it. wait_connection_end = true; - buf = qubesd_call(target + 8, "admin.vm.CreateDisposable", "", &resp_len); + buf = qubesd_call2(target + QREXEC_DISPVM_PREFIX_SIZE, + "admin.vm.CreateDisposable", "", + "uuid", use_uuid ? 4 : 0, &resp_len); if (buf == NULL) // error already printed by qubesd_call return false; if (memcmp(buf, "0", 2) == 0) { - /* we exit later so memory leaks do not matter */ + if (strlen(buf + 2) != resp_len - 2) { + LOG(ERROR, "NUL byte in qubesd response"); + return false; + } + if (use_uuid) { + if (resp_len != sizeof("uuid:00000000-0000-0000-0000-000000000000") + 1) { + LOG(ERROR, "invalid UUID length"); + return false; + } + if (memcmp(buf + 2, "uuid:", 5) != 0) { + LOG(ERROR, "invalid UUID target %s", buf + 2); + return false; + } + } target = buf + 2; } else { if (memcmp(buf, "2", 2) == 0) { diff --git a/daemon/qrexec-daemon-common.h b/daemon/qrexec-daemon-common.h index c7f9e660..02b36397 100644 --- a/daemon/qrexec-daemon-common.h +++ b/daemon/qrexec-daemon-common.h @@ -142,6 +142,8 @@ int prepare_local_fds(struct qrexec_parsed_command *command, struct buffer *stdi __attribute__((warn_unused_result)) bool qrexec_execute_vm(const char *target, bool autostart, int remote_domain_id, const char *cmd, size_t service_length, const char *request_id, - bool just_exec, bool wait_connection_end); + bool just_exec, bool wait_connection_end, bool use_uuid); /** FD for stdout of remote process */ extern int local_stdin_fd; +__attribute__((warn_unused_result)) +bool target_refers_to_dom0(const char *target); diff --git a/daemon/qrexec-daemon.c b/daemon/qrexec-daemon.c index 5dfbe3c1..91966db0 100644 --- a/daemon/qrexec-daemon.c +++ b/daemon/qrexec-daemon.c @@ -20,6 +20,7 @@ */ #include +#include #include #include #include @@ -135,24 +136,44 @@ static #endif int protocol_version; -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_or_exit(const char *path) +{ + int v = unlink(path); + if (v != 0 && !(v == -1 && errno == ENOENT)) + err(1, "unlink(%s)", path); +} + +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(); + return res; +} + static void unlink_qrexec_socket(void) { - char *socket_address; - char *link_to_socket_name; - - if (asprintf(&socket_address, "%s/qrexec.%d", socket_dir, remote_domain_id) < 0) - err(1, "asprintf"); - if (unlink(socket_address) != 0 && errno != ENOENT) - err(1, "unlink(%s)", socket_address); - free(socket_address); - if (asprintf(&link_to_socket_name, "%s/qrexec.%s", socket_dir, remote_domain_name) < 0) - err(1, "asprintf"); - if (unlink(link_to_socket_name) != 0 && errno != ENOENT) - err(1, "unlink(%s)", link_to_socket_name); - free(link_to_socket_name); + 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(); + unlink_or_exit(socket_name); + free(socket_name); } static void handle_vchan_error(const char *op) @@ -161,27 +182,25 @@ 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; - - snprintf(socket_address, sizeof(socket_address), - "%s/qrexec.%d", socket_dir, domid); - if (asprintf(&link_to_socket_name, - "%s/qrexec.%s", socket_dir, domname) < 0) - err(1, "asprintf"); - unlink(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); - free(link_to_socket_name); return fd; } @@ -407,7 +426,7 @@ static void init(int xid, bool opt_direct) 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, @@ -856,11 +875,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 @@ -905,6 +925,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) @@ -1001,6 +1027,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 @@ -1028,7 +1055,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 @@ -1099,7 +1126,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); } } @@ -1132,11 +1159,11 @@ _Noreturn static void handle_execute_service_child( 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) daemon__exit(QREXEC_EXIT_REQUEST_REFUSED); @@ -1152,8 +1179,7 @@ _Noreturn static void handle_execute_service_child( 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] == '@'; @@ -1178,6 +1204,8 @@ _Noreturn static void handle_execute_service_child( 5 /* 5 second timeout */, false /* return 0 not remote status code */)); } else { + bool const use_uuid = target_uuid != NULL; + const char *const selected_target = use_uuid ? target_uuid : target; int service_length = asprintf(&cmd, "%s:QUBESRPC %s%s %s", user, service_name, @@ -1185,10 +1213,12 @@ _Noreturn static void handle_execute_service_child( remote_domain_name); if (service_length < 0) daemon__exit(QREXEC_EXIT_PROBLEM); - daemon__exit(qrexec_execute_vm(target, autostart, remote_domain_id, + daemon__exit(qrexec_execute_vm(selected_target, autostart, + remote_domain_id, cmd, (size_t)service_length + 1, - request_id->ident, false, false) + request_id->ident, false, false, + use_uuid) ? 0 : QREXEC_EXIT_PROBLEM); } } @@ -1511,7 +1541,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; } @@ -1521,6 +1551,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 }, }; @@ -1556,7 +1587,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; @@ -1572,6 +1603,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 33dacf9b..18d9b141 100644 --- a/libqrexec/libqrexec-utils.h +++ b/libqrexec/libqrexec-utils.h @@ -371,18 +371,34 @@ void qrexec_log(int level, int errnoval, const char *file, int line, __attribute__((visibility("default"))) void setup_logging(const char *program_name); +/** + * 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. */ __attribute__((visibility("default"))) char *qubesd_call(const char *dest, char *method, char *arg, size_t *len); +__attribute__((visibility("default"))) +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..f23dbb62 100644 --- a/qrexec/policy/parser.py +++ b/qrexec/policy/parser.py @@ -48,7 +48,7 @@ ) from .. import POLICYPATH, RPCNAME_ALLOWED_CHARSET, POLICYSUFFIX -from ..utils import FullSystemInfo +from ..utils import FullSystemInfo, SystemInfo, uuid_to_name from .. import exc from ..exc import ( AccessDenied, @@ -208,6 +208,20 @@ def __init__(cls, name: str, bases: Tuple[type], dict_: Dict[str, str]): if "PREFIX" in dict_: cls.prefixes[dict_["PREFIX"]] = cls # type: ignore +def match_strings(info: SystemInfo, self: str, other: str) -> bool: + 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 class VMToken(str, metaclass=VMTokenMeta): """A domain specification @@ -235,11 +249,11 @@ def __new__(cls, token: str, *, filepath: Optional[pathlib.Path]=None, orig_token = token # first, adjust some aliases - if token == "dom0": - # TODO: log a warning in Qubes 4.1 + if token in {"dom0", "uuid:00000000-0000-0000-0000-000000000000"}: + # TODO: log a warning in Qubes 4.3 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,14 +314,16 @@ 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 - return self == other + # pylint: disable=unused-argument,too-many-return-statements + if self == "@adminvm": + return other == "@adminvm" + return match_strings(system_info["domains"], self, other) def is_special_value(self) -> bool: """Check if the token specification is special (keyword) value""" @@ -339,9 +355,9 @@ 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"]: - yield IntendedTarget(self) - + info = system_info["domains"] + if self in info: + yield IntendedTarget(uuid_to_name(info, self)) class Target(_BaseTarget): # pylint: disable=missing-docstring @@ -365,7 +381,7 @@ def __new__( # 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 +426,7 @@ class WildcardVM(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -419,6 +435,8 @@ def match( def expand(self, *, system_info: FullSystemInfo) -> Iterable[VMToken]: for name, domain in system_info["domains"].items(): + if name.startswith("uuid:"): + continue yield IntendedTarget(name) if domain["template_for_dispvms"]: yield DispVMTemplate("@dispvm:" + name) @@ -443,7 +461,7 @@ class AnyVM(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -452,6 +470,8 @@ def match( def expand(self, *, system_info: FullSystemInfo) -> Iterable[VMToken]: for name, domain in system_info["domains"].items(): + if name.startswith("uuid:"): + continue if domain["type"] != "AdminVM": yield IntendedTarget(name) if domain["template_for_dispvms"]: @@ -476,19 +496,18 @@ class TypeVM(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None ) -> bool: - _system_info = system_info["domains"] - return ( - other in _system_info - and self.value == _system_info[other]["type"] - ) + info = system_info["domains"] + return (other in info and self.value == info[other]["type"]) def expand(self, *, system_info: FullSystemInfo) -> Iterable[IntendedTarget]: for name, domain in system_info["domains"].items(): + if name.startswith("uuid:"): + continue if domain["type"] == self.value: yield IntendedTarget(name) @@ -499,19 +518,18 @@ class TagVM(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None ) -> bool: - _system_info = system_info["domains"] - return ( - other in _system_info - and self.value in _system_info[other]["tags"] - ) + info = system_info["domains"] + return (other in info and self.value in info[other]["tags"]) def expand(self, *, system_info: FullSystemInfo) -> Iterable[IntendedTarget]: for name, domain in system_info["domains"].items(): + if name.startswith("uuid:"): + continue if self.value in domain["tags"]: yield IntendedTarget(name) @@ -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,20 +574,24 @@ class DispVMTemplate(Source, Target, Redirect, IntendedTarget): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None ) -> bool: + assert self.startswith("@dispvm:"), f"missing prefix in {self!r}" if isinstance(other, DispVM) and source is not None: - return self == other.get_dispvm_template( + return match_strings(system_info["domains"], self, other.get_dispvm_template( source, system_info=system_info - ) - return self == other + )) + if not isinstance(other, DispVMTemplate): + return False # not a disposable VM template + return match_strings(system_info["domains"], self[8:], other[8:]) def expand(self, *, system_info: FullSystemInfo) -> Iterable["DispVMTemplate"]: - if system_info["domains"][self.value]["template_for_dispvms"]: - yield self + info = system_info["domains"] + if info[self.value]["template_for_dispvms"]: + yield uuid_to_name(info, self) # else: log a warning? def verify(self, *, system_info: FullSystemInfo) -> "DispVMTemplate": @@ -590,7 +612,7 @@ class DispVMTag(Source, Target): def match( self, - other: Optional[str], + other: str, *, system_info: FullSystemInfo, source: Optional[VMToken]=None @@ -614,6 +636,8 @@ def match( def expand(self, *, system_info: FullSystemInfo) -> Iterable[VMToken]: for name, domain in system_info["domains"].items(): + if name.startswith("uuid:"): + continue if self.value in domain["tags"] and domain["template_for_dispvms"]: yield DispVMTemplate("@dispvm:" + name) @@ -698,12 +722,30 @@ async def execute(self) -> str: if request.source == target: raise AccessDenied("loopback qrexec connection not supported") + 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={request.target}""" + 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={self.request.target}""" +requested_target={request.target}""" class AskResolution(AbstractResolution): """Resolution returned for :py:class:`Rule` with :py:class:`Ask`. @@ -1642,6 +1684,12 @@ def collect_targets_for_ask(self, request): can also contains @dispvm like keywords. """ targets: Set[str] = set() + info: SystemInfo = request.system_info["domains"] + source = request.source + if source.startswith("uuid:"): + source_uuid, source_name = source, info[source]["name"] + else: + source_uuid, source_name = "uuid:" + info[source]["uuid"], source # iterate over rules in reversed order to easier handle 'deny' # actions - simply remove matching domains from allowed set @@ -1651,10 +1699,19 @@ def collect_targets_for_ask(self, request): rule_target = ( getattr(rule.action, "target", None) or rule.target ) - expansion = set( - rule_target.expand(system_info=request.system_info) - ) - + expansion = set() + for potential_target in rule_target.expand( + system_info=request.system_info): + try: + # The policy agent cannot handle UUIDs (and rightly so, + # those are meaningless to humans). Convert them to names. + # VM names can be reused, but in practice, this is unlikely + # to happen except by user request or DispVM name reuse. + # The latter will not happen for a week and so is unlikely + # to confuse humans. + expansion.add(uuid_to_name(info, potential_target)) + except KeyError: + continue if isinstance(rule.action, Action.deny.value): targets.difference_update(expansion) else: @@ -1664,7 +1721,7 @@ def collect_targets_for_ask(self, request): if "@dispvm" in targets: targets.remove("@dispvm") dispvm = DispVM("@dispvm").get_dispvm_template( - request.source, system_info=request.system_info + source, system_info=request.system_info ) if dispvm is not None: targets.add(dispvm) @@ -1675,8 +1732,10 @@ def collect_targets_for_ask(self, request): targets.add("dom0") # XXX remove when #951 gets fixed - if request.source in targets: - targets.remove(request.source) + if source_name in targets: + targets.remove(source_name) + if source_uuid in targets: + targets.remove(source_uuid) return targets diff --git a/qrexec/tests/cli.py b/qrexec/tests/cli.py index bb6c88d9..26c6cf25 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_graph.py b/qrexec/tests/policy_graph.py index 19891433..13ad3f21 100644 --- a/qrexec/tests/policy_graph.py +++ b/qrexec/tests/policy_graph.py @@ -21,57 +21,74 @@ import pytest from unittest import mock +from types import MappingProxyType as Proxy from ..tools.qrexec_policy_graph import main @pytest.fixture(autouse=True) def system_info(): system_info = { - "domains": { - "dom0": { - "icon": "black", - "template_for_dispvms": False, - "guivm": None, - "type": "AdminVM", - "tags": [], - }, - "work": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - "type": "AppVM", - "tags": [], - }, - "personal": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - "type": "AppVM", - "tags": [], - }, - "sys-usb": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - "type": "AppVM", - "tags": [], - }, - "sys-usb-2": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - "type": "AppVM", - "tags": [], - }, - "dvm_template": { - "icon": "red", - "template_for_dispvms": True, - "guivm": None, - "type": "AppVM", - "tags": [], - }, + "dom0": { + "icon": "black", + "template_for_dispvms": False, + "guivm": None, + "type": "AdminVM", + "tags": [], + "uuid": "00000000-0000-0000-0000-000000000000", + }, + "work": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "bab9a66f-57b1-4cc3-8e8f-3d2368a8a68d", + }, + "personal": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "4fe7ad96-1db2-4523-9c3d-3abc31d55427", + }, + "sys-usb": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "d9f594c1-85a4-443f-81e7-aeadf0d0a8fd", + }, + "sys-usb-2": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "03958b59-8ea0-43c7-9aa6-afaf4bf482f0", + }, + "dvm_template": { + "icon": "red", + "template_for_dispvms": True, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "29a7a455-9d71-426f-80ed-932d381cec0a", }, } + + for i, j in list(system_info.items()): + assert not i.startswith("uuid:") + j["name"] = i + j["tags"] = tuple(j["tags"]) + assert not j["name"].startswith("uuid:") + k = system_info["uuid:" + j["uuid"]] = system_info[i] = Proxy(j) + assert not k["name"].startswith("uuid:") + for i in system_info.values(): + assert not i["name"].startswith("uuid:") + system_info = Proxy({"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 diff --git a/qrexec/tests/policy_parser.py b/qrexec/tests/policy_parser.py index bb9a631b..79f8d506 100644 --- a/qrexec/tests/policy_parser.py +++ b/qrexec/tests/policy_parser.py @@ -25,6 +25,7 @@ import unittest.mock import asyncio import pytest +from types import MappingProxyType from .. import QREXEC_CLIENT, QUBESD_INTERNAL_SOCK from .. import exc, utils @@ -38,6 +39,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 +47,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Running", + "uuid": "c9024a97-9b15-46cc-8341-38d75d5d421b", }, "test-vm2": { "tags": ["tag2"], @@ -52,6 +55,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Running", + "uuid": "b3eb69d0-f9d9-4c3c-ad5c-454500303ea4", }, "test-vm3": { "tags": ["tag3"], @@ -59,6 +63,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": True, "power_state": "Halted", + "uuid": "fa6d56e8-a89d-4106-aa62-22e172a43c8b", }, "default-dvm": { "tags": [], @@ -66,6 +71,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 +79,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 +87,7 @@ "default_dispvm": None, "template_for_dispvms": False, "power_state": "Halted", + "uuid": "53a450b9-a454-4416-8adb-46812257ad29", }, "test-template": { "tags": ["tag1", "tag2"], @@ -87,6 +95,7 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Halted", + "uuid": "a9fe2b04-9fd5-4e95-be20-162433d64de0", }, "test-standalone": { "tags": ["tag1", "tag2"], @@ -94,10 +103,35 @@ "default_dispvm": "default-dvm", "template_for_dispvms": False, "power_state": "Halted", + "uuid": "6d7a02b5-532b-467f-b9fb-6596bae03c33", }, }, } +def patch_system_info(): + """ + We *really* do not want any code to modify SYSTEM_INFO, + so replace all of its contents with versions that do not + allow mutation. Lists are replaced with tuples. + + This also adds keys for uuid:UUID and adds "name" + fields to all dictionaries that need it. + """ + global SYSTEM_INFO + system_info = SYSTEM_INFO["domains"] + for i, j in list(system_info.items()): + assert not i.startswith("uuid:") + j["name"] = i + j["tags"] = tuple(j["tags"]) + system_info["uuid:" + j["uuid"]] = system_info[i] = \ + MappingProxyType(j) + SYSTEM_INFO["domains"] = MappingProxyType(system_info) + for i in system_info.values(): + assert not i["name"].startswith("uuid:") + SYSTEM_INFO = MappingProxyType(SYSTEM_INFO) + +patch_system_info() + # a generic request helper _req = functools.partial( parser.Request, "test.Service", "+argument", system_info=SYSTEM_INFO @@ -110,6 +144,9 @@ async def __call__(self, *args, **kwargs): class TC_00_VMToken(unittest.TestCase): + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + def test_010_Source(self): # with self.assertRaises(exc.PolicySyntaxError): # parser.Source(None) @@ -120,6 +157,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 +192,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 +206,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", @@ -190,21 +236,22 @@ def test_021_Target_expand(self): "@dispvm", ], ) - self.assertCountEqual( - parser.Target("*").expand(system_info=SYSTEM_INFO), + self.maxDiff = None + self.assertEqual( + sorted(set(parser.Target("*").expand(system_info=SYSTEM_INFO))), [ "@adminvm", - "test-vm1", - "test-vm2", - "test-vm3", + "@dispvm", + "@dispvm:default-dvm", "@dispvm:test-vm3", "default-dvm", - "@dispvm:default-dvm", "test-invalid-dvm", "test-no-dvm", - "test-template", "test-standalone", - "@dispvm", + "test-template", + "test-vm1", + "test-vm2", + "test-vm3", ], ) self.assertCountEqual( @@ -286,6 +333,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 +361,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 +393,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), @@ -382,6 +435,12 @@ def test_100_match_single(self): ("@dispvm", "test-vm1", False), ("@dispvm", "default-dvm", False), ("@dispvm:default-dvm", "default-dvm", False), + ("uuid:6d7a02b5-532b-467f-b9fb-6596bae03c33", "test-standalone", True), + ("uuid:f3e538bd-4427-4697-bed7-45ef3270df21", "default-dvm", True), + ("@dispvm:uuid:f3e538bd-4427-4697-bed7-45ef3270df21", "@dispvm:uuid:f3e538bd-4427-4697-bed7-45ef3270df21", True), + ("@dispvm:uuid:f3e538bd-4427-4697-bed7-45ef3270df21", "@dispvm:default-dvm", True), + ("@dispvm:default-dvm", "@dispvm:uuid:f3e538bd-4427-4697-bed7-45ef3270df21", True), + ("test-standalone", "uuid:6d7a02b5-532b-467f-b9fb-6596bae03c33", True), ] for token, target, expected_result in cases: @@ -1183,41 +1242,42 @@ def test_020_collect_targets_for_ask(self): * * test-no-dvm @dispvm allow * * test-standalone @dispvm allow * * test-standalone @adminvm allow + * * uuid:c9024a97-9b15-46cc-8341-38d75d5d421b bogus2 deny """ ) self.assertCountEqual( - policy.collect_targets_for_ask(_req("test-vm1", "@default")), + sorted(policy.collect_targets_for_ask(_req("test-vm1", "@default"))), [ - "test-vm2", - "test-vm3", + "@dispvm:default-dvm", "@dispvm:test-vm3", "default-dvm", - "@dispvm:default-dvm", "test-invalid-dvm", "test-no-dvm", - "test-template", "test-standalone", + "test-template", + "test-vm2", + "test-vm3", ], ) - self.assertCountEqual( + self.assertEqual( policy.collect_targets_for_ask(_req("test-vm2", "@default")), - ["test-vm3"], + {"test-vm3"}, ) self.assertCountEqual( policy.collect_targets_for_ask(_req("test-vm3", "@default")), [] ) - self.assertCountEqual( - policy.collect_targets_for_ask(_req("test-standalone", "@default")), + self.assertEqual( + sorted(policy.collect_targets_for_ask(_req("test-standalone", "@default"))), [ + "@dispvm:default-dvm", + "default-dvm", + "dom0", + "test-invalid-dvm", + "test-no-dvm", "test-vm1", "test-vm2", "test-vm3", - "default-dvm", - "test-no-dvm", - "test-invalid-dvm", - "@dispvm:default-dvm", - "dom0", ], ) self.assertCountEqual( @@ -1440,8 +1500,8 @@ def test_040_eval_ask(self): self.assertEqual(resolution.rule, self.policy.rules[2]) self.assertEqual(resolution.request.target, "test-vm2") self.assertCountEqual( - resolution.targets_for_ask, - [ + sorted(resolution.targets_for_ask), + sorted([ "test-vm1", "test-vm2", "test-vm3", @@ -1451,29 +1511,31 @@ def test_040_eval_ask(self): "test-invalid-dvm", "test-no-dvm", "test-template", - ], + ]), ) def test_041_eval_ask(self): - resolution = self.policy.evaluate(_req("test-standalone", "test-vm3")) + for i in SYSTEM_INFO["domains"].values(): + assert not i["name"].startswith("uuid:") + resolution = self.policy.evaluate(_req("uuid:6d7a02b5-532b-467f-b9fb-6596bae03c33", "test-vm3")) self.assertIsInstance(resolution, parser.AskResolution) self.assertEqual(resolution.rule, self.policy.rules[3]) self.assertEqual(resolution.default_target, "test-vm3") self.assertEqual(resolution.request.target, "test-vm3") - self.assertCountEqual( - resolution.targets_for_ask, + self.assertEqual( [ - "test-vm1", - "test-vm2", - "test-vm3", + "@dispvm:default-dvm", "@dispvm:test-vm3", "default-dvm", - "@dispvm:default-dvm", "test-invalid-dvm", "test-no-dvm", "test-template", + "test-vm1", + "test-vm2", + "test-vm3", ], + sorted(resolution.targets_for_ask), ) def test_042_eval_ask_no_targets(self): @@ -1794,6 +1856,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\ """, @@ -1816,7 +1879,7 @@ async def _test_121_execute_dom0(self): """\ user=DEFAULT result=allow -target=dom0 +target=@adminvm autostart=True requested_target=@adminvm\ """ @@ -1870,6 +1933,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\ """) @@ -1886,12 +1950,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/qrexec_legacy_convert.py b/qrexec/tests/qrexec_legacy_convert.py index 7d083225..07bbb1f4 100644 --- a/qrexec/tests/qrexec_legacy_convert.py +++ b/qrexec/tests/qrexec_legacy_convert.py @@ -22,56 +22,72 @@ from unittest import mock import pytest from pathlib import Path +from types import MappingProxyType as Proxy from ..tools import qrexec_legacy_convert @pytest.fixture(autouse=True) def system_info(): system_info = { - "domains": { - "dom0": { - "icon": "black", - "template_for_dispvms": False, - "guivm": None, - "type": "AdminVM", - "tags": [], - }, - "work": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - "type": "AppVM", - "tags": [], - }, - "personal": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - "type": "AppVM", - "tags": [], - }, - "sys-usb": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - "type": "AppVM", - "tags": [], - }, - "sys-usb-2": { - "icon": "red", - "template_for_dispvms": False, - "guivm": None, - "type": "AppVM", - "tags": [], - }, - "dvm_template": { - "icon": "red", - "template_for_dispvms": True, - "guivm": None, - "type": "AppVM", - "tags": [], - }, + "dom0": { + "icon": "black", + "template_for_dispvms": False, + "guivm": None, + "type": "AdminVM", + "tags": [], + "uuid": "00000000-0000-0000-0000-000000000000", + }, + "work": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "8079de03-ab79-424c-8fb6-d8adcfda19d1", + }, + "personal": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "bf7cdc8a-1dc5-4fcc-b01f-bd648ff8d903", + }, + "sys-usb": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "46fd27ac-46e3-44d7-918e-986f095900f9", + }, + "sys-usb-2": { + "icon": "red", + "template_for_dispvms": False, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "b65fceec-2d84-4665-99cb-50c2e7d91dc6", + }, + "dvm_template": { + "icon": "red", + "template_for_dispvms": True, + "guivm": None, + "type": "AppVM", + "tags": [], + "uuid": "b41c5829-3ea9-475e-b5e6-b404ea409f29", }, } + + for i, j in list(system_info.items()): + assert not i.startswith("uuid:") + j["name"] = i + j["tags"] = tuple(j["tags"]) + system_info["uuid:" + j["uuid"]] = system_info[i] = Proxy(j) + for i in system_info.values(): + assert not i["name"].startswith("uuid:") + assert "uuid" in i + system_info = Proxy({"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 diff --git a/qrexec/tests/socket/daemon.py b/qrexec/tests/socket/daemon.py index f605d70c..d95a2ad1 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. POLICY_PROGRAM = """\ @@ -75,6 +76,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", @@ -579,12 +581,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 @@ -602,10 +608,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() @@ -638,11 +645,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 b7b72b7e..c0e60141 100644 --- a/qrexec/tests/socket/qrexec.py +++ b/qrexec/tests/socket/qrexec.py @@ -140,22 +140,23 @@ 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.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) 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 cabd9de9..816b05aa 100644 --- a/qrexec/tools/qrexec_policy_exec.py +++ b/qrexec/tools/qrexec_policy_exec.py @@ -73,9 +73,11 @@ async def execute(self) -> str: assert False, "handle_user_response should throw" # prepare icons - icons = {name: domains[name]["icon"] for name in domains.keys()} + icons = {name: domains[name]["icon"] for name in domains.keys() + if not name.startswith("uuid:")} for dispvm_base in domains: - if not domains[dispvm_base]["template_for_dispvms"]: + if (dispvm_base.startswith("uuid:") + or not domains[dispvm_base]["template_for_dispvms"]): continue dispvm_api_name = "@dispvm:" + dispvm_base icons[dispvm_api_name] = domains[dispvm_base]["icon"] @@ -288,10 +290,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") # pylint: disable=possibly-used-before-assignment diff --git a/qrexec/tools/qrexec_policy_graph.py b/qrexec/tools/qrexec_policy_graph.py index f01b5f29..08a11021 100644 --- a/qrexec/tools/qrexec_policy_graph.py +++ b/qrexec/tools/qrexec_policy_graph.py @@ -136,16 +136,18 @@ def main(args=None, output=sys.stdout): else: system_info = utils.get_system_info() - sources = list(system_info["domains"].keys()) + targets = [key for key in system_info["domains"] if not key.startswith("uuid")] if args.source: sources = args.source + else: + sources = list(targets) - targets = list(system_info["domains"].keys()) targets.append("@dispvm") targets.extend( "@dispvm:" + dom for dom in system_info["domains"] - if system_info["domains"][dom]["template_for_dispvms"] + if (not dom.startswith("uuid:") + and system_info["domains"][dom]["template_for_dispvms"]) ) targets.append("@default") diff --git a/qrexec/utils.py b/qrexec/utils.py index 348b36d0..0600e9c6 100644 --- a/qrexec/utils.py +++ b/qrexec/utils.py @@ -115,12 +115,21 @@ class SystemInfoEntry(TypedDict): power_state: str icon: str guivm: Optional[str] + uuid: Optional[str] + name: str SystemInfo: 'TypeAlias' = Dict[str, SystemInfoEntry] class FullSystemInfo(TypedDict): domains: SystemInfo +def uuid_to_name(info: SystemInfo, uuid_or_name: str) -> str: + if uuid_or_name.startswith("uuid:"): + return info[uuid_or_name[5:]]["name"] + if uuid_or_name.startswith("@dispvm:uuid:"): + return "@dispvm:" + info[uuid_or_name[13:]]["name"] + return uuid_or_name + def get_system_info() -> FullSystemInfo: """Get system information @@ -136,7 +145,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]: