Fix scriptlet to reenable unit when obsolete wantedbys are removed.

Also improve tests and add Tox for mypy and pytest.

.gitignore

@@ -10,3 +10,4 @@ build
 *.egg-info
 src/*.service
 .mypy_cache
+.tox

Jenkinsfile

@@ -2,10 +2,4 @@
 @Library('shared-jenkins-libraries@master') _
 
 
-def test_step() {
-    return {
-        println "Tests disabled"
-    }
-}
-
-genericFedoraRPMPipeline(null, null, null, null, test_step())
+genericFedoraRPMPipeline(null, null, null, null, TestStrategySkipTests())

Makefile

@@ -11,7 +11,7 @@ src/qubes-routing-manager.service: src/qubes-routing-manager.service.in
 
 ROOT_DIR := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
 
-.PHONY: clean dist rpm srpm install-template install-dom0
+.PHONY: clean dist rpm srpm install-template install-dom0 test
 
 clean:
 	cd $(ROOT_DIR) || exit $$? ; find -name '*.pyc' -o -name '*~' -print0 | xargs -0 rm -f
@@ -42,3 +42,6 @@ install-dom0:
 	PYTHONDONTWRITEBYTECODE=1 python3 networkserversetup.py install $(PYTHON_PREFIX_ARG) -O0 --root $(DESTDIR)
 
 install: install-dom0 install-template
+
+test:
+	tox --current-env

README.md

@@ -8,6 +8,11 @@ of setting up your own Xen server.
 This release is only intended for use with Qubes OS 4.2.  Older Qubes OS releases
 will not support it.  For Qubes OS 4.1, check branch `r4.1`.
 
+**Important note about upgrades**: when you upgrade your system from Qubes OS 4.1 to
+Qubes OS 4.2, if you have this package installed in your template, the template will
+likely **fail to update**.  Please consult [our upgrade instructions](doc/distupgrade.md)
+for information on how to proceed.
+
 ## Why?
 
 Qubes OS is a magnificent operating system.  That said, there are many use cases its networking

build.parameters

@@ -1 +1 @@
-["RELEASE": "q4.2 37 38 39"]
+["QUBES_RELEASES": "4.2"]

doc/distupgrade.md

@@ -0,0 +1,64 @@
+# How to upgrade a Qubes network server from Qubes OS 4.1 to Qubes OS 4.2
+
+The [standard instructions to upgrade Qubes OS systems](https://www.qubes-os.org/doc/upgrade/4.2/)
+will fail to work.  The instructions tell you to run something to the effect of:
+
+```
+qubes-dist-upgrade --all-pre-reboot <other flags>
+```
+
+then reboot, then run:
+
+```
+qubes-dist-upgrade --all-post-reboot <other flags>
+```
+
+The pre-reboot phase will fail if run without the following precautions.
+
+## Step by step instructions
+
+First, build a `qubes-network-server` RPM with the instructions provided
+by this package's [README.md](../README.md) file.  Then, for each template
+where `qubes-network-server` is installed, deposit your build of the
+`qubes-network-server` RPM in a folder `/root/update` of the template,
+and run the command `createrepo_c /root/update` (you may have to install
+package `createrepo_c` via `dnf` to run it).
+
+Now build a `qubes-core-admin-addon-network-server` package for your dom0,
+then copy the file to your profile directory into dom0.  Remember this
+package has to be built *in the same Fedora release (37)* as the Qubes OS
+4.2 dom0 (the `toolbox` command in a disposable qube is handy for this!).
+
+Now open the file `/etc/dnf/dnf.conf` on every template qube where you
+did the above, then add an `exclude=qubes-network-server` setting under
+its `[main]` section.
+
+Remove the currently-installed `qubes-core-admin-addon-network-server`
+package from your dom0 (using `dnf remove`).
+
+Run the pre-reboot phase.
+
+Install the recently-built `qubes-core-admin-addon-network-server` package
+into dom0 (using `dnf install` with the path to the RPM file).
+
+Reboot.
+
+Before running the post-reboot phase, remove the setting you added to the
+`dnf.conf` file of each template you modified.  Finally, add the file
+`/etc/yum.repos.d/local.repo` with the following contents:
+
+```
+[local]
+name=Local packages
+baseurl=file:///root/update
+enabled=1
+gpgcheck=0
+metadata_expire=15
+```
+
+Now run the post-reboot phase.  The template upgrade should succeed now.
+
+To finalize, delete folder `/root/update` and file `/etc/yum.repos.d/local.repo`
+from every template that has it.
+
+You are now updated to Qubes OS 4.2 and `qubes-network-server` is ready.

qubes-network-server.spec

@@ -3,7 +3,7 @@
 %define mybuildnumber %{?build_number}%{?!build_number:1}
 
 Name:           qubes-network-server
-Version:        0.1.1
+Version:        0.1.6
 Release:        %{mybuildnumber}%{?dist}
 Summary:        Turn your Qubes OS into a network server
 BuildArch:      noarch
@@ -19,8 +19,12 @@ BuildRequires:  findutils
 BuildRequires:  python3
 BuildRequires:  python3-rpm-macros
 BuildRequires:  systemd-rpm-macros
+BuildRequires:  python3-tox-current-env
+BuildRequires:  python3-mypy
+BuildRequires:  python3-pytest
 
 Requires:       qubes-core-agent-networking >= 4.2
+Conflicts:      qubes-core-agent < 4.2
 Requires:       python3
 Requires:       python3-qubesdb
 Requires:       nftables
@@ -46,6 +50,7 @@ BuildRequires:  python3-setuptools
 
 Requires:       python3
 Requires:       qubes-core-dom0 >= 4.2
+Conflicts:      qubes-core-dom0 < 4.2
 
 %description -n qubes-core-admin-addon-network-server
 This package lets you turn your Qubes OS into a network server.  Install this
@@ -69,6 +74,9 @@ make install DESTDIR=$RPM_BUILD_ROOT SBINDIR=%{_sbindir} UNITDIR=%{_unitdir} PYT
 mkdir -p "$RPM_BUILD_ROOT"/%{_presetdir}
 echo 'enable qubes-routing-manager.service' > "$RPM_BUILD_ROOT"/%{_presetdir}/75-%{name}.preset
 
+%check
+tox --current-env
+
 %files
 %attr(0755, root, root) %{_sbindir}/qubes-routing-manager
 %attr(0644, root, root) %{python3_sitelib}/qubesroutingmanager/*
@@ -84,6 +92,26 @@ echo 'enable qubes-routing-manager.service' > "$RPM_BUILD_ROOT"/%{_presetdir}/75
 %post
 %systemd_post  qubes-routing-manager.service
 
+%posttrans
+# Remove old unit enablement paths.
+reenable=0
+if [ -h %{_sysconfdir}/systemd/system/multi-user.target.wants/qubes-routing-manager.service ]
+then
+    reenable=1
+    rm -f %{_sysconfdir}/systemd/system/multi-user.target.wants/qubes-routing-manager.service
+fi
+if [ -h %{_sysconfdir}/systemd/system/qubes-iptables.service.wants/qubes-routing-manager.service ]
+then
+    reenable=1
+    rm -f %{_sysconfdir}/systemd/system/qubes-iptables.service.wants/qubes-routing-manager.service
+fi
+if [ $reenable = 1 ]
+then
+    mkdir -p %{_sysconfdir}/systemd/system/qubes-network.service.wants
+    ln -sf %{_unitdir}/qubes-routing-manager.service %{_sysconfdir}/systemd/system/qubes-network.service.wants/qubes-routing-manager.service
+fi
+exit 0
+
 %preun
 %systemd_preun qubes-routing-manager.service
 

qubesroutingmanager/__init__.py

@@ -4,10 +4,10 @@ import json
 import logging
 import subprocess
 
-from typing import TypedDict, Any, cast, Literal
+from typing import TypedDict, Any, cast, Literal, Union
 
 
-ADDRESS_FAMILIES = Literal["ip"] | Literal["ip6"]
+ADDRESS_FAMILIES = Union[Literal["ip"], Literal["ip6"]]
 
 
 class Chain(TypedDict):
@@ -69,7 +69,6 @@ POSTROUTING_CHAIN_NAME = "postrouting"
 ROUTING_MANAGER_CHAIN_NAME = "qubes-routing-manager"
 ROUTING_MANAGER_POSTROUTING_CHAIN_NAME = "qubes-routing-manager-postrouting"
 NFTABLES_CMD = "nft"
-ADD_FORWARD_RULE_AFTER_THIS_RULE = "custom-forward"
 
 
 def get_table(address_family: ADDRESS_FAMILIES, table: str) -> NFTablesOutput:
@@ -139,15 +138,20 @@ def append_counter_at_end(
     )
 
 
-def append_rule_after(
-    address_family: ADDRESS_FAMILIES, table: str, chain: str, handle: int, *rest: str
+def _append_or_insert_rule(
+    where: Literal["add"] | Literal["insert"],
+    address_family: ADDRESS_FAMILIES,
+    table: str,
+    chain: str,
+    handle: int,
+    *rest: str,
 ) -> None:
     subprocess.check_output(
         [
             NFTABLES_CMD,
             "-n",
             "-j",
-            "add",
+            where,
             "rule",
             address_family,
             table,
@@ -160,6 +164,18 @@ def append_rule_after(
     )
 
 
+def append_rule_after(
+    address_family: ADDRESS_FAMILIES, table: str, chain: str, handle: int, *rest: str
+) -> None:
+    _append_or_insert_rule("add", address_family, table, chain, handle, *rest)
+
+
+def insert_rule_before(
+    address_family: ADDRESS_FAMILIES, table: str, chain: str, handle: int, *rest: str
+) -> None:
+    _append_or_insert_rule("insert", address_family, table, chain, handle, *rest)
+
+
 def delete_rule(
     address_family: ADDRESS_FAMILIES, table: str, chain: str, handle: int
 ) -> None:
@@ -237,39 +253,42 @@ def setup_plain_forwarding_for_address(source: str, enable: bool, family: int) -
                 chain_name,
             )
 
-    def is_forward_jump_to_custom_forward(rule):
+    def is_oifgroup_2(rule):
         return (
             rule["chain"] == forward_chain["name"]
-            and len(rule["expr"]) == 1
-            and rule["expr"][0].get("jump", {}).get("target")
-            == ADD_FORWARD_RULE_AFTER_THIS_RULE
+            and len(rule["expr"]) == 3
+            and (
+                rule["expr"][0].get("match", {}).get("op") == "=="
+                and rule["expr"][0]
+                .get("match", {})
+                .get("left", {})
+                .get("meta", {})
+                .get("key")
+                == "oifgroup"
+                and rule["expr"][0].get("match", {}).get("right") == 2
+            )
+            and (rule["expr"][-1].get("drop", "not none") is None)
         )
 
-    def is_postrouting_lo_accept(rule):
+    def is_postrouting_masquerade(rule):
         return (
             rule["chain"] == postrouting_chain["name"]
-            and len(rule["expr"]) == 2
-            and rule["expr"][0].get("match", {}).get("op", "") == "=="
-            and rule["expr"][0]
-            .get("match", {})
-            .get("left", {})
-            .get("meta", {})
-            .get("key")
-            == "oif"
-            and rule["expr"][0].get("match", {}).get("right", "") == "lo"
-            and "accept" in rule["expr"][1]
+            and len(rule["expr"]) == 1
+            and "masquerade" in rule["expr"][0]
         )
 
-    for parent_chain, child_chain_name, previous_rule_detector in [
+    for parent_chain, child_chain_name, previous_rule_detector, insertor in [
         (
             forward_chain,
             ROUTING_MANAGER_CHAIN_NAME,
-            is_forward_jump_to_custom_forward,
+            is_oifgroup_2,
+            insert_rule_before,
         ),
         (
             postrouting_chain,
             ROUTING_MANAGER_POSTROUTING_CHAIN_NAME,
-            is_postrouting_lo_accept,
+            is_postrouting_masquerade,
+            insert_rule_before,
         ),
     ]:
         jump_rule: None | Rule = None
@@ -302,7 +321,7 @@ def setup_plain_forwarding_for_address(source: str, enable: bool, family: int) -
                 child_chain_name,
                 TABLE_NAME,
             )
-            append_rule_after(
+            insertor(
                 af,
                 TABLE_NAME,
                 parent_chain["name"],

qubesroutingmanager/test_firewalling.py

@@ -50,13 +50,13 @@ def test_partial_add_completes_the_add():
             "counter",
         ],
         [
-            "add",
+            "insert",
             "rule",
             "ip",
             "qubes",
             "postrouting",
             "position",
-            "66",
+            "67",
             "jump",
             "qubes-routing-manager-postrouting",
         ],
@@ -78,6 +78,79 @@ def test_partial_add_completes_the_add():
     assert got == expected
 
 
+def test_forward_rule_added_before_oifgroup_2():
+    got, MockedPopen = mock_collector(get_fixture("no_routing_manager.json"))
+    expected = [
+        ["list", "table", "ip", "qubes"],
+        ["add", "chain", "ip", "qubes", "qubes-routing-manager"],
+        [
+            "add",
+            "rule",
+            "ip",
+            "qubes",
+            "qubes-routing-manager",
+            "counter",
+        ],
+        ["add", "chain", "ip", "qubes", "qubes-routing-manager-postrouting"],
+        [
+            "add",
+            "rule",
+            "ip",
+            "qubes",
+            "qubes-routing-manager-postrouting",
+            "counter",
+        ],
+        [
+            "insert",
+            "rule",
+            "ip",
+            "qubes",
+            "forward",
+            "position",
+            "79",
+            "jump",
+            "qubes-routing-manager",
+        ],
+        [
+            "insert",
+            "rule",
+            "ip",
+            "qubes",
+            "postrouting",
+            "position",
+            "67",
+            "jump",
+            "qubes-routing-manager-postrouting",
+        ],
+        [
+            "add",
+            "rule",
+            "ip",
+            "qubes",
+            "qubes-routing-manager",
+            "ip",
+            "daddr",
+            "10.250.4.13",
+            "accept",
+        ],
+        [
+            "add",
+            "rule",
+            "ip",
+            "qubes",
+            "qubes-routing-manager-postrouting",
+            "ip",
+            "saddr",
+            "10.250.4.13",
+            "accept",
+        ],
+    ]
+    with mock.patch("subprocess.Popen", MockedPopen):
+        setup_plain_forwarding_for_address("10.250.4.13", True, 4)
+
+    assert got == expected
+
+
 def test_forwarding_does_not_add_twice():
     got, MockedPopen = mock_collector(get_fixture("fully_added.json"))
     expected = [

qubesroutingmanager/worker.py

@@ -14,7 +14,7 @@ import logging
 import os
 import socket
 
-import qubesdb
+import qubesdb  # type: ignore
 
 from qubesroutingmanager import setup_plain_forwarding_for_address
 

src/qubes-routing-manager.service.in

@@ -1,12 +1,13 @@
 [Unit]
 Description=Configure the network to allow network server VMs
 Documentation=https://github.com/Rudd-O/qubes-network-server
-After=qubes-iptables.service
+After=qubes-network.service qubes-iptables.service
 BindsTo=qubes-iptables.service
+ConditionPathExists=/var/run/qubes-service/qubes-network
 
 [Service]
 Type=notify
 ExecStart=@SBINDIR@/qubes-routing-manager
 
 [Install]
-WantedBy=qubes-iptables.service
+WantedBy=qubes-network.service

tox.ini

@@ -0,0 +1,10 @@
+[tox]
+envlist = basepython
+
+[testenv]
+deps =
+    pytest
+    mypy
+commands =
+    pytest -vv
+    mypy -p qubesroutingmanager