Fix scriptlet to reenable unit when obsolete wantedbys are removed.

Also improve tests and add Tox for mypy and pytest.

.gitignore

@@ -9,3 +9,5 @@ pkgs/
 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,12 +11,12 @@ 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
 	cd $(ROOT_DIR) || exit $$? ; rm -rf *.tar.gz *.rpm
-	cd $(ROOT_DIR) || exit $$? ; rm -rf *.egg-info build
+	cd $(ROOT_DIR) || exit $$? ; rm -rf *.egg-info build .mypy_cache
 
 dist: clean
 	@which rpmspec || { echo 'rpmspec is not available.  Please install the rpm-build package with the command `dnf install rpm-build` to continue, then rerun this step.' ; exit 1 ; }
@@ -32,12 +32,16 @@ rpm: dist
 	cd $(ROOT_DIR) ; mv -f builddir.rpm/*/* . && rm -rf builddir.rpm
 
 install-template: all
+	PYTHONDONTWRITEBYTECODE=1 python3 routingmanagersetup.py install $(PYTHON_PREFIX_ARG) -O0 --root $(DESTDIR)
 	install -Dm 755 src/qubes-routing-manager -t $(DESTDIR)/$(SBINDIR)/
 	sed -i "s,^#!.*,#!$(PYTHON)," $(DESTDIR)/$(SBINDIR)/qubes-routing-manager
 	install -Dm 644 src/qubes-routing-manager.service -t $(DESTDIR)/$(UNITDIR)/
 
 # Python 3 is always used for Qubes admin package.
 install-dom0:
-	PYTHONDONTWRITEBYTECODE=1 python3 setup.py install $(PYTHON_PREFIX_ARG) -O0 --root $(DESTDIR)
+	PYTHONDONTWRITEBYTECODE=1 python3 networkserversetup.py install $(PYTHON_PREFIX_ARG) -O0 --root $(DESTDIR)
 
 install: install-dom0 install-template
+
+test:
+	tox --current-env

README.md

@@ -1,12 +1,17 @@
 # Qubes network server
 
-This software lets you turn your [Qubes OS 4.1](https://www.qubes-os.org/) machine into
+This software lets you turn your [Qubes OS 4.2](https://www.qubes-os.org/) machine into
 a network server, enjoying all the benefits of Qubes OS (isolation, secure
 inter-VM process communication, ease of use) with none of the drawbacks
 of setting up your own Xen server.
 
-This release is only intended for use with Qubes OS 4.1.  Older Qubes OS releases
-will not support it.  For Qubes OS 4.0, check branch `r4.0`.
+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?
 
@@ -38,15 +43,18 @@ connectivity for ProxyVMs and other user VMs alike.
 
 No provision is made for running a server in a virtualized environment,
 such that the server's ports are accessible by (a) other VMs (b) machines
-beyond the perimeter of the NetVM.  To the extent that such a thing is
-possible, it is only possible by painstakingly maintaining firewall rules
-for multiple VMs, which need to carefully override the existing firewall
-rules, and require careful thought not to open the system to unexpected
-attack vectors.  The Qubes OS user interface provides no help either.
+beyond the perimeter of the NetVM.  By default, firewall rules in NetVMs
+prevent traffic from reaching any VM attached to them.  Furthermore, even
+with custom, permissive firewall rules, the IP addresses of VMs attached
+to any NetVM are not visible "on the other side of the NetVM", so firewall
+rules can only help with something like DNAT.  Finally, such custom firewalls
+require careful thought not to open the system to unexpected attack vectors.
+The Qubes OS user interface provides no means to set this up either.
 
 ### The Qubes network server networking model
 
-Qubes network server builds on that.
+Qubes network server builds on the Qubes security model and enhances it
+to optionally permit traffic to user VMs.
 
 ![Qubes network server model](./doc/Qubes%20network%20server%20model.png)
 
@@ -55,7 +63,8 @@ network servers in user VMs available to other machines, be them
 peer VMs in the same Qubes OS system or machines connected to
 a physical link shared by a NetVM.  Those network server VMs also
 obey the Qubes OS outbound firewall rules controls, letting you run
-services with outbound connections restricted.
+services with outbound connections restricted using the standard Qubes OS
+firewall system.
 
 This is all, of course, opt-in, so the standard Qubes OS network security
 model remains in effect until you decide to enable the feature on any
@@ -73,43 +82,55 @@ the software is straightforward.
 
 These sample instructions assume:
 
-* you understand the distinction between dom0 and qubes
-* you already have an AppVM VM set up, named `testvm`,
+* the software is properly installed (see below),
+* you understand the distinction between dom0 and qubes,
+* you already have an AppVM VM set up, named `testvm`, and
 * your `sys-net` VM is attached to a network with subnet `192.168.16.0/24`
   — this, of course, may vary depending on your local router configuration.
 
 *Do not proceed any further if you do not yet meet these requirements.*
   
-First, attach the VM you want to expose to the network
+First, **attach** the VM you want to expose to the network
 to a NetVM that has an active network connection:
 
 `qvm-prefs -s testvm netvm sys-net`
 
-Then, set an IP address on the VM:
+Set an **IP** address on `testvm` belonging to the same LAN as `sys-net`:
 
 `qvm-prefs -s testvm ip 192.168.16.25`
 
-(The step above requires you restart the `testvm` VM if it was running.)
+**Restart** the `testvm` VM if it was already running.
 
-Then, to enable the network server feature for your `testvm` VM, all you have
-to do in your AdminVM (`dom0`) is run the following command:
+**Configure** routing method; to enable the network server feature for
+your `testvm` VM, all you have to do in your AdminVM (`dom0`) is run
+the following command:
 
 `qvm-features testvm routing-method forward`
 
 Now `testvm` is exposed to the network with address `192.168.16.25`, as well
 as to other VMs attached to `NetVM`.
 
-Do note that `testvm` will have the standard Qubes OS firewall rules stopping
-inbound traffic.  To solve that issue, you can
-[use the standard `rc.local` Qubes OS mechanism to alter the firewall rules](https://www.qubes-os.org/doc/firewall/#where-to-put-firewall-rules)
-in your `testvm` AppVM.
+Finally, adjust **input firewall rules** on `testvm` to permit traffic coming from
+machines in your LAN.  `testvm` will have the standard Qubes OS firewall
+rules stopping inbound traffic.  To solve that issue, you can use a sample
+rule in `testvm`:
+
+```
+sudo nft add rule qubes custom-input ip saddr 192.168.16.0/24 ct state new,established,related counter accept
+```
+
+You can make these rules persistent by [following instructions on the Qubes
+OS firewall documentation page](https://www.qubes-os.org/doc/firewall/#enabling-networking-between-two-qubes).
+Note that you do not need to change the `custom-forward` chain at all
+on any qube -- Qubes network server manages that for you transparently
+in your `NetVM`.
 
 Here are documents that will help you take advantage of Qubes network server:
 
 * [Setting up your first server](doc/Setting up your first server.md)
 * [Setting up an SSH server](doc/Setting up an SSH server.md)
 
-## Installation of packages
+## Setup
 
 Package installation consists of two steps (**the package creation instructions are below**):
 
@@ -130,30 +151,38 @@ a terminal in your NetVM, then typing the following:
 systemctl status qubes-routing-manager.service
 ```
 
-The routing manager should show as `enabled` and `active` in the terminal output.
+The routing manager should show as `enabled` and `active` in the terminal
+output, with no errors.  You can now follow the usage instructions above.
 
 ### How to build the packages to install
 
 You will first build the `qubes-core-admin-addon-network-server` RPM.
 
 To build this package, you will need to use a `chroot` jail containing
-a Fedora installation of the exact same release as your `dom0` (Fedora 25
-for Qubes release 4.0, Fedora 32 for Qubes release 4.1).
+a Fedora installation of the exact same release as your `dom0` (Fedora 37
+for Qubes 4.2).  You can do this using `toolbox` (for maximum safety
+within a disposable qube):
 
-Copy the source of the package to your `chroot`.  Then start a shell in
-your `chroot`, and type `make rpm`.  You may have to install some packages
-in your `chroot` -- use `dnf install git rpm-build make coreutils tar gawk findutils systemd systemd-rpm-macros`
-to get the minimum dependency set installed.
+```
+dnf install -y toolbox
+toolbox create -r 37
+toolbox enter fedora-toolbox-37
+# Bam!  You have a shell in an isolated Fedora 37 instance now.
+```
 
-Once built, in the source directory you will find the RPM built for the
-exact release of Qubes you need.
+Within the toolbox, all your normal files from your home directory are
+visible.  Change into the directory that contains this source code,
+then type `make rpm`.  You may have to install some packages within your
+toolbox -- use `dnf install git rpm-build make coreutils tar gawk findutils systemd systemd-rpm-macros`
+to get the minimum dependency set installed.  Don't worry -- nothing within
+the toolbox affects
 
-Alternatively, you may first create a source RPM using `make srpm` on your
-regular workstation, then use `mock` to rebuild the source RPM produced
-in the source directory, using a Fedora release compatible with your `dom0`.
+Once built, in the source directory you will find the
+`qubes-core-admin-addon-network-server` RPM built for your dom0.
 
-To build the `qubes-network-server` RPM, you can use a DisposableVM running
-the same Fedora release as your NetVM.  Build said package as follows:
+To build the `qubes-network-server` RPM that goes in your template, you
+can simply use a DisposableVM running the same Fedora release as your NetVM.
+Build said package as follows:
 
 ```
 # Dependencies
@@ -165,8 +194,8 @@ make rpm
 ```
 
 The process will output a `qubes-network-server-*.noarch.rpm` in the
-directory where it ran.  Fish it out and save it into the VM where you'll
-install it.
+directory where it ran.  Fish it out and copy it into the template where
+you'll install it.
 
 You can power off the DisposableVM now.
 

build.parameters

@@ -1 +1 @@
-["RELEASE": "q4.1 36"]
+["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.

networkserversetup.py

@@ -0,0 +1,26 @@
+import os
+import setuptools
+
+if __name__ == "__main__":
+    version = (
+        open(os.path.join(os.path.dirname(__file__), "qubes-network-server.spec"))
+        .read()
+        .strip()
+    )
+    version = [v for v in version.splitlines() if v.startswith("Version:")][0]
+    version = version.split()[-1]
+    setuptools.setup(
+        name="qubesnetworkserver",
+        version=version,
+        author="Manuel Amador (Rudd-O)",
+        author_email="rudd-o@rudd-o.com",
+        description="Qubes network server dom0 component",
+        license="GPL2+",
+        url="https://github.com/Rudd-O/qubes-network-server",
+        packages=("qubesnetworkserver",),
+        entry_points={
+            "qubes.ext": [
+                "qubesnetworkserver = qubesnetworkserver:QubesNetworkServerExtension",
+            ],
+        },
+    )

qubes-network-server.spec

@@ -3,7 +3,7 @@
 %define mybuildnumber %{?build_number}%{?!build_number:1}
 
 Name:           qubes-network-server
-Version:        0.0.19
+Version:        0.1.6
 Release:        %{mybuildnumber}%{?dist}
 Summary:        Turn your Qubes OS into a network server
 BuildArch:      noarch
@@ -19,10 +19,15 @@ 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.1
+Requires:       qubes-core-agent-networking >= 4.2
+Conflicts:      qubes-core-agent < 4.2
 Requires:       python3
 Requires:       python3-qubesdb
+Requires:       nftables
 
 %description
 This package lets you turn your Qubes OS into a network server.  Install this
@@ -44,7 +49,8 @@ BuildRequires:  python3-rpm-macros
 BuildRequires:  python3-setuptools
 
 Requires:       python3
-Requires:       qubes-core-dom0 >= 4.1
+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
@@ -68,8 +74,13 @@ 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/*
+%{python3_sitelib}/qubesroutingmanager-*.egg-info
 %attr(0644, root, root) %{_presetdir}/75-%{name}.preset
 %config %attr(0644, root, root) %{_unitdir}/qubes-routing-manager.service
 %doc README.md TODO
@@ -81,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

@@ -0,0 +1,379 @@
+#!/usr/bin/python3
+
+import json
+import logging
+import subprocess
+
+from typing import TypedDict, Any, cast, Literal, Union
+
+
+ADDRESS_FAMILIES = Union[Literal["ip"], Literal["ip6"]]
+
+
+class Chain(TypedDict):
+    name: str
+    family: str
+    table: str
+    handle: int
+    type: str
+    hook: str
+    prio: int
+    policy: str
+
+
+class Table(TypedDict):
+    family: str
+    name: str
+    handle: int
+
+
+class Metainfo(TypedDict):
+    version: str
+    release_name: str
+    json_schema_version: int
+
+
+class Rule(TypedDict):
+    family: str
+    table: str
+    chain: str
+    handle: int
+    expr: list[dict[str, Any]]
+
+
+class ChainContainer(TypedDict):
+    chain: Chain
+
+
+class MetainfoContainer(TypedDict):
+    metainfo: Metainfo
+
+
+class TableContainer(TypedDict):
+    table: Table
+
+
+class RuleContainer(TypedDict):
+    rule: Rule
+
+
+class NFTablesOutput(TypedDict):
+    nftables: list[ChainContainer | MetainfoContainer | TableContainer | RuleContainer]
+
+
+ADDRESS_FAMILY_IPV6 = "ip6"
+ADDRESS_FAMILY_IPV4 = "ip"
+TABLE_NAME = "qubes"
+FORWARD_CHAIN_NAME = "forward"
+POSTROUTING_CHAIN_NAME = "postrouting"
+ROUTING_MANAGER_CHAIN_NAME = "qubes-routing-manager"
+ROUTING_MANAGER_POSTROUTING_CHAIN_NAME = "qubes-routing-manager-postrouting"
+NFTABLES_CMD = "nft"
+
+
+def get_table(address_family: ADDRESS_FAMILIES, table: str) -> NFTablesOutput:
+    return cast(
+        NFTablesOutput,
+        json.loads(
+            subprocess.check_output(
+                [NFTABLES_CMD, "-n", "-j", "list", "table", address_family, table],
+                text=True,
+            )
+        ),
+    )
+
+
+def add_chain(address_family: ADDRESS_FAMILIES, table: str, chain: str) -> None:
+    subprocess.check_output(
+        [
+            NFTABLES_CMD,
+            "-n",
+            "-j",
+            "add",
+            "chain",
+            address_family,
+            table,
+            chain,
+        ],
+        text=True,
+    )
+
+
+def append_rule_at_end(
+    address_family: ADDRESS_FAMILIES, table: str, chain: str, *rest: str
+) -> None:
+    subprocess.check_output(
+        [
+            NFTABLES_CMD,
+            "-n",
+            "-j",
+            "add",
+            "rule",
+            address_family,
+            table,
+            chain,
+        ]
+        + list(rest),
+        text=True,
+    )
+
+
+def append_counter_at_end(
+    address_family: ADDRESS_FAMILIES, table: str, chain: str, *rest: str
+) -> None:
+    subprocess.check_output(
+        [
+            NFTABLES_CMD,
+            "-n",
+            "-j",
+            "add",
+            "rule",
+            address_family,
+            table,
+            chain,
+            "counter",
+        ]
+        + list(rest),
+        text=True,
+    )
+
+
+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",
+            where,
+            "rule",
+            address_family,
+            table,
+            chain,
+            "position",
+            str(handle),
+        ]
+        + list(rest),
+        text=True,
+    )
+
+
+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:
+    subprocess.check_output(
+        [
+            NFTABLES_CMD,
+            "-n",
+            "-j",
+            "delete",
+            "rule",
+            address_family,
+            table,
+            chain,
+            "handle",
+            str(handle),
+        ],
+        text=True,
+    )
+
+
+def setup_plain_forwarding_for_address(source: str, enable: bool, family: int) -> None:
+    logging.info("Handling forwarding for address %s family %s.", source, family)
+
+    af = cast(
+        ADDRESS_FAMILIES,
+        ADDRESS_FAMILY_IPV6 if family == 6 else ADDRESS_FAMILY_IPV4,
+    )
+
+    # table ip qubes {
+    #     set downstream {
+    #         type ipv4_addr
+    #         elements = { 10.137.0.10, 10.250.4.13 }
+    #     }
+    #     ...
+    existing_table_output = get_table(af, TABLE_NAME)
+    existing_table_items = existing_table_output["nftables"]
+
+    existing_chains = [x["chain"] for x in existing_table_items if "chain" in x]  # type: ignore
+    existing_rules = [x["rule"] for x in existing_table_items if "rule" in x]  # type: ignore
+
+    try:
+        forward_chain = [x for x in existing_chains if x["name"] == FORWARD_CHAIN_NAME][
+            0
+        ]
+        postrouting_chain = [
+            x for x in existing_chains if x["name"] == POSTROUTING_CHAIN_NAME
+        ][0]
+    except IndexError:
+        logging.warn(
+            "No forward or postrouting chains in table %s, not setting up forwarding",
+            TABLE_NAME,
+        )
+        return
+
+    for chain_name in [
+        ROUTING_MANAGER_CHAIN_NAME,
+        ROUTING_MANAGER_POSTROUTING_CHAIN_NAME,
+    ]:
+        chain: None | Chain = None
+        try:
+            chain = [x for x in existing_chains if x["name"] == chain_name].pop()
+        except IndexError:
+            pass
+
+        if not chain:
+            logging.info(
+                "Adding %s chain to table %s and counter to chain",
+                chain_name,
+                TABLE_NAME,
+            )
+            add_chain(af, TABLE_NAME, chain_name)
+            append_counter_at_end(
+                af,
+                TABLE_NAME,
+                chain_name,
+            )
+
+    def is_oifgroup_2(rule):
+        return (
+            rule["chain"] == forward_chain["name"]
+            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_masquerade(rule):
+        return (
+            rule["chain"] == postrouting_chain["name"]
+            and len(rule["expr"]) == 1
+            and "masquerade" in rule["expr"][0]
+        )
+
+    for parent_chain, child_chain_name, previous_rule_detector, insertor in [
+        (
+            forward_chain,
+            ROUTING_MANAGER_CHAIN_NAME,
+            is_oifgroup_2,
+            insert_rule_before,
+        ),
+        (
+            postrouting_chain,
+            ROUTING_MANAGER_POSTROUTING_CHAIN_NAME,
+            is_postrouting_masquerade,
+            insert_rule_before,
+        ),
+    ]:
+        jump_rule: None | Rule = None
+        try:
+            jump_rule = [
+                x
+                for x in existing_rules
+                if x["chain"] == parent_chain["name"]
+                and x["family"] == af
+                and len(x["expr"]) == 1
+                and x["expr"][0].get("jump", {}).get("target") == child_chain_name
+            ].pop()
+        except IndexError:
+            pass
+
+        if not jump_rule:
+            try:
+                previous_rule = [
+                    x for x in existing_rules if previous_rule_detector(x)
+                ][0]
+            except IndexError:
+                logging.warn(
+                    "Cannot find appropriate previous rule in chain %s of table %s, not setting up forwarding",
+                    parent_chain["name"],
+                    TABLE_NAME,
+                )
+            logging.info(
+                "Adding rule to jump from chain %s to chain %s in table %s",
+                parent_chain["name"],
+                child_chain_name,
+                TABLE_NAME,
+            )
+            insertor(
+                af,
+                TABLE_NAME,
+                parent_chain["name"],
+                previous_rule["handle"],
+                "jump",
+                child_chain_name,
+            )
+
+    def detect_ip_rule(rule: Rule, chain_name: str, ip: str, mode: str):
+        return (
+            rule["chain"] == chain_name
+            and len(rule["expr"]) == 2
+            and rule["expr"][0].get("match", {}).get("op", {}) == "=="
+            and rule["expr"][0]["match"]
+            .get("left", {})
+            .get("payload", {})
+            .get("protocol", "")
+            == af
+            and rule["expr"][0]["match"]["left"]["payload"].get("field", "") == mode
+            and rule["expr"][0].get("match", {}).get("right", []) == ip
+            and "accept" in rule["expr"][1]
+        )
+
+    for chain_name, mode in [
+        (ROUTING_MANAGER_CHAIN_NAME, "daddr"),
+        (ROUTING_MANAGER_POSTROUTING_CHAIN_NAME, "saddr"),
+    ]:
+        address_rules = [
+            x for x in existing_rules if detect_ip_rule(x, chain_name, source, mode)
+        ]
+
+        if enable and not address_rules:
+            logging.info(
+                "Adding accept rule on chain %s for %s.",
+                chain_name,
+                source,
+            )
+            append_rule_at_end(
+                af,
+                TABLE_NAME,
+                chain_name,
+                af,
+                mode,
+                source,
+                "accept",
+            )
+        elif not enable and address_rules:
+            logging.info(
+                "Removing %s accept rules from chain %s for %s.",
+                len(address_rules),
+                chain_name,
+                source,
+            )
+            for rule in reversed(sorted(address_rules, key=lambda r: r["handle"])):
+                delete_rule(af, TABLE_NAME, chain_name, rule["handle"])

qubesroutingmanager/test_firewalling.py

@@ -0,0 +1,162 @@
+import os
+
+from unittest import mock
+from qubesroutingmanager import setup_plain_forwarding_for_address
+
+
+def get_fixture(name):
+    with open(os.path.join(os.path.dirname(__file__), "fixtures", name)) as f:
+        return f.read()
+
+
+def mock_collector(output: str):
+    final_args = []
+
+    class MockedPopen:
+        def __init__(self, args, **kwargs):
+            final_args.append(args[3:])
+            self.args = args
+            self.returncode = 0
+
+        def __enter__(self):
+            return self
+
+        def __exit__(self, exc_type, value, traceback):
+            pass
+
+        def communicate(self, input=None, timeout=None):
+            stdout = output
+            stderr = ""
+            self.returncode = 1
+            return stdout, stderr
+
+        def poll(self):
+            return 0
+
+    return final_args, MockedPopen
+
+
+def test_partial_add_completes_the_add():
+    got, MockedPopen = mock_collector(get_fixture("partially_added.json"))
+    expected = [
+        ["list", "table", "ip", "qubes"],
+        ["add", "chain", "ip", "qubes", "qubes-routing-manager-postrouting"],
+        [
+            "add",
+            "rule",
+            "ip",
+            "qubes",
+            "qubes-routing-manager-postrouting",
+            "counter",
+        ],
+        [
+            "insert",
+            "rule",
+            "ip",
+            "qubes",
+            "postrouting",
+            "position",
+            "67",
+            "jump",
+            "qubes-routing-manager-postrouting",
+        ],
+        [
+            "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_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 = [
+        ["list", "table", "ip", "qubes"],
+    ]
+    with mock.patch("subprocess.Popen", MockedPopen):
+        setup_plain_forwarding_for_address("10.250.4.13", True, 4)
+
+    assert got == expected

qubesroutingmanager/worker.py

@@ -0,0 +1,135 @@
+#!/usr/bin/python3
+
+"""
+This program reads the /qubes-firewall/{ip}/qubes-routing-method file
+for any firewall configuration, then configures the network to obey
+the routing method for the VM.  If the routing method is "masquerade",
+then nothing happens.  If, however, the routing method is "forward",
+then VM-specific rules are enacted in the VM's attached NetVM to allow
+traffic coming from other VMs and the outside world to reach this VM.
+"""
+
+import glob
+import logging
+import os
+import socket
+
+import qubesdb  # type: ignore
+
+from qubesroutingmanager import setup_plain_forwarding_for_address
+
+
+FORWARD_ROUTING_METHOD = "forward"
+
+
+def _s(v):
+    if isinstance(v, bytes):
+        return v.decode("utf-8")
+    return v
+
+
+class AdjunctWorker(object):
+    def __init__(self):
+        self.qdb = qubesdb.QubesDB()
+
+    @staticmethod
+    def is_ip6(addr):
+        return addr.count(":") > 0
+
+    def setup_proxy_arp_ndp(self, enabled, family):
+        # If any of the IP addresses is assigned the forward routing method,
+        # then enable proxy ARP/NDP on the upstream interfaces, so that the
+        # interfaces in question will impersonate the IP addresses in question.
+        # Ideally, this impersonation would be exclusively done for the
+        # specific IP addresses in question, but it is not clear to me how
+        # to cause this outcome to take place.
+        if family == 6:
+            globber = "/proc/sys/net/ipv6/conf/*/proxy_ndp"
+            name = "proxy NDP"
+        elif family == 4:
+            globber = "/proc/sys/net/ipv4/conf/*/proxy_arp"
+            name = "proxy ARP"
+        else:
+            return
+
+        if enabled:
+            action = "Enabling"
+            val = "1\n"
+        else:
+            action = "Disabling"
+            val = "0\n"
+
+        matches = glob.glob(globber)
+        for m in matches:
+            iface = m.split("/")[6]
+            if iface in ("all", "lo") or iface.startswith("vif"):
+                # No need to enable it for "all", or VIFs, or loopback.
+                continue
+            with open(m, "w+") as f:
+                oldval = f.read()
+                if oldval != val:
+                    logging.info("%s %s on interface %s.", action, name, iface)
+                    f.seek(0)
+                    f.write(val)
+
+    def handle_addr(self, addr):
+        # Setup plain forwarding for this specific address.
+        routing_method = _s(self.qdb.read("/qubes-routing-method/{}".format(addr)))
+        setup_plain_forwarding_for_address(
+            addr,
+            routing_method == FORWARD_ROUTING_METHOD,
+            6 if self.is_ip6(addr) else 4,
+        )
+
+        # Manipulate proxy ARP for all known addresses.
+        methods = [
+            (_s(k).split("/")[2], _s(v))
+            for k, v in self.qdb.multiread("/qubes-routing-method/").items()
+        ]
+        mmethods = {
+            4: [m[1] for m in methods if not self.is_ip6(m[0])],
+            6: [m[1] for m in methods if self.is_ip6(m[0])],
+        }
+        for family, methods in mmethods.items():
+            self.setup_proxy_arp_ndp(
+                FORWARD_ROUTING_METHOD in methods,
+                family,
+            )
+
+    def list_targets(self):
+        return set(_s(t).split("/")[2] for t in self.qdb.list("/qubes-routing-method/"))
+
+    def sd_notify(self, state):
+        """Send notification to systemd, if available"""
+        # based on sdnotify python module
+        if "NOTIFY_SOCKET" not in os.environ:
+            return
+        addr = os.environ["NOTIFY_SOCKET"]
+        if addr[0] == "@":
+            addr = "\0" + addr[1:]
+        try:
+            sock = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)
+            sock.connect(addr)
+            sock.sendall(state.encode())
+        except BaseException:
+            # generally ignore error on systemd notification
+            pass
+
+    def main(self):
+        logging.basicConfig(level=logging.INFO)
+        self.qdb.watch("/qubes-routing-method/")
+        for source_addr in self.list_targets():
+            self.handle_addr(source_addr)
+        self.sd_notify("READY=1")
+        try:
+            for watch_path in iter(self.qdb.read_watch, None):
+                # ignore writing rules itself - wait for final write at
+                # source_addr level empty write (/qubes-firewall/SOURCE_ADDR)
+                watch_path = _s(watch_path)
+                if watch_path.count("/") != 2:
+                    continue
+                source_addr = watch_path.split("/")[2]
+                self.handle_addr(source_addr)
+        except OSError:  # EINTR
+            # signal received, don't continue the loop
+            return

routingmanagersetup.py

@@ -0,0 +1,21 @@
+import os
+import setuptools
+
+if __name__ == "__main__":
+    version = (
+        open(os.path.join(os.path.dirname(__file__), "qubes-network-server.spec"))
+        .read()
+        .strip()
+    )
+    version = [v for v in version.splitlines() if v.startswith("Version:")][0]
+    version = version.split()[-1]
+    setuptools.setup(
+        name="qubesroutingmanager",
+        version=version,
+        author="Manuel Amador (Rudd-O)",
+        author_email="rudd-o@rudd-o.com",
+        description="Qubes network server network qube (template) component",
+        license="GPL2+",
+        url="https://github.com/Rudd-O/qubes-network-server",
+        packages=("qubesroutingmanager",),
+    )

setup.py

@@ -1,24 +0,0 @@
-import os
-import setuptools
-
-if __name__ == '__main__':
-    version = open(os.path.join(os.path.dirname(__file__), 'qubes-network-server.spec')).read().strip()
-    version = [v for v in version.splitlines() if v.startswith("Version:")][0]
-    version = version.split()[-1]
-    setuptools.setup(
-        name='qubesnetworkserver',
-        version=version,
-        author='Manuel Amador (Rudd-O)',
-        author_email='rudd-o@rudd-o.com',
-        description='Qubes network server dom0 component',
-        license='GPL2+',
-        url='https://github.com/Rudd-O/qubes-network-server',
-
-        packages=('qubesnetworkserver',),
-
-        entry_points={
-            'qubes.ext': [
-                'qubesnetworkserver = qubesnetworkserver:QubesNetworkServerExtension',
-            ],
-        }
-    )

src/qubes-routing-manager

@@ -1,265 +1,8 @@
 #!/usr/bin/python3
 
-"""
-This program reads the /qubes-firewall/{ip}/qubes-routing-method file
-for any firewall configuration, then configures the network to obey
-the routing method for the VM.  If the routing method is "masquerade",
-then nothing happens.  If, however, the routing method is "forward",
-then VM-specific rules are enacted in the VM's attached NetVM to allow
-traffic coming from other VMs and the outside world to reach this VM.
-"""
-
-import glob
-import logging
-import os
-import socket
-import subprocess
-
-import qubesdb
-
-
-def _s(v):
-    if isinstance(v, bytes):
-        return v.decode("utf-8")
-    return v
-
-
-FORWARD_ROUTING_METHOD = "forward"
-
-
-class AdjunctWorker(object):
-    def __init__(self):
-        self.qdb = qubesdb.QubesDB()
-
-    @staticmethod
-    def is_ip6(addr):
-        return addr.count(":") > 0
-
-    def setup_plain_forwarding_for_address(self, source, enable, family):
-        def find_pos_of_first_rule(table, startswith):
-            rules = [n for n, l in enumerate(table) if l.startswith(startswith)]
-            if rules:
-                return rules[0]
-            return None
-
-        cmd = "ip6tables" if family == 6 else "iptables"
-        mask = "/128" if family == 6 else "/32"
-
-        def run_ipt(*args):
-            return subprocess.check_call([cmd, "-w"] + list(args))
-
-        out_nat = subprocess.check_output(
-            [cmd + "-save", "-t", "nat"], universal_newlines=True
-        ).splitlines()
-        out_filter = subprocess.check_output(
-            [cmd + "-save", "-t", "filter"], universal_newlines=True
-        ).splitlines()
-        
-        if enable:
-            # Create necessary prerouting chain.
-            if not find_pos_of_first_rule(out_nat, ":PR-PLAIN-FORWARDING - "):
-                logging.info("Creating chain PR-PLAIN-FORWARDING on table nat.")
-                run_ipt("-t", "nat", "-N", "PR-PLAIN-FORWARDING")
-
-            # Route prerouting traffic to necessary chain.
-            if not find_pos_of_first_rule(out_nat, "-A POSTROUTING -j PR-PLAIN-FORWARDING"):
-                rule_num = find_pos_of_first_rule(out_nat, "-A POSTROUTING -j MASQUERADE")
-                if not rule_num:
-                    # This table does not contain the masquerading rule.
-                    # Accordingly, we will not do anything.
-                    return
-                first_rule_num = find_pos_of_first_rule(out_nat, "-A POSTROUTING")
-                pos = rule_num - first_rule_num + 1
-                logging.info("Adding chain PR-PLAIN-FORWARDING to chain POSTROUTING on table nat.")
-                run_ipt(
-                    "-t",
-                    "nat",
-                    "-I",
-                    "POSTROUTING",
-                    str(pos),
-                    "-j",
-                    "PR-PLAIN-FORWARDING",
-                )
-
-            # Create necessary forward chain.
-            if not find_pos_of_first_rule(out_filter, ":PLAIN-FORWARDING - "):
-                logging.info("Creating chain PLAIN-FORWARDING on table filter.")
-                run_ipt("-t", "filter", "-N", "PLAIN-FORWARDING")
-
-            # Route forward traffic to necessary chain.
-            if not find_pos_of_first_rule(out_filter, "-A FORWARD -j PLAIN-FORWARDING"):
-                rule_num = find_pos_of_first_rule(
-                    out_filter, "-A FORWARD -i vif+ -o vif+ -j DROP"
-                )
-                if not rule_num:
-                    # This table does not contain the masquerading rule.
-                    # Accordingly, we will not do anything.
-                    return
-                first_rule_num = find_pos_of_first_rule(out_filter, "-A FORWARD")
-                pos = rule_num - first_rule_num + 1
-                logging.info("Adding chain PLAIN-FORWARDING to chain FORWARD on table filter.")
-                run_ipt(
-                    "-t", "filter", "-I", "FORWARD", str(pos), "-j", "PLAIN-FORWARDING"
-                )
-
-        rule = find_pos_of_first_rule(
-            out_nat, "-A PR-PLAIN-FORWARDING -s {}{} -j ACCEPT".format(source, mask)
-        )
-        if enable:
-            if rule:
-                pass
-            else:
-                logging.info(
-                    "Adding PR-PLAIN-FORWARDING rule on table nat to forward traffic from %s.", source
-                )
-                run_ipt(
-                    "-t",
-                    "nat",
-                    "-A",
-                    "PR-PLAIN-FORWARDING",
-                    "-s",
-                    "{}{}".format(source, mask),
-                    "-j",
-                    "ACCEPT",
-                )
-        else:
-            if rule:
-                first_rule = find_pos_of_first_rule(out_nat, "-A PR-PLAIN-FORWARDING")
-                pos = rule - first_rule + 1
-                logging.info(
-                    "Removing PR-PLAIN-FORWARDING rule on table nat forwarding traffic from %s.", source
-                )
-                run_ipt("-t", "nat", "-D", "PR-PLAIN-FORWARDING", str(pos))
-            else:
-                pass
-
-        rule = find_pos_of_first_rule(
-            out_filter, "-A PLAIN-FORWARDING -d {}{} -o vif+ -j ACCEPT".format(source, mask)
-        )
-        if enable:
-            if rule:
-                pass
-            else:
-                logging.info("Adding PLAIN-FORWARDING rule on table filter to allow traffic to %s.", source)
-                run_ipt(
-                    "-t",
-                    "filter",
-                    "-A",
-                    "PLAIN-FORWARDING",
-                    "-d",
-                    "{}{}".format(source, mask),
-                    "-o",
-                    "vif+",
-                    "-j",
-                    "ACCEPT",
-                )
-        else:
-            if rule:
-                logging.info("Removing PLAIN-FORWARDING rule on table filter allowing traffic to %s.", source)
-                first_rule = find_pos_of_first_rule(out_filter, "-A PLAIN-FORWARDING")
-                pos = rule - first_rule + 1
-                run_ipt("-t", "filter", "-D", "PLAIN-FORWARDING", str(pos))
-            else:
-                pass
-
-    def setup_proxy_arp_ndp(self, enabled, family):
-        # If any of the IP addresses is assigned the forward routing method,
-        # then enable proxy ARP/NDP on the upstream interfaces, so that the
-        # interfaces in question will impersonate the IP addresses in question.
-        # Ideally, this impersonation would be exclusively done for the
-        # specific IP addresses in question, but it is not clear to me how
-        # to cause this outcome to take place.
-        if family == 6:
-            globber = "/proc/sys/net/ipv6/conf/*/proxy_ndp"
-            name = "proxy NDP"
-        elif family == 4:
-            globber = "/proc/sys/net/ipv4/conf/*/proxy_arp"
-            name = "proxy ARP"
-        else:
-            return
-
-        if enabled:
-            action = "Enabling"
-            val = "1\n"
-        else:
-            action = "Disabling"
-            val = "0\n"
-
-        matches = glob.glob(globber)
-        for m in matches:
-            iface = m.split("/")[6]
-            if iface in ("all", "lo") or iface.startswith("vif"):
-                # No need to enable it for "all", or VIFs, or loopback.
-                continue
-            with open(m, "w+") as f:
-                oldval = f.read()
-                if oldval != val:
-                    logging.info("%s %s on interface %s.", action, name, iface)
-                    f.seek(0)
-                    f.write(val)
-
-    def handle_addr(self, addr):
-        # Setup plain forwarding for this specific address.
-        routing_method = _s(self.qdb.read("/qubes-routing-method/{}".format(addr)))
-        self.setup_plain_forwarding_for_address(
-            addr,
-            routing_method == FORWARD_ROUTING_METHOD,
-            6 if self.is_ip6(addr) else 4,
-        )
-
-        # Manipulate proxy ARP for all known addresses.
-        methods = [
-            (_s(k).split("/")[2], _s(v))
-            for k, v in self.qdb.multiread("/qubes-routing-method/").items()
-        ]
-        mmethods = {
-            4: [m[1] for m in methods if not self.is_ip6(m[0])],
-            6: [m[1] for m in methods if self.is_ip6(m[0])],
-        }
-        for family, methods in mmethods.items():
-            self.setup_proxy_arp_ndp(
-                FORWARD_ROUTING_METHOD in methods, family,
-            )
-
-    def list_targets(self):
-        return set(_s(t).split("/")[2] for t in self.qdb.list("/qubes-routing-method/"))
-
-    def sd_notify(self, state):
-        """Send notification to systemd, if available"""
-        # based on sdnotify python module
-        if not "NOTIFY_SOCKET" in os.environ:
-            return
-        addr = os.environ["NOTIFY_SOCKET"]
-        if addr[0] == "@":
-            addr = "\0" + addr[1:]
-        try:
-            sock = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)
-            sock.connect(addr)
-            sock.sendall(state.encode())
-        except:
-            # generally ignore error on systemd notification
-            pass
-
-    def main(self):
-        logging.basicConfig(level=logging.INFO)
-        self.qdb.watch("/qubes-routing-method/")
-        for source_addr in self.list_targets():
-            self.handle_addr(source_addr)
-        self.sd_notify("READY=1")
-        try:
-            for watch_path in iter(self.qdb.read_watch, None):
-                # ignore writing rules itself - wait for final write at
-                # source_addr level empty write (/qubes-firewall/SOURCE_ADDR)
-                watch_path = _s(watch_path)
-                if watch_path.count("/") != 2:
-                    continue
-                source_addr = watch_path.split("/")[2]
-                self.handle_addr(source_addr)
-        except OSError:  # EINTR
-            # signal received, don't continue the loop
-            return
-
 
 if __name__ == "__main__":
+    from qubesroutingmanager.worker import AdjunctWorker
+
     w = AdjunctWorker()
     w.main()

src/qubes-routing-manager.service.in

@@ -1,13 +1,13 @@
 [Unit]
 Description=Configure the network to allow network server VMs
 Documentation=https://github.com/Rudd-O/qubes-network-server
-ConditionPathExists=/var/run/qubes-service/qubes-firewall
-After=qubes-firewall.service
-BindsTo=qubes-firewall.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=multi-user.target
+WantedBy=qubes-network.service

tox.ini

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