Reapply the firewall ruleset in place when the package is upgraded

On upgrade the package reloaded nothing: fips-firewall.service kept the ruleset
loaded at boot, so a changed /etc/fips/fips.nft did not take effect until the
next reboot or a manual restart, and a restart runs ExecStop, which deletes the
fips table and leaves the mesh interface unfiltered until ExecStart loads it
again.

fips-firewall.service, in both the Debian and the plain systemd unit, gains an
ExecReload that runs the same nft -f. The file adds and then flushes the table
before defining it, so one run replaces the ruleset in a single transaction.
postinst now runs try-reload-or-restart on the unit before it starts the
daemon. That acts only when the unit is already active, so it never turns the
firewall on for a host that has not opted in, and a reload that fails leaves the
previous ruleset in place, so it is reported and the upgrade goes on.

The upgrade scenario gains a host with the firewall enabled. Its newer package
carries a ruleset with an extra named counter; after the upgrade the counter
must be loaded, and an nft monitor running across the upgrade, proven to be
recording first, must show no deletion of the fips table. The host that never
opted in must still have the firewall inactive, disabled and its table absent.

packaging/debian/fips-firewall.service

@@ -9,6 +9,9 @@ ConditionPathExists=/etc/fips/fips.nft
 Type=oneshot
 RemainAfterExit=yes
 ExecStart=/usr/sbin/nft -f /etc/fips/fips.nft
+# Reapplies the ruleset in place: the file adds then flushes the table, so one
+# nft run replaces it in a single transaction, with no moment without it.
+ExecReload=/usr/sbin/nft -f /etc/fips/fips.nft
 ExecStop=-/usr/sbin/nft delete table inet fips
 StandardOutput=journal
 StandardError=journal

packaging/debian/postinst

@@ -127,6 +127,19 @@ case "$1" in
             # is not a failure, but the units that require it are not started
             # either.
             if [ -n "$2" ]; then
+                # Reapply the firewall ruleset in place, before the daemon
+                # starts, and only where the operator has it running: "try"
+                # leaves a unit that is not active alone, so this never opts
+                # a host in. The daemon-reload above has loaded the unit's
+                # ExecReload, so this reloads rather than restarts; a restart
+                # would run ExecStop, which deletes the table. A reload that
+                # fails leaves the previous ruleset loaded, so it is reported
+                # and the upgrade goes on.
+                if ! systemctl try-reload-or-restart fips-firewall.service; then
+                    echo "fips: reloading fips-firewall.service failed; the ruleset loaded before the upgrade stays in force" >&2
+                    echo "fips: check /etc/fips/fips.nft and the rules in /etc/fips/fips.d/" >&2
+                fi
+
                 daemon_rc=0
                 unit_bounded start fips.service "$UNIT_START_LIMIT" || daemon_rc=$?
                 if [ "$daemon_rc" -eq 1 ]; then

packaging/systemd/fips-firewall.service

@@ -9,6 +9,9 @@ ConditionPathExists=/etc/fips/fips.nft
 Type=oneshot
 RemainAfterExit=yes
 ExecStart=/usr/sbin/nft -f /etc/fips/fips.nft
+# Reapplies the ruleset in place: the file adds then flushes the table, so one
+# nft run replaces it in a single transaction, with no moment without it.
+ExecReload=/usr/sbin/nft -f /etc/fips/fips.nft
 ExecStop=-/usr/sbin/nft delete table inet fips
 StandardOutput=journal
 StandardError=journal

testing/deb-install/test.sh

@@ -744,6 +744,90 @@ check_active() {
     return 0
 }
 
+# Pass or fail on whether a unit the host never enabled is still neither
+# running nor enabled.
+check_left_off() {
+    local name="$1" unit="$2" what="$3" state
+    state=$(cexec "$name" systemctl is-enabled "$unit" 2>/dev/null || true)
+    if ! cexec "$name" systemctl is-active --quiet "$unit" && [ "$state" = "disabled" ]; then
+        pass "$what: $unit inactive and disabled"
+    else
+        fail "$what: $unit is $(cexec "$name" systemctl is-active "$unit" 2>/dev/null) and '$state' (want inactive and disabled)"
+    fi
+    return 0
+}
+
+# Start `nft monitor tables` in the background, writing to
+# /root/nft-monitor.log, and prove it is recording by adding and deleting a
+# table of its own. An empty log from a monitor that never ran would otherwise
+# read as a ruleset that was never removed. The probe table's name does not
+# begin with "fips", so it cannot match a check on the fips table.
+start_nft_monitor() {
+    local name="$1"
+    if ! timeout "$EXEC_TIMEOUT" docker exec -d "$name" \
+            sh -c 'exec nft monitor tables > /root/nft-monitor.log 2>&1'; then
+        return 1
+    fi
+    sleep 1
+    cexec "$name" sh -c 'nft add table inet monprobe && nft delete table inet monprobe' || return 1
+    local _i
+    for _i in 1 2 3 4 5; do
+        if cexec "$name" grep -Eq '^delete table inet monprobe( |$)' /root/nft-monitor.log; then
+            return 0
+        fi
+        sleep 1
+    done
+    cexec "$name" cat /root/nft-monitor.log 2>&1 | tail -10
+    return 1
+}
+
+# Host that opted in to the firewall: the upgrade must apply the new ruleset in
+# place, with no moment at which the fips table is absent.
+_upgrade_opted_in() {
+    local name="$1" image="$2" deb="$3"
+    log "upgrade on a host that opted in ($name)"
+    upgrade_boot "$name" "$image" "$deb" || { cleanup_container "$name"; return 0; }
+    make_next_package "$name" "$deb" || { cleanup_container "$name"; return 0; }
+    if ! cexec "$name" systemctl enable --now fips-firewall.service >/dev/null 2>&1 ||
+        ! start_daemon_units "$name"; then
+        fail "opted in: the firewall, fips and fips-dns did not all start before the upgrade"
+        cexec "$name" systemctl status --no-pager fips-firewall.service 2>&1 | tail -15
+        cleanup_container "$name"
+        return 0
+    fi
+    if ! start_nft_monitor "$name"; then
+        fail "opted in: nft monitor is not observing table changes"
+        cleanup_container "$name"
+        return 0
+    fi
+
+    run_apt "$name" install -y ./next.deb
+    echo "  upgrade took ${APT_SECS}s"
+    if [ "$APT_RC" -eq 0 ]; then
+        pass "opted in: upgrade exits 0"
+    else
+        fail "opted in: upgrade exited $APT_RC"
+        echo "$APT_OUT" | tail -20
+    fi
+    check_active "$name" fips.service "opted in, after upgrade"
+    check_active "$name" fips-dns.service "opted in, after upgrade"
+    check_active "$name" fips-firewall.service "opted in, after upgrade"
+    if cexec "$name" nft list counter inet fips fips_upgrade_probe >/dev/null 2>&1; then
+        pass "opted in: the upgraded ruleset is loaded"
+    else
+        fail "opted in: the upgraded ruleset is not loaded (no fips_upgrade_probe counter)"
+    fi
+    if cexec "$name" grep -Eq '^delete table inet fips( |$)' /root/nft-monitor.log; then
+        fail "opted in: the fips table was deleted during the upgrade"
+        cexec "$name" cat /root/nft-monitor.log 2>&1 | tail -10
+    else
+        pass "opted in: the fips table was never deleted during the upgrade"
+    fi
+
+    cleanup_container "$name"
+    return 0
+}
+
 # Host that never opted in to the firewall or enabled the gateway: the upgrade
 # must leave both as they were. Then the package is reinstalled twice: with the
 # daemon masked, when apt must succeed and start nothing, and with a daemon
@@ -770,6 +854,12 @@ _upgrade_not_opted_in() {
     fi
     check_active "$name" fips.service "not opted in, after upgrade"
     check_active "$name" fips-dns.service "not opted in, after upgrade"
+    check_left_off "$name" fips-firewall.service "not opted in, after upgrade"
+    if cexec "$name" nft list table inet fips >/dev/null 2>&1; then
+        fail "not opted in, after upgrade: the fips firewall table is loaded"
+    else
+        pass "not opted in, after upgrade: no fips firewall table"
+    fi
 
     # A host that masked the daemon on purpose: the upgrade must skip it with a
     # message, not fail. The package before this change printed nothing for a
@@ -849,6 +939,7 @@ DOCKERFILE
 
     # Named under the install scenario's prefix, so a CI step that collects
     # that scenario's container logs on failure collects these too.
+    _upgrade_opted_in "fips-deb-test-${distro_label}-upg-a${FIPS_CI_NAME_SUFFIX:-}" "$image" "$deb"
     _upgrade_not_opted_in "fips-deb-test-${distro_label}-upg-b${FIPS_CI_NAME_SUFFIX:-}" "$image" "$deb"
     docker rmi "$image" >/dev/null 2>&1 || true
     return 0