diff --git a/doc/qubes-api.rst b/doc/qubes-api.rst new file mode 100644 index 000000000..a121cceb6 --- /dev/null +++ b/doc/qubes-api.rst @@ -0,0 +1,140 @@ +:py:mod:`qubes.api` -- API +================================= + +API sanitization is very important to maintain the AdminVM and everything it +controls, secure. + +**Simple rules**: + +* Handle exceptions gracefully. Unhandled exceptions have the traceback only in + the AdminVM, not allowing clients to know what happened. More information + about exceptions can be found at :py:class:`qubes.exc`. +* Failure to serve a request must raise exceptions that can be understood by the + client. Each stage below allows a different class of exception to be shown. + Be careful to not reveal more information than what the client is allowed to + know at each stage. + +**Stages**: + +.# Sanitize data + + * Must never print, log or store untrusted data. Be careful when throwing an + exception. + * Qrexec sanitizes the argument, but if you will use the argument to + something that you know has an even stricter syntax, it must be sanitized + also. + * The payload is received raw, in bytes. It must always be sanitized before + being passed to functions that expect it to be already trusted. + * When sanitizing data, if it may reveal information from the system, such as + object existence, then, throw :py:class:`qubes.exc.PermissionDenied` to + avoid leaking object existence or delay reveal till after + `admin-permission`. + +.# Fire permission event + + * Must only pass sanitized information. + * The most commonly used is + :py:meth:`qubes.api.AbstractQubesAPI.fire_event_for_permission`, which + fires event `admin-permission` directly, used by + :py:class:`qubes.api.admin.AdminExtension`. For more complex cases, + involving global information such as fetching objects from different + destinations, :py:meth:`qubes.api.AbstractQubesAPI.fire_event_for_filter` + is more appropriate, as it fires `admin-permission` for each operation + required. + +.# Action + + * The client is fully authorized at this stage, it passed Qrexec policy + evaluation and Qubesd `admin-permission`. The server may be aware that some + resource could not be served before the `admin-permisison`, but it was not + allowed to reveal at that stage, now it can reveal what and why it failed. + A custom exception derived from :py:class:`qubes.exc.QubesException` can be + used to allow the client to handle it gracefully. + * Act. + +.. code-block:: python + + @qubes.api.method( + "dest.feat.Set", # RPC name + wants_arg=True, # Argument must be provided + wants_payload=None, # Payload can be provided + dest_adminvm=False, # Target must not be AdminVM + scope="global", # Applies to the whole system + read=True, # Will read system information + write=True, # Will write information to the system + ) + async def dest_feat_set(self, untrusted_payload): + """ + Set destination feature + + name: self.arg + value: untrusted_payload + """ + # Qrexec sanitizes self.arg, but our feature name can only be made of + # letters. The client should have know to not make such a request in the + # first case, therefore it throws qubes.exc.ProtocolError. + allowed_chars = string.ascii_letters + self.enforce( + all(c in allowed_chars for c in self.arg), + reason="Feature name must be in safe set: " + allowed_chars, + ) + + # Payload is in bytes and we receive it without being sanitized + # previously. We only want to allow values to be in ASCII. + try: + untrusted_value = untrusted_payload.decode("ascii", errors="strict") + except UnicodeDecodeError: + raise qubes.exc.ProtocolError("Value contains non-ASCII characters") + # Delete untrusted payload prevent using it. + del untrusted_payload + + # Second sanitization of the value + if re.match(r"\A[\x20-\x7E]*\Z", untrusted_value) is None: + raise qubes.exc.ProtocolError( + f"{self.arg} value contains illegal characters" + ) + # Delete untrusted value prevent using it. + value = untrusted_value + del untrusted_value + + # In this case, we just want to allow setting value to features that are + # already set. We want to hide hide "absent" feature from "prohibited" + # feature (see "admin-permission" below), therefore, it throws + # qubes.exc.PermissionDenied. + self.enforce_arg( + wants=self.dest.features.keys(), + short_reason="destination features", + ) + + # Event "admin-permission" is used to prohibit certain API calls from + # qubesd when qrexec cannot possibly be that restrictive, as it doesn't + # have full knowledge of the system nor the policy is expressive enough. + # Only trusted data should be passed to this method. + # Throws qubes.exc.PermissionDenied if call is prohibited. + self.fire_event_for_permission(value=value) + + # The server is in a bad mood today. Let the user know we will not + # serve them today. + if True: + raise qubes.exc.QubesException( + "Not in a good mood today, feature '%r' doesn't look nice" % + self.arg + ) + + # All validation has passed, we can return the requested data. + self.dest.features[self.arg] = value + self.app.save() + +Inheritance diagram +------------------- + +.. inheritance-diagram:: qubes.api + +Module contents +--------------- + +.. autoclass:: qubes.api.admin +.. autoclass:: qubes.api.internal +.. autoclass:: qubes.api.misc + +.. vim: ts=3 sw=3 et tw=80 diff --git a/doc/qubes-exc.rst b/doc/qubes-exc.rst index f3a644b14..0b1c45e40 100644 --- a/doc/qubes-exc.rst +++ b/doc/qubes-exc.rst @@ -43,7 +43,7 @@ call for effect). Instead consider writing in negative form, implying expected state: "Domain is not running" instead of "Domain is paused" (yeah, what's wrong with that?). -Also avoid implying the personhood of the computer, including adressing user in +Also avoid implying the personhood of the computer, including addressing user in second person. For example, write "Sending message failed" instead of "I failed to send the message". diff --git a/qubes-rpc-policy/generate-admin-policy b/qubes-rpc-policy/generate-admin-policy index a724a375c..31ff0b196 100755 --- a/qubes-rpc-policy/generate-admin-policy +++ b/qubes-rpc-policy/generate-admin-policy @@ -43,19 +43,19 @@ parser.add_argument('--exclude', action='store', nargs='*', parser.add_argument('service', nargs='*', action='store', help='Generate policy for those services (default: all)') -def write_default_policy(args, apiname, clasifiers, f): +def write_default_policy(args, apiname, classifiers, f): ''' Write single default policy for given API call ''' - assert 'scope' in clasifiers, \ + assert 'scope' in classifiers, \ 'Method {} lack scope classifier'.format(apiname) - assert any(attr in clasifiers for attr in ('read', 'write', 'execute')), \ + assert any(attr in classifiers for attr in ('read', 'write', 'execute')), \ 'Method {} lack read/write/execute classifier'.format(apiname) - assert clasifiers['scope'] in ('local', 'global'), \ - 'Method {} have invalid scope: {}'.format(apiname, clasifiers['scope']) + assert classifiers['scope'] in ('local', 'global'), \ + 'Method {} have invalid scope: {}'.format(apiname, classifiers['scope']) file_to_include = 'admin-{scope}-{rwx}'.format( - scope=clasifiers['scope'], - rwx=('rwx' if clasifiers.get('write', False) or - clasifiers.get('execute', False) + scope=classifiers['scope'], + rwx=('rwx' if classifiers.get('write', False) or + classifiers.get('execute', False) else 'ro')) if args.verbose: diff --git a/qubes/api/__init__.py b/qubes/api/__init__.py index 764c98679..491bd2f9b 100644 --- a/qubes/api/__init__.py +++ b/qubes/api/__init__.py @@ -27,16 +27,28 @@ import re import shutil import socket +import string import struct import traceback -from typing import Union, Any +from typing import Union, Any, Literal, Generator import uuid import qubes.exc from qubes.exc import ProtocolError, PermissionDenied -def method(name, *, no_payload=False, endpoints=None, **classifiers): +def method( + name, + *, + dest_adminvm: bool | None, + wants_arg: bool | None, + wants_payload: bool | None, + scope: Literal["local", "global"] | None = None, + read: bool = True, + write: bool = False, + execute: bool = False, + endpoints: Generator[str, None, None] | None = None, +): """Decorator factory for methods intended to appear in API. The decorated method can be called from public API using a child of @@ -44,10 +56,27 @@ def method(name, *, no_payload=False, endpoints=None, **classifiers): called using remote management interface. :param str name: qrexec rpc method name - :param bool no_payload: if :py:obj:`True`, will barf on non-empty payload; \ - also will not pass payload at all to the method + :param bool dest_adminvm: if :py:obj:`True`, will barf if destination is \ + not AdminVM; If :py:obj:`False`, will barf if destination is AdminVM. \ + If :py:obj:`None`, checks are disabled. + :param bool wants_arg: if :py:obj:`False`, will barf on non-empty \ + argument; also will not pass argument at all to the method. If \ + :py:obj:`True`, will barf on empty argument. If :py:obj:`None`, \ + checks are disabled. + :param bool wants_payload: if :py:obj:`False`, will barf on non-empty \ + payload; also will not pass payload at all to the method. If \ + :py:obj:`True`, will barf on empty payload. If :py:obj:`None`, \ + checks are disabled. :param iterable endpoints: if specified, method serve multiple API calls generated by replacing `{endpoint}` with each value in this iterable + :param bool scope: if `global` or `local`, will include the service to the \ + global section of included policy directories. + :param bool read: if :py:obj:`True` will add the service to the included \ + policy directories with read capabilities. + :param bool write: if :py:obj:`True` will add the service to the included \ + policy directories with write capabilities. + :param bool read: if :py:obj:`True` will add the service to the included \ + policy directories with execute capabilities. The expected function method should have one argument (other than usual *self*), ``untrusted_payload``, which will contain the payload. @@ -55,22 +84,27 @@ def method(name, *, no_payload=False, endpoints=None, **classifiers): .. warning:: This argument has to be named such, to remind the programmer that the content of this variable is indeed untrusted. - - If *no_payload* is true, then the method is called with no arguments. """ def decorator(func): - if no_payload: - # the following assignment is needed for how closures work in Python - _func = func - - @functools.wraps(_func) - def wrapper(self, untrusted_payload, **kwargs): - if untrusted_payload != b"": - raise ProtocolError("unexpected payload") - return _func(self, **kwargs) - - func = wrapper + if wants_arg is not None and not isinstance(wants_arg, bool): + raise ValueError("Invalid value for 'wants_arg'") + if wants_payload is not None and not isinstance(wants_payload, bool): + raise ValueError("Invalid value for 'wants_payload'") + if dest_adminvm is not None and not isinstance(dest_adminvm, bool): + raise ValueError("Invalid value for 'dest_adminvm'") + if not isinstance(read, bool): + raise ValueError("Invalid value for 'read'") + if not isinstance(write, bool): + raise ValueError("Invalid value for 'write'") + if not isinstance(execute, bool): + raise ValueError("Invalid value for 'execute'") + if scope not in [None, "local", "global"]: + raise ValueError("Invalid value for 'scope'") + + func.api_wants_arg = wants_arg + func.api_wants_payload = wants_payload + func.api_dest_adminvm = dest_adminvm # pylint: disable=protected-access if endpoints is None: @@ -81,8 +115,12 @@ def wrapper(self, untrusted_payload, **kwargs): for endpoint in endpoints ) - func.classifiers = classifiers - + func.classifiers = { + "read": read, + "write": write, + "execute": execute, + "scope": scope, + } return func return decorator @@ -189,6 +227,9 @@ def __init__( self._handler = candidates[0] self._running_handler = None + # pylint: disable=invalid-name + self.EXC_ARG_NOT_IN_DEST_VOLUMES = "{} volumes".format(self.dest.name) + @classmethod def list_methods(cls, select_method=None): for attr in dir(cls): @@ -215,9 +256,30 @@ def execute(self, *, untrusted_payload): kwargs = {} if endpoint is not None: kwargs["endpoint"] = endpoint - self._running_handler = asyncio.ensure_future( - handler(self, untrusted_payload=untrusted_payload, **kwargs) - ) + + if handler.api_wants_payload is True: + if untrusted_payload == b"": + raise ProtocolError("expected payload") + kwargs["untrusted_payload"] = untrusted_payload + if handler.api_wants_payload is False and untrusted_payload != b"": + raise ProtocolError("unexpected payload") + if handler.api_wants_payload is None: + kwargs["untrusted_payload"] = untrusted_payload + del untrusted_payload + + if handler.api_wants_arg is True and not self.arg: + raise ProtocolError("expected an argument") + if handler.api_wants_arg is False: + if self.arg: + raise ProtocolError("unexpected argument") + del self.arg + + if handler.api_dest_adminvm is True and self.dest.klass != "AdminVM": + raise ProtocolError("expected destination to be AdminVM") + if handler.api_dest_adminvm is False and self.dest.klass == "AdminVM": + raise ProtocolError("expected destination to not be AdminVM") + + self._running_handler = asyncio.ensure_future(handler(self, **kwargs)) return self._running_handler def cancel(self): @@ -231,42 +293,70 @@ def fire_event_for_permission(self, **kwargs): "admin-permission:" + self.method, pre_event=True, dest=self.dest, - arg=self.arg, - **kwargs + arg=getattr(self, "arg", ""), + **kwargs, ) def fire_event_for_filter(self, iterable, **kwargs): """Fire an event on the source qube to filter for permission""" return apply_filters(iterable, self.fire_event_for_permission(**kwargs)) + def enforce_arg( + self, + wants: Union[list, tuple], + short_reason: str = "", + ) -> None: + """Enforce argument to be absent or be in iterable.""" + assert isinstance(self.arg, str) + assert short_reason + if self.arg not in wants: + raise PermissionDenied("Argument not in {!r}".format(short_reason)) + @staticmethod - def enforce(predicate): - """An assert replacement, but works even with optimisations.""" + def enforce(predicate, reason: str = "") -> None: + """If predicate is false, raise an exception to terminate handling + the request. + + This will raise :py:class:`ProtocolError` if the predicate is false. + See the documentation of that class for details. + + :param str reason: Exception motive. + """ if not predicate: - raise PermissionDenied() + raise ProtocolError(reason) - def validate_size( - self, untrusted_size: bytes, allow_negative: bool = False + def validate_number( + self, untrusted_number: bytes, name: str, allow_negative: bool = False ) -> int: - self.enforce(isinstance(untrusted_size, bytes)) + allowed_chars = string.ascii_letters + string.digits + "-_. " + assert all(c in allowed_chars for c in name) + if not isinstance(untrusted_number, bytes): + raise ProtocolError( + "Expected {!r} to be of type bytes, got {!r}".format( + name, type(untrusted_number).__name__ + ) + ) coefficient = 1 - if allow_negative and untrusted_size.startswith(b"-"): + if allow_negative and untrusted_number.startswith(b"-"): coefficient = -1 - untrusted_size = untrusted_size[1:] - if not untrusted_size.isdigit(): - raise qubes.exc.ProtocolError("Size must be ASCII digits (only)") - if len(untrusted_size) >= 20: - raise qubes.exc.ProtocolError("Sizes limited to 19 decimal digits") - if untrusted_size[0] == 48 and untrusted_size != b"0": - raise qubes.exc.ProtocolError("Spurious leading zeros not allowed") - return int(untrusted_size) * coefficient + untrusted_number = untrusted_number[1:] + name_cap = name.capitalize() + if not untrusted_number.isdigit(): + raise ProtocolError(name_cap + " contains non ASCII digits") + if len(untrusted_number) >= 20: + raise ProtocolError(name_cap + " is bigger than 19 decimal digits") + if untrusted_number[0] == 48 and untrusted_number != b"0": + raise ProtocolError(name_cap + " contains spurious leading zeros") + return int(untrusted_number) * coefficient class QubesDaemonProtocol(asyncio.Protocol): buffer_size = 65536 header = struct.Struct("Bx") - def __init__(self, handler, *args, app, debug=False, **kwargs): + def __init__( + self, handler, *args, app, debug=False, log_dom0_call=False, **kwargs + ): super().__init__(*args, **kwargs) self.handler = handler self.app = app @@ -274,6 +364,7 @@ def __init__(self, handler, *args, app, debug=False, **kwargs): self.len_untrusted_buffer = 0 self.transport = None self.debug = debug + self.log_dom0_call = log_dom0_call self.event_sent = False self.mgmt = None @@ -332,6 +423,20 @@ def eof_received(self): return True async def respond(self, src, meth, dest, arg, *, untrusted_payload): + if self.log_dom0_call: + call_fmt = "received call %r+%r (%r → %r) with payload of %d bytes" + self.app.log.debug( + call_fmt, + meth, + arg, + src, + dest, + len(untrusted_payload), + ) + exc_fmt = ( + "failed call %r+%r (%r → %r) with payload of %d bytes due to %r" + ) + success = False try: self.mgmt = self.handler( self.app, src, meth, dest, arg, self.send_event @@ -343,62 +448,47 @@ async def respond(self, src, meth, dest, arg, *, untrusted_payload): if self.transport is None: return - # except clauses will fall through to transport.abort() below - - except PermissionDenied: + except (PermissionDenied, ProtocolError) as exc: self.app.log.warning( - "permission denied for call %s+%s (%s → %s) " - "with payload of %d bytes", + exc_fmt, meth, arg, src, dest, len(untrusted_payload), + exc, ) + if self.transport is not None: + self.send_exception(exc) + self.transport.write_eof() - except ProtocolError: - self.app.log.warning( - "protocol error for call %s+%s (%s → %s) " - "with payload of %d bytes", - meth, - arg, - src, - dest, - len(untrusted_payload), - ) - - except qubes.exc.QubesException as err: - msg = ( - "%r while calling " - "src=%r meth=%r dest=%r arg=%r len(untrusted_payload)=%d" - ) - + except qubes.exc.QubesException as exc: if self.debug: self.app.log.debug( - msg, - err, - src, + exc_fmt, meth, - dest, arg, + src, + dest, len(untrusted_payload), + exc, exc_info=1, ) if self.transport is not None: - self.send_exception(err) + self.send_exception(exc) self.transport.write_eof() self.transport.close() return except Exception: # pylint: disable=broad-except self.app.log.exception( - "unhandled exception while calling " - "src=%r meth=%r dest=%r arg=%r len(untrusted_payload)=%d", - src, + exc_fmt, meth, - dest, arg, + src, + dest, len(untrusted_payload), + "unhandled exception", ) else: @@ -408,13 +498,12 @@ async def respond(self, src, meth, dest, arg, *, untrusted_payload): self.transport.write_eof() except NotImplementedError: pass - self.transport.close() - return - - # this is reached if from except: blocks; do not put it in finally:, - # because this will prevent the good case from sending the reply - if self.transport: - self.transport.abort() + success = True + finally: + if success: + self.transport.close() + elif self.transport: + self.transport.abort() def send_header(self, *args): self.transport.write(self.header.pack(*args)) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index 2b86b4cee..fc0cb6add 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -112,13 +112,15 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): SOCKNAME = "/var/run/qubesd.sock" @qubes.api.method( - "admin.vmclass.List", no_payload=True, scope="global", read=True + "admin.vmclass.List", + wants_payload=False, + wants_arg=False, + dest_adminvm=True, + scope="global", + read=True, ) async def vmclass_list(self): """List all VM classes""" - self.enforce(not self.arg) - self.enforce(self.dest.name == "dom0") - entrypoints = self.fire_event_for_filter( importlib.metadata.entry_points(group=qubes.vm.VM_ENTRY_POINT) ) @@ -126,12 +128,15 @@ async def vmclass_list(self): return "".join("{}\n".format(ep.name) for ep in entrypoints) @qubes.api.method( - "admin.vm.List", no_payload=True, scope="global", read=True + "admin.vm.List", + wants_payload=False, + wants_arg=False, + dest_adminvm=None, + scope="global", + read=True, ) async def vm_list(self): """List all the domains""" - self.enforce(not self.arg) - if self.dest.name == "dom0": domains = self.fire_event_for_filter(self.app.domains) else: @@ -145,40 +150,56 @@ async def vm_list(self): ) @qubes.api.method( - "admin.vm.property.List", no_payload=True, scope="local", read=True + "admin.vm.property.List", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_property_list(self): """List all properties on a qube""" return self._property_list(self.dest) @qubes.api.method( - "admin.property.List", no_payload=True, scope="global", read=True + "admin.property.List", + wants_arg=False, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def property_list(self): """List all global properties""" - self.enforce(self.dest.name == "dom0") return self._property_list(self.app) def _property_list(self, dest): - self.enforce(not self.arg) - properties = self.fire_event_for_filter(dest.property_list()) return "".join("{}\n".format(prop.__name__) for prop in properties) @qubes.api.method( - "admin.vm.property.Get", no_payload=True, scope="local", read=True + "admin.vm.property.Get", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_property_get(self): """Get a value of one property""" return self._property_get(self.dest) @qubes.api.method( - "admin.property.Get", no_payload=True, scope="global", read=True + "admin.property.Get", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def property_get(self): """Get a value of one global property""" - self.enforce(self.dest.name == "dom0") return self._property_get(self.app) def _property_get(self, dest): @@ -215,23 +236,30 @@ def _serialize_property(dest, prop): ) @qubes.api.method( - "admin.vm.property.GetAll", no_payload=True, scope="local", read=True + "admin.vm.property.GetAll", + wants_payload=False, + wants_arg=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_property_get_all(self): """Get values of all VM properties""" return self._property_get_all(self.dest) @qubes.api.method( - "admin.property.GetAll", no_payload=True, scope="global", read=True + "admin.property.GetAll", + wants_arg=False, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def property_get_all(self): """Get value all global properties""" - self.enforce(self.dest.name == "dom0") return self._property_get_all(self.app) def _property_get_all(self, dest): - self.enforce(not self.arg) - properties = dest.property_list() properties = self.fire_event_for_filter(properties) @@ -248,7 +276,9 @@ def _property_get_all(self, dest): @qubes.api.method( "admin.vm.property.GetDefault", - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -258,12 +288,16 @@ async def vm_property_get_default(self): return self._property_get_default(self.dest) @qubes.api.method( - "admin.property.GetDefault", no_payload=True, scope="global", read=True + "admin.property.GetDefault", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def property_get_default(self): """Get default value of a global property (what value will be set if this property is set to default).""" - self.enforce(self.dest.name == "dom0") return self._property_get_default(self.app) def _property_get_default(self, dest): @@ -293,17 +327,30 @@ def _property_get_default(self, dest): property_type, str(value) if value is not None else "" ) - @qubes.api.method("admin.vm.property.Set", scope="local", write=True) + @qubes.api.method( + "admin.vm.property.Set", + wants_arg=True, + wants_payload=None, + dest_adminvm=None, + scope="local", + write=True, + ) async def vm_property_set(self, untrusted_payload): """Set property value""" return self._property_set( self.dest, untrusted_payload=untrusted_payload ) - @qubes.api.method("admin.property.Set", scope="global", write=True) + @qubes.api.method( + "admin.property.Set", + wants_arg=True, + wants_payload=None, + dest_adminvm=True, + scope="global", + write=True, + ) async def property_set(self, untrusted_payload): """Set property value""" - self.enforce(self.dest.name == "dom0") return self._property_set(self.app, untrusted_payload=untrusted_payload) def _property_set(self, dest, untrusted_payload): @@ -319,18 +366,27 @@ def _property_set(self, dest, untrusted_payload): self.app.save() @qubes.api.method( - "admin.vm.property.Help", no_payload=True, scope="local", read=True + "admin.vm.property.Help", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_property_help(self): """Get help for one property""" return self._property_help(self.dest) @qubes.api.method( - "admin.property.Help", no_payload=True, scope="global", read=True + "admin.property.Help", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def property_help(self): """Get help for one property""" - self.enforce(self.dest.name == "dom0") return self._property_help(self.app) def _property_help(self, dest): @@ -347,18 +403,27 @@ def _property_help(self, dest): return qubes.utils.format_doc(doc) @qubes.api.method( - "admin.vm.property.Reset", no_payload=True, scope="local", write=True + "admin.vm.property.Reset", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_property_reset(self): """Reset a property to a default value""" return self._property_reset(self.dest) @qubes.api.method( - "admin.property.Reset", no_payload=True, scope="global", write=True + "admin.property.Reset", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + write=True, ) async def property_reset(self): """Reset a property to a default value""" - self.enforce(self.dest.name == "dom0") return self._property_reset(self.app) def _property_reset(self, dest): @@ -371,11 +436,14 @@ def _property_reset(self, dest): self.app.save() @qubes.api.method( - "admin.vm.volume.List", no_payload=True, scope="local", read=True + "admin.vm.volume.List", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_volume_list(self): - self.enforce(not self.arg) - volume_names = ( self.fire_event_for_filter(self.dest.volumes.keys()) if isinstance(self.dest, qubes.vm.qubesvm.QubesVM) @@ -384,10 +452,18 @@ async def vm_volume_list(self): return "".join("{}\n".format(name) for name in volume_names) @qubes.api.method( - "admin.vm.volume.Info", no_payload=True, scope="local", read=True + "admin.vm.volume.Info", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_volume_info(self): - self.enforce(self.arg in self.dest.volumes.keys()) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) self.fire_event_for_permission() @@ -426,12 +502,17 @@ def _serialize(value): @qubes.api.method( "admin.vm.volume.ListSnapshots", - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) async def vm_volume_listsnapshots(self): - self.enforce(self.arg in self.dest.volumes.keys()) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) volume = self.dest.volumes[self.arg] id_to_timestamp = volume.revisions @@ -440,28 +521,55 @@ async def vm_volume_listsnapshots(self): return "".join("{}\n".format(revision) for revision in revisions) - @qubes.api.method("admin.vm.volume.Revert", scope="local", write=True) + @qubes.api.method( + "admin.vm.volume.Revert", + wants_arg=True, + wants_payload=True, + dest_adminvm=None, + scope="local", + write=True, + ) async def vm_volume_revert(self, untrusted_payload): - self.enforce(self.arg in self.dest.volumes.keys()) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) untrusted_revision = untrusted_payload.decode("ascii").strip() del untrusted_payload + allowed_chars = string.ascii_letters + string.digits + ":-_." + self.enforce( + all(c in allowed_chars for c in untrusted_revision), + reason="Revision name must be in safe set: " + allowed_chars, + ) + revision = untrusted_revision + del untrusted_revision volume = self.dest.volumes[self.arg] - snapshots = volume.revisions - self.enforce(untrusted_revision in snapshots) - revision = untrusted_revision self.fire_event_for_permission(volume=volume, revision=revision) + + snapshots = volume.revisions + if revision not in snapshots: + raise qubes.exc.QubesVolumeRevisionNotFoundError() + await qubes.utils.coro_maybe(volume.revert(revision)) self.app.save() # write=True because this allow to clone VM - and most likely modify that # one - still having the same data @qubes.api.method( - "admin.vm.volume.CloneFrom", no_payload=True, scope="local", write=True + "admin.vm.volume.CloneFrom", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_volume_clone_from(self): - self.enforce(self.arg in self.dest.volumes.keys()) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) volume = self.dest.volumes[self.arg] @@ -473,19 +581,38 @@ async def vm_volume_clone_from(self): self.app.api_admin_pending_clone = {} # don't handle collisions any better - if someone is so much out of # luck, can try again anyway - self.enforce(token not in self.app.api_admin_pending_clone) + if token in self.app.api_admin_pending_clone: + raise qubes.exc.QubesVolumeCopyInUseError( + vm=self.dest, volume=self.arg + ) self.app.api_admin_pending_clone[token] = volume return token - @qubes.api.method("admin.vm.volume.CloneTo", scope="local", write=True) + @qubes.api.method( + "admin.vm.volume.CloneTo", + wants_arg=True, + wants_payload=True, + dest_adminvm=None, + scope="local", + write=True, + ) async def vm_volume_clone_to(self, untrusted_payload): - self.enforce(self.arg in self.dest.volumes.keys()) - untrusted_token = untrusted_payload.decode("ascii").strip() - del untrusted_payload - self.enforce( - untrusted_token in getattr(self.app, "api_admin_pending_clone", {}) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, ) + try: + untrusted_token = untrusted_payload.decode("ascii").strip() + except UnicodeDecodeError: + raise qubes.exc.ProtocolError("Token contains non-ASCII characters") + + del untrusted_payload + if untrusted_token not in getattr( + self.app, "api_admin_pending_clone", {} + ): + raise qubes.exc.QubesVolumeCopyTokenNotFoundError() + token = untrusted_token del untrusted_token @@ -493,8 +620,10 @@ async def vm_volume_clone_to(self, untrusted_payload): del self.app.api_admin_pending_clone[token] # make sure the volume still exists, but invalidate token anyway - self.enforce(str(src_volume.pool) in self.app.pools) - self.enforce(src_volume in self.app.pools[str(src_volume.pool)].volumes) + if str(src_volume.pool) not in self.app.pools: + raise qubes.exc.QubesPoolNotFoundError() + if src_volume not in self.app.pools[str(src_volume.pool)].volumes: + raise qubes.exc.QubesVolumeNotFoundError() dst_volume = self.dest.volumes[self.arg] @@ -506,10 +635,22 @@ async def vm_volume_clone_to(self, untrusted_payload): ) self.app.save() - @qubes.api.method("admin.vm.volume.Resize", scope="local", write=True) + @qubes.api.method( + "admin.vm.volume.Resize", + wants_arg=True, + wants_payload=True, + dest_adminvm=None, + scope="local", + write=True, + ) async def vm_volume_resize(self, untrusted_payload): - self.enforce(self.arg in self.dest.volumes.keys()) - size = self.validate_size(untrusted_payload.strip()) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) + size = self.validate_number( + untrusted_payload.strip(), name="volume size" + ) self.fire_event_for_permission(size=size) try: await self.dest.storage.resize(self.arg, size) @@ -517,10 +658,18 @@ async def vm_volume_resize(self, untrusted_payload): self.app.save() @qubes.api.method( - "admin.vm.volume.Clear", no_payload=True, scope="local", write=True + "admin.vm.volume.Clear", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_volume_clear(self): - self.enforce(self.arg in self.dest.volumes.keys()) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) self.fire_event_for_permission() @@ -546,24 +695,44 @@ async def vm_volume_clear(self): self.app.save() @qubes.api.method( - "admin.vm.volume.Set.revisions_to_keep", scope="local", write=True + "admin.vm.volume.Set.revisions_to_keep", + wants_arg=True, + wants_payload=True, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_volume_set_revisions_to_keep(self, untrusted_payload): - self.enforce(self.arg in self.dest.volumes.keys()) - newvalue = self.validate_size(untrusted_payload, allow_negative=True) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) + newvalue = self.validate_number( + untrusted_payload, name="revisions to keep", allow_negative=True + ) self.fire_event_for_permission(newvalue=newvalue) self.dest.storage.set_revisions_to_keep(self.arg, newvalue) self.app.save() - @qubes.api.method("admin.vm.volume.Set.rw", scope="local", write=True) + @qubes.api.method( + "admin.vm.volume.Set.rw", + wants_arg=True, + wants_payload=True, + dest_adminvm=None, + scope="local", + write=True, + ) async def vm_volume_set_rw(self, untrusted_payload): - self.enforce(self.arg in self.dest.volumes.keys()) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) try: newvalue = qubes.property.bool( None, None, untrusted_payload.decode("ascii") ) except (UnicodeDecodeError, ValueError): - raise qubes.exc.ProtocolError("Invalid value") + raise qubes.exc.ProtocolError("Payload is not boolean ASCII") del untrusted_payload self.fire_event_for_permission(newvalue=newvalue) @@ -575,16 +744,24 @@ async def vm_volume_set_rw(self, untrusted_payload): self.app.save() @qubes.api.method( - "admin.vm.volume.Set.ephemeral", scope="local", write=True + "admin.vm.volume.Set.ephemeral", + wants_arg=True, + wants_payload=True, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_volume_set_ephemeral(self, untrusted_payload): - self.enforce(self.arg in self.dest.volumes.keys()) + self.enforce_arg( + wants=self.dest.volumes.keys(), + short_reason=self.EXC_ARG_NOT_IN_DEST_VOLUMES, + ) try: newvalue = qubes.property.bool( None, None, untrusted_payload.decode("ascii") ) except (UnicodeDecodeError, ValueError): - raise qubes.exc.ProtocolError("Invalid value") + raise qubes.exc.ProtocolError("Payload is not boolean ASCII") del untrusted_payload self.fire_event_for_permission(newvalue=newvalue) @@ -596,11 +773,14 @@ async def vm_volume_set_ephemeral(self, untrusted_payload): self.app.save() @qubes.api.method( - "admin.vm.tag.List", no_payload=True, scope="local", read=True + "admin.vm.tag.List", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_tag_list(self): - self.enforce(not self.arg) - tags = self.dest.tags tags = self.fire_event_for_filter(tags) @@ -608,7 +788,12 @@ async def vm_tag_list(self): return "".join("{}\n".format(tag) for tag in sorted(tags)) @qubes.api.method( - "admin.vm.tag.Get", no_payload=True, scope="local", read=True + "admin.vm.tag.Get", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_tag_get(self): qubes.vm.Tags.validate_tag(self.arg) @@ -618,7 +803,12 @@ async def vm_tag_get(self): return "1" if self.arg in self.dest.tags else "0" @qubes.api.method( - "admin.vm.tag.Set", no_payload=True, scope="local", write=True + "admin.vm.tag.Set", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_tag_set(self): qubes.vm.Tags.validate_tag(self.arg) @@ -629,7 +819,12 @@ async def vm_tag_set(self): self.app.save() @qubes.api.method( - "admin.vm.tag.Remove", no_payload=True, scope="local", write=True + "admin.vm.tag.Remove", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_tag_remove(self): qubes.vm.Tags.validate_tag(self.arg) @@ -643,11 +838,14 @@ async def vm_tag_remove(self): self.app.save() @qubes.api.method( - "admin.vm.Console", no_payload=True, scope="local", write=True + "admin.vm.Console", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_console(self): - self.enforce(not self.arg) - self.fire_event_for_permission() if not self.dest.is_running(): @@ -660,23 +858,27 @@ async def vm_console(self): return ttypath @qubes.api.method( - "admin.pool.List", no_payload=True, scope="global", read=True + "admin.pool.List", + wants_arg=False, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def pool_list(self): - self.enforce(not self.arg) - self.enforce(self.dest.name == "dom0") - pools = self.fire_event_for_filter(self.app.pools) return "".join("{}\n".format(pool) for pool in pools) @qubes.api.method( - "admin.pool.ListDrivers", no_payload=True, scope="global", read=True + "admin.pool.ListDrivers", + wants_arg=False, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def pool_listdrivers(self): - self.enforce(self.dest.name == "dom0") - self.enforce(not self.arg) - drivers = self.fire_event_for_filter(qubes.storage.pool_drivers()) return "".join( @@ -687,11 +889,15 @@ async def pool_listdrivers(self): ) @qubes.api.method( - "admin.pool.Info", no_payload=True, scope="global", read=True + "admin.pool.Info", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def pool_info(self): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg in self.app.pools.keys()) + self.enforce_arg(wants=self.app.pools.keys(), short_reason="pools") pool = self.app.pools[self.arg] @@ -723,11 +929,15 @@ async def pool_info(self): ) @qubes.api.method( - "admin.pool.UsageDetails", no_payload=True, scope="global", read=True + "admin.pool.UsageDetails", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def pool_usage(self): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg in self.app.pools.keys()) + self.enforce_arg(wants=self.app.pools.keys(), short_reason="pools") pool = self.app.pools[self.arg] @@ -742,17 +952,32 @@ async def pool_usage(self): return usage - @qubes.api.method("admin.pool.Add", scope="global", write=True) + @qubes.api.method( + "admin.pool.Add", + wants_arg=True, + wants_payload=True, + dest_adminvm=True, + scope="global", + write=True, + ) async def pool_add(self, untrusted_payload): - self.enforce(self.dest.name == "dom0") - if self.arg not in qubes.storage.pool_drivers(): - raise qubes.exc.QubesException( - "unexpected driver name: " + self.arg - ) + self.enforce_arg( + wants=qubes.storage.pool_drivers(), short_reason="available drivers" + ) - untrusted_pool_config = untrusted_payload.decode("ascii").splitlines() + try: + untrusted_pool_config = untrusted_payload.decode( + "ascii" + ).splitlines() + except UnicodeDecodeError: + raise qubes.exc.ProtocolError( + "Pool parameters contains non-ASCII characters" + ) del untrusted_payload - self.enforce(all(("=" in line) for line in untrusted_pool_config)) + self.enforce( + all(("=" in line) for line in untrusted_pool_config), + reason="Options must be separated by equal sign", + ) # pairs of (option, value) untrusted_pool_config = [ line.split("=", 1) for line in untrusted_pool_config @@ -760,17 +985,26 @@ async def pool_add(self, untrusted_payload): # reject duplicated options self.enforce( len(set(x[0] for x in untrusted_pool_config)) - == len([x[0] for x in untrusted_pool_config]) + == len([x[0] for x in untrusted_pool_config]), + reason="Options must be unique", ) # and convert to dict untrusted_pool_config = dict(untrusted_pool_config) - self.enforce("name" in untrusted_pool_config) + self.enforce( + "name" in untrusted_pool_config, + reason="Pool configuration must contain key: name", + ) untrusted_pool_name = untrusted_pool_config.pop("name") allowed_chars = string.ascii_letters + string.digits + "-_." - self.enforce(all(c in allowed_chars for c in untrusted_pool_name)) + self.enforce( + all(c in allowed_chars for c in untrusted_pool_name), + reason="Pool name must be in safe set: " + allowed_chars, + ) pool_name = untrusted_pool_name - self.enforce(pool_name not in self.app.pools) + invalid_pool = False + if pool_name in self.app.pools: + invalid_pool = True driver_parameters = qubes.storage.driver_parameters(self.arg) dp_names = driver_parameters.keys() @@ -778,9 +1012,7 @@ async def pool_add(self, untrusted_payload): key for key in untrusted_pool_config if key not in driver_parameters ] if unexpected_parameters: - raise qubes.exc.QubesException( - "unexpected driver options: " + " ".join(unexpected_parameters) - ) + raise qubes.exc.ProtocolError("Unexpected driver options found") required_parameters_unmet = [ key for key in dp_names @@ -796,15 +1028,22 @@ async def pool_add(self, untrusted_payload): self.fire_event_for_permission(name=pool_name, pool_config=pool_config) + if invalid_pool: + raise qubes.exc.QubesPoolInUseError(pool_name=pool_name) + await self.app.add_pool(name=pool_name, driver=self.arg, **pool_config) self.app.save() @qubes.api.method( - "admin.pool.Remove", no_payload=True, scope="global", write=True + "admin.pool.Remove", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + write=True, ) async def pool_remove(self): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg in self.app.pools.keys()) + self.enforce_arg(wants=self.app.pools.keys(), short_reason="pools") self.fire_event_for_permission() @@ -812,11 +1051,15 @@ async def pool_remove(self): self.app.save() @qubes.api.method( - "admin.pool.volume.List", no_payload=True, scope="global", read=True + "admin.pool.volume.List", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def pool_volume_list(self): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg in self.app.pools.keys()) + self.enforce_arg(wants=self.app.pools.keys(), short_reason="pools") pool = self.app.pools[self.arg] @@ -824,16 +1067,21 @@ async def pool_volume_list(self): return "".join("{}\n".format(name) for name in volume_names) @qubes.api.method( - "admin.pool.Set.revisions_to_keep", scope="global", write=True + "admin.pool.Set.revisions_to_keep", + wants_arg=True, + wants_payload=True, + dest_adminvm=True, + scope="global", + write=True, ) async def pool_set_revisions_to_keep(self, untrusted_payload): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg in self.app.pools.keys()) + self.enforce_arg(wants=self.app.pools.keys(), short_reason="pools") pool = self.app.pools[self.arg] - newvalue = self.validate_size(untrusted_payload, allow_negative=True) + newvalue = self.validate_number( + untrusted_payload, name="revisions to keep", allow_negative=True + ) - if newvalue < -1: - raise qubes.api.ProtocolError("Invalid value for revisions_to_keep") + self.enforce(newvalue >= -1, reason="Value can't be lower than -1") self.fire_event_for_permission(newvalue=newvalue) @@ -841,18 +1089,22 @@ async def pool_set_revisions_to_keep(self, untrusted_payload): self.app.save() @qubes.api.method( - "admin.pool.Set.ephemeral_volatile", scope="global", write=True + "admin.pool.Set.ephemeral_volatile", + wants_arg=True, + wants_payload=True, + dest_adminvm=True, + scope="global", + write=True, ) async def pool_set_ephemeral(self, untrusted_payload): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg in self.app.pools.keys()) + self.enforce_arg(wants=self.app.pools.keys(), short_reason="pools") pool = self.app.pools[self.arg] try: newvalue = qubes.property.bool( None, None, untrusted_payload.decode("ascii", "strict") ) except (UnicodeDecodeError, ValueError): - raise qubes.exc.ProtocolError("Invalid value") + raise qubes.exc.ProtocolError("Payload is not boolean ASCII") del untrusted_payload self.fire_event_for_permission(newvalue=newvalue) @@ -861,72 +1113,80 @@ async def pool_set_ephemeral(self, untrusted_payload): self.app.save() @qubes.api.method( - "admin.label.List", no_payload=True, scope="global", read=True + "admin.label.List", + wants_arg=False, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def label_list(self): - self.enforce(self.dest.name == "dom0") - self.enforce(not self.arg) - labels = self.fire_event_for_filter(self.app.labels.values()) return "".join("{}\n".format(label.name) for label in labels) @qubes.api.method( - "admin.label.Get", no_payload=True, scope="global", read=True + "admin.label.Get", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def label_get(self): - self.enforce(self.dest.name == "dom0") + qubes.utils.validate_label_name(untrusted_label=self.arg) - try: - label = self.app.get_label(self.arg) - except KeyError: - raise qubes.exc.QubesValueError + label = self.app.get_label(self.arg) self.fire_event_for_permission(label=label) return label.color @qubes.api.method( - "admin.label.Index", no_payload=True, scope="global", read=True + "admin.label.Index", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def label_index(self): - self.enforce(self.dest.name == "dom0") + qubes.utils.validate_label_name(untrusted_label=self.arg) - try: - label = self.app.get_label(self.arg) - except KeyError: - raise qubes.exc.QubesValueError + label = self.app.get_label(self.arg) self.fire_event_for_permission(label=label) return str(label.index) - @qubes.api.method("admin.label.Create", scope="global", write=True) + @qubes.api.method( + "admin.label.Create", + wants_arg=True, + wants_payload=True, + dest_adminvm=True, + scope="global", + write=True, + ) async def label_create(self, untrusted_payload): - self.enforce(self.dest.name == "dom0") + qubes.utils.validate_label_name(untrusted_label=self.arg, creation=True) + + qubes.utils.validate_label_value( + untrusted_label_value=untrusted_payload + ) + + # TODO: avoid creating too-similar qube labels: #2732 + color = untrusted_payload + del untrusted_payload + + self.fire_event_for_permission(color=color) - # don't confuse label name with label index - self.enforce(not self.arg.isdigit()) - allowed_chars = string.ascii_letters + string.digits + "-_." - self.enforce(all(c in allowed_chars for c in self.arg)) try: self.app.get_label(self.arg) except KeyError: # ok, no such label yet pass else: - raise qubes.exc.QubesValueError("label already exists") - - untrusted_payload = untrusted_payload.decode("ascii", "strict").strip() - self.enforce(len(untrusted_payload) == 8) - self.enforce(untrusted_payload.startswith("0x")) - # besides prefix, only hex digits are allowed - self.enforce(all(x in string.hexdigits for x in untrusted_payload[2:])) - - # SEE: #2732 - color = untrusted_payload - - self.fire_event_for_permission(color=color) + raise qubes.exc.QubesLabelInUseError(label=self.arg) # allocate new index, but make sure it's outside of default labels set new_index = ( @@ -938,33 +1198,42 @@ async def label_create(self, untrusted_payload): self.app.save() @qubes.api.method( - "admin.label.Remove", no_payload=True, scope="global", write=True + "admin.label.Remove", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + write=True, ) async def label_remove(self): - self.enforce(self.dest.name == "dom0") + qubes.utils.validate_label_name(untrusted_label=self.arg) - try: - label = self.app.get_label(self.arg) - except KeyError: - raise qubes.exc.QubesValueError - # don't allow removing default labels - self.enforce(label.index > qubes.config.max_default_label) + label = self.app.get_label(self.arg) + + self.fire_event_for_permission(label=label) + + self.enforce( + label.index > qubes.config.max_default_label, + reason="Can only remove custom labels", + ) # FIXME: this should be in app.add_label() for vm in self.app.domains: if vm.label == label: - raise qubes.exc.QubesException("label still in use") - - self.fire_event_for_permission(label=label) + raise qubes.exc.QubesLabelInUseError(label=label) del self.app.labels[label.index] self.app.save() @qubes.api.method( - "admin.vm.Start", no_payload=True, scope="local", execute=True + "admin.vm.Start", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + execute=True, ) async def vm_start(self): - self.enforce(not self.arg) self.fire_event_for_permission() try: await self.dest.start() @@ -977,65 +1246,97 @@ async def vm_start(self): ) @qubes.api.method( - "admin.vm.Shutdown", no_payload=True, scope="local", execute=True + "admin.vm.Shutdown", + wants_arg=None, + wants_payload=False, + dest_adminvm=None, + scope="local", + execute=True, ) async def vm_shutdown(self): if self.arg: args = self.arg.split("+") else: args = [] - self.enforce(all(arg in ("force", "wait") for arg in args)) + allowed_args = ("force", "wait") + self.enforce( + all(arg in allowed_args for arg in args), + reason="Argument must match: " + ", ".join(allowed_args), + ) force = "force" in args wait = "wait" in args self.fire_event_for_permission(force=force, wait=wait) await self.dest.shutdown(force=force, wait=wait) @qubes.api.method( - "admin.vm.Pause", no_payload=True, scope="local", execute=True + "admin.vm.Pause", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + execute=True, ) async def vm_pause(self): - self.enforce(not self.arg) self.fire_event_for_permission() await self.dest.pause() @qubes.api.method( - "admin.vm.Unpause", no_payload=True, scope="local", execute=True + "admin.vm.Unpause", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + execute=True, ) async def vm_unpause(self): - self.enforce(not self.arg) self.fire_event_for_permission() await self.dest.unpause() @qubes.api.method( - "admin.vm.Suspend", no_payload=True, scope="local", execute=True + "admin.vm.Suspend", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + execute=True, ) async def vm_suspend(self): - self.enforce(not self.arg) self.fire_event_for_permission() await self.dest.suspend() @qubes.api.method( - "admin.vm.Resume", no_payload=True, scope="local", execute=True + "admin.vm.Resume", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + execute=True, ) async def vm_resume(self): - self.enforce(not self.arg) self.fire_event_for_permission() await self.dest.resume() @qubes.api.method( - "admin.vm.Kill", no_payload=True, scope="local", execute=True + "admin.vm.Kill", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + execute=True, ) async def vm_kill(self): - self.enforce(not self.arg) self.fire_event_for_permission() await self.dest.kill() @qubes.api.method( - "admin.Events", no_payload=True, scope="global", read=True + "admin.Events", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="global", + read=True, ) async def events(self): - self.enforce(not self.arg) - # run until client connection is terminated self.cancellable = True wait_for_cancel = asyncio.get_event_loop().create_future() @@ -1074,15 +1375,24 @@ async def events(self): self.dest.remove_handler("*", dispatcher.vm_handler) @qubes.api.method( - "admin.vm.feature.List", no_payload=True, scope="local", read=True + "admin.vm.feature.List", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_feature_list(self): - self.enforce(not self.arg) features = self.fire_event_for_filter(self.dest.features.keys()) return "".join("{}\n".format(feature) for feature in features) @qubes.api.method( - "admin.vm.feature.Get", no_payload=True, scope="local", read=True + "admin.vm.feature.Get", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_feature_get(self): # validation of self.arg done by qrexec-policy is enough @@ -1096,7 +1406,9 @@ async def vm_feature_get(self): @qubes.api.method( "admin.vm.feature.CheckWithTemplate", - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -1112,7 +1424,9 @@ async def vm_feature_checkwithtemplate(self): @qubes.api.method( "admin.vm.feature.CheckWithNetvm", - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -1128,7 +1442,9 @@ async def vm_feature_checkwithnetvm(self): @qubes.api.method( "admin.vm.feature.CheckWithAdminVM", - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -1144,7 +1460,9 @@ async def vm_feature_checkwithadminvm(self): @qubes.api.method( "admin.vm.feature.CheckWithTemplateAndAdminVM", - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -1159,7 +1477,12 @@ async def vm_feature_checkwithtpladminvm(self): return value @qubes.api.method( - "admin.vm.feature.Remove", no_payload=True, scope="local", write=True + "admin.vm.feature.Remove", + wants_arg=True, + wants_payload=False, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_feature_remove(self): # validation of self.arg done by qrexec-policy is enough @@ -1171,16 +1494,29 @@ async def vm_feature_remove(self): raise qubes.exc.QubesFeatureNotFoundError(self.dest, self.arg) self.app.save() - @qubes.api.method("admin.vm.feature.Set", scope="local", write=True) + @qubes.api.method( + "admin.vm.feature.Set", + wants_arg=True, + wants_payload=None, + dest_adminvm=None, + scope="local", + write=True, + ) async def vm_feature_set(self, untrusted_payload): - untrusted_value = untrusted_payload.decode("ascii", errors="strict") + try: + untrusted_value = untrusted_payload.decode("ascii", errors="strict") + except UnicodeDecodeError: + raise qubes.exc.ProtocolError( + "Feature value contains non-ASCII characters" + ) + del untrusted_payload if re.match(r"\A[a-zA-Z0-9_.-]+\Z", self.arg) is None: - raise qubes.exc.QubesValueError( + raise qubes.exc.ProtocolError( "feature name contains illegal characters" ) if re.match(r"\A[\x20-\x7E]*\Z", untrusted_value) is None: - raise qubes.exc.QubesValueError( + raise qubes.exc.ProtocolError( f"{self.arg} value contains illegal characters" ) value = untrusted_value @@ -1198,6 +1534,9 @@ async def vm_feature_set(self, untrusted_payload): group=qubes.vm.VM_ENTRY_POINT ) ), + wants_arg=None, + wants_payload=None, + dest_adminvm=True, scope="global", write=True, ) @@ -1214,6 +1553,9 @@ def vm_create(self, endpoint, untrusted_payload=None): group=qubes.vm.VM_ENTRY_POINT ) ), + wants_arg=None, + wants_payload=None, + dest_adminvm=True, scope="global", write=True, ) @@ -1225,8 +1567,6 @@ def vm_create_in_pool(self, endpoint, untrusted_payload=None): async def _vm_create( self, vm_type, allow_pool=False, untrusted_payload=None ): - self.enforce(self.dest.name == "dom0") - kwargs = {} pool = None pools = {} @@ -1238,73 +1578,108 @@ async def _vm_create( # if argument is given, it needs to be a valid template, and only # when given VM class do need a template + invalid_template = False if self.arg: - if hasattr(vm_class, "template"): - if self.arg not in self.app.domains: - raise qubes.exc.QubesVMNotFoundError(self.arg) - kwargs["template"] = self.app.domains[self.arg] - else: - raise qubes.exc.QubesValueError( + if not hasattr(vm_class, "template"): + raise qubes.exc.ProtocolError( "{} cannot be based on template".format(vm_type) ) + if self.arg in self.app.domains: + kwargs["template"] = self.app.domains[self.arg] + else: + invalid_template = True + invalid_label = False for untrusted_param in untrusted_payload.decode( "ascii", errors="strict" ).split(" "): untrusted_key, untrusted_value = untrusted_param.split("=", 1) - if untrusted_key in kwargs: - raise qubes.exc.ProtocolError("duplicated parameters") + self.enforce( + untrusted_key not in kwargs, reason="Options must be unique" + ) if untrusted_key == "name": qubes.vm.validate_name(None, None, untrusted_value) kwargs["name"] = untrusted_value elif untrusted_key == "label": - # don't confuse label name with label index - self.enforce(not untrusted_value.isdigit()) - allowed_chars = string.ascii_letters + string.digits + "-_." - self.enforce(all(c in allowed_chars for c in untrusted_value)) - # this may raise QubesLabelNotFoundError - kwargs["label"] = self.app.get_label(untrusted_value) + qubes.utils.validate_label_name( + untrusted_label=untrusted_value, creation=True + ) + try: + kwargs["label"] = self.app.get_label(untrusted_value) + except qubes.exc.QubesLabelNotFoundError: + invalid_label = True elif untrusted_key == "pool" and allow_pool: - if pool is not None: - raise qubes.exc.ProtocolError("duplicated pool parameter") + self.enforce( + pool is None, reason="Option 'pool' must be unique" + ) pool = self.app.get_pool(untrusted_value) + elif untrusted_key.startswith("pool:") and allow_pool: untrusted_volume = untrusted_key.split(":", 1)[1] # kind of ugly, but actual list of volumes is available only # after creating a VM + allowed_volumes = ["root", "private", "volatile", "kernel"] self.enforce( - untrusted_volume - in ["root", "private", "volatile", "kernel"] + untrusted_volume in allowed_volumes, + reason="Volume must be one of: " + + ", ".join(allowed_volumes), ) volume = untrusted_volume - if volume in pools: - raise qubes.exc.ProtocolError( - "duplicated pool:{} parameter".format(volume) - ) + self.enforce( + volume not in pools, + reason="Option 'pool:{}' must be unique".format(volume), + ) pools[volume] = self.app.get_pool(untrusted_value) else: - raise qubes.exc.ProtocolError("Invalid param name") + if not allow_pool and ( + untrusted_key == "pool" or untrusted_key.startswith("pool:") + ): + raise qubes.exc.ProtocolError( + "Pool option specified but 'allow_pool' is False" + ) + allowed_options = ["name", "label", "pool", "pool:"] + raise qubes.exc.ProtocolError( + "Options does not match: " + ", ".join(allowed_options) + ) del untrusted_payload - if "name" not in kwargs or "label" not in kwargs: - raise qubes.exc.ProtocolError("Missing name or label") - - if pool and pools: - raise qubes.exc.ProtocolError( - "Only one of 'pool=' and 'pool:volume=' can be used" + for required_key in ["name", "label"]: + self.enforce( + required_key in kwargs, + reason="Creation requested without key: " + required_key, ) + self.enforce( + not (pool and pools), + reason="Conflicting options specified: 'pool=', 'pool:volume='", + ) + + invalid_name = False if kwargs["name"] in self.app.domains: - raise qubes.exc.QubesValueError( - "VM {} already exists".format(kwargs["name"]) - ) + invalid_name = True self.fire_event_for_permission(pool=pool, pools=pools, **kwargs) + # Avoids leaking qube existence before admin-permission. + if invalid_name: + raise qubes.exc.QubesVMAlreadyExistsError( + "Qube already exists: {}".format(kwargs["name"]) + ) + + # Avoids leaking template existence before admin-permission. + if invalid_template: + raise qubes.exc.QubesVMNotFoundError(self.arg) + + # Avoids leaking label existence before admin-permission. + if invalid_label: + # If label is invalid, it came from payload and should not be + # logged. + raise qubes.exc.QubesLabelNotFoundError(label="untrusted label") + vm = self.app.add_new_vm(vm_class, **kwargs) # TODO: move this to extension (in race-free fashion) vm.tags.add("created-by-" + str(self.src)) @@ -1317,20 +1692,29 @@ async def _vm_create( raise self.app.save() - @qubes.api.method("admin.vm.CreateDisposable", scope="global", write=True) + @qubes.api.method( + "admin.vm.CreateDisposable", + wants_arg=None, + wants_payload=None, + dest_adminvm=None, + scope="global", + write=True, + ) async def create_disposable(self, untrusted_payload): """ Create a disposable. If the RPC argument is ``preload``, cleanse the preload list and start preloading fresh disposables. """ - self.enforce(self.arg in [None, "", "preload"]) + self.enforce_arg( + [None, "", "preload"], + short_reason="'', 'preload'", + ) preload = False if self.arg == "preload": preload = True if untrusted_payload not in (b"", b"uuid"): - raise qubes.exc.QubesValueError( - "Invalid payload for admin.vm.CreateDisposable: " - "expected the empty string or 'uuid'" + raise qubes.exc.ProtocolError( + "Invalid payload, expected empty string or 'uuid'" ) if self.dest.name == "dom0": @@ -1353,11 +1737,14 @@ async def create_disposable(self, untrusted_payload): ) @qubes.api.method( - "admin.vm.Remove", no_payload=True, scope="global", write=True + "admin.vm.Remove", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="global", + write=True, ) async def vm_remove(self): - self.enforce(not self.arg) - self.fire_event_for_permission() if self.dest.name == "dom0": @@ -1386,13 +1773,15 @@ async def vm_remove(self): self.app.save() @qubes.api.method( - "admin.deviceclass.List", no_payload=True, scope="global", read=True + "admin.deviceclass.List", + wants_arg=False, + wants_payload=False, + dest_adminvm=True, + scope="global", + read=True, ) async def deviceclass_list(self): """List all DEVICES classes""" - self.enforce(not self.arg) - self.enforce(self.dest.name == "dom0") - entrypoints = self.fire_event_for_filter( importlib.metadata.entry_points(group="qubes.devices") ) @@ -1405,7 +1794,9 @@ async def deviceclass_list(self): ep.name for ep in importlib.metadata.entry_points(group="qubes.devices") ), - no_payload=True, + wants_arg=None, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -1422,7 +1813,8 @@ async def vm_device_available(self, endpoint): devices = [dev for dev in devices if dev.port_id == self.arg] # no duplicated devices, but device may not exist, in which case # the list is empty - self.enforce(len(devices) <= 1) + if len(devices) > 1: + raise qubes.exc.DeviceNotFound() devices = self.fire_event_for_filter(devices, devclass=devclass) dev_info = { f"{dev.port_id}:{dev.device_id}": dev.serialize().decode() @@ -1438,7 +1830,9 @@ async def vm_device_available(self, endpoint): ep.name for ep in importlib.metadata.entry_points(group="qubes.devices") ), - no_payload=True, + wants_arg=None, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -1463,7 +1857,8 @@ async def vm_device_list(self, endpoint): ] # no duplicated devices, but device may not exist, in which case # the list is empty - self.enforce(len(device_assignments) <= 1) + if len(device_assignments) > 1: + raise qubes.exc.DeviceNotFound() device_assignments = self.fire_event_for_filter( device_assignments, devclass=devclass ) @@ -1487,7 +1882,9 @@ async def vm_device_list(self, endpoint): ep.name for ep in importlib.metadata.entry_points(group="qubes.devices") ), - no_payload=True, + wants_arg=None, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -1512,7 +1909,8 @@ async def vm_device_attached(self, endpoint): ] # no duplicated devices, but device may not exist, in which case # the list is empty - self.enforce(len(device_assignments) <= 1) + if len(device_assignments) > 1: + raise qubes.exc.DeviceNotFound() device_assignments = [ a.clone( device=self.app.domains[a.backend_name].devices[devclass][ @@ -1545,6 +1943,9 @@ async def vm_device_attached(self, endpoint): ep.name for ep in importlib.metadata.entry_points(group="qubes.devices") ), + wants_arg=True, + wants_payload=True, + dest_adminvm=None, scope="local", write=True, ) @@ -1591,7 +1992,9 @@ def load_device_info(self, devclass) -> VirtualDevice: ep.name for ep in importlib.metadata.entry_points(group="qubes.devices") ), - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=None, scope="local", write=True, ) @@ -1613,6 +2016,9 @@ async def vm_device_unassign(self, endpoint): ep.name for ep in importlib.metadata.entry_points(group="qubes.devices") ), + wants_arg=True, + wants_payload=None, + dest_adminvm=None, scope="local", execute=True, ) @@ -1637,7 +2043,9 @@ async def vm_device_attach(self, endpoint, untrusted_payload): ep.name for ep in importlib.metadata.entry_points(group="qubes.devices") ), - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=None, scope="local", execute=True, ) @@ -1658,6 +2066,9 @@ async def vm_device_detach(self, endpoint): ep.name for ep in importlib.metadata.entry_points(group="qubes.devices") ), + wants_arg=True, + wants_payload=True, + dest_adminvm=None, scope="local", write=True, ) @@ -1666,8 +2077,9 @@ async def vm_device_set_required(self, endpoint, untrusted_payload): Update `required` flag of an already assigned device. Payload: - `False` -> device will be auto-attached to qube - `True` -> device is required to start qube + `required` -> device is required to start qube + `ask-to-attach` -> TODO + `auto-attack` -> device will be auto-attached to qube """ devclass = endpoint @@ -1679,7 +2091,9 @@ async def vm_device_set_required(self, endpoint, untrusted_payload): try: mode = allowed_values[untrusted_payload] except KeyError: - raise qubes.exc.PermissionDenied() + raise qubes.exc.QubesUnrecognizedDeviceAssignmentMode( + "Invalid assignment mode" + ) dev = VirtualDevice.from_qarg(self.arg, devclass, self.app.domains) @@ -1690,7 +2104,9 @@ async def vm_device_set_required(self, endpoint, untrusted_payload): @qubes.api.method( "admin.vm.device.denied.List", - no_payload=True, + wants_arg=False, + wants_payload=False, + dest_adminvm=None, scope="local", read=True, ) @@ -1700,14 +2116,19 @@ async def vm_device_denied_list(self): Returns a newline-separated string. """ - self.enforce(not self.arg) - self.fire_event_for_permission() denied = self.dest.devices_denied return "\n".join(map(repr, DeviceInterface.from_str_bulk(denied))) - @qubes.api.method("admin.vm.device.denied.Add", scope="local", write=True) + @qubes.api.method( + "admin.vm.device.denied.Add", + wants_arg=False, + wants_payload=None, + dest_adminvm=None, + scope="local", + write=True, + ) async def vm_device_denied_add(self, untrusted_payload): """ Add device interface(s) to the denied list for the VM. @@ -1715,11 +2136,16 @@ async def vm_device_denied_add(self, untrusted_payload): Payload: Encoded device interface (can be repeated without any separator). """ - payload = untrusted_payload.decode("ascii", errors="strict") + try: + payload = untrusted_payload.decode("ascii", errors="strict") + except UnicodeDecodeError: + raise qubes.exc.ProtocolError( + "Device interface contains non-ASCII characters" + ) to_add = DeviceInterface.from_str_bulk(payload) if len(set(to_add)) != len(to_add): - raise qubes.exc.QubesValueError( + raise qubes.exc.ProtocolError( "Duplicated device interfaces in payload." ) @@ -1735,7 +2161,12 @@ async def vm_device_denied_add(self, untrusted_payload): self.app.save() @qubes.api.method( - "admin.vm.device.denied.Remove", scope="local", write=True + "admin.vm.device.denied.Remove", + wants_arg=False, + wants_payload=None, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_device_denied_remove(self, untrusted_payload): """ @@ -1747,14 +2178,19 @@ async def vm_device_denied_remove(self, untrusted_payload): """ denied = DeviceInterface.from_str_bulk(self.dest.devices_denied) - payload = untrusted_payload.decode("ascii", errors="strict") + try: + payload = untrusted_payload.decode("ascii", errors="strict") + except UnicodeDecodeError: + raise qubes.exc.ProtocolError( + "Device interface contains non-ASCII characters" + ) if payload != "all": to_remove = DeviceInterface.from_str_bulk(payload) else: to_remove = denied.copy() if len(set(to_remove)) != len(to_remove): - raise qubes.exc.QubesValueError( + raise qubes.exc.ProtocolError( "Duplicated device interfaces in payload." ) @@ -1769,11 +2205,14 @@ async def vm_device_denied_remove(self, untrusted_payload): self.app.save() @qubes.api.method( - "admin.vm.firewall.Get", no_payload=True, scope="local", read=True + "admin.vm.firewall.Get", + wants_arg=False, + wants_payload=False, + dest_adminvm=False, + scope="local", + read=True, ) async def vm_firewall_get(self): - self.enforce(not self.arg) - self.fire_event_for_permission() return "".join( @@ -1782,17 +2221,26 @@ async def vm_firewall_get(self): if rule.api_rule is not None ) - @qubes.api.method("admin.vm.firewall.Set", scope="local", write=True) + @qubes.api.method( + "admin.vm.firewall.Set", + wants_arg=False, + wants_payload=True, + dest_adminvm=False, + scope="local", + write=True, + ) async def vm_firewall_set(self, untrusted_payload): - self.enforce(not self.arg) rules = [] - for untrusted_line in untrusted_payload.decode( - "ascii", errors="strict" - ).splitlines(): - rule = qubes.firewall.Rule.from_api_string( - untrusted_rule=untrusted_line - ) - rules.append(rule) + try: + for untrusted_line in untrusted_payload.decode( + "ascii", errors="strict" + ).splitlines(): + rule = qubes.firewall.Rule.from_api_string( + untrusted_rule=untrusted_line + ) + rules.append(rule) + except UnicodeDecodeError: + raise qubes.exc.ProtocolError("Rules contains non-ASCII characters") self.fire_event_for_permission(rules=rules) @@ -1801,13 +2249,13 @@ async def vm_firewall_set(self, untrusted_payload): @qubes.api.method( "admin.vm.firewall.Reload", - no_payload=True, + wants_arg=False, + wants_payload=False, + dest_adminvm=False, scope="local", execute=True, ) async def vm_firewall_reload(self): - self.enforce(not self.arg) - self.fire_event_for_permission() self.dest.fire_event("firewall-changed") @@ -1823,6 +2271,10 @@ async def _load_backup_profile(self, profile_name, skip_passphrase=False): profile_path = os.path.join( qubes.config.backup_profile_dir, profile_name + ".conf" ) + if not os.path.exists(profile_path): + raise qubes.exc.QubesBackupProfileNotFoundError( + "Backup profile does not exist: {}".format(profile_name) + ) with open(profile_path, encoding="utf-8") as profile_file: profile_data = yaml.safe_load(profile_file) @@ -1867,11 +2319,14 @@ async def _load_backup_profile(self, profile_name, skip_passphrase=False): ) try: passphrase, _ = await passphrase_vm.run_service_for_stdio( - "qubes.BackupPassphrase+" + self.arg + "qubes.BackupPassphrase+" + profile_name ) # make it foolproof against "echo passphrase" implementation passphrase = passphrase.strip() - self.enforce(b"\n" not in passphrase) + self.enforce( + b"\n" not in passphrase, + reason="Passphrase must not contain newline character", + ) except subprocess.CalledProcessError: raise qubes.exc.QubesException( "Failed to retrieve passphrase from '{}' VM".format( @@ -1927,26 +2382,21 @@ def _backup_progress_callback(self, profile_name, progress): @qubes.api.method( "admin.backup.Execute", - no_payload=True, + wants_arg=True, + wants_payload=False, + dest_adminvm=True, scope="global", read=True, execute=True, ) async def backup_execute(self): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg) - self.enforce("/" not in self.arg) + self.enforce( + "/" not in self.arg, + reason="Backup profile ID must not contain forward slash character", + ) self.fire_event_for_permission() - profile_path = os.path.join( - qubes.config.backup_profile_dir, self.arg + ".conf" - ) - if not os.path.exists(profile_path): - raise qubes.exc.PermissionDenied( - "Backup profile {} does not exist".format(self.arg) - ) - if not hasattr(self.app, "api_admin_running_backups"): self.app.api_admin_running_backups = {} @@ -1964,17 +2414,23 @@ async def backup_execute(self): try: await backup_task except asyncio.CancelledError: - raise qubes.exc.QubesException("Backup cancelled") + raise qubes.exc.BackupCancelledError() finally: del self.app.api_admin_running_backups[self.arg] @qubes.api.method( - "admin.backup.Cancel", no_payload=True, scope="global", execute=True + "admin.backup.Cancel", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="global", + execute=True, ) async def backup_cancel(self): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg) - self.enforce("/" not in self.arg) + self.enforce( + "/" not in self.arg, + reason="Backup profile ID must not contain forward slash character", + ) self.fire_event_for_permission() @@ -1987,23 +2443,21 @@ async def backup_cancel(self): self.app.api_admin_running_backups[self.arg].cancel() @qubes.api.method( - "admin.backup.Info", no_payload=True, scope="local", read=True + "admin.backup.Info", + wants_arg=True, + wants_payload=False, + dest_adminvm=True, + scope="local", + read=True, ) async def backup_info(self): - self.enforce(self.dest.name == "dom0") - self.enforce(self.arg) - self.enforce("/" not in self.arg) + self.enforce( + "/" not in self.arg, + reason="Backup profile ID must not contain forward slash character", + ) self.fire_event_for_permission() - profile_path = os.path.join( - qubes.config.backup_profile_dir, self.arg + ".conf" - ) - if not os.path.exists(profile_path): - raise qubes.exc.PermissionDenied( - "Backup profile {} does not exist".format(self.arg) - ) - backup = await self._load_backup_profile(self.arg, skip_passphrase=True) return backup.get_backup_summary() @@ -2057,11 +2511,14 @@ def _send_stats_single( return info_time, info @qubes.api.method( - "admin.vm.Stats", no_payload=True, scope="global", read=True + "admin.vm.Stats", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="global", + read=True, ) async def vm_stats(self): - self.enforce(not self.arg) - # run until client connection is terminated self.cancellable = True @@ -2088,10 +2545,14 @@ async def vm_stats(self): pass @qubes.api.method( - "admin.vm.CurrentState", no_payload=True, scope="local", read=True + "admin.vm.CurrentState", + wants_arg=False, + wants_payload=False, + dest_adminvm=None, + scope="local", + read=True, ) async def vm_current_state(self): - self.enforce(not self.arg) self.fire_event_for_permission() state = { @@ -2103,19 +2564,29 @@ async def vm_current_state(self): return " ".join("{}={}".format(k, v) for k, v in state.items()) @qubes.api.method( - "admin.vm.notes.Get", no_payload=True, scope="local", read=True + "admin.vm.notes.Get", + wants_arg=False, + wants_payload=False, + dest_adminvm=False, + scope="local", + read=True, ) async def vm_notes_get(self): """Get qube notes""" - self.enforce(self.dest.name != "dom0") self.fire_event_for_permission() notes = self.dest.get_notes() return notes - @qubes.api.method("admin.vm.notes.Set", scope="local", write=True) + @qubes.api.method( + "admin.vm.notes.Set", + wants_arg=False, + wants_payload=True, + dest_adminvm=False, + scope="local", + write=True, + ) async def vm_notes_set(self, untrusted_payload): """Set qube notes""" - self.enforce(self.dest.name != "dom0") self.fire_event_for_permission() if len(untrusted_payload) > 256000: raise qubes.exc.ProtocolError( diff --git a/qubes/api/internal.py b/qubes/api/internal.py index e74462f75..3b7362c10 100644 --- a/qubes/api/internal.py +++ b/qubes/api/internal.py @@ -166,17 +166,23 @@ class QubesInternalAPI(qubes.api.AbstractQubesAPI): SOCKNAME = "/var/run/qubesd.internal.sock" - @qubes.api.method("internal.GetSystemInfo", no_payload=True) + @qubes.api.method( + "internal.GetSystemInfo", + dest_adminvm=True, + wants_arg=False, + wants_payload=False, + ) async def getsysteminfo(self): - self.enforce(self.dest.name == "dom0") - self.enforce(not self.arg) - system_info = SystemInfoCache.get_system_info(self.app) - return json.dumps(system_info) @qubes.api.method( - "internal.vm.volume.ImportBegin", scope="local", write=True + "internal.vm.volume.ImportBegin", + wants_arg=True, + wants_payload=None, + dest_adminvm=None, + scope="local", + write=True, ) async def vm_volume_import(self, untrusted_payload): """Begin importing volume data. Payload is either size of new data @@ -191,7 +197,10 @@ async def vm_volume_import(self, untrusted_payload): payload) and response from that call will be actually send to the caller. """ - self.enforce(self.arg in self.dest.volumes.keys()) + if self.arg not in self.dest.volumes.keys(): + raise qubes.exc.ProtocolError( + "Argument not in {}".format(self.EXC_ARG_NOT_IN_DEST_VOLUMES) + ) if untrusted_payload: original_method = "admin.vm.volume.ImportWithSize" @@ -208,12 +217,14 @@ async def vm_volume_import(self, untrusted_payload): raise qubes.exc.QubesVMNotHaltedError(self.dest) requested_size = ( - self.validate_size(untrusted_payload) if untrusted_payload else None + self.validate_number(untrusted_payload, name="size of new data") + if untrusted_payload + else None ) del untrusted_payload path = await self.dest.storage.import_data(self.arg, requested_size) - self.enforce(" " not in path) + self.enforce(" " not in path, reason="Path contains whitespace") if requested_size is None: size = self.dest.volumes[self.arg].size else: @@ -227,7 +238,12 @@ async def vm_volume_import(self, untrusted_payload): return "{} {}".format(size, path) - @qubes.api.method("internal.vm.volume.ImportEnd") + @qubes.api.method( + "internal.vm.volume.ImportEnd", + wants_arg=True, + wants_payload=True, + dest_adminvm=None, + ) async def vm_volume_import_end(self, untrusted_payload): """ This is second half of admin.vm.volume.Import handling. It is called @@ -236,7 +252,10 @@ async def vm_volume_import_end(self, untrusted_payload): The payload is either 'ok', or 'fail\n'. """ - self.enforce(self.arg in self.dest.volumes.keys()) + if self.arg not in self.dest.volumes.keys(): + raise qubes.exc.ProtocolError( + "Argument not in {}".format(self.EXC_ARG_NOT_IN_DEST_VOLUMES) + ) success = untrusted_payload == b"ok" try: @@ -260,7 +279,12 @@ async def vm_volume_import_end(self, untrusted_payload): "Data import failed: {}".format(error) ) - @qubes.api.method("internal.SuspendPre", no_payload=True) + @qubes.api.method( + "internal.SuspendPre", + wants_payload=False, + wants_arg=False, + dest_adminvm=True, + ) async def suspend_pre(self): """ Method called before host system goes to sleep. @@ -343,7 +367,12 @@ async def suspend_pre(self): "Failed to suspend some qubes: {}".format(failed) ) - @qubes.api.method("internal.SuspendPost", no_payload=True) + @qubes.api.method( + "internal.SuspendPost", + wants_payload=False, + wants_arg=False, + dest_adminvm=True, + ) async def suspend_post(self): """ Method called after host system wake up from sleep. diff --git a/qubes/api/misc.py b/qubes/api/misc.py index eddf21e7d..ecd81291d 100644 --- a/qubes/api/misc.py +++ b/qubes/api/misc.py @@ -32,7 +32,12 @@ class QubesMiscAPI(qubes.api.AbstractQubesAPI): SOCKNAME = "/var/run/qubesd.misc.sock" - @qubes.api.method("qubes.FeaturesRequest", no_payload=True) + @qubes.api.method( + "qubes.FeaturesRequest", + wants_payload=False, + wants_arg=False, + dest_adminvm=True, + ) async def qubes_features_request(self): """qubes.FeaturesRequest handler @@ -45,9 +50,6 @@ async def qubes_features_request(self): appropriate extensions. Requests not explicitly handled by some extension are ignored. """ - self.enforce(self.dest.name == "dom0") - self.enforce(not self.arg) - prefix = "/features-request/" keys = self.src.untrusted_qdb.list(prefix) @@ -61,21 +63,26 @@ async def qubes_features_request(self): safe_set = string.ascii_letters + string.digits + "-.,_= " for untrusted_key in untrusted_features: untrusted_value = untrusted_features[untrusted_key] - self.enforce(all((c in safe_set) for c in untrusted_value)) + self.enforce( + all((c in safe_set) for c in untrusted_value), + reason="Value outside safe set: " + safe_set, + ) await self.src.fire_event_async( "features-request", untrusted_features=untrusted_features ) self.app.save() - @qubes.api.method("qubes.NotifyTools", no_payload=True) + @qubes.api.method( + "qubes.NotifyTools", + wants_payload=False, + wants_arg=False, + dest_adminvm=True, + ) async def qubes_notify_tools(self): """ Legacy version of qubes.FeaturesRequest, used by Qubes Windows Tools """ - self.enforce(self.dest.name == "dom0") - self.enforce(not self.arg) - untrusted_features = {} safe_set = string.ascii_letters + string.digits expected_features = ( @@ -93,7 +100,10 @@ async def qubes_notify_tools(self): untrusted_value = untrusted_value.decode( "ascii", errors="strict" ) - self.enforce(all((c in safe_set) for c in untrusted_value)) + self.enforce( + all((c in safe_set) for c in untrusted_value), + reason="Value outside safe set: " + safe_set, + ) untrusted_features[feature] = untrusted_value del untrusted_value @@ -102,7 +112,12 @@ async def qubes_notify_tools(self): ) self.app.save() - @qubes.api.method("qubes.NotifyUpdates") + @qubes.api.method( + "qubes.NotifyUpdates", + wants_payload=True, + wants_arg=False, + dest_adminvm=True, + ) async def qubes_notify_updates(self, untrusted_payload): """ Receive VM notification about updates availability @@ -112,7 +127,10 @@ async def qubes_notify_updates(self, untrusted_payload): """ untrusted_update_count = untrusted_payload.strip() - self.enforce(untrusted_update_count.isdigit()) + self.enforce( + untrusted_update_count.isdigit(), + reason="Update count is not a digit", + ) # now sanitized update_count = int(untrusted_update_count) del untrusted_update_count diff --git a/qubes/exc.py b/qubes/exc.py index dcd145c00..c05346fc3 100644 --- a/qubes/exc.py +++ b/qubes/exc.py @@ -27,6 +27,31 @@ class QubesException(Exception): """Exception that can be shown to the user""" +class ProtocolError(AssertionError): + """Raised deliberately by handlers to indicate a malformed client request. + + Client programming errors. The client made a request that it should have + known better than to send in the first place. Includes things like passing + an argument or payload to a service documented to not take one. The only + way that a correctly behaving client can get ProtocolError is qubesd is + either buggy or too old. In HTTP, this would be 400 Bad Request. + + It should not be used to reject requests that are valid, but which qubesd + is refusing to process. Instead, raise a subclass of + :py:class:`QubesException` with a useful error message. + """ + + +class PermissionDenied(PermissionError): + """Raised deliberately by handlers to inform the request is prohibited. + + The request is valid, but the client does not have permission to perform + the operation. Clients in dom0 should usually not get this error. It must + only be raised by "admin-permission" events. In HTTP, this would be 403 + Forbidden. + """ + + class QubesVMNotFoundError(QubesException, KeyError): """Domain cannot be found in the system""" @@ -50,6 +75,10 @@ def __init__(self, uuid: str) -> None: self.vmname = uuid +class QubesVMAlreadyExistsError(QubesException): + """Domain was requested to be created, but it already exists.""" + + class QubesVMError(QubesException): """Some problem with domain state.""" @@ -167,9 +196,9 @@ def __init__(self, vm, msg=None): class QubesPoolInUseError(QubesException): """VM is in use, cannot remove.""" - def __init__(self, pool, msg=None): + def __init__(self, pool_name, msg=None): super().__init__( - msg or "Storage pool is in use: {!r}".format(pool.name) + msg or "Storage pool is in use: {!r}".format(pool_name) ) @@ -212,6 +241,15 @@ def __init__(self, msg=None): super().__init__(msg or "This feature is not available") +class QubesBackupProfileNotFoundError(QubesException): + """Requested backup profile does not exist.""" + + def __init__(self, msg=None, profile=None): + super().__init__( + msg or "Backup profile {!r} does not exist".format(profile) + ) + + class BackupCancelledError(QubesException): """Thrown at user when backup was manually cancelled""" @@ -276,14 +314,6 @@ def __str__(self): return QubesException.__str__(self) -class ProtocolError(AssertionError): - """Raised when something is wrong with data received""" - - -class PermissionDenied(Exception): - """Raised deliberately by handlers when we decide not to cooperate""" - - class DeviceNotAssigned(QubesException, KeyError): """ Trying to unassign not assigned device. @@ -308,6 +338,12 @@ class DeviceAlreadyAssigned(QubesException, KeyError): """ +class QubesUnrecognizedDeviceAssignmentMode(ProtocolError): + """ + Device assignment is not as expected. + """ + + class UnrecognizedDevice(QubesException, ValueError): """ Device identity is not as expected. @@ -322,3 +358,56 @@ class UnexpectedDeviceProperty(QubesException, ValueError): class StoragePoolException(QubesException): """A general storage exception""" + + +class QubesVolumeCopyTokenNotFoundError(ProtocolError, KeyError): + """Domain volume copy did not specify a configured token.""" + + def __init__(self, msg=None): + super().__init__(msg or "Token to clone volume of qube was not found") + + +class QubesVolumeCopyInUseError(QubesException): + """Domain volume is already being cloned.""" + + def __init__(self, vm, volume, msg=None): + super().__init__( + vm, + msg + or "Domain volume is being cloned already: {!r}:{!r}".format( + vm.name, volume + ), + ) + + +class QubesVolumeRevisionNotFoundError(KeyError): + """Specified revision not found in qube volume.""" + + +class QubesPoolNotFoundError(KeyError): + """Pool does not exist.""" + + +class QubesVolumeNotFoundError(KeyError): + """Pool does not exist.""" + + +class QubesInvalidTagError(ProtocolError): + """Domain tag is invalid.""" + + +class QubesInvalidLabelError(ProtocolError): + """Domain label is invalid.""" + + +class QubesInvalidLabelValueError(ProtocolError): + """Domain label value is invalid.""" + + +class QubesLabelInUseError(QubesException): + """Cannot remove or add label as it is still in use.""" + + def __init__(self, label, msg=None): + super().__init__( + msg or "Label is still in use: {!r}".format(label), + ) diff --git a/qubes/log.py b/qubes/log.py index 8a2c8f97b..d3795af25 100644 --- a/qubes/log.py +++ b/qubes/log.py @@ -43,7 +43,7 @@ def formatMessage(self, record): return fmt % record.__dict__ -def enable(log_level: int = logging.INFO): +def enable(log_level: int = logging.INFO, enable_debug_libvirt: bool = False): """Enable global logging Use :py:mod:`logging` module from standard library to log messages. @@ -64,6 +64,8 @@ def enable(log_level: int = logging.INFO): if log_level == logging.DEBUG: for handler in logging.root.handlers: handler.setFormatter(Formatter(debug=True)) + if enable_debug_libvirt: + logging.getLogger("virEventAsyncIOImpl").setLevel(logging.INFO) logging.root.setLevel(log_level) diff --git a/qubes/tests/api.py b/qubes/tests/api.py index 228fde64f..4c2e420d9 100644 --- a/qubes/tests/api.py +++ b/qubes/tests/api.py @@ -201,3 +201,88 @@ def test_006_target_adminvm(self): response, b"0\0src: b'src', dest: b'dom0', arg: b'arg', payload: b'payload'", ) + + +class TC_10_QubesAPIValidation(qubes.tests.QubesTestCase): + + def test_000_method_param__wants_arg(self): + func = qubes.api.method( + "rpc", + wants_arg="test", + wants_payload=None, + dest_adminvm=None, + ) + with self.assertRaisesRegex(ValueError, "wants_arg"): + func("rpc") + + def test_000_method_param__wants_payload(self): + func = qubes.api.method( + "rpc", + wants_arg=None, + wants_payload="test", + dest_adminvm=None, + ) + with self.assertRaisesRegex(ValueError, "wants_payload"): + func("rpc") + + def test_000_method_param__dest_adminvm(self): + func = qubes.api.method( + "rpc", + wants_arg=None, + wants_payload=None, + dest_adminvm="test", + ) + with self.assertRaisesRegex(ValueError, "dest_adminvm"): + func("rpc") + + def test_000_method_param__read(self): + func = qubes.api.method( + "rpc", + wants_arg=None, + wants_payload=None, + dest_adminvm=None, + read="test", + ) + with self.assertRaisesRegex(ValueError, "read"): + func("rpc") + + def test_000_method_param__write(self): + func = qubes.api.method( + "rpc", + wants_arg=None, + wants_payload=None, + dest_adminvm=None, + write="test", + ) + with self.assertRaisesRegex(ValueError, "write"): + func("rpc") + + def test_000_method_param__execute(self): + func = qubes.api.method( + "rpc", + wants_arg=None, + wants_payload=None, + dest_adminvm=None, + execute="test", + ) + with self.assertRaisesRegex(ValueError, "execute"): + func("rpc") + + def test_000_method_param__scope(self): + func = qubes.api.method( + "rpc", + wants_arg=None, + wants_payload=None, + dest_adminvm=None, + scope="test", + ) + with self.assertRaisesRegex(ValueError, "scope"): + func("rpc") + + def test_030_validate_number(self): + with self.assertRaisesRegex( + qubes.exc.ProtocolError, "of type bytes, got 'int'" + ): + qubes.api.AbstractQubesAPI.validate_number( + "", untrusted_number=30, name="size" + ) diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index bc14f32fe..38db1408a 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -713,7 +713,7 @@ def test_100_vm_volume_snapshot_invalid_volume(self): }, } self.vm.volumes.configure_mock(**volumes_conf) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.volume.Snapshots", b"test-vm1", b"no-such-volume" ) @@ -728,7 +728,7 @@ def test_100_vm_volume_snapshot_invalid_revision(self): "keys.return_value": ["root", "private", "volatile", "kernel"] } self.vm.volumes.configure_mock(**volumes_conf) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.volume.Snapshots", b"test-vm1", @@ -783,20 +783,13 @@ def test_110_vm_volume_revert_invalid_rev(self): } self.vm.volumes.configure_mock(**volumes_conf) self.vm.storage = unittest.mock.Mock() - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.QubesVolumeRevisionNotFoundError): self.call_mgmt_func( b"admin.vm.volume.Revert", b"test-vm1", b"private", b"no-such-rev", ) - self.assertEqual( - self.vm.volumes.mock_calls, - [ - unittest.mock.call.keys(), - unittest.mock.call.__getattr__("__getitem__")("private"), - ], - ) self.assertFalse(self.vm.storage.called) def test_120_vm_volume_resize(self): @@ -1053,6 +1046,32 @@ def test_160_pool_add(self, mock_parameters, mock_drivers): ) self.assertTrue(self.app.save.called) + @unittest.mock.patch("qubes.storage.pool_drivers") + @unittest.mock.patch("qubes.storage.driver_parameters") + def test_160_pool_add_wrong_payload(self, mock_parameters, mock_drivers): + self.app.pools = { + "file": unittest.mock.Mock(), + "lvm": unittest.mock.Mock(), + } + + mock_drivers.return_value = ["driver1", "driver2"] + mock_parameters.side_effect = lambda driver: { + "driver1": {"param1": True, "param2": True}, + "driver2": {"param3": True, "param4": True}, + }[driver] + + add_pool_mock, self.app.add_pool = self.coroutine_mock() + + with self.assertRaises(qubes.exc.ProtocolError): + self.call_mgmt_func( + b"admin.pool.Add", + b"dom0", + b"driver1", + "name=test-pool\nparam1=\u00f6\n".encode(), + ) + self.assertEqual(add_pool_mock.mock_calls, []) + self.assertFalse(self.app.save.called) + @unittest.mock.patch("qubes.storage.pool_drivers") @unittest.mock.patch("qubes.storage.driver_parameters") def test_160_pool_add_missing_param(self, mock_parameters, mock_drivers): @@ -1095,7 +1114,7 @@ def test_160_pool_add_invalid_driver(self, mock_parameters, mock_drivers): add_pool_mock, self.app.add_pool = self.coroutine_mock() - with self.assertRaises(qubes.exc.QubesException): + with self.assertRaises(qubes.exc.PermissionDenied): self.call_mgmt_func( b"admin.pool.Add", b"dom0", @@ -1123,7 +1142,7 @@ def test_160_pool_add_invalid_param(self, mock_parameters, mock_drivers): add_pool_mock, self.app.add_pool = self.coroutine_mock() - with self.assertRaises(qubes.exc.QubesException): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.pool.Add", b"dom0", @@ -1153,7 +1172,7 @@ def test_160_pool_add_missing_name(self, mock_parameters, mock_drivers): add_pool_mock, self.app.add_pool = self.coroutine_mock() - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.pool.Add", b"dom0", @@ -1181,7 +1200,7 @@ def test_160_pool_add_existing_pool(self, mock_parameters, mock_drivers): add_pool_mock, self.app.add_pool = self.coroutine_mock() - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.QubesPoolInUseError): self.call_mgmt_func( b"admin.pool.Add", b"dom0", @@ -1189,7 +1208,6 @@ def test_160_pool_add_existing_pool(self, mock_parameters, mock_drivers): b"name=file\nparam1=value\nparam2=some-value\n", ) self.assertEqual(mock_drivers.mock_calls, [unittest.mock.call()]) - self.assertEqual(mock_parameters.mock_calls, []) self.assertEqual(add_pool_mock.mock_calls, []) self.assertFalse(self.app.save.called) @@ -1211,7 +1229,7 @@ def test_160_pool_add_invalid_config_format( add_pool_mock, self.app.add_pool = self.coroutine_mock() - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.pool.Add", b"dom0", @@ -1267,6 +1285,11 @@ def test_190_label_get(self): ) self.assertFalse(self.app.save.called) + def test_191_label_get_invalid(self): + with self.assertRaises(qubes.exc.QubesInvalidLabelError): + self.call_mgmt_func(b"admin.label.Get", b"dom0", b" ") + self.assertFalse(self.app.save.called) + def test_195_label_index(self): self.app.get_label = unittest.mock.Mock() self.app.get_label.configure_mock(**{"return_value.index": 1}) @@ -1311,13 +1334,31 @@ def test_200_label_create_invalid_color(self): "keys.return_value": range(1, 9), } self.app.labels.configure_mock(**labels_config) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaisesRegex( + qubes.exc.QubesInvalidLabelValueError, "encoded in ASCII" + ): self.call_mgmt_func( - b"admin.label.Create", b"dom0", b"cyan", b"abcd" + b"admin.label.Create", b"dom0", b"cyan", "\u00f6".encode() + ) + with self.assertRaisesRegex( + qubes.exc.QubesInvalidLabelValueError, "length of 8" + ): + self.call_mgmt_func( + b"admin.label.Create", b"dom0", b"cyan", b"0x00fffff" + ) + with self.assertRaisesRegex( + qubes.exc.QubesInvalidLabelValueError, "must start with: 0x" + ): + self.call_mgmt_func( + b"admin.label.Create", b"dom0", b"cyan", b"0X00ffff" + ) + with self.assertRaisesRegex( + qubes.exc.QubesInvalidLabelValueError, + "hexadecimal digits after prefix", + ): + self.call_mgmt_func( + b"admin.label.Create", b"dom0", b"cyan", b"0x00fffg" ) - self.assertEqual( - self.app.get_label.mock_calls, [unittest.mock.call("cyan")] - ) self.assertEqual(self.app.labels.mock_calls, []) self.assertFalse(self.app.save.called) @@ -1329,15 +1370,15 @@ def test_200_label_create_invalid_name(self): "keys.return_value": range(1, 9), } self.app.labels.configure_mock(**labels_config) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.label.Create", b"dom0", b"01", b"0xff0000" ) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.label.Create", b"dom0", b"../xxx", b"0xff0000" ) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.label.Create", b"dom0", @@ -1351,8 +1392,10 @@ def test_200_label_create_invalid_name(self): def test_200_label_create_already_exists(self): self.app.get_label = unittest.mock.Mock(wraps=self.app.get_label) - with self.assertRaises(qubes.exc.QubesValueError): - self.call_mgmt_func(b"admin.label.Create", b"dom0", b"red", b"abcd") + with self.assertRaises(qubes.exc.QubesLabelInUseError): + self.call_mgmt_func( + b"admin.label.Create", b"dom0", b"red", b"0xff0000" + ) self.assertEqual( self.app.get_label.mock_calls, [unittest.mock.call("red")] ) @@ -1373,8 +1416,19 @@ def test_210_label_remove(self): ) self.assertTrue(self.app.save.called) + def test_211_label_remove_in_use(self): + label = qubes.Label(9, "0x00ffff", "cyan") + self.app.labels[9] = label + self.app.get_label = unittest.mock.Mock(**{"return_value.index": 9}) + self.app.labels = unittest.mock.MagicMock(wraps=self.app.labels) + self.vm.label = "cyan" + with self.assertRaises(qubes.exc.QubesLabelInUseError): + self.call_mgmt_func(b"admin.label.Remove", b"dom0", b"cyan") + self.assertEqual(self.app.labels.mock_calls, []) + self.assertFalse(self.app.save.called) + def test_210_label_remove_invalid_label(self): - with self.assertRaises(qubes.exc.QubesValueError): + with self.assertRaises(qubes.exc.QubesLabelNotFoundError): self.call_mgmt_func( b"admin.label.Remove", b"dom0", b"no-such-label" ) @@ -1383,7 +1437,7 @@ def test_210_label_remove_invalid_label(self): def test_210_label_remove_default_label(self): self.app.labels = unittest.mock.MagicMock(wraps=self.app.labels) self.app.get_label = unittest.mock.Mock(**{"return_value.index": 6}) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"admin.label.Remove", b"dom0", b"blue") self.assertEqual(self.app.labels.mock_calls, []) self.assertFalse(self.app.save.called) @@ -1391,7 +1445,7 @@ def test_210_label_remove_default_label(self): def test_210_label_remove_in_use(self): self.app.labels = unittest.mock.MagicMock(wraps=self.app.labels) self.app.get_label = unittest.mock.Mock(**{"return_value.index": 1}) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"admin.label.Remove", b"dom0", b"red") self.assertEqual(self.app.labels.mock_calls, []) self.assertFalse(self.app.save.called) @@ -1473,7 +1527,7 @@ async def coroutine_mock(*args, **kwargs): return func_mock(*args, **kwargs) self.vm.shutdown = coroutine_mock - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"admin.vm.Shutdown", b"test-vm1", b"forcewait") func_mock.assert_not_called() @@ -1657,6 +1711,56 @@ async def fire_event(): ], ) + def test_273_events_single(self): + vm2 = self.app.add_new_vm( + "AppVM", + label="red", + name="test-vm2", + template="test-template", + ) + + with tempfile.TemporaryDirectory() as tmpdir: + tmpdir = pathlib.Path(tmpdir) + with unittest.mock.patch( + "qubes.ext.admin.AdminExtension._instance.policy_cache.paths", + [pathlib.Path(tmpdir)], + ): + with (tmpdir / "admin.policy").open("w") as file: + file.write("admin.Events * test-vm1 test-vm2 allow") + + send_event = unittest.mock.Mock(spec=[]) + mgmt_obj = qubes.api.admin.QubesAdminAPI( + self.app, + b"test-vm1", + b"admin.Events", + b"test-vm2", + b"", + send_event=send_event, + ) + + async def fire_event(): + vm2.fire_event("test-event2", arg1="abc") + self.vm.fire_event("test-event", arg1="abc") + mgmt_obj.cancel() + return vm2 + + loop = asyncio.get_event_loop() + execute_task = asyncio.ensure_future( + mgmt_obj.execute(untrusted_payload=b"") + ) + event_task = asyncio.ensure_future(fire_event()) + loop.run_until_complete(execute_task) + event_task.result() + self.assertIsNone(execute_task.result()) + + self.assertEqual( + send_event.mock_calls, + [ + unittest.mock.call(self.app, "connection-established"), + unittest.mock.call(vm2, "test-event2", arg1="abc"), + ], + ) + def test_280_feature_list(self): self.vm.features["test-feature"] = "some-value" value = self.call_mgmt_func(b"admin.vm.feature.List", b"test-vm1") @@ -1817,8 +1921,8 @@ def test_321_feature_set_empty(self): self.assertEqual(self.vm.features["test-feature"], "") self.assertTrue(self.app.save.called) - def test_322_feature_set_invalid(self): - with self.assertRaises(UnicodeDecodeError): + def test_322_feature_set_invalid_value(self): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.feature.Set", b"test-vm1", @@ -1828,6 +1932,43 @@ def test_322_feature_set_invalid(self): self.assertNotIn("test-feature", self.vm.features) self.assertFalse(self.app.save.called) + with self.assertRaisesRegex( + qubes.exc.ProtocolError, "non-ASCII characters" + ): + self.call_mgmt_func( + b"admin.vm.feature.Set", + b"test-vm1", + b"test-feature", + "\u00f6".encode(), + ) + self.assertNotIn("test-feature", self.vm.features) + self.assertFalse(self.app.save.called) + + def test_322_feature_set_invalid_key(self): + with self.assertRaisesRegex( + qubes.exc.ProtocolError, "expected an argument" + ): + self.call_mgmt_func( + b"admin.vm.feature.Set", + b"test-vm1", + b"", + b"value", + ) + self.assertNotIn("test-feature", self.vm.features) + self.assertFalse(self.app.save.called) + + with self.assertRaisesRegex( + qubes.exc.ProtocolError, "illegal characters" + ): + self.call_mgmt_func( + b"admin.vm.feature.Set", + b"test-vm1", + b" ", + b"", + ) + self.assertNotIn("test-feature", self.vm.features) + self.assertFalse(self.app.save.called) + def test_323_feature_set_service_too_long(self): with self.assertRaises(qubes.exc.QubesValueError): self.call_mgmt_func( @@ -1887,7 +2028,7 @@ def test_330_vm_create_standalone(self, storage_mock): @unittest.mock.patch("qubes.storage.Storage.create") def test_331_vm_create_standalone_spurious_template(self, storage_mock): storage_mock.side_effect = self.dummy_coro - with self.assertRaises(qubes.exc.QubesValueError): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.Create.StandaloneVM", b"dom0", @@ -1932,6 +2073,26 @@ def test_332_vm_create_app(self, storage_mock): self.assertTrue(self.app.save.called) + @unittest.mock.patch("qubes.storage.Storage.create") + def test_333_vm_create_appvm_invalid_template(self, storage_mock): + storage_mock.side_effect = self.dummy_coro + with self.assertRaises(qubes.exc.QubesVMNotFoundError): + self.call_mgmt_func( + b"admin.vm.Create.AppVM", + b"dom0", + b"test-template-inexistent", + b"name=test-vm2 label=red", + ) + self.assertNotIn("test-vm2", self.app.domains) + self.assertEqual(storage_mock.mock_calls, []) + self.assertFalse( + os.path.exists( + os.path.join(self.test_base_dir, "appvms", "test-vm2") + ) + ) + self.assertNotIn("test-vm2", self.app.domains) + self.assertFalse(self.app.save.called) + @unittest.mock.patch("qubes.storage.Storage.create") def test_333_vm_create_app_default_template(self, storage_mock): storage_mock.side_effect = self.dummy_coro @@ -2014,7 +2175,7 @@ def test_336_vm_create_spurious_pool(self, storage_mock): @unittest.mock.patch("qubes.storage.Storage.create") def test_337_vm_create_duplicate_name(self, storage_mock): storage_mock.side_effect = self.dummy_coro - with self.assertRaises(qubes.exc.QubesException): + with self.assertRaises(qubes.exc.QubesVMAlreadyExistsError): self.call_mgmt_func( b"admin.vm.Create.AppVM", b"dom0", @@ -2039,6 +2200,20 @@ def test_338_vm_create_name_twice(self, storage_mock): self.assertNotIn("test-vm3", self.app.domains) self.assertFalse(self.app.save.called) + @unittest.mock.patch("qubes.storage.Storage.create") + def test_339_vm_create_spurious_params(self, storage_mock): + storage_mock.side_effect = self.dummy_coro + with self.assertRaisesRegex(qubes.exc.ProtocolError, "does not match"): + self.call_mgmt_func( + b"admin.vm.Create.AppVM", + b"dom0", + b"test-template", + b"name=test-vm2 label=red what=is", + ) + self.assertNotIn("test-vm2", self.app.domains) + self.assertNotIn("test-vm3", self.app.domains) + self.assertFalse(self.app.save.called) + @unittest.mock.patch("qubes.storage.Storage.create") def test_340_vm_create_in_pool_app(self, storage_mock): storage_mock.side_effect = self.dummy_coro @@ -2140,7 +2315,7 @@ def test_343_vm_create_in_pool_invalid_pool2(self, storage_mock): @unittest.mock.patch("qubes.storage.Storage.create") def test_344_vm_create_in_pool_invalid_volume(self, storage_mock): storage_mock.side_effect = self.dummy_coro - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.CreateInPool.AppVM", b"dom0", @@ -2200,6 +2375,41 @@ def test_410_property_get_str(self): ) self.assertEqual(value, "default=False type=str 1.0") + def test_411_property_get_all(self): + expected = """check_updates_vm default=True type=bool True +clockvm default=True type=vm +default_audiovm default=True type=vm dom0 +default_dispvm default=True type=vm +default_guivm default=True type=vm dom0 +default_kernel default=False type=str 1.0 +default_netvm default=False type=vm +default_pool default=False type=str varlibqubes +default_pool_kernel default=False type=str linux-kernel +default_pool_private default=True type=str varlibqubes +default_pool_root default=True type=str varlibqubes +default_pool_volatile default=True type=str varlibqubes +default_qrexec_timeout default=True type=int 60 +default_shutdown_timeout default=True type=int 60 +default_template default=False type=vm test-template +management_dispvm default=True type=vm +stats_interval default=True type=int 3 +updatevm default=True type=vm \n""" + value = self.call_mgmt_func(b"admin.property.GetAll", b"dom0", b"") + self.maxDiff = None + self.assertEqual(value, expected) + + def test_411_property_get_default_none(self): + value = self.call_mgmt_func( + b"admin.property.GetDefault", b"dom0", b"default_template" + ) + self.assertEqual(value, None) + + def test_411_property_get_default_bool(self): + value = self.call_mgmt_func( + b"admin.property.GetDefault", b"dom0", b"updatevm" + ) + self.assertEqual(value, "type=vm ") + def test_420_propert_set_str(self): # actual function tested for admin.vm.property.* already with unittest.mock.patch("qubes.property.__set__") as mock: @@ -3065,6 +3275,20 @@ def test_503_vm_remove_dom0(self, mock_rmtree, mock_remove): # Import tests # (internal methods, normally called from qubes-rpc script) + def test_509_vm_volume_import_begin_wrong_volume(self): + with self.assertRaises(qubes.exc.ProtocolError): + self.call_internal_mgmt_func( + b"internal.vm.volume.ImportBegin", b"test-vm1", b"inexistent" + ) + self.assertFalse(self.app.save.called) + + def test_509_vm_volume_import_end_wrong_volume(self): + with self.assertRaises(qubes.exc.ProtocolError): + self.call_internal_mgmt_func( + b"internal.vm.volume.ImportEnd", b"test-vm1", b"inexistent" + ) + self.assertFalse(self.app.save.called) + def test_510_vm_volume_import(self): value = self.call_internal_mgmt_func( b"internal.vm.volume.ImportBegin", b"test-vm1", b"private" @@ -3258,6 +3482,64 @@ def test_522_vm_volume_clone_invalid_volume2(self): ) self.assertFalse(self.app.save.called) + def test_522_vm_volume_clone_invalid_token(self): + self.setup_for_clone() + + self.vm.volumes["private"].is_running = lambda: False + self.vm.storage.get_volume = lambda x: x + self.vm2.storage.get_volume = lambda x: x + + token = self.call_mgmt_func( + b"admin.vm.volume.CloneFrom", b"test-vm1", b"private", b"" + ) + with self.assertRaises(qubes.exc.ProtocolError): + self.call_mgmt_func( + b"admin.vm.volume.CloneTo", + b"test-vm1", + b"private", + token.encode() + "\u00f6".encode(), + ) + self.assertNotIn( + "init_volume().import_volume", + map(operator.itemgetter(0), self.pool.mock_calls), + ) + self.assertFalse(self.app.save.called) + + def test_523_vm_volume_clone_wrong_pool(self): + self.setup_for_clone() + + self.vm.volumes["private"].is_running = lambda: False + self.vm.storage.get_volume = lambda x: x + self.vm2.storage.get_volume = lambda x: x + + token = self.call_mgmt_func( + b"admin.vm.volume.CloneFrom", b"test-vm1", b"private", b"" + ) + + self.app.pools = { + "pool1": unittest.mock.Mock( + config={"param1": "value1", "param2": "value2"}, + usage=102400, + size=204800, + volumes={ + "vol1": unittest.mock.Mock(), + "vol2": unittest.mock.Mock(), + }, + ) + } + with self.assertRaises(qubes.exc.QubesPoolNotFoundError): + self.call_mgmt_func( + b"admin.vm.volume.CloneTo", + b"test-vm1", + b"private", + token.encode(), + ) + self.assertNotIn( + "init_volume().import_volume", + map(operator.itemgetter(0), self.pool.mock_calls), + ) + self.assertFalse(self.app.save.called) + def test_523_vm_volume_clone_removed_volume(self): self.setup_for_clone() @@ -3276,7 +3558,7 @@ def get_volume(vid): return unittest.mock.DEFAULT self.pool.get_volume.side_effect = get_volume - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.QubesVolumeNotFoundError): self.call_mgmt_func( b"admin.vm.volume.CloneTo", b"test-vm1", @@ -3296,7 +3578,7 @@ def test_524_vm_volume_clone_invlid_token(self): self.vm.storage.get_volume = lambda x: x self.vm2.storage.get_volume = lambda x: x - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.volume.CloneTo", b"test-vm1", @@ -3368,11 +3650,22 @@ def test_560_tag_set(self): self.assertTrue(self.app.save.called) def test_561_tag_set_invalid(self): - with self.assertRaises(ValueError): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"admin.vm.tag.Set", b"test-vm1", b"+.some-tag") self.assertNotIn("+.some-tag", self.vm.tags) self.assertFalse(self.app.save.called) + def test_562_tag_set_invalid_empty(self): + with self.assertRaises(qubes.exc.ProtocolError): + self.call_mgmt_func(b"admin.vm.tag.Set", b"test-vm1", b"") + self.assertNotIn("", self.vm.tags) + self.assertFalse(self.app.save.called) + + with self.assertRaises(qubes.exc.QubesInvalidTagError): + self.call_mgmt_func(b"admin.vm.tag.Set", b"test-vm1", b"+") + self.assertNotIn("", self.vm.tags) + self.assertFalse(self.app.save.called) + def test_570_firewall_get(self): self.vm.firewall.save = unittest.mock.Mock() value = self.call_mgmt_func(b"admin.vm.firewall.Get", b"test-vm1", b"") @@ -3380,6 +3673,13 @@ def test_570_firewall_get(self): self.assertFalse(self.vm.firewall.save.called) self.assertFalse(self.app.save.called) + def test_570_firewall_get_invalid_dest(self): + self.vm.firewall.save = unittest.mock.Mock() + with self.assertRaisesRegex( + qubes.exc.ProtocolError, "destination to not be AdminVM" + ): + self.call_mgmt_func(b"admin.vm.firewall.Get", b"dom0", b"") + def test_571_firewall_get_non_default(self): self.vm.firewall.save = unittest.mock.Mock() self.vm.firewall.rules = [ @@ -3487,7 +3787,7 @@ def test_585_firewall_set_invalid(self): rules_txt = ( "action=accept dstports=1-1024 comment=ążźł\n" "action=drop\n" ) - with self.assertRaises(UnicodeDecodeError): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.firewall.Set", b"test-vm1", b"", rules_txt.encode() ) @@ -3581,6 +3881,18 @@ def test_602_backup_info_profile_missing_destination_vm(self): b"admin.backup.Info", b"dom0", b"testprofile" ) + def test_603_backup_info_profile_missing(self): + with tempfile.TemporaryDirectory() as profile_dir: + with unittest.mock.patch( + "qubes.config.backup_profile_dir", profile_dir + ): + with self.assertRaises( + qubes.exc.QubesBackupProfileNotFoundError + ): + self.call_mgmt_func( + b"admin.backup.Info", b"dom0", b"testprofile" + ) + def test_610_backup_cancel_not_running(self): with self.assertRaises(qubes.exc.QubesException): self.call_mgmt_func(b"admin.backup.Cancel", b"dom0", b"testprofile") @@ -4003,6 +4315,17 @@ def test_642_vm_create_disposable_not_allowed(self, storage_mock): self.call_mgmt_func(b"admin.vm.CreateDisposable", b"test-vm1") self.assertFalse(self.app.save.called) + @unittest.mock.patch("qubes.storage.Storage.create") + def test_642_vm_create_disposable_wrong_arg(self, mock_storage): + mock_storage.side_effect = self.dummy_coro + self.vm.template_for_dispvms = True + self.app.default_dispvm = self.vm + with self.assertRaises(qubes.exc.PermissionDenied): + self.call_mgmt_func( + b"admin.vm.CreateDisposable", b"test-vm1", b"oops" + ) + self.assertFalse(self.app.save.called) + @unittest.mock.patch("qubes.vm.dispvm.DispVM.start") @unittest.mock.patch("qubes.storage.Storage.verify") @unittest.mock.patch("qubes.storage.Storage.create") @@ -4230,7 +4553,7 @@ def test_655_vm_device_set_mode_invalid_value(self): with unittest.mock.patch.object( qubes.vm.qubesvm.QubesVM, "is_halted", lambda _: False ): - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.device.testclass.Set.assignment", b"test-vm1", @@ -4284,7 +4607,7 @@ def test_663_vm_device_denied_add_multiple(self): def test_664_vm_device_denied_add_repeated(self): self.vm.devices_denied = "b******p012345p53**2*" - with self.assertRaises(qubes.exc.QubesValueError): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.device.denied.Add", b"test-vm1", @@ -4294,6 +4617,20 @@ def test_664_vm_device_denied_add_repeated(self): self.assertEqual(self.vm.devices_denied, "b******p012345p53**2*") self.assertFalse(self.app.save.called) + def test_664_vm_device_denied_add_invalid(self): + self.vm.devices_denied = "b******p012345p53**2*" + with self.assertRaisesRegex( + qubes.exc.ProtocolError, "non-ASCII characters" + ): + self.call_mgmt_func( + b"admin.vm.device.denied.Add", + b"test-vm1", + b"", + "u112233u112233\u00f6\n".encode(), + ) + self.assertEqual(self.vm.devices_denied, "b******p012345p53**2*") + self.assertFalse(self.app.save.called) + def test_665_vm_device_denied_add_present(self): self.vm.devices_denied = "b******p012345p53**2*" self.call_mgmt_func( @@ -4320,7 +4657,7 @@ def test_670_vm_device_denied_remove(self): def test_671_vm_device_denied_remove_repeated(self): self.vm.devices_denied = "b******p012345p53**2*" - with self.assertRaises(qubes.exc.QubesValueError): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( b"admin.vm.device.denied.Remove", b"test-vm1", @@ -4330,6 +4667,20 @@ def test_671_vm_device_denied_remove_repeated(self): self.assertEqual(self.vm.devices_denied, "b******p012345p53**2*") self.assertFalse(self.app.save.called) + def test_671_vm_device_denied_remove_invalid(self): + self.vm.devices_denied = "b******p012345p53**2*" + with self.assertRaisesRegex( + qubes.exc.ProtocolError, "non-ASCII characters" + ): + self.call_mgmt_func( + b"admin.vm.device.denied.Remove", + b"test-vm1", + b"", + "u112233u112233\u00f6\n".encode(), + ) + self.assertEqual(self.vm.devices_denied, "b******p012345p53**2*") + self.assertFalse(self.app.save.called) + def test_672_vm_device_denied_remove_all(self): self.vm.devices_denied = "b******p012345p53**2*" self.call_mgmt_func( @@ -4841,17 +5192,16 @@ def test_991_vm_unexpected_argument(self): vm_mock.qid = self.vm.qid vm_mock.__lt__ = lambda x, y: x.qid < y.qid self.app.domains._dict[self.vm.qid] = vm_mock - exceptions = (qubes.exc.PermissionDenied, qubes.exc.ProtocolError) for method in methods_with_no_argument: # should reject argument regardless of having payload or not with self.subTest(method.decode("ascii")): - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(method, b"test-vm1", b"some-arg", b"") self.assertFalse(vm_mock.called) self.assertFalse(self.app.save.called) with self.subTest(method.decode("ascii") + "+payload"): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( method, b"test-vm1", b"unexpected-arg", b"some-payload" ) @@ -4920,17 +5270,16 @@ def test_993_dom0_unexpected_argument(self): vm_mock.qid = self.vm.qid vm_mock.__lt__ = lambda x, y: x.qid < y.qid self.app.domains._dict[self.vm.qid] = vm_mock - exceptions = (qubes.exc.PermissionDenied, qubes.exc.ProtocolError) for method in methods_with_no_argument: # should reject argument regardless of having payload or not with self.subTest(method.decode("ascii")): - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(method, b"dom0", b"some-arg", b"") self.assertFalse(vm_mock.called) self.assertFalse(self.app.save.called) with self.subTest(method.decode("ascii") + "+payload"): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( method, b"dom0", b"unexpected-arg", b"some-payload" ) @@ -4976,29 +5325,28 @@ def test_994_dom0_only_calls(self): vm_mock.qid = self.vm.qid vm_mock.__lt__ = lambda x, y: x.qid < y.qid self.app.domains._dict[self.vm.qid] = vm_mock - exceptions = (qubes.exc.PermissionDenied, qubes.exc.ProtocolError) for method in methods_for_dom0_only: # should reject call regardless of having payload or not with self.subTest(method.decode("ascii")): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(method, b"test-vm1", b"", b"") self.assertFalse(vm_mock.called) self.assertFalse(self.app.save.called) with self.subTest(method.decode("ascii") + "+arg"): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(method, b"test-vm1", b"some-arg", b"") self.assertFalse(vm_mock.called) self.assertFalse(self.app.save.called) with self.subTest(method.decode("ascii") + "+payload"): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(method, b"test-vm1", b"", b"payload") self.assertFalse(vm_mock.called) self.assertFalse(self.app.save.called) with self.subTest(method.decode("ascii") + "+arg+payload"): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( method, b"test-vm1", b"some-arg", b"some-payload" ) @@ -5061,30 +5409,29 @@ def test_995_vm_only_calls(self): vm_mock.qid = self.vm.qid vm_mock.__lt__ = lambda x, y: x.qid < y.qid self.app.domains._dict[self.vm.qid] = vm_mock - exceptions = (qubes.exc.PermissionDenied, qubes.exc.ProtocolError) for method in methods_for_vm_only: # should reject payload regardless of having argument or not # should reject call regardless of having payload or not with self.subTest(method.decode("ascii")): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(method, b"dom0", b"", b"") self.assertFalse(vm_mock.called) self.assertFalse(self.app.save.called) with self.subTest(method.decode("ascii") + "+arg"): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(method, b"dom0", b"some-arg", b"") self.assertFalse(vm_mock.called) self.assertFalse(self.app.save.called) with self.subTest(method.decode("ascii") + "+payload"): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(method, b"dom0", b"", b"payload") self.assertFalse(vm_mock.called) self.assertFalse(self.app.save.called) with self.subTest(method.decode("ascii") + "+arg+payload"): - with self.assertRaises(exceptions): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func( method, b"dom0", b"some-arg", b"some-payload" ) diff --git a/qubes/tests/api_misc.py b/qubes/tests/api_misc.py index fafeb4ecf..c3d29a2b0 100644 --- a/qubes/tests/api_misc.py +++ b/qubes/tests/api_misc.py @@ -39,6 +39,7 @@ def setUp(self): self.app = mock.NonCallableMock() self.dest = mock.NonCallableMock() self.dest.name = "dom0" + self.dest.klass = "AdminVM" self.app.configure_mock( domains={ "dom0": self.dest, @@ -117,7 +118,7 @@ def test_002_features_request_invalid1(self): "/features-request/feature1": b"test$chars", } self.configure_qdb(qdb_entries) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"qubes.FeaturesRequest") self.assertEqual(self.app.mock_calls, []) self.assertEqual( @@ -217,7 +218,7 @@ def test_015_notify_tools_invalid_value_qrexec(self): "/qubes-tools/default-user": b"user", } self.configure_qdb(qdb_entries) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"qubes.NotifyTools") self.assertEqual(self.app.mock_calls, []) self.assertEqual( @@ -236,7 +237,7 @@ def test_016_notify_tools_invalid_value_gui(self): "/qubes-tools/default-user": b"user", } self.configure_qdb(qdb_entries) - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"qubes.NotifyTools") self.assertEqual(self.app.mock_calls, []) self.assertEqual( @@ -296,7 +297,7 @@ def test_021_notify_updates_standalone2(self): def test_022_notify_updates_invalid(self): del self.src.template - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"qubes.NotifyUpdates", payload=b"") self.assertEqual(self.src.mock_calls, []) self.assertEqual(self.tpl.mock_calls, []) @@ -304,7 +305,7 @@ def test_022_notify_updates_invalid(self): def test_023_notify_updates_invalid2(self): del self.src.template - with self.assertRaises(qubes.exc.PermissionDenied): + with self.assertRaises(qubes.exc.ProtocolError): self.call_mgmt_func(b"qubes.NotifyUpdates", payload=b"no updates") self.assertEqual(self.src.mock_calls, []) self.assertEqual(self.tpl.mock_calls, []) diff --git a/qubes/tests/vm/dispvm.py b/qubes/tests/vm/dispvm.py index 393dc16b9..138f9d7d0 100644 --- a/qubes/tests/vm/dispvm.py +++ b/qubes/tests/vm/dispvm.py @@ -467,7 +467,7 @@ def test_011_create_direct_reject(self): "__getitem__.side_effect": orig_getitem, } ) - with self.assertRaises(qubes.exc.QubesException): + with self.assertRaises(qubes.exc.QubesValueError): self.app.add_new_vm( qubes.vm.dispvm.DispVM, name="test-dispvm", diff --git a/qubes/tests/vm/qubesvm.py b/qubes/tests/vm/qubesvm.py index 492621639..4900f4ba7 100644 --- a/qubes/tests/vm/qubesvm.py +++ b/qubes/tests/vm/qubesvm.py @@ -64,6 +64,15 @@ def __init__(self): self.host = unittest.mock.Mock() self.host.memory_total = 4096 * 1024 + def get_label(self, label): + # pylint: disable=unused-argument + if label in self.labels: + return self.labels[label] + for l in self.labels.values(): + if l.name == label: + return l + raise qubes.exc.QubesLabelNotFoundError(label) + class TestFeatures(dict): def __init__(self, vm, **kwargs) -> None: diff --git a/qubes/tools/__init__.py b/qubes/tools/__init__.py index 8c13c8955..56c83da56 100644 --- a/qubes/tools/__init__.py +++ b/qubes/tools/__init__.py @@ -475,6 +475,15 @@ def set_qubes_verbosity(namespace): This is done by configuring global logging. :param argparse.Namespace args: args as parsed by parser """ + if namespace.debug: + qubes.log.enable( + log_level=logging.DEBUG, + enable_debug_libvirt=getattr( + namespace, "enable_debug_libvirt", False + ), + ) + return + verbose = namespace.verbose - namespace.quiet levels = [logging.NOTSET, logging.INFO, logging.DEBUG] log_level = levels[min(verbose, 2)] diff --git a/qubes/tools/qubesd.py b/qubes/tools/qubesd.py index d0ff7f5df..8ff3d5c09 100644 --- a/qubes/tools/qubesd.py +++ b/qubes/tools/qubesd.py @@ -38,6 +38,18 @@ def sighandler(loop, signame, servers, app): help="Enable verbose error logging (all exceptions with full " "tracebacks) and also send tracebacks to Admin API clients", ) +parser.add_argument( + "--enable-debug-libvirt", + action="store_true", + default=False, + help="Enables verbose logging of libvirtaio virEventAsyncIOImpl handler", +) +parser.add_argument( + "--log-dom0-call", + action="store_true", + default=False, + help="Enable logging API calls made by dom0", +) def main(args=None): @@ -54,9 +66,6 @@ def main(args=None): # Stop storage for domains not currently running loop.run_until_complete(args.app.stop_storage()) - if args.debug: - qubes.log.enable(log_level=10) - servers = loop.run_until_complete( qubes.api.create_servers( qubes.api.admin.QubesAdminAPI, @@ -64,6 +73,8 @@ def main(args=None): qubes.api.misc.QubesMiscAPI, app=args.app, debug=args.debug, + log_dom0_call=args.log_dom0_call + or os.environ.get("QUBES_LOG_DOM0_CALL"), ) ) diff --git a/qubes/utils.py b/qubes/utils.py index 49e0d0e89..5a324f1df 100644 --- a/qubes/utils.py +++ b/qubes/utils.py @@ -510,3 +510,49 @@ def is_pci_path(device_id: str): r"(-[0-9a-f]{2}_[0-9a-f]{2}\.[0-9a-f])*\Z" ) return bool(path_re.match(device_id)) + + +def validate_label_name(untrusted_label, creation: bool = False) -> None: + if not untrusted_label: + raise qubes.exc.QubesInvalidLabelError("Label name must be set") + + # don't confuse label name with label index + if creation and str(untrusted_label).isdigit(): + raise qubes.exc.QubesInvalidLabelError("Label must not be a digit") + + allowed_chars = string.ascii_letters + string.digits + "-_." + if not all(c in allowed_chars for c in untrusted_label): + raise qubes.exc.QubesInvalidLabelError( + "Label must be in safe set: " + allowed_chars + ) + + +def validate_label_value(untrusted_label_value) -> None: + if not untrusted_label_value: + raise qubes.exc.QubesInvalidLabelValueError("Label value must be set") + + try: + untrusted_value = untrusted_label_value.decode( + "ascii", "strict" + ).strip() + except UnicodeDecodeError: + raise qubes.exc.QubesInvalidLabelValueError( + "Label value must be encoded in ASCII" + ) + + if len(untrusted_value) != 8: + raise qubes.exc.QubesInvalidLabelValueError( + "Label value must have length of 8" + ) + + if not untrusted_value.startswith("0x"): + raise qubes.exc.QubesInvalidLabelValueError( + "Label value must start with: 0x" + ) + + # besides prefix, only hex digits are allowed + if not all(x in string.hexdigits for x in untrusted_value[2:]): + raise qubes.exc.QubesInvalidLabelValueError( + "Label value must only contain hexadecimal digits after prefix: " + + string.hexdigits + ) diff --git a/qubes/vm/__init__.py b/qubes/vm/__init__.py index de9431dc7..adfa97929 100644 --- a/qubes/vm/__init__.py +++ b/qubes/vm/__init__.py @@ -100,8 +100,7 @@ def setter_label(self, prop, value): if isinstance(value, qubes.Label): return value if isinstance(value, str) and value.startswith("label-"): - return self.app.labels[int(value.split("-", 1)[1])] - + value = int(value.split("-", 1)[1]) return self.app.get_label(value) @@ -195,9 +194,11 @@ def remove(self, elem): @staticmethod def validate_tag(tag): + if not tag: + raise qubes.exc.QubesInvalidTagError("tag cannot be empty") safe_set = string.ascii_letters + string.digits + "-_" if not all((x in safe_set) for x in tag): - raise ValueError("disallowed characters") + raise qubes.exc.QubesInvalidTagError("disallowed characters") class BaseVM(qubes.PropertyHolder):