From 1759bca00f62a6325c3a68e3c700c287012e5df5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Fri, 21 Jul 2017 23:11:24 +0200 Subject: [PATCH] Rename vm.qdb to vm.untrusted_qdb QubesDB can be freely modified by a VM, so one should take care when reading any data retrieved from it. Fixes QubesOS/qubes-issues#2934 --- qubes/api/misc.py | 7 +++--- qubes/ext/block.py | 10 ++++----- qubes/ext/r3compatibility.py | 18 ++++++++------- qubes/storage/__init__.py | 3 ++- qubes/tests/api_misc.py | 43 ++++++++++++++++++------------------ qubes/tests/devices_block.py | 2 +- qubes/vm/adminvm.py | 2 +- qubes/vm/mix/net.py | 15 +++++++------ qubes/vm/qubesvm.py | 42 +++++++++++++++++------------------ 9 files changed, 74 insertions(+), 68 deletions(-) diff --git a/qubes/api/misc.py b/qubes/api/misc.py index 12438f2a..64dbc114 100644 --- a/qubes/api/misc.py +++ b/qubes/api/misc.py @@ -52,9 +52,9 @@ class QubesMiscAPI(qubes.api.AbstractQubesAPI): prefix = '/features-request/' keys = [key.decode('ascii', errors='strict') - for key in self.src.qdb.list(prefix)] + for key in self.src.untrusted_qdb.list(prefix)] untrusted_features = {key[len(prefix):]: - self.src.qdb.read(key).decode('ascii', errors='strict') + self.src.untrusted_qdb.read(key).decode('ascii', errors='strict') for key in keys} safe_set = string.ascii_letters + string.digits @@ -79,7 +79,8 @@ class QubesMiscAPI(qubes.api.AbstractQubesAPI): safe_set = string.ascii_letters + string.digits expected_features = ('qrexec', 'gui', 'default-user') for feature in expected_features: - untrusted_value = self.src.qdb.read('/qubes-tools/' + feature) + untrusted_value = self.src.untrusted_qdb.read( + '/qubes-tools/' + feature) if untrusted_value: untrusted_value = untrusted_value.decode('ascii', errors='strict') diff --git a/qubes/ext/block.py b/qubes/ext/block.py index 060e93f2..8b1224f6 100644 --- a/qubes/ext/block.py +++ b/qubes/ext/block.py @@ -56,7 +56,7 @@ class BlockDevice(qubes.devices.DeviceInfo): return self.ident safe_set = {ord(c) for c in string.ascii_letters + string.digits + '()+,-.:=_/ '} - untrusted_desc = self.backend_domain.qdb.read( + untrusted_desc = self.backend_domain.untrusted_qdb.read( '/qubes-block-devices/{}/desc'.format(self.ident)) desc = ''.join((chr(c) if c in safe_set else '_') for c in untrusted_desc) @@ -69,7 +69,7 @@ class BlockDevice(qubes.devices.DeviceInfo): if self._mode is None: if not self.backend_domain.is_running(): return 'w' - untrusted_mode = self.backend_domain.qdb.read( + untrusted_mode = self.backend_domain.untrusted_qdb.read( '/qubes-block-devices/{}/mode'.format(self.ident)) if untrusted_mode is None: self._mode = 'w' @@ -87,7 +87,7 @@ class BlockDevice(qubes.devices.DeviceInfo): if self._size is None: if not self.backend_domain.is_running(): return None - untrusted_size = self.backend_domain.qdb.read( + untrusted_size = self.backend_domain.untrusted_qdb.read( '/qubes-block-devices/{}/size'.format(self.ident)) if untrusted_size is None: self._size = 0 @@ -114,7 +114,7 @@ class BlockDeviceExtension(qubes.ext.Extension): :param ident: device identifier :returns BlockDevice''' - untrusted_qubes_device_attrs = vm.qdb.list( + untrusted_qubes_device_attrs = vm.untrusted_qdb.list( '/qubes-block-devices/{}/'.format(ident)) if not untrusted_qubes_device_attrs: return None @@ -128,7 +128,7 @@ class BlockDeviceExtension(qubes.ext.Extension): string.ascii_letters + string.digits} if not vm.is_running(): return - untrusted_qubes_devices = vm.qdb.list('/qubes-block-devices/') + untrusted_qubes_devices = vm.untrusted_qdb.list('/qubes-block-devices/') untrusted_idents = set(untrusted_path.split(b'/', 3)[2] for untrusted_path in untrusted_qubes_devices) for untrusted_ident in untrusted_idents: diff --git a/qubes/ext/r3compatibility.py b/qubes/ext/r3compatibility.py index 5e627679..4e494e99 100644 --- a/qubes/ext/r3compatibility.py +++ b/qubes/ext/r3compatibility.py @@ -60,9 +60,9 @@ class R3Compatibility(qubes.ext.Extension): vmtype = 'NetVM' else: vmtype = 'AppVM' - vm.qdb.write('/qubes-vm-type', vmtype) + vm.untrusted_qdb.write('/qubes-vm-type', vmtype) - vm.qdb.write("/qubes-iptables-error", '') + vm.untrusted_qdb.write("/qubes-iptables-error", '') self.write_iptables_qubesdb_entry(vm) self.write_services(vm) @@ -81,7 +81,7 @@ class R3Compatibility(qubes.ext.Extension): def write_iptables_qubesdb_entry(self, firewallvm): # pylint: disable=no-self-use - firewallvm.qdb.rm("/qubes-iptables-domainrules/") + firewallvm.untrusted_qdb.rm("/qubes-iptables-domainrules/") iptables = "# Generated by Qubes Core on {0}\n".format( datetime.datetime.now().ctime()) iptables += "*filter\n" @@ -102,7 +102,7 @@ class R3Compatibility(qubes.ext.Extension): # Deny inter-VMs networking iptables += "-A FORWARD -i vif+ -o vif+ -j DROP\n" iptables += "COMMIT\n" - firewallvm.qdb.write("/qubes-iptables-header", iptables) + firewallvm.untrusted_qdb.write("/qubes-iptables-header", iptables) for vm in firewallvm.connected_vms: iptables = "*filter\n" @@ -154,11 +154,12 @@ class R3Compatibility(qubes.ext.Extension): iptables += '-A FORWARD -s {0} -j {1}\n'.format(ip, str(conf.policy).upper()) iptables += 'COMMIT\n' - firewallvm.qdb.write('/qubes-iptables-domainrules/' + str(xid), + firewallvm.untrusted_qdb.write( + '/qubes-iptables-domainrules/' + str(xid), iptables) # no need for ending -A FORWARD -j DROP, cause default action is DROP - firewallvm.qdb.write('/qubes-iptables', 'reload') + firewallvm.untrusted_qdb.write('/qubes-iptables', 'reload') def write_services(self, vm): for feature, value in vm.features.items(): @@ -166,8 +167,9 @@ class R3Compatibility(qubes.ext.Extension): if service is None: continue # forcefully convert to '0' or '1' - vm.qdb.write('/qubes-service/{}'.format(service), + vm.untrusted_qdb.write('/qubes-service/{}'.format(service), str(int(bool(value)))) if 'updates-proxy-setup' in vm.features.keys(): - vm.qdb.write('/qubes-service/{}'.format('yum-proxy-setup'), + vm.untrusted_qdb.write( + '/qubes-service/{}'.format('yum-proxy-setup'), str(int(bool(vm.features['updates-proxy-setup'])))) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index 8bde3b36..a48485e9 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -419,7 +419,8 @@ class Storage(object): # trigger watches to update device status # FIXME: this should be removed once libvirt will report such # events itself - # self.vm.qdb.write('/qubes-block-devices', '') ← do we need this? + # self.vm.untrusted_qdb.write('/qubes-block-devices', '') + # ← do we need this? def _is_already_attached(self, volume): ''' Checks if the given volume is already attached ''' diff --git a/qubes/tests/api_misc.py b/qubes/tests/api_misc.py index d153623b..340c79be 100644 --- a/qubes/tests/api_misc.py +++ b/qubes/tests/api_misc.py @@ -37,9 +37,10 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): def configure_qdb(self, entries): self.src.configure_mock(**{ - 'qdb.read.side_effect': (lambda path: entries.get(path, None)), - 'qdb.list.side_effect': (lambda path: - sorted(map(str.encode, entries.keys()))), + 'untrusted_qdb.read.side_effect': ( + lambda path: entries.get(path, None)), + 'untrusted_qdb.list.side_effect': ( + lambda path: sorted(map(str.encode, entries.keys()))), }) def call_mgmt_func(self, method, arg=b'', payload=b''): @@ -64,10 +65,10 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): mock.call.save() ]) self.assertEqual(self.src.mock_calls, [ - mock.call.qdb.list('/features-request/'), - mock.call.qdb.read('/features-request/feature1'), - mock.call.qdb.read('/features-request/feature2'), - mock.call.qdb.read('/features-request/feature3'), + mock.call.untrusted_qdb.list('/features-request/'), + mock.call.untrusted_qdb.read('/features-request/feature1'), + mock.call.untrusted_qdb.read('/features-request/feature2'), + mock.call.untrusted_qdb.read('/features-request/feature3'), mock.call.fire_event('features-request', untrusted_features={ 'feature1': '1', 'feature2': '', 'feature3': 'other'}) ]) @@ -80,7 +81,7 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): mock.call.save() ]) self.assertEqual(self.src.mock_calls, [ - mock.call.qdb.list('/features-request/'), + mock.call.untrusted_qdb.list('/features-request/'), mock.call.fire_event('features-request', untrusted_features={}) ]) @@ -93,8 +94,8 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): self.call_mgmt_func(b'qubes.FeaturesRequest') self.assertEqual(self.app.mock_calls, []) self.assertEqual(self.src.mock_calls, [ - mock.call.qdb.list('/features-request/'), - mock.call.qdb.read('/features-request/feature1'), + mock.call.untrusted_qdb.list('/features-request/'), + mock.call.untrusted_qdb.read('/features-request/feature1'), ]) def test_003_features_request_invalid2(self): @@ -106,8 +107,8 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): self.call_mgmt_func(b'qubes.FeaturesRequest') self.assertEqual(self.app.mock_calls, []) self.assertEqual(self.src.mock_calls, [ - mock.call.qdb.list('/features-request/'), - mock.call.qdb.read('/features-request/feature1'), + mock.call.untrusted_qdb.list('/features-request/'), + mock.call.untrusted_qdb.read('/features-request/feature1'), ]) def test_010_notify_tools(self): @@ -125,9 +126,9 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): mock.call.save() ]) self.assertEqual(self.src.mock_calls, [ - mock.call.qdb.read('/qubes-tools/qrexec'), - mock.call.qdb.read('/qubes-tools/gui'), - mock.call.qdb.read('/qubes-tools/default-user'), + mock.call.untrusted_qdb.read('/qubes-tools/qrexec'), + mock.call.untrusted_qdb.read('/qubes-tools/gui'), + mock.call.untrusted_qdb.read('/qubes-tools/default-user'), mock.call.fire_event('features-request', untrusted_features={ 'gui': '1', 'default-user': 'user', @@ -146,9 +147,9 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): response = self.call_mgmt_func(b'qubes.NotifyTools') self.assertIsNone(response) self.assertEqual(self.src.mock_calls, [ - mock.call.qdb.read('/qubes-tools/qrexec'), - mock.call.qdb.read('/qubes-tools/gui'), - mock.call.qdb.read('/qubes-tools/default-user'), + mock.call.untrusted_qdb.read('/qubes-tools/qrexec'), + mock.call.untrusted_qdb.read('/qubes-tools/gui'), + mock.call.untrusted_qdb.read('/qubes-tools/default-user'), mock.call.fire_event('features-request', untrusted_features={ 'gui': '1', 'default-user': 'user', @@ -169,7 +170,7 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): self.call_mgmt_func(b'qubes.NotifyTools') self.assertEqual(self.app.mock_calls, []) self.assertEqual(self.src.mock_calls, [ - mock.call.qdb.read('/qubes-tools/qrexec'), + mock.call.untrusted_qdb.read('/qubes-tools/qrexec'), ]) def test_016_notify_tools_invalid_value_gui(self): @@ -185,8 +186,8 @@ class TC_00_API_Misc(qubes.tests.QubesTestCase): self.call_mgmt_func(b'qubes.NotifyTools') self.assertEqual(self.app.mock_calls, []) self.assertEqual(self.src.mock_calls, [ - mock.call.qdb.read('/qubes-tools/qrexec'), - mock.call.qdb.read('/qubes-tools/gui'), + mock.call.untrusted_qdb.read('/qubes-tools/qrexec'), + mock.call.untrusted_qdb.read('/qubes-tools/gui'), ]) def test_020_notify_updates_standalone(self): diff --git a/qubes/tests/devices_block.py b/qubes/tests/devices_block.py index 3ff76c14..bacf2dfc 100644 --- a/qubes/tests/devices_block.py +++ b/qubes/tests/devices_block.py @@ -120,7 +120,7 @@ class TestApp(object): class TestVM(object): def __init__(self, qdb, domain_xml=None, running=True, name='test-vm'): self.name = name - self.qdb = TestQubesDB(qdb) + self.untrusted_qdb = TestQubesDB(qdb) self.libvirt_domain = mock.Mock() self.is_running = lambda: running self.log = mock.Mock() diff --git a/qubes/vm/adminvm.py b/qubes/vm/adminvm.py index fd80cbb2..7a336afe 100644 --- a/qubes/vm/adminvm.py +++ b/qubes/vm/adminvm.py @@ -167,7 +167,7 @@ class AdminVM(qubes.vm.BaseVM): return None @property - def qdb(self): + def untrusted_qdb(self): '''QubesDB handle for this domain.''' if self._qdb_connection is None: import qubesdb # pylint: disable=import-error diff --git a/qubes/vm/mix/net.py b/qubes/vm/mix/net.py index 167238d5..525f36e6 100644 --- a/qubes/vm/mix/net.py +++ b/qubes/vm/mix/net.py @@ -291,12 +291,12 @@ class NetVMMixin(qubes.events.Emitter): base_dir = '/qubes-firewall/' + vm.ip + '/' # remove old entries if any (but don't touch base empty entry - it # would trigger reload right away - self.qdb.rm(base_dir) + self.untrusted_qdb.rm(base_dir) # write new rules for key, value in vm.firewall.qdb_entries(addr_family=4).items(): - self.qdb.write(base_dir + key, value) + self.untrusted_qdb.write(base_dir + key, value) # signal its done - self.qdb.write(base_dir[:-1], '') + self.untrusted_qdb.write(base_dir[:-1], '') def set_mapped_ip_info_for_vm(self, vm): ''' @@ -307,14 +307,15 @@ class NetVMMixin(qubes.events.Emitter): # add info about remapped IPs (VM IP hidden from the VM itself) mapped_ip_base = '/mapped-ip/{}'.format(vm.ip) if vm.visible_ip: - self.qdb.write(mapped_ip_base + '/visible-ip', vm.visible_ip) + self.untrusted_qdb.write(mapped_ip_base + '/visible-ip', + vm.visible_ip) else: - self.qdb.rm(mapped_ip_base + '/visible-ip') + self.untrusted_qdb.rm(mapped_ip_base + '/visible-ip') if vm.visible_gateway: - self.qdb.write(mapped_ip_base + '/visible-gateway', + self.untrusted_qdb.write(mapped_ip_base + '/visible-gateway', vm.visible_gateway) else: - self.qdb.rm(mapped_ip_base + '/visible-gateway') + self.untrusted_qdb.rm(mapped_ip_base + '/visible-gateway') @qubes.events.handler('property-del:netvm') diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index 69febe4a..47df0413 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -590,7 +590,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM): yield block_dev @property - def qdb(self): + def untrusted_qdb(self): '''QubesDB handle for this domain.''' if self._qdb_connection is None: if self.is_running(): @@ -1714,53 +1714,53 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM): ''' # pylint: disable=no-member - self.qdb.write('/name', self.name) - self.qdb.write('/type', self.__class__.__name__) - self.qdb.write('/qubes-vm-updateable', str(self.updateable)) - self.qdb.write('/qubes-vm-persistence', + self.untrusted_qdb.write('/name', self.name) + self.untrusted_qdb.write('/type', self.__class__.__name__) + self.untrusted_qdb.write('/qubes-vm-updateable', str(self.updateable)) + self.untrusted_qdb.write('/qubes-vm-persistence', 'full' if self.updateable else 'rw-only') - self.qdb.write('/qubes-debug-mode', str(int(self.debug))) + self.untrusted_qdb.write('/qubes-debug-mode', str(int(self.debug))) try: - self.qdb.write('/qubes-base-template', self.template.name) + self.untrusted_qdb.write('/qubes-base-template', self.template.name) except AttributeError: - self.qdb.write('/qubes-base-template', '') + self.untrusted_qdb.write('/qubes-base-template', '') - self.qdb.write('/qubes-random-seed', + self.untrusted_qdb.write('/qubes-random-seed', base64.b64encode(qubes.utils.urandom(64))) if self.provides_network: # '/qubes-netvm-network' value is only checked for being non empty - self.qdb.write('/qubes-netvm-network', self.gateway) - self.qdb.write('/qubes-netvm-gateway', self.gateway) - self.qdb.write('/qubes-netvm-netmask', self.netmask) + self.untrusted_qdb.write('/qubes-netvm-network', self.gateway) + self.untrusted_qdb.write('/qubes-netvm-gateway', self.gateway) + self.untrusted_qdb.write('/qubes-netvm-netmask', self.netmask) for i, addr in zip(('primary', 'secondary'), self.dns): - self.qdb.write('/qubes-netvm-{}-dns'.format(i), addr) + self.untrusted_qdb.write('/qubes-netvm-{}-dns'.format(i), addr) if self.netvm is not None: - self.qdb.write('/qubes-ip', self.visible_ip) - self.qdb.write('/qubes-netmask', self.visible_netmask) - self.qdb.write('/qubes-gateway', self.visible_gateway) + self.untrusted_qdb.write('/qubes-ip', self.visible_ip) + self.untrusted_qdb.write('/qubes-netmask', self.visible_netmask) + self.untrusted_qdb.write('/qubes-gateway', self.visible_gateway) for i, addr in zip(('primary', 'secondary'), self.dns): - self.qdb.write('/qubes-{}-dns'.format(i), addr) + self.untrusted_qdb.write('/qubes-{}-dns'.format(i), addr) tzname = qubes.utils.get_timezone() if tzname: - self.qdb.write('/qubes-timezone', tzname) + self.untrusted_qdb.write('/qubes-timezone', tzname) for feature, value in self.features.items(): if not feature.startswith('service.'): continue service = feature[len('service.'):] # forcefully convert to '0' or '1' - self.qdb.write('/qubes-service/{}'.format(service), + self.untrusted_qdb.write('/qubes-service/{}'.format(service), str(int(bool(value)))) - self.qdb.write('/qubes-block-devices', '') + self.untrusted_qdb.write('/qubes-block-devices', '') - self.qdb.write('/qubes-usb-devices', '') + self.untrusted_qdb.write('/qubes-usb-devices', '') # TODO: Currently the whole qmemman is quite Xen-specific, so stay with # xenstore for it until decided otherwise