Skip to content

Commit

Permalink
Support using UUIDs instead of VM names
Browse files Browse the repository at this point in the history
This supports using UUIDs (instead of VM names) in the Admin API.  It
also provides the UUIDs to qrexec-policy-daemon, which will use them to
support uuid:<UUID> syntax as an alternative way of naming a VM.

All admin APIs now support UUIDs as source and destination.  Specific
APIs changed include:

- admin.vm.CreateDisposable: if the payload is "uuid" used, the VM UUID
  (with "uuid:" prefix) is returned instead of the name.
- admin.vm.List: the UUID is included in the returned list.
- internal.GetSystemInfo: the UUID is returned in the "uuid" key of each
  VM's JSON object.
- A new stubdom_uuid property contains the UUID of the stubdomain.

Fixes: QubesOS/qubes-issues#8862
  • Loading branch information
DemiMarie committed Jun 28, 2024
1 parent 488752c commit 8039442
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 31 deletions.
59 changes: 43 additions & 16 deletions qubes/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@
import functools
import io
import os
import re
import shutil
import socket
import struct
import traceback
from typing import Union, Any
import uuid

import qubes.exc
from qubes.exc import ProtocolError, PermissionDenied
Expand Down Expand Up @@ -88,6 +91,32 @@ def apply_filters(iterable, filters):
iterable = filter(selector, iterable)
return iterable

# This regex allows incorrect-length UUIDs,
# but there is an explicit length check to catch that.
_uuid_regex = re.compile(rb"\Auuid:[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]*")
def decode_vm(
untrusted_input: bytes, domains: qubes.app.VMCollection
) -> qubes.vm.qubesvm.QubesVM:
lookup: Union[uuid.UUID, str]
vm = untrusted_input.decode("ascii", "strict")
if untrusted_input.startswith(b"uuid:"):
if (len(untrusted_input) != 41 or
not _uuid_regex.match(untrusted_input)):
raise qubes.exc.QubesVMInvalidUUIDError(vm[5:])
lookup = uuid.UUID(vm[5:])
else:
# throws if name is invalid
qubes.vm.validate_name(None, None, vm)
lookup = vm
try:
return domains[lookup]
except KeyError:

Check warning on line 113 in qubes/api/__init__.py

View check run for this annotation

Codecov / codecov/patch

qubes/api/__init__.py#L113

Added line #L113 was not covered by tests
# normally this should filtered out by qrexec policy, but there are
# two cases it might not be:
# 1. The call comes from dom0, which bypasses qrexec policy
# 2. Domain was removed between checking the policy and here
# we inform the client accordingly
raise qubes.exc.QubesVMNotFoundError(vm)

Check warning on line 119 in qubes/api/__init__.py

View check run for this annotation

Codecov / codecov/patch

qubes/api/__init__.py#L119

Added line #L119 was not covered by tests

class AbstractQubesAPI:
'''Common code for Qubes Management Protocol handling
Expand All @@ -108,25 +137,23 @@ class AbstractQubesAPI:
#: the preferred socket location (to be overridden in child's class)
SOCKNAME = None

def __init__(self, app, src, method_name, dest, arg, send_event=None):
app: qubes.Qubes
src: qubes.vm.qubesvm.QubesVM
def __init__(self,
app: qubes.Qubes,
src: bytes,
method_name: bytes,
dest: bytes,
arg: qubes.Qubes,
send_event: Any = None) -> None:
#: :py:class:`qubes.Qubes` object
self.app = app

try:
vm = src.decode('ascii')
#: source qube
self.src = self.app.domains[vm]

vm = dest.decode('ascii')
#: destination qube
self.dest = self.app.domains[vm]
except KeyError:
# normally this should be filtered out by qrexec policy, but there
# are two cases it might not be:
# 1. The call comes from dom0, which bypasses qrexec policy
# 2. Domain was removed between checking the policy and here
# we inform the client accordingly
raise qubes.exc.QubesVMNotFoundError(vm)
#: source qube
self.src = decode_vm(src, app.domains)

#: destination qube
self.dest = decode_vm(dest, app.domains)

#: argument
self.arg = arg.decode('ascii')
Expand Down
17 changes: 12 additions & 5 deletions qubes/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,11 @@ async def vm_list(self):
else:
domains = self.fire_event_for_filter([self.dest])

return ''.join('{} class={} state={}\n'.format(
return ''.join('{} class={} state={} uuid={}\n'.format(
vm.name,
vm.__class__.__name__,
vm.get_power_state())
vm.get_power_state(),
vm.uuid)
for vm in sorted(domains))

@qubes.api.method('admin.vm.property.List', no_payload=True,
Expand Down Expand Up @@ -1138,10 +1139,14 @@ async def _vm_create(self, vm_type, allow_pool=False,
raise
self.app.save()

@qubes.api.method('admin.vm.CreateDisposable', no_payload=True,
@qubes.api.method('admin.vm.CreateDisposable',
scope='global', write=True)
async def create_disposable(self):
async def create_disposable(self, untrusted_payload):
self.enforce(not self.arg)
if untrusted_payload not in (b"", b"uuid"):
raise qubes.exc.QubesValueError(

Check warning on line 1147 in qubes/api/admin.py

View check run for this annotation

Codecov / codecov/patch

qubes/api/admin.py#L1147

Added line #L1147 was not covered by tests
"Invalid payload for admin.vm.CreateDisposable: "
"expected the empty string or 'uuid'")

if self.dest.name == 'dom0':
dispvm_template = self.src.default_dispvm
Expand All @@ -1155,7 +1160,9 @@ async def create_disposable(self):
dispvm.tags.add('created-by-' + str(self.src))
dispvm.tags.add('disp-created-by-' + str(self.src))

return dispvm.name
return (("uuid:" + str(dispvm.uuid))
if untrusted_payload
else dispvm.name)

@qubes.api.method(
'admin.vm.Remove', no_payload=True, scope='global', write=True)
Expand Down
1 change: 1 addition & 0 deletions qubes/api/internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def get_system_info(app):
'guivm': (domain.guivm.name if getattr(domain, 'guivm', None)
else None),
'power_state': domain.get_power_state(),
'uuid': str(domain.uuid),
} for domain in app.domains
}}
return system_info
Expand Down
3 changes: 2 additions & 1 deletion qubes/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@ def __getitem__(self, key):

if isinstance(key, uuid.UUID):
for vm in self:
assert isinstance(vm.uuid, uuid.UUID)
if vm.uuid == key:
return vm
raise KeyError(key)
Expand All @@ -540,7 +541,7 @@ def __delitem__(self, key):
self.app.fire_event('domain-delete', vm=vm)

def __contains__(self, key):
return any((key in (vm, vm.qid, vm.name))
return any((key in (vm, vm.qid, vm.name, vm.uuid))
for vm in self)

def __len__(self):
Expand Down
8 changes: 8 additions & 0 deletions qubes/exc.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ def __str__(self):
# KeyError overrides __str__ method
return QubesException.__str__(self)

class QubesVMInvalidUUIDError(QubesException):
"""Domain UUID is invalid"""
# pylint: disable = super-init-not-called
def __init__(self, uuid: str) -> None:
# QubesVMNotFoundError overrides __init__ method
# pylint: disable = non-parent-init-called
QubesException.__init__(self, f"VM UUID is not valid: {uuid!r}")
self.vmname = uuid

class QubesVMError(QubesException):
'''Some problem with domain state.'''
Expand Down
57 changes: 51 additions & 6 deletions qubes/tests/api_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
import asyncio
import operator
import os
import re
import shutil
import tempfile
import unittest.mock
import uuid

import libvirt
import copy
Expand All @@ -46,6 +48,8 @@
'pool', 'vid', 'size', 'usage', 'rw', 'source', 'path',
'save_on_stop', 'snap_on_start', 'revisions_to_keep', 'ephemeral']

_uuid_regex = re.compile(
r"\Auuid:[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}\Z")

class AdminAPITestCase(qubes.tests.QubesTestCase):
def setUp(self):
Expand Down Expand Up @@ -78,6 +82,7 @@ def setUp(self):
app.save = unittest.mock.Mock()
self.vm = app.add_new_vm('AppVM', label='red', name='test-vm1',
template='test-template')
self.uuid = b"uuid:" + str(self.vm.uuid).encode("ascii", "strict")
self.app = app
libvirt_attrs = {
'libvirt_conn.lookupByUUID.return_value.isActive.return_value':
Expand Down Expand Up @@ -130,14 +135,33 @@ class TC_00_VMs(AdminAPITestCase):
def test_000_vm_list(self):
value = self.call_mgmt_func(b'admin.vm.List', b'dom0')
self.assertEqual(value,
'dom0 class=AdminVM state=Running\n'
'test-template class=TemplateVM state=Halted\n'
'test-vm1 class=AppVM state=Halted\n')
'dom0 class=AdminVM state=Running '
'uuid=00000000-0000-0000-0000-000000000000\n'
'test-template class=TemplateVM state=Halted '
f'uuid={self.template.uuid}\n'
f'test-vm1 class=AppVM state=Halted uuid={self.vm.uuid}\n')

def test_001_vm_list_single(self):
value = self.call_mgmt_func(b'admin.vm.List', b'test-vm1')
self.assertEqual(value,
'test-vm1 class=AppVM state=Halted\n')
f"test-vm1 class=AppVM state=Halted uuid={self.vm.uuid}\n")

def test_001_vm_list_single_uuid(self):
value = self.call_mgmt_func(b'admin.vm.List', self.uuid)
self.assertEqual(value,
f"test-vm1 class=AppVM state=Halted uuid={self.vm.uuid}\n")

def test_001_vm_list_single_invalid_name(self):
with self.assertRaisesRegex(qubes.exc.QubesValueError,
r"\AVM name contains illegal characters\Z"):
self.call_mgmt_func(b'admin.vm.CreateDisposable', b'.test-vm1')
self.assertFalse(self.app.save.called)

def test_001_vm_list_single_invalid_uuid(self):
with self.assertRaisesRegex(qubes.exc.QubesVMInvalidUUIDError,
r"\AVM UUID is not valid: ''\Z"):
self.call_mgmt_func(b'admin.vm.CreateDisposable', b"uuid:")
self.assertFalse(self.app.save.called)

def test_002_vm_list_filter(self):
with tempfile.TemporaryDirectory() as tmpdir:
Expand All @@ -154,8 +178,9 @@ def test_002_vm_list_filter(self):
value = loop.run_until_complete(
mgmt_obj.execute(untrusted_payload=b''))
self.assertEqual(value,
'dom0 class=AdminVM state=Running\n'
'test-vm1 class=AppVM state=Halted\n')
'dom0 class=AdminVM state=Running '
'uuid=00000000-0000-0000-0000-000000000000\n'
f"test-vm1 class=AppVM state=Halted uuid={self.vm.uuid}\n")

def test_010_vm_property_list(self):
# this test is kind of stupid, but at least check if appropriate
Expand Down Expand Up @@ -2816,6 +2841,26 @@ def test_640_vm_create_disposable(self, mock_storage):
mock_storage.assert_called_once_with()
self.assertTrue(self.app.save.called)

@unittest.mock.patch('qubes.storage.Storage.create')
def test_640_vm_create_disposable_uuid(self, mock_storage):
mock_storage.side_effect = self.dummy_coro
self.vm.template_for_dispvms = True
retval = self.call_mgmt_func(b'admin.vm.CreateDisposable',
b'test-vm1', payload=b'uuid')
self.assertRegex(retval, _uuid_regex)
found = False
for i in self.app.domains:
print(i.uuid)
if i.uuid == uuid.UUID(retval):
found = True
self.assertIsInstance(i.uuid, uuid.UUID)
self.assertTrue(found)
self.assertIn(uuid.UUID(retval), self.app.domains)
dispvm = self.app.domains[uuid.UUID(retval)]
self.assertEqual(dispvm.template, self.vm)
mock_storage.assert_called_once_with()
self.assertTrue(self.app.save.called)

@unittest.mock.patch('qubes.storage.Storage.create')
def test_641_vm_create_disposable_default(self, mock_storage):
mock_storage.side_effect = self.dummy_coro
Expand Down
7 changes: 7 additions & 0 deletions qubes/tests/api_internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@
import qubes.vm.adminvm
from unittest import mock
import json
import uuid

def mock_coro(f):
async def coro_f(*args, **kwargs):
return f(*args, **kwargs)

return coro_f

TEST_UUID = uuid.UUID("50c7dad4-5f1e-4586-9f6a-bf10a86ba6f0")

class TC_00_API_Misc(qubes.tests.QubesTestCase):
def setUp(self):
super().setUp()
Expand Down Expand Up @@ -150,6 +153,7 @@ def test_010_get_system_info(self):
self.dom0.template_for_dispvms = False
self.dom0.label.icon = 'icon-dom0'
self.dom0.get_power_state.return_value = 'Running'
self.dom0.uuid = uuid.UUID("00000000-0000-0000-0000-000000000000")
del self.dom0.guivm

vm = mock.NonCallableMock(spec=qubes.vm.qubesvm.QubesVM)
Expand All @@ -160,6 +164,7 @@ def test_010_get_system_info(self):
vm.label.icon = 'icon-vm'
vm.guivm = vm
vm.get_power_state.return_value = 'Halted'
vm.uuid = TEST_UUID
self.domains['vm'] = vm

ret = json.loads(self.call_mgmt_func(b'internal.GetSystemInfo'))
Expand All @@ -173,6 +178,7 @@ def test_010_get_system_info(self):
'icon': 'icon-dom0',
'guivm': None,
'power_state': 'Running',
'uuid': "00000000-0000-0000-0000-000000000000",
},
'vm': {
'tags': ['tag3', 'tag4'],
Expand All @@ -182,6 +188,7 @@ def test_010_get_system_info(self):
'icon': 'icon-vm',
'guivm': 'vm',
'power_state': 'Halted',
"uuid": str(TEST_UUID),
}
}
})
3 changes: 2 additions & 1 deletion qubes/vm/adminvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import grp
import subprocess
import libvirt
import uuid

import qubes
import qubes.exc
Expand All @@ -45,7 +46,7 @@ class AdminVM(BaseVM):
default=0, type=int, setter=qubes.property.forbidden)

uuid = qubes.property('uuid',
default='00000000-0000-0000-0000-000000000000',
default=uuid.UUID('00000000-0000-0000-0000-000000000000'),
setter=qubes.property.forbidden)

default_dispvm = qubes.VMProperty('default_dispvm',
Expand Down
18 changes: 16 additions & 2 deletions qubes/vm/qubesvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
MEM_OVERHEAD_BASE = (3 + 1) * 1024 * 1024
MEM_OVERHEAD_PER_VCPU = 3 * 1024 * 1024 / 2

_vm_uuid_re = re.compile(rb"\A/vm/[0-9a-f]{8}(?:-[0-9a-f]{4}){4}[0-9a-f]{8}\Z")

def _setter_kernel(self, prop, value):
""" Helper for setting the domain kernel and running sanity checks on it.
Expand Down Expand Up @@ -801,6 +802,17 @@ def xid(self):
e.get_error_code()))
raise

@qubes.stateless_property
def stubdom_uuid(self):
stubdom_xid = self.stubdom_xid
if stubdom_xid == -1:
return ""
stubdom_uuid = self.app.vmm.xs.read(

Check warning on line 810 in qubes/vm/qubesvm.py

View check run for this annotation

Codecov / codecov/patch

qubes/vm/qubesvm.py#L807-L810

Added lines #L807 - L810 were not covered by tests
'', '/local/domain/{}/vm'.format(
stubdom_xid))
assert _vm_uuid_re.match(stubdom_uuid), "Invalid UUID in XenStore"
return stubdom_uuid[4:].decode("ascii", "strict")

Check warning on line 814 in qubes/vm/qubesvm.py

View check run for this annotation

Codecov / codecov/patch

qubes/vm/qubesvm.py#L813-L814

Added lines #L813 - L814 were not covered by tests

@qubes.stateless_property
def stubdom_xid(self):
if not self.is_running():
Expand Down Expand Up @@ -1807,9 +1819,11 @@ async def start_qrexec_daemon(self, stubdom=False):

self.log.debug('Starting the qrexec daemon')
if stubdom:
qrexec_args = [str(self.stubdom_xid), self.name + '-dm', 'root']
qrexec_args = ["-u", self.stubdom_uuid, "--",

Check warning on line 1822 in qubes/vm/qubesvm.py

View check run for this annotation

Codecov / codecov/patch

qubes/vm/qubesvm.py#L1822

Added line #L1822 was not covered by tests
str(self.stubdom_xid), self.name + '-dm', 'root']
else:
qrexec_args = [str(self.xid), self.name, self.default_user]
qrexec_args = ["-u", str(self.uuid), "--",

Check warning on line 1825 in qubes/vm/qubesvm.py

View check run for this annotation

Codecov / codecov/patch

qubes/vm/qubesvm.py#L1825

Added line #L1825 was not covered by tests
str(self.xid), self.name, self.default_user]

if not self.debug:
qrexec_args.insert(0, "-q")
Expand Down

0 comments on commit 8039442

Please sign in to comment.