Redo netvm setup after unpause

Applying deferred netvm for preloaded disposables is handled separately
to ensure that before a disposable is returned to the user, the
networking is already set up. If the domain-unpaused event of the
NetVMMixin kicks in before the preload is used, it is ignored by the
"is_preload" attribute, if it kicks after, it is ignored by the absent
"deferred-netvm-original" feature.

Fixes: QubesOS/qubes-issues#10173
For: QubesOS/qubes-issues#1512

Author: ben-grande

qubes/tests/integ/network.py

@@ -191,6 +191,37 @@ def test_000_simple_networking(self):
         self.assertEqual(self.run_cmd(self.testvm1, self.ping_ip), 0)
         self.assertEqual(self.run_cmd(self.testvm1, self.ping_name), 0)
 
+    def test_001_simple_networking_paused(self):
+        """
+        :type self: qubes.tests.SystemTestCase | VMNetworkingMixin
+        """
+        self.testvm1.netvm = None
+        self.loop.run_until_complete(self.start_vm(self.testvm1))
+        self.loop.run_until_complete(self.testvm1.pause())
+        self.testvm1.netvm = self.testnetvm
+        self.assertEqual(
+            self.testvm1.features.get("deferred-netvm-original", None), ""
+        )
+        self.loop.run_until_complete(self.testvm1.unpause())
+        self.assertEqual(self.run_cmd(self.testvm1, self.ping_ip), 0)
+        self.assertEqual(self.run_cmd(self.testvm1, self.ping_name), 0)
+        self.assertEqual(
+            self.testvm1.features.get("deferred-netvm-original", None), None
+        )
+
+        self.loop.run_until_complete(self.testvm1.pause())
+        self.testvm1.netvm = None
+        self.assertEqual(
+            self.testvm1.features.get("deferred-netvm-original", None),
+            self.testnetvm.name,
+        )
+        self.loop.run_until_complete(self.testvm1.unpause())
+        self.assertNotEqual(self.run_cmd(self.testvm1, self.ping_ip), 0)
+        self.assertNotEqual(self.run_cmd(self.testvm1, self.ping_name), 0)
+        self.assertEqual(
+            self.testvm1.features.get("deferred-netvm-original", None), None
+        )
+
     def test_010_simple_proxyvm(self):
         """
         :type self: qubes.tests.SystemTestCase | VMNetworkingMixin
@@ -389,7 +420,7 @@ def test_030_firewallvm_firewall(self):
             # block all except ICMP
 
             self.testvm1.firewall.rules = [
-                (qubes.firewall.Rule(None, action="accept", proto="icmp"))
+                qubes.firewall.Rule(None, action="accept", proto="icmp")
             ]
             self.testvm1.firewall.save()
             # Ugly hack b/c there is no feedback when the rules are actually

qubes/tests/vm/mix/net.py

@@ -159,6 +159,78 @@ def test_145_netvm_change(self):
                 mock_detach.reset_mock()
                 mock_attach.reset_mock()
 
+    def test_146_netvm_defer(self):
+        vm = self.get_vm()
+        self.setup_netvms(vm)
+        with (
+            patch("qubes.vm.qubesvm.QubesVM.is_running", lambda x: True),
+            patch("qubes.vm.qubesvm.QubesVM.is_paused", lambda x: True),
+            patch("qubes.vm.mix.net.NetVMMixin.attach_network") as mock_attach,
+            patch("qubes.vm.mix.net.NetVMMixin.detach_network") as mock_detach,
+            patch("qubes.vm.qubesvm.QubesVM.create_qdb_entries"),
+        ):
+
+            with self.subTest("changing netvm"):
+                original_netvm = vm.netvm.name
+                vm.netvm = self.netvm2
+                mock_detach.assert_not_called()
+                mock_attach.assert_not_called()
+                self.assertEqual(
+                    vm.features.get("deferred-netvm-original", None),
+                    original_netvm,
+                )
+                with patch(
+                    "qubes.vm.qubesvm.QubesVM.is_paused", lambda x: False
+                ):
+                    vm.apply_deferred_netvm()
+                    self.assertEqual(
+                        vm.features.get("deferred-netvm-original", None), None
+                    )
+                    mock_detach.assert_called()
+                    mock_attach.assert_called()
+                    mock_detach.reset_mock()
+                    mock_attach.reset_mock()
+
+            with self.subTest("setting netvm to none"):
+                original_netvm = vm.netvm.name
+                vm.netvm = None
+                mock_detach.assert_not_called()
+                mock_attach.assert_not_called()
+                self.assertEqual(
+                    vm.features.get("deferred-netvm-original", None),
+                    original_netvm,
+                )
+                with patch(
+                    "qubes.vm.qubesvm.QubesVM.is_paused", lambda x: False
+                ):
+                    vm.apply_deferred_netvm()
+                    self.assertEqual(
+                        vm.features.get("deferred-netvm-original", None), None
+                    )
+                    mock_detach.assert_called()
+                    mock_attach.assert_not_called()
+                    mock_detach.reset_mock()
+
+            with self.subTest("resetting netvm to default"):
+                original_netvm = vm.netvm.name if vm.netvm else ""
+                del vm.netvm
+                mock_detach.assert_not_called()
+                mock_attach.assert_not_called()
+                self.assertEqual(
+                    vm.features.get("deferred-netvm-original", None),
+                    original_netvm,
+                )
+                with patch(
+                    "qubes.vm.qubesvm.QubesVM.is_paused", lambda x: False
+                ):
+                    vm.apply_deferred_netvm()
+                    self.assertEqual(
+                        vm.features.get("deferred-netvm-original", None), None
+                    )
+                    mock_detach.assert_not_called()
+                    mock_attach.assert_called_once()
+                    mock_attach.reset_mock()
+
     def test_150_ip(self):
         vm = self.get_vm()
         self.setup_netvms(vm)

qubes/tests/vm/qubesvm.py

@@ -1691,7 +1691,12 @@ def test_600_libvirt_xml_hvm_pcidev(self):
         )
         dom0 = self.get_vm(name="dom0", qid=0)
         vm = self.get_vm(uuid=my_uuid)
-        vm.netvm = None
+        vm.paused = lambda x: False
+        with unittest.mock.patch(
+            "qubes.vm.qubesvm.QubesVM.is_paused"
+        ) as mock_is_paused:
+            mock_is_paused.return_value = False
+            vm.netvm = None
         vm.virt_mode = "hvm"
         vm.kernel = None
         # even with meminfo-writer enabled, should have memory==maxmem
@@ -1803,7 +1808,11 @@ def test_600_libvirt_xml_hvm_pcidev_s0ix(self):
         dom0 = self.get_vm(name="dom0", qid=0)
         dom0.features["suspend-s0ix"] = True
         vm = self.get_vm(uuid=my_uuid)
-        vm.netvm = None
+        with unittest.mock.patch(
+            "qubes.vm.qubesvm.QubesVM.is_paused"
+        ) as mock_is_paused:
+            mock_is_paused.return_value = False
+            vm.netvm = None
         vm.virt_mode = "hvm"
         vm.kernel = None
         # even with meminfo-writer enabled, should have memory==maxmem

qubes/vm/dispvm.py

@@ -577,13 +577,15 @@ def use_preload(self):
             self.log.info("Using preloaded qube")
             if not appvm.features.get("internal", None):
                 del self.features["internal"]
+            self.apply_deferred_netvm()
             self.preload_requested = None
             del self.features["preload-dispvm-in-progress"]
         else:
             # Happens when unpause/resume occurs without qube being requested.
             self.log.warning("Using a preloaded qube before requesting it")
             if not appvm.features.get("internal", None):
                 del self.features["internal"]
+            self.apply_deferred_netvm()
             appvm.remove_preload_from_list([self.name])
             self.features["preload-dispvm-in-progress"] = False
         self.app.save()

qubes/vm/mix/net.py

@@ -20,12 +20,13 @@
 # License along with this library; if not, see <https://www.gnu.org/licenses/>.
 #
 
-""" This module contains the NetVMMixin """
+"""This module contains the NetVMMixin"""
 import ipaddress
 import os
 import re
 
 import libvirt  # pylint: disable=import-error
+import lxml.etree
 import qubes
 import qubes.config
 import qubes.events
@@ -341,6 +342,45 @@ def on_domain_started_net(self, event, **kwargs):
             except (qubes.exc.QubesException, libvirt.libvirtError):
                 vm.log.warning("Cannot attach network", exc_info=1)
 
+    def apply_deferred_netvm(self):
+        """Apply deferred netvm changes in case qube could not apply it at the
+        time it was requested."""
+        deferred_from = self.features.get("deferred-netvm-original", None)
+        if deferred_from is None:
+            return
+        oldvalue = None
+        if deferred_from in self.app.domains:
+            oldvalue = self.app.domains[deferred_from]
+        if oldvalue:
+            self.fire_event(
+                "property-pre-set:netvm",
+                pre_event=True,
+                name="netvm",
+                newvalue=self.netvm,
+                oldvalue=oldvalue,
+            )
+        if self.netvm:
+            self.fire_event(
+                "property-set:netvm",
+                name="netvm",
+                newvalue=self.netvm,
+                oldvalue=oldvalue,
+            )
+        del self.features["deferred-netvm-original"]
+
+    @qubes.events.handler("domain-unpaused")
+    def on_domain_unpaused_net(
+        self, event, **kwargs
+    ):  # pylint: disable=unused-argument
+        """Check for deferred netvm changes in case qube was paused while
+        changes happened."""
+        if getattr(self, "is_preload", False):
+            # Networking of preloaded disposable must be done when it is being
+            # marked as used, so we guarantee that it finishes before the user
+            # can use the disposable.
+            return
+        self.apply_deferred_netvm()
+
     @qubes.events.handler("domain-pre-shutdown")
     def on_domain_pre_shutdown(self, event, force=False):
         """Checks before NetVM shutdown if any connected domains are running.
@@ -373,6 +413,12 @@ def attach_network(self):
             self.netvm.start()
 
         self.netvm.set_mapped_ip_info_for_vm(self)
+
+        if self.is_paused():
+            self.log.warning(
+                "Deferred attaching libvirt net device because qube is paused"
+            )
+            return
         self.libvirt_domain.attachDevice(
             self.app.env.get_template("libvirt/devices/net.xml").render(vm=self)
         )
@@ -383,13 +429,24 @@ def detach_network(self):
         if not self.is_running():
             raise qubes.exc.QubesVMNotRunningError(self)
         if self.netvm is None:
-            raise qubes.exc.QubesVMError(
-                self, "netvm should not be {}".format(self.netvm)
+            deferred_from = self.features.get("deferred-netvm", None)
+            if deferred_from is not None:
+                raise qubes.exc.QubesVMError(
+                    self, "netvm should not be {}".format(self.netvm)
+                )
+
+        if self.is_paused():
+            self.log.warning(
+                "Deferred detaching libvirt net device because qube is paused"
             )
+            return
 
-        self.libvirt_domain.detachDevice(
-            self.app.env.get_template("libvirt/devices/net.xml").render(vm=self)
-        )
+        # Properties extracted from libvirt_domain to support deferred netvm.
+        root = lxml.etree.fromstring(self.libvirt_domain.XMLDesc())
+        eth = root.find(".//interface[@type='ethernet']")
+        if eth is None:
+            return
+        self.libvirt_domain.detachDevice(lxml.etree.tostring(eth).decode())
 
     def is_networked(self):
         """Check whether this VM can reach network (firewall notwithstanding).
@@ -515,18 +572,27 @@ def on_property_pre_set_netvm(self, event, name, newvalue, oldvalue=None):
                     ),
                 )
 
-        # don't check oldvalue, because it's missing if it was default
-        if self.netvm is not None:
-            if self.is_running() and self.netvm.is_running():
-                self.detach_network()
+        if self.is_paused():
+            if "deferred-netvm-original" not in self.features:
+                self.features["deferred-netvm-original"] = (
+                    oldvalue.name if oldvalue else None
+                )
+            return
+        deferred_from = self.features.get("deferred-netvm-original", None)
+        if deferred_from is None:
+            # don't check oldvalue, because it's missing if it was default
+            if self.netvm is None:
+                return
+            if not (self.is_running() and self.netvm.is_running()):
+                return
+        self.detach_network()
 
     @qubes.events.handler("property-set:netvm")
     def on_property_set_netvm(self, event, name, newvalue, oldvalue=None):
         """Replaces the current NetVM with a new one and fires
         net-domain-connect event
         """
         # pylint: disable=unused-argument
-
         if oldvalue is not None and oldvalue.is_running():
             oldvalue.reload_connected_ips()
 
@@ -539,7 +605,8 @@ def on_property_set_netvm(self, event, name, newvalue, oldvalue=None):
         if self.is_running():
             # refresh IP, DNS etc
             self.create_qdb_entries()
-            self.attach_network()
+            if not self.is_paused():
+                self.attach_network()
 
             newvalue.fire_event("net-domain-connect", vm=self)
 

templates/libvirt/devices/net.xml

@@ -8,4 +8,4 @@
     <script path="vif-route-qubes" />
 </interface>
 
-{# vim : set ft=jinja ts=4 sts=4 sw=4 et : #}
+{# vim: set ft=jinja ts=4 sts=4 sw=4 et : #}