qubes/features: check_with_(template_and_)adminvm

- Two new methods: .features.check_with_adminvm() and
  .check_with_template_and_adminvm(). Common code refactored.

- Two new AdminAPI calls to take advantage of the methods:
  - admin.vm.feature.CheckWithAdminVM
  - admin.vm.feature.CheckWithTemplateAndAdminVM

- Features manager moved to separate module in anticipation of features
  on app object in R5.0. The attribute Features.vm renamed to
  Features.subject.

- Documentation, tests.

Makefile

@@ -72,6 +72,8 @@ ADMIN_API_METHODS_SIMPLE = \
 	admin.vm.device.mic.Set.persistent \
 	admin.vm.feature.CheckWithNetvm \
 	admin.vm.feature.CheckWithTemplate \
+	admin.vm.feature.CheckWithAdminVM \
+	admin.vm.feature.CheckWithTemplateAndAdminVM \
 	admin.vm.feature.Get \
 	admin.vm.feature.List \
 	admin.vm.feature.Remove \

doc/qubes-features.rst

@@ -1,5 +1,5 @@
-:py:class:`qubes.vm.Features` - Qubes VM features, services
+:py:mod:`qubes.features` - Qubes VM features, services
-============================================================
+======================================================
 
 Features are generic mechanism for storing key-value pairs attached to a
 VM. The primary use case for them is data storage for extensions (you can think
@@ -18,12 +18,12 @@ however if you assign instances of :py:class:`bool`, they are converted as
 described above. Be aware that assigning the number `0` (which is considered
 false in Python) will result in string `'0'`, which is considered true.
 
-:py:class:`qubes.vm.Features` inherits from :py:class:`dict`, so provide all the
+:py:class:`qubes.features.Features` inherits from :py:class:`dict`, so provide
-standard functions to get, list and set values.  Additionally provide helper
+all the standard functions to get, list and set values.  Additionally provide
-functions to check if given feature is set on the VM and default to the value
+helper functions to check if given feature is set on the VM and default to the
-on the VM's template or netvm. This is useful for features which nature is
+value on the VM's template or netvm. This is useful for features which nature is
-inherited from other VMs, like "is package X is installed" or "is VM behind a
+inherited from other VMs, like "is package X is installed" or "is VM behind
-VPN".
+a VPN".
 
 Example usage of features in extension:
 
@@ -31,12 +31,12 @@ Example usage of features in extension:
 
    import qubes.exc
    import qubes.ext
-   
+
    class ExampleExtension(qubes.ext.Extension):
       @qubes.ext.handler('domain-pre-start')
       def on_domain_start(self, vm, event, **kwargs):
          if vm.features.get('do-not-start', False):
-            raise qubes.exc.QubesVMError(vm, 
+            raise qubes.exc.QubesVMError(vm,
                'Start prohibited because of do-not-start feature')
 
          if vm.features.check_with_template('something-installed', False):
@@ -45,7 +45,8 @@ Example usage of features in extension:
 The above extension does two things:
 
  - prevent starting a qube with ``do-not-start`` feature set
- - do something when ``something-installed`` feature is set on the qube, or its template
+ - do something when ``something-installed`` feature is set on the qube, or its
+   template
 
 
 qvm-features-request, qubes.PostInstall service
@@ -53,9 +54,9 @@ qvm-features-request, qubes.PostInstall service
 
 When some package in the VM want to request feature to be set (aka advertise
 support for it), it should place a shell script in ``/etc/qubes/post-install.d``.
-This script should call :program:`qvm-features-request` with ``FEATURE=VALUE`` pair(s) as
+This script should call :program:`qvm-features-request` with ``FEATURE=VALUE``
-arguments to request those features. It is recommended to use very simple
+pair(s) as arguments to request those features. It is recommended to use very
-values here (for example ``1``). The script should be named in form
+simple values here (for example ``1``). The script should be named in form
 ``XX-package-name.sh`` where ``XX`` is two-digits number below 90 and
 ``package-name`` is unique name specific to this package (preferably actual
 package name). The script needs executable bit set.
@@ -65,17 +66,17 @@ installation and also after initial template installation.
 This way package have a chance to report to dom0 if any feature is
 added/removed.
 
-The features flow to dom0 according to the diagram below. Important part is
+The features flow to dom0 according to the diagram below. Important part is that
-that qubes core :py:class:`qubes.ext.Extension` is responsible for handling such request in
+qubes core :py:class:`qubes.ext.Extension` is responsible for handling such
-``features-request`` event handler. If no extension handles given feature request,
+request in ``features-request`` event handler. If no extension handles given
-it will be ignored. The extension should carefuly validate requested
+feature request, it will be ignored. The extension should carefuly validate
-features (ignoring those not recognized - may be for another extension) and
+requested features (ignoring those not recognized - may be for another
-only then set appropriate value on VM object
+extension) and only then set appropriate value on VM object
 (:py:attr:`qubes.vm.BaseVM.features`). It is recommended to make the
 verification code as bulletproof  as possible (for example allow only specific
-simple values, instead of complex structures), because feature requests
+simple values, instead of complex structures), because feature requests come
-come from untrusted sources. The features actually set on the VM in some cases
+from untrusted sources. The features actually set on the VM in some cases may
-may not be necessary those requested. Similar for values.
+not be necessary those requested. Similar for values.
 
 .. graphviz::
 
@@ -185,7 +186,7 @@ Services and features can be then inspected from dom0 using
 Module contents
 ---------------
 
-.. autoclass:: qubes.vm.Features
+.. automodule:: qubes.features
    :members:
    :show-inheritance:
 

qubes/api/admin.py

@@ -938,6 +938,32 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
             raise qubes.exc.QubesFeatureNotFoundError(self.dest, self.arg)
         return value
 
+    @qubes.api.method('admin.vm.feature.CheckWithAdminVM', no_payload=True,
+        scope='local', read=True)
+    @asyncio.coroutine
+    def vm_feature_checkwithadminvm(self):
+        # validation of self.arg done by qrexec-policy is enough
+
+        self.fire_event_for_permission()
+        try:
+            value = self.dest.features.check_with_adminvm(self.arg)
+        except KeyError:
+            raise qubes.exc.QubesFeatureNotFoundError(self.dest, self.arg)
+        return value
+
+    @qubes.api.method('admin.vm.feature.CheckWithTemplateAndAdminVM',
+        no_payload=True, scope='local', read=True)
+    @asyncio.coroutine
+    def vm_feature_checkwithtpladminvm(self):
+        # validation of self.arg done by qrexec-policy is enough
+
+        self.fire_event_for_permission()
+        try:
+            value = self.dest.features.check_with_template_and_adminvm(self.arg)
+        except KeyError:
+            raise qubes.exc.QubesFeatureNotFoundError(self.dest, self.arg)
+        return value
+
     @qubes.api.method('admin.vm.feature.Remove', no_payload=True,
         scope='local', write=True)
     @asyncio.coroutine

qubes/features.py

@@ -0,0 +1,180 @@
+#
+# The Qubes OS Project, https://www.qubes-os.org/
+#
+# Copyright (C) 2010-2015  Joanna Rutkowska <joanna@invisiblethingslab.com>
+# Copyright (C) 2011-2015  Marek Marczykowski-Górecki
+#                              <marmarek@invisiblethingslab.com>
+# Copyright (C) 2014-2018  Wojtek Porczyk <woju@invisiblethingslab.com>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, see <https://www.gnu.org/licenses/>.
+#
+
+from . import vm as _vm
+
+_NO_DEFAULT = object()
+
+class Features(dict):
+    '''Manager of the features.
+
+    Features can have three distinct values: no value (not present in mapping,
+    which is closest thing to :py:obj:`None`), empty string (which is
+    interpreted as :py:obj:`False`) and non-empty string, which is
+    :py:obj:`True`. Anything assigned to the mapping is coerced to strings,
+    however if you assign instances of :py:class:`bool`, they are converted as
+    described above. Be aware that assigning the number `0` (which is considered
+    false in Python) will result in string `'0'`, which is considered true.
+
+    This class inherits from dict, but has most of the methods that manipulate
+    the item disarmed (they raise NotImplementedError). The ones that are left
+    fire appropriate events on the qube that owns an instance of this class.
+    '''
+
+    #
+    # Those are the methods that affect contents. Either disarm them or make
+    # them report appropriate events. Good approach is to rewrite them carefully
+    # using official documentation, but use only our (overloaded) methods.
+    #
+    def __init__(self, subject, other=None, **kwargs):
+        super().__init__()
+        self.subject = subject
+        self.update(other, **kwargs)
+
+    def __delitem__(self, key):
+        super().__delitem__(key)
+        self.subject.fire_event('domain-feature-delete:' + key, feature=key)
+
+    def __setitem__(self, key, value):
+        if value is None or isinstance(value, bool):
+            value = '1' if value else ''
+        else:
+            value = str(value)
+        try:
+            oldvalue = self[key]
+            has_oldvalue = True
+        except KeyError:
+            has_oldvalue = False
+        super().__setitem__(key, value)
+        if has_oldvalue:
+            self.subject.fire_event('domain-feature-set:' + key, feature=key,
+                value=value, oldvalue=oldvalue)
+        else:
+            self.subject.fire_event('domain-feature-set:' + key, feature=key,
+                value=value)
+
+    def clear(self):
+        for key in tuple(self):
+            del self[key]
+
+    def pop(self, _key, _default=None):
+        '''Not implemented
+        :raises: NotImplementedError
+        '''
+        raise NotImplementedError()
+
+    def popitem(self):
+        '''Not implemented
+        :raises: NotImplementedError
+        '''
+        raise NotImplementedError()
+
+    def setdefault(self, _key, _default=None):
+        '''Not implemented
+        :raises: NotImplementedError
+        '''
+        raise NotImplementedError()
+
+    def update(self, other=None, **kwargs):
+        if other is not None:
+            if hasattr(other, 'keys'):
+                for key in other:
+                    self[key] = other[key]
+            else:
+                for key, value in other:
+                    self[key] = value
+
+        for key in kwargs:
+            self[key] = kwargs[key]
+
+    #
+    # end of overriding
+    #
+
+    def _recursive_check(self, attr=None, *, feature, default,
+            check_adminvm=False, check_app=False):
+        '''Recursive search for a feature.
+
+        Traverse domains along one attribute, like
+        :py:attr:`qubes.vm.qubesvm.QubesVM.netvm` or
+        :py:attr:`qubes.vm.appvm.AppVM.template`, starting with current domain
+        (:py:attr:`subject`). Search stops when first `feature` is found. If
+        a qube has no attribute, or if the attribute is :py:obj:`None`, the
+        *default* is returned, or if not specified, :py:class:`KeyError` is
+        raised.
+
+        If `check_adminvm` is true, before returning default, also AdminVM is
+        consulted (the recursion does not restart).
+
+        If `check_app` is true, also the app feature is checked. This is not
+        implemented, as app does not have features yet.
+        '''
+        if check_app:
+            raise NotImplementedError('app does not have features yet')
+
+        assert isinstance(self.subject, _vm.BaseVM), (
+            'recursive checks do not work for {}'.format(
+                type(self.subject).__name__))
+
+        subject = self.subject
+        while subject is not None:
+            try:
+                return subject.features[feature]
+            except KeyError:
+                if attr is None:
+                    break
+                subject = getattr(subject, attr, None)
+
+        if check_adminvm:
+            adminvm = self.subject.app.domains['dom0']
+            if adminvm not in (None, self.subject):
+                try:
+                    return adminvm.features[feature]
+                except KeyError:
+                    pass
+
+        # TODO check_app
+
+        if default is not _NO_DEFAULT:
+            return default
+
+        raise KeyError(feature)
+
+    def check_with_template(self, feature, default=_NO_DEFAULT):
+        '''Check if the subject's template has the specified feature.'''
+        return self._recursive_check('template',
+            feature=feature, default=default)
+
+    def check_with_netvm(self, feature, default=_NO_DEFAULT):
+        '''Check if the subject's netvm has the specified feature.'''
+        return self._recursive_check('netvm',
+            feature=feature, default=default)
+
+    def check_with_adminvm(self, feature, default=_NO_DEFAULT):
+        '''Check if the AdminVM has the specified feature.'''
+        return self._recursive_check(check_adminvm=True,
+            feature=feature, default=default)
+
+    def check_with_template_and_adminvm(self, feature, default=_NO_DEFAULT):
+        '''Check if the template and AdminVM has the specified feature.'''
+        return self._recursive_check('template', check_adminvm=True,
+            feature=feature, default=default)

qubes/tests/api_admin.py

@@ -1123,6 +1123,28 @@ class TC_00_VMs(AdminAPITestCase):
                 b'test-vm1', b'test-feature')
         self.assertFalse(self.app.save.called)
 
+    def test_318_feature_checkwithadminvm(self):
+        self.app.domains['dom0'].features['test-feature'] = 'some-value'
+        value = self.call_mgmt_func(b'admin.vm.feature.CheckWithAdminVM',
+            b'test-vm1', b'test-feature')
+        self.assertEqual(value, 'some-value')
+        self.assertFalse(self.app.save.called)
+
+    def test_319_feature_checkwithtpladminvm(self):
+        self.app.domains['dom0'].features['test-feature'] = 'some-value'
+        value = self.call_mgmt_func(
+            b'admin.vm.feature.CheckWithTemplateAndAdminVM',
+            b'test-vm1', b'test-feature')
+        self.assertEqual(value, 'some-value')
+
+        self.template.features['test-feature'] = 'some-value2'
+        value = self.call_mgmt_func(
+            b'admin.vm.feature.CheckWithTemplateAndAdminVM',
+            b'test-vm1', b'test-feature')
+        self.assertEqual(value, 'some-value2')
+
+        self.assertFalse(self.app.save.called)
+
     def test_320_feature_set(self):
         value = self.call_mgmt_func(b'admin.vm.feature.Set',
             b'test-vm1', b'test-feature', b'some-value')

qubes/tests/vm/init.py

@@ -213,7 +213,7 @@ class TC_21_Features(qubes.tests.QubesTestCase):
     def setUp(self):
         super(TC_21_Features, self).setUp()
         self.vm = qubes.tests.TestEmitter()
-        self.features = qubes.vm.Features(self.vm)
+        self.features = qubes.features.Features(self.vm)
 
     def test_000_set(self):
         self.features['testfeature'] = 'value'

qubes/vm/__init__.py

@@ -33,6 +33,7 @@ import lxml.etree
 import qubes
 import qubes.devices
 import qubes.events
+import qubes.features
 import qubes.log
 
 VM_ENTRY_POINT = 'qubes.vm'
@@ -85,122 +86,6 @@ def _setter_qid(self, prop, value):
                 prop.__name__))
     return value
 
-class Features(dict):
-    '''Manager of the features.
-
-    Features can have three distinct values: no value (not present in mapping,
-    which is closest thing to :py:obj:`None`), empty string (which is
-    interpreted as :py:obj:`False`) and non-empty string, which is
-    :py:obj:`True`. Anything assigned to the mapping is coerced to strings,
-    however if you assign instances of :py:class:`bool`, they are converted as
-    described above. Be aware that assigning the number `0` (which is considered
-    false in Python) will result in string `'0'`, which is considered true.
-
-    This class inherits from dict, but has most of the methods that manipulate
-    the item disarmed (they raise NotImplementedError). The ones that are left
-    fire appropriate events on the qube that owns an instance of this class.
-    '''
-
-    #
-    # Those are the methods that affect contents. Either disarm them or make
-    # them report appropriate events. Good approach is to rewrite them carefully
-    # using official documentation, but use only our (overloaded) methods.
-    #
-    def __init__(self, vm, other=None, **kwargs):
-        super(Features, self).__init__()
-        self.vm = vm
-        self.update(other, **kwargs)
-
-    def __delitem__(self, key):
-        super(Features, self).__delitem__(key)
-        self.vm.fire_event('domain-feature-delete:' + key, feature=key)
-
-    def __setitem__(self, key, value):
-        if value is None or isinstance(value, bool):
-            value = '1' if value else ''
-        else:
-            value = str(value)
-        try:
-            oldvalue = self[key]
-            has_oldvalue = True
-        except KeyError:
-            has_oldvalue = False
-        super(Features, self).__setitem__(key, value)
-        if has_oldvalue:
-            self.vm.fire_event('domain-feature-set:' + key, feature=key,
-                value=value, oldvalue=oldvalue)
-        else:
-            self.vm.fire_event('domain-feature-set:' + key, feature=key,
-                value=value)
-
-    def clear(self):
-        for key in tuple(self):
-            del self[key]
-
-    def pop(self, _key, _default=None):
-        '''Not implemented
-        :raises: NotImplementedError
-        '''
-        raise NotImplementedError()
-
-    def popitem(self):
-        '''Not implemented
-        :raises: NotImplementedError
-        '''
-        raise NotImplementedError()
-
-    def setdefault(self, _key, _default=None):
-        '''Not implemented
-        :raises: NotImplementedError
-        '''
-        raise NotImplementedError()
-
-    def update(self, other=None, **kwargs):
-        if other is not None:
-            if hasattr(other, 'keys'):
-                for key in other:
-                    self[key] = other[key]
-            else:
-                for key, value in other:
-                    self[key] = value
-
-        for key in kwargs:
-            self[key] = kwargs[key]
-
-    #
-    # end of overriding
-    #
-
-    _NO_DEFAULT = object()
-
-    def check_with_template(self, feature, default=_NO_DEFAULT):
-        ''' Check if the vm's template has the specified feature. '''
-        if feature in self:
-            return self[feature]
-
-        if hasattr(self.vm, 'template') and self.vm.template is not None:
-            return self.vm.template.features.check_with_template(feature,
-                default)
-
-        if default is self._NO_DEFAULT:
-            raise KeyError(feature)
-
-        return default
-
-    def check_with_netvm(self, feature, default=_NO_DEFAULT):
-        ''' Check if the vm's netvm has the specified feature. '''
-        if feature in self:
-            return self[feature]
-
-        if hasattr(self.vm, 'netvm') and self.vm.netvm is not None:
-            return self.vm.netvm.features.check_with_netvm(feature,
-                default)
-
-        if default is self._NO_DEFAULT:
-            raise KeyError(feature)
-
-        return default
-
 
 class Tags(set):
     '''Manager of the tags.
@@ -332,7 +217,7 @@ class BaseVM(qubes.PropertyHolder):
         super(BaseVM, self).__init__(xml, **kwargs)
 
         #: dictionary of features of this qube
-        self.features = Features(self, features)
+        self.features = qubes.features.Features(self, features)
 
         #: :py:class:`DeviceManager` object keeping devices that are attached to
         #: this domain

rpm_spec/core-dom0.spec.in

@@ -223,6 +223,7 @@ fi
 %{python3_sitelib}/qubes/dochelpers.py
 %{python3_sitelib}/qubes/events.py
 %{python3_sitelib}/qubes/exc.py
+%{python3_sitelib}/qubes/features.py
 %{python3_sitelib}/qubes/firewall.py
 %{python3_sitelib}/qubes/log.py
 %{python3_sitelib}/qubes/rngdoc.py