Browse Source

admin: raise QubesNoSuchPropertyError for non-existing properties

Accessing non-existing property is a common action (for example
hasattr() do try to access the property). So, introduce specific
exception, inheriting from AttributeError. It will behave very similar
to standard (non-Admin-API) property access.

This exception is reported to the Admin API user, so it will be possible
to distinguish between non-existing property and access denied. But it
isn't any significant information leak, as list of valid properties is
publicly available in the source code.

QubesOS/qubes-issues#853
Marek Marczykowski-Górecki 7 years ago
parent
commit
64b83fa95a
3 changed files with 21 additions and 6 deletions
  1. 8 4
      qubes/api/admin.py
  2. 11 0
      qubes/exc.py
  3. 2 2
      qubes/tests/api_admin.py

+ 8 - 4
qubes/api/admin.py

@@ -137,7 +137,8 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
         return self._property_get(self.app)
 
     def _property_get(self, dest):
-        assert self.arg in dest.property_list()
+        if self.arg not in dest.property_list():
+            raise qubes.exc.QubesNoSuchPropertyError(dest, self.arg)
 
         self.fire_event_for_permission()
 
@@ -180,7 +181,8 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
             untrusted_payload=untrusted_payload)
 
     def _property_set(self, dest, untrusted_payload):
-        assert self.arg in dest.property_list()
+        if self.arg not in dest.property_list():
+            raise qubes.exc.QubesNoSuchPropertyError(dest, self.arg)
 
         property_def = dest.property_get_def(self.arg)
         newvalue = property_def.sanitize(untrusted_newvalue=untrusted_payload)
@@ -204,7 +206,8 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
         return self._property_help(self.app)
 
     def _property_help(self, dest):
-        assert self.arg in dest.property_list()
+        if self.arg not in dest.property_list():
+            raise qubes.exc.QubesNoSuchPropertyError(dest, self.arg)
 
         self.fire_event_for_permission()
 
@@ -229,7 +232,8 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
         return self._property_reset(self.app)
 
     def _property_reset(self, dest):
-        assert self.arg in dest.property_list()
+        if self.arg not in dest.property_list():
+            raise qubes.exc.QubesNoSuchPropertyError(dest, self.arg)
 
         self.fire_event_for_permission()
 

+ 11 - 0
qubes/exc.py

@@ -122,6 +122,17 @@ class QubesPropertyValueError(QubesValueError):
         self.value = value
 
 
+class QubesNoSuchPropertyError(QubesException, AttributeError):
+    '''Requested property does not exist
+    '''
+    def __init__(self, holder, prop_name, msg=None):
+        super(QubesNoSuchPropertyError, self).__init__(
+            msg or 'Invalid property {!r} of {!s}'.format(
+                prop_name, holder))
+        self.holder = holder
+        self.prop = prop_name
+
+
 class QubesNotImplementedError(QubesException, NotImplementedError):
     '''Thrown at user when some feature is not implemented'''
     def __init__(self, msg=None):

+ 2 - 2
qubes/tests/api_admin.py

@@ -266,7 +266,7 @@ class TC_00_VMs(AdminAPITestCase):
         self.assertFalse(self.app.save.called)
 
     def test_052_vm_property_help_invalid_property(self):
-        with self.assertRaises(AssertionError):
+        with self.assertRaises(qubes.exc.QubesNoSuchPropertyError):
             self.call_mgmt_func(b'admin.vm.property.Help', b'test-vm1',
                 b'no-such-property')
 
@@ -282,7 +282,7 @@ class TC_00_VMs(AdminAPITestCase):
 
     def test_062_vm_property_reset_invalid_property(self):
         with unittest.mock.patch('qubes.property.__delete__') as mock:
-            with self.assertRaises(AssertionError):
+            with self.assertRaises(qubes.exc.QubesNoSuchPropertyError):
                 self.call_mgmt_func(b'admin.vm.property.Help', b'test-vm1',
                     b'no-such-property')
             self.assertFalse(mock.called)