Skip to content

Commit

Permalink
admin: raise QubesNoSuchPropertyError for non-existing properties
Browse files Browse the repository at this point in the history
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
  • Loading branch information
marmarek committed May 23, 2017
1 parent f42cd28 commit 64b83fa
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 6 deletions.
12 changes: 8 additions & 4 deletions qubes/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ def property_get(self):
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()

Expand Down Expand Up @@ -180,7 +181,8 @@ def property_set(self, untrusted_payload):
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)
Expand All @@ -204,7 +206,8 @@ def property_help(self):
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()

Expand All @@ -229,7 +232,8 @@ def property_reset(self):
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()

Expand Down
11 changes: 11 additions & 0 deletions qubes/exc.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,17 @@ def __init__(self, holder, prop, value, msg=None):
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):
Expand Down
4 changes: 2 additions & 2 deletions qubes/tests/api_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def test_050_vm_property_help(self):
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')

Expand All @@ -282,7 +282,7 @@ def test_060_vm_property_reset(self):

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)
Expand Down

0 comments on commit 64b83fa

Please sign in to comment.