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 Apr 19, 2024
1 parent ec10371 commit 8315e54
Show file tree
Hide file tree
Showing 9 changed files with 134 additions and 32 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

Expand Down Expand Up @@ -94,6 +97,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:
# 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)

class AbstractQubesAPI:
'''Common code for Qubes Management Protocol handling
Expand All @@ -114,25 +143,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 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
15 changes: 10 additions & 5 deletions qubes/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,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 @@ -1134,10 +1135,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(
"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 @@ -1150,7 +1155,7 @@ async def create_disposable(self):
# TODO: move this to extension (in race-free fashion, better than here)
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
54 changes: 47 additions & 7 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 @@ -45,6 +47,7 @@
'pool', 'vid', 'size', 'usage', 'rw', 'source', 'path',
'save_on_stop', 'snap_on_start', 'revisions_to_keep', 'ephemeral']

_uuid_regex = re.compile(r"\A[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 @@ -77,6 +80,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 @@ -123,19 +127,35 @@ def call_internal_mgmt_func(self, method, dest, arg=b'', payload=b''):
mgmt_obj.execute(untrusted_payload=payload))
return response


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'
f'test-template class=TemplateVM state=Halted 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 @@ -152,8 +172,8 @@ 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 @@ -2619,6 +2639,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
16 changes: 14 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 @@ -784,6 +785,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(
'', '/local/domain/{}/vm'.format(
stubdom_xid))
assert _vm_uuid_re.match(stubdom_uuid), "Invalid UUID in XenStore"
return stubdom_uuid[4:].decode("ascii", "strict")

@qubes.stateless_property
def stubdom_xid(self):
if not self.is_running():
Expand Down Expand Up @@ -1740,9 +1752,9 @@ 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, "--", 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), "--", str(self.xid), self.name, self.default_user]

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

0 comments on commit 8315e54

Please sign in to comment.