From 9fcc46c6f3bf19c23852b1967ddf9a648623acc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Pierret=20=28fepitre=29?= Date: Mon, 4 Mar 2024 11:33:47 +0100 Subject: [PATCH 1/8] ext/audio: reformat --- qubes/ext/audio.py | 78 ++++++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/qubes/ext/audio.py b/qubes/ext/audio.py index c1ac6a005..f7b0470cc 100644 --- a/qubes/ext/audio.py +++ b/qubes/ext/audio.py @@ -20,69 +20,81 @@ import qubes.config import qubes.ext + class AUDIO(qubes.ext.Extension): # pylint: disable=unused-argument @staticmethod def attached_vms(vm): for domain in vm.app.domains: - if getattr(domain, 'audiovm', None) and domain.audiovm == vm: + if getattr(domain, "audiovm", None) and domain.audiovm == vm: yield domain - @qubes.ext.handler('domain-pre-shutdown') + @qubes.ext.handler("domain-pre-shutdown") def on_domain_pre_shutdown(self, vm, event, **kwargs): - attached_vms = [domain for domain in self.attached_vms(vm) if - domain.is_running()] - if attached_vms and not kwargs.get('force', False): + attached_vms = [ + domain for domain in self.attached_vms(vm) if domain.is_running() + ] + if attached_vms and not kwargs.get("force", False): raise qubes.exc.QubesVMError( - self, 'There are running VMs using this VM as AudioVM: ' - '{}'.format(', '.join(vm.name for vm in attached_vms))) + self, + "There are running VMs using this VM as AudioVM: " + "{}".format(", ".join(vm.name for vm in attached_vms)), + ) - @qubes.ext.handler('domain-init', 'domain-load') + @qubes.ext.handler("domain-init", "domain-load") def on_domain_init_load(self, vm, event): - if getattr(vm, 'audiovm', None): - if 'audiovm-' + vm.audiovm.name not in vm.tags: - self.on_property_set(vm, event, name='audiovm', - newvalue=vm.audiovm) + if getattr(vm, "audiovm", None): + if "audiovm-" + vm.audiovm.name not in vm.tags: + self.on_property_set( + vm, event, name="audiovm", newvalue=vm.audiovm + ) - @qubes.ext.handler('property-reset:audiovm') + @qubes.ext.handler("property-reset:audiovm") def on_property_reset(self, subject, event, name, oldvalue=None): - newvalue = getattr(subject, 'audiovm', None) + newvalue = getattr(subject, "audiovm", None) self.on_property_set(subject, event, name, newvalue, oldvalue) - @qubes.ext.handler('property-set:audiovm') + @qubes.ext.handler("property-set:audiovm") def on_property_set(self, subject, event, name, newvalue, oldvalue=None): # Clean other 'audiovm-XXX' tags. # pulseaudio agent (module-vchan-sink) can connect to only one domain tags_list = list(subject.tags) for tag in tags_list: - if tag.startswith('audiovm-'): + if tag.startswith("audiovm-"): subject.tags.remove(tag) if newvalue: - audiovm = 'audiovm-' + newvalue.name + audiovm = "audiovm-" + newvalue.name subject.tags.add(audiovm) - @qubes.ext.handler('domain-qdb-create') + @qubes.ext.handler("domain-qdb-create") def on_domain_qdb_create(self, vm, event): # Add AudioVM Xen ID for gui-agent - if getattr(vm, 'audiovm', None): + if getattr(vm, "audiovm", None): if vm != vm.audiovm and vm.audiovm.is_running(): - vm.untrusted_qdb.write('/qubes-audio-domain-xid', - str(vm.audiovm.xid)) + vm.untrusted_qdb.write( + "/qubes-audio-domain-xid", str(vm.audiovm.xid) + ) - @qubes.ext.handler('property-set:default_audiovm', system=True) - def on_property_set_default_audiovm(self, app, event, name, newvalue, - oldvalue=None): + @qubes.ext.handler("property-set:default_audiovm", system=True) + def on_property_set_default_audiovm( + self, app, event, name, newvalue, oldvalue=None + ): for vm in app.domains: - if hasattr(vm, 'audiovm') and vm.property_is_default('audiovm'): - vm.fire_event('property-set:audiovm', - name='audiovm', newvalue=newvalue, - oldvalue=oldvalue) + if hasattr(vm, "audiovm") and vm.property_is_default("audiovm"): + vm.fire_event( + "property-set:audiovm", + name="audiovm", + newvalue=newvalue, + oldvalue=oldvalue, + ) - @qubes.ext.handler('domain-start') + @qubes.ext.handler("domain-start") def on_domain_start(self, vm, event, **kwargs): - attached_vms = [domain for domain in self.attached_vms(vm) if - domain.is_running()] + attached_vms = [ + domain for domain in self.attached_vms(vm) if domain.is_running() + ] for attached_vm in attached_vms: - attached_vm.untrusted_qdb.write('/qubes-audio-domain-xid', - str(vm.xid)) + attached_vm.untrusted_qdb.write( + "/qubes-audio-domain-xid", str(vm.xid) + ) From 24d9371fc48ce815e5b3b157c17b847005ddd679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Pierret=20=28fepitre=29?= Date: Mon, 4 Mar 2024 11:38:45 +0100 Subject: [PATCH 2/8] ext/audio: update audiovm qubesdb entry Fixes https://github.com/QubesOS/qubes-issues/issues/7650 --- qubes/ext/audio.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/qubes/ext/audio.py b/qubes/ext/audio.py index f7b0470cc..ed1755a30 100644 --- a/qubes/ext/audio.py +++ b/qubes/ext/audio.py @@ -29,6 +29,15 @@ def attached_vms(vm): if getattr(domain, "audiovm", None) and domain.audiovm == vm: yield domain + @staticmethod + def set_qubesdb_audiovm(vm): + # Add AudioVM Xen ID for gui-agent + if getattr(vm, "audiovm", None): + if vm != vm.audiovm and vm.audiovm.is_running(): + vm.untrusted_qdb.write( + "/qubes-audio-domain-xid", str(vm.audiovm.xid) + ) + @qubes.ext.handler("domain-pre-shutdown") def on_domain_pre_shutdown(self, vm, event, **kwargs): attached_vms = [ @@ -66,15 +75,11 @@ def on_property_set(self, subject, event, name, newvalue, oldvalue=None): if newvalue: audiovm = "audiovm-" + newvalue.name subject.tags.add(audiovm) + self.set_qubesdb_audiovm(subject) @qubes.ext.handler("domain-qdb-create") def on_domain_qdb_create(self, vm, event): - # Add AudioVM Xen ID for gui-agent - if getattr(vm, "audiovm", None): - if vm != vm.audiovm and vm.audiovm.is_running(): - vm.untrusted_qdb.write( - "/qubes-audio-domain-xid", str(vm.audiovm.xid) - ) + self.set_qubesdb_audiovm(vm) @qubes.ext.handler("property-set:default_audiovm", system=True) def on_property_set_default_audiovm( From b9333d787b8becbe07dbf5672c803d1061f8c6a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Pierret=20=28fepitre=29?= Date: Mon, 4 Mar 2024 14:49:54 +0100 Subject: [PATCH 3/8] ext/audio: ensure vm.is_running() is available --- qubes/ext/audio.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qubes/ext/audio.py b/qubes/ext/audio.py index ed1755a30..3d38c8cf8 100644 --- a/qubes/ext/audio.py +++ b/qubes/ext/audio.py @@ -75,6 +75,10 @@ def on_property_set(self, subject, event, name, newvalue, oldvalue=None): if newvalue: audiovm = "audiovm-" + newvalue.name subject.tags.add(audiovm) + + # It is needed to filter these events because + # vm.is_running() is not yet available. + if event not in ("domain-init", "domain-load"): self.set_qubesdb_audiovm(subject) @qubes.ext.handler("domain-qdb-create") From 6e039595139780d1ac13d53b40c9d41c86afc1d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Pierret=20=28fepitre=29?= Date: Mon, 4 Mar 2024 15:10:29 +0100 Subject: [PATCH 4/8] ext/audio: ensure qubes's qdb is ready --- qubes/ext/audio.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qubes/ext/audio.py b/qubes/ext/audio.py index 3d38c8cf8..cee04ecd6 100644 --- a/qubes/ext/audio.py +++ b/qubes/ext/audio.py @@ -31,6 +31,10 @@ def attached_vms(vm): @staticmethod def set_qubesdb_audiovm(vm): + # Ensure that qube is ready + if not vm.untrusted_qdb: + return + # Add AudioVM Xen ID for gui-agent if getattr(vm, "audiovm", None): if vm != vm.audiovm and vm.audiovm.is_running(): From b789cbd48c312ecb003400ce0449ea5d91395a24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Pierret=20=28fepitre=29?= Date: Mon, 4 Mar 2024 15:52:07 +0100 Subject: [PATCH 5/8] ext/audio: handle del of audiovm --- qubes/ext/audio.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qubes/ext/audio.py b/qubes/ext/audio.py index cee04ecd6..c3ae646ba 100644 --- a/qubes/ext/audio.py +++ b/qubes/ext/audio.py @@ -41,6 +41,8 @@ def set_qubesdb_audiovm(vm): vm.untrusted_qdb.write( "/qubes-audio-domain-xid", str(vm.audiovm.xid) ) + else: + vm.untrusted_qdb.rm("/qubes-audio-domain-xid") @qubes.ext.handler("domain-pre-shutdown") def on_domain_pre_shutdown(self, vm, event, **kwargs): @@ -67,7 +69,7 @@ def on_property_reset(self, subject, event, name, oldvalue=None): newvalue = getattr(subject, "audiovm", None) self.on_property_set(subject, event, name, newvalue, oldvalue) - @qubes.ext.handler("property-set:audiovm") + @qubes.ext.handler("property-set:audiovm", "property-del:audiovm") def on_property_set(self, subject, event, name, newvalue, oldvalue=None): # Clean other 'audiovm-XXX' tags. # pulseaudio agent (module-vchan-sink) can connect to only one domain From c8571a4db71ef82caf67360aaef4fb4547a94c77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Pierret=20=28fepitre=29?= Date: Mon, 4 Mar 2024 16:31:25 +0100 Subject: [PATCH 6/8] ext/audio: add test for change of audiovm --- qubes/tests/vm/qubesvm.py | 70 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/qubes/tests/vm/qubesvm.py b/qubes/tests/vm/qubesvm.py index 8bd2fb338..497b5cacf 100644 --- a/qubes/tests/vm/qubesvm.py +++ b/qubes/tests/vm/qubesvm.py @@ -1961,7 +1961,73 @@ def test_623_qdb_audiovm(self, mock_qubesdb, mock_urandom, @unittest.mock.patch('qubes.utils.get_timezone') @unittest.mock.patch('qubes.utils.urandom') @unittest.mock.patch('qubes.vm.qubesvm.QubesVM.untrusted_qdb') - def test_624_qdb_guivm_invalid_keyboard_layout(self, mock_qubesdb, + def test_624_qdb_audiovm_change_to_new_and_none(self, mock_qubesdb, mock_urandom, + mock_timezone): + mock_urandom.return_value = b'A' * 64 + mock_timezone.return_value = 'UTC' + template = self.get_vm( + cls=qubes.vm.templatevm.TemplateVM, name='template') + template.netvm = None + audiovm = self.get_vm(cls=qubes.vm.appvm.AppVM, template=template, + name='sys-audio', qid=2, provides_network=False) + audiovm_new = self.get_vm(cls=qubes.vm.appvm.AppVM, template=template, + name='sys-audio-new', qid=3, provides_network=False) + vm = self.get_vm(cls=qubes.vm.appvm.AppVM, template=template, + name='appvm', qid=3) + vm.netvm = None + vm.audiovm = audiovm + vm.is_running = lambda: True + vm._qubesprop_xid = 2 + audiovm.is_running = lambda: True + audiovm._libvirt_domain = unittest.mock.Mock(**{'ID.return_value': 2}) + audiovm_new.is_running = lambda: True + audiovm_new._libvirt_domain = unittest.mock.Mock(**{'ID.return_value': 3}) + vm.events_enabled = True + test_qubesdb = TestQubesDB() + mock_qubesdb.write.side_effect = test_qubesdb.write + mock_qubesdb.rm.side_effect = test_qubesdb.rm + vm.create_qdb_entries() + self.maxDiff = None + expected = { + '/name': 'test-inst-appvm', + '/type': 'AppVM', + '/default-user': 'user', + '/qubes-vm-type': 'AppVM', + '/qubes-audio-domain-xid': '{}'.format(audiovm.xid), + '/qubes-debug-mode': '0', + '/qubes-base-template': 'test-inst-template', + '/qubes-timezone': 'UTC', + '/qubes-random-seed': base64.b64encode(b'A' * 64), + '/qubes-vm-persistence': 'rw-only', + '/qubes-vm-updateable': 'False', + '/qubes-block-devices': '', + '/qubes-usb-devices': '', + '/qubes-iptables': 'reload', + '/qubes-iptables-error': '', + '/qubes-iptables-header': unittest.mock.ANY, + '/qubes-service/qubes-update-check': '0', + '/qubes-service/meminfo-writer': '1', + '/connected-ips': '', + '/connected-ips6': '', + } + + with self.subTest('default'): + self.assertEqual(test_qubesdb.data, expected) + + with self.subTest('value_change'): + vm.audiovm = None + del expected['/qubes-audio-domain-xid'] + self.assertEqual(test_qubesdb.data, expected) + + with self.subTest('value_change'): + vm.audiovm = audiovm_new + expected['/qubes-audio-domain-xid'] = "3" + self.assertEqual(test_qubesdb.data, expected) + + @unittest.mock.patch('qubes.utils.get_timezone') + @unittest.mock.patch('qubes.utils.urandom') + @unittest.mock.patch('qubes.vm.qubesvm.QubesVM.untrusted_qdb') + def test_625_qdb_guivm_invalid_keyboard_layout(self, mock_qubesdb, mock_urandom, mock_timezone): mock_urandom.return_value = b'A' * 64 mock_timezone.return_value = 'UTC' @@ -1986,7 +2052,7 @@ def test_624_qdb_guivm_invalid_keyboard_layout(self, mock_qubesdb, @unittest.mock.patch('qubes.utils.get_timezone') @unittest.mock.patch('qubes.utils.urandom') @unittest.mock.patch('qubes.vm.qubesvm.QubesVM.untrusted_qdb') - def test_625_qdb_keyboard_layout_change(self, mock_qubesdb, mock_urandom, + def test_626_qdb_keyboard_layout_change(self, mock_qubesdb, mock_urandom, mock_timezone): mock_urandom.return_value = b'A' * 64 mock_timezone.return_value = 'UTC' From 28e71f35d9c77846175658429ce54edb3fbf840b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Pierret=20=28fepitre=29?= Date: Sun, 10 Mar 2024 10:32:01 +0100 Subject: [PATCH 7/8] ext/audio: few improvements from @DemiMarie's suggestions --- qubes/ext/audio.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/qubes/ext/audio.py b/qubes/ext/audio.py index c3ae646ba..0948a2d46 100644 --- a/qubes/ext/audio.py +++ b/qubes/ext/audio.py @@ -26,7 +26,7 @@ class AUDIO(qubes.ext.Extension): @staticmethod def attached_vms(vm): for domain in vm.app.domains: - if getattr(domain, "audiovm", None) and domain.audiovm == vm: + if getattr(domain, "audiovm", None) == vm: yield domain @staticmethod @@ -36,10 +36,11 @@ def set_qubesdb_audiovm(vm): return # Add AudioVM Xen ID for gui-agent - if getattr(vm, "audiovm", None): - if vm != vm.audiovm and vm.audiovm.is_running(): + audiovm = getattr(vm, "audiovm", None) + if audiovm is not None: + if audiovm != vm and audiovm.is_running(): vm.untrusted_qdb.write( - "/qubes-audio-domain-xid", str(vm.audiovm.xid) + "/qubes-audio-domain-xid", str(audiovm.xid) ) else: vm.untrusted_qdb.rm("/qubes-audio-domain-xid") From 3caf3931c7ad932146791bebd17d7777068c38b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Pierret=20=28fepitre=29?= Date: Sun, 10 Mar 2024 11:11:14 +0100 Subject: [PATCH 8/8] ext/audio: split set/unset properties --- qubes/ext/audio.py | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/qubes/ext/audio.py b/qubes/ext/audio.py index 0948a2d46..c882b6c43 100644 --- a/qubes/ext/audio.py +++ b/qubes/ext/audio.py @@ -45,6 +45,23 @@ def set_qubesdb_audiovm(vm): else: vm.untrusted_qdb.rm("/qubes-audio-domain-xid") + def set_tag_and_qubesdb_entry(self, subject, event, newvalue=None): + # Clean other 'audiovm-XXX' tags. + # pulseaudio agent (module-vchan-sink) can connect to only one domain + tags_list = list(subject.tags) + for tag in tags_list: + if tag.startswith("audiovm-"): + subject.tags.remove(tag) + + if newvalue: + audiovm = "audiovm-" + newvalue.name + subject.tags.add(audiovm) + + # It is needed to filter these events because + # vm.is_running() is not yet available. + if event not in ("domain-init", "domain-load"): + self.set_qubesdb_audiovm(subject) + @qubes.ext.handler("domain-pre-shutdown") def on_domain_pre_shutdown(self, vm, event, **kwargs): attached_vms = [ @@ -70,23 +87,15 @@ def on_property_reset(self, subject, event, name, oldvalue=None): newvalue = getattr(subject, "audiovm", None) self.on_property_set(subject, event, name, newvalue, oldvalue) - @qubes.ext.handler("property-set:audiovm", "property-del:audiovm") + @qubes.ext.handler("property-set:audiovm") def on_property_set(self, subject, event, name, newvalue, oldvalue=None): - # Clean other 'audiovm-XXX' tags. - # pulseaudio agent (module-vchan-sink) can connect to only one domain - tags_list = list(subject.tags) - for tag in tags_list: - if tag.startswith("audiovm-"): - subject.tags.remove(tag) + self.set_tag_and_qubesdb_entry( + subject=subject, event=event, newvalue=newvalue + ) - if newvalue: - audiovm = "audiovm-" + newvalue.name - subject.tags.add(audiovm) - - # It is needed to filter these events because - # vm.is_running() is not yet available. - if event not in ("domain-init", "domain-load"): - self.set_qubesdb_audiovm(subject) + @qubes.ext.handler("property-del:audiovm") + def on_property_del(self, subject, event, name, oldvalue=None): + self.set_tag_and_qubesdb_entry(subject=subject, event=event) @qubes.ext.handler("domain-qdb-create") def on_domain_qdb_create(self, vm, event):