fix(ipsec): stop charon on nodes that do not use OVN IPsec (#768)

The daemon only called EnableOVNIPSec when network.ipsec_enabled was true, so
the disabled case was never reached and nothing ever turned the host's IPsec
services off. Since setup.sh installs openvswitch-ipsec and strongswan-charon
unconditionally and the packages enable their units, charon listened on
0.0.0.0:500 and 0.0.0.0:4500 on every node -- including single-node clusters,
where EnableOVNIPSec short-circuits and IPsec is never switched on at all.

ReconcileOVNIPSec runs on every startup and resolves the state both ways. It
self-heals: a single-node box that later joins an IPsec-on cluster gets the
services back through the same call, which a static install-time mask could not
do because setup.sh runs before the topology is known.

The unit changes go through a new fixed-verb root helper rather than systemctl.
spinifex-daemon holds three narrow sudo grants and none of them is systemctl,
which is deliberate: a NOPASSWD systemctl rule takes an arbitrary unit name and
is root-equivalent. The helper takes only on|off and names the units itself,
matching the existing spinifex-set-endpoint-sysctl pattern. Only the read-only
is-enabled and is-active probes are run directly, and a test pins that.

strongswan-starter.service is disabled in both states. ovs-monitor-ipsec execs
the strongSwan starter itself rather than going through that unit, so it only
ever competes for UDP 500/4500 -- confirmed on a live IPsec-enabled cluster,
where charon's parent is /usr/lib/ipsec/starter and the unit is inactive.

A nil cluster config leaves the services alone rather than guessing.

This is the bind-address half only. On a multi-node cluster charon must run,
and it still binds the wildcard; restricting that reach to the encap plane is
the host firewall's job.

scripts/setup.sh

@@ -24,8 +24,9 @@ set -e
 INSTALL_SPINIFEX_CHANNEL="${INSTALL_SPINIFEX_CHANNEL:-latest}"
 INSTALL_BASE_URL="${INSTALL_BASE_URL:-https://install.mulgadc.com}"
 
-# Referenced by both the sudoers grant and the daemon, so the path is fixed here.
+# Referenced by both the sudoers grant and the daemon, so the paths are fixed here.
 ENDPOINT_SYSCTL_HELPER="/usr/local/lib/spinifex/spinifex-set-endpoint-sysctl"
+IPSEC_STATE_HELPER="/usr/local/lib/spinifex/spinifex-set-ipsec-state"
 
 # --- Colors ---
 RED='\033[0;31m'
@@ -234,9 +235,54 @@ HELPER
     $SUDO chmod 0755 "${ENDPOINT_SYSCTL_HELPER}"
 }
 
+# The daemon turns OVN IPsec on and off as the cluster topology resolves, so it
+# needs unit control. A NOPASSWD systemctl grant takes an arbitrary unit name and
+# is root-equivalent, so the units are fixed here and only the state is an input.
+install_ipsec_state_helper() {
+    $SUDO install -d -m 0755 /usr/local/lib/spinifex
+    $SUDO tee "${IPSEC_STATE_HELPER}" > /dev/null << 'HELPER'
+#!/bin/sh
+# Turns the host's OVN IPsec services on or off. Runs as root under a NOPASSWD
+# grant, so it must never act on a unit the caller names.
+set -eu
+
+if [ "$#" -ne 1 ]; then
+    echo "usage: $0 <on|off>" >&2
+    exit 2
+fi
+
+# mask/unmask rather than disable/enable. The daemon's unit sets
+# ProtectSystem=full, so /etc is read-only in the namespace its children inherit,
+# and disabling a unit that also ships a SysV script shells out to update-rc.d
+# there and fails. Masking is done by PID 1 over D-Bus and is unaffected.
+
+# ovs-monitor-ipsec execs the strongSwan starter itself rather than going through
+# this unit, so leaving it enabled only contends for UDP 500/4500. Off in both
+# states. Absent on a host without strongswan, which is not an error.
+systemctl mask --now strongswan-starter.service >/dev/null 2>&1 || true
+
+case "$1" in
+    on)
+        systemctl unmask openvswitch-ipsec.service
+        exec systemctl start openvswitch-ipsec.service
+        ;;
+    off)
+        exec systemctl mask --now openvswitch-ipsec.service
+        ;;
+    *)
+        echo "state not permitted: $1" >&2
+        exit 2
+        ;;
+esac
+HELPER
+    $SUDO chown root:root "${IPSEC_STATE_HELPER}"
+    $SUDO chmod 0755 "${IPSEC_STATE_HELPER}"
+}
+
 install_sudoers() {
     stage "installing scoped sudoers rules"
     install_endpoint_sysctl_helper
+    install_ipsec_state_helper
 
     if sudo_is_sudo_rs; then
         $SUDO tee /etc/sudoers.d/spinifex-network > /dev/null << 'SUDOERS'
@@ -282,6 +328,7 @@ SUDOERS
     # backticks that an expanding one would run as command substitution.
     $SUDO tee -a /etc/sudoers.d/spinifex-network > /dev/null << SUDOERS
 spinifex-daemon ALL=(root) NOPASSWD: ${ENDPOINT_SYSCTL_HELPER}
+spinifex-daemon ALL=(root) NOPASSWD: ${IPSEC_STATE_HELPER}
 SUDOERS
     $SUDO chmod 0440 /etc/sudoers.d/spinifex-network
     $SUDO visudo -cf /etc/sudoers.d/spinifex-network || fatal "Invalid sudoers syntax in spinifex-network"

spinifex/daemon/daemon.go

@@ -1393,11 +1393,10 @@ func (d *Daemon) startCluster() error {
 		}
 	}
 
-	// Enable OVN native IPsec when configured (idempotent).
-	if d.clusterConfig != nil && d.clusterConfig.Network.IPSecEnabled {
-		if err := host.EnableOVNIPSec(d.configPath, d.clusterConfig); err != nil {
-			slog.Warn("Failed to enable OVN native IPsec; intra-AZ Geneve will be plaintext", "err", err)
-		}
+	// Reconcile OVN native IPsec both ways (idempotent): a node that does not
+	// use it should not be left running charon on 500/4500 either.
+	if err := host.ReconcileOVNIPSec(d.configPath, d.clusterConfig); err != nil {
+		slog.Warn("Failed to reconcile OVN native IPsec", "err", err)
 	}
 
 	// Write service manifest so other nodes know what this node runs

spinifex/network/host/ipsec_enable.go

@@ -19,6 +19,83 @@ var systemctlActiveTimeout = 5 * time.Second
 // ovnNBSocketPath gates the NB_Global ipsec write to the management node.
 var ovnNBSocketPath = "/run/ovn/ovnnb_db.sock"
 
+const (
+	// ovsIPSecUnit owns charon: ovs-monitor-ipsec execs the strongSwan starter
+	// itself rather than going through the starter's own unit.
+	ovsIPSecUnit = "openvswitch-ipsec.service"
+
+	// strongswanStarterUnit would start a second charon competing for UDP
+	// 500/4500. Nothing here needs it, so it stays off in both states.
+	strongswanStarterUnit = "strongswan-starter.service"
+)
+
+// ipsecStateHelper is the fixed-verb root helper installed by setup.sh. The
+// daemon holds no systemctl grant, and a NOPASSWD rule for one would be
+// root-equivalent, so unit changes go through this instead.
+var ipsecStateHelper = "/usr/local/lib/spinifex/spinifex-set-ipsec-state"
+
+// ReconcileOVNIPSec brings the host's IPsec services in line with the cluster
+// config, then enables OVN IPsec if it is wanted. Runs on every startup so the
+// disabled case is reached too, which EnableOVNIPSec alone never is.
+func ReconcileOVNIPSec(configPath string, clusterConfig *config.ClusterConfig) error {
+	// A nil config means the intent is unknown; leave the host's services alone
+	// rather than guessing and tearing down working tunnels.
+	if clusterConfig == nil {
+		return nil
+	}
+
+	want := clusterConfig.Network.IPSecEnabled && len(clusterConfig.Nodes) > 1
+
+	if err := ensureIPSecServices(want); err != nil {
+		return err
+	}
+	if !want {
+		slog.Info("ipsec: not in use, IKE and NAT-T listeners stopped")
+		return nil
+	}
+	return EnableOVNIPSec(configPath, clusterConfig)
+}
+
+// ensureIPSecServices runs the helper only when the host does not already match
+// the wanted state, so a steady-state startup executes nothing.
+func ensureIPSecServices(want bool) error {
+	if ipsecServicesMatch(want) {
+		return nil
+	}
+
+	state := "off"
+	if want {
+		state = "on"
+	}
+	out, err := utils.SudoCommand(ipsecStateHelper, state).CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("set ipsec state %s: %w: %s", state, err, strings.TrimSpace(string(out)))
+	}
+	slog.Info("ipsec: host services reconciled", "state", state)
+	return nil
+}
+
+func ipsecServicesMatch(want bool) bool {
+	if unitIsEnabled(strongswanStarterUnit) || unitIsActive(strongswanStarterUnit) {
+		return false
+	}
+	return unitIsEnabled(ovsIPSecUnit) == want && unitIsActive(ovsIPSecUnit) == want
+}
+
+// unitIsEnabled treats every state other than enabled as not enabled, which
+// covers masked, static and a unit the distro does not ship at all. The helper
+// masks rather than disables, so the off state reads back as "masked".
+func unitIsEnabled(unit string) bool {
+	out, _ := utils.SudoCommand("systemctl", "is-enabled", unit).CombinedOutput()
+	state := strings.TrimSpace(string(out))
+	return state == "enabled" || state == "enabled-runtime" || state == "alias"
+}
+
+func unitIsActive(unit string) bool {
+	out, _ := utils.SudoCommand("systemctl", "is-active", unit).CombinedOutput()
+	return strings.TrimSpace(string(out)) == "active"
+}
+
 // EnableOVNIPSec wires the local IPsec peer cert and flips ipsec_encapsulation=true.
 // Idempotent. Single-node clusters short-circuit (no Geneve tunnels to encrypt).
 // Lives in L0 per ADR-0006 S8 (IPSec is OVN-native only; SA lifecycle invisible above L0).
@@ -75,7 +152,7 @@ func ensureOVSMonitorIPSecActive() error {
 	deadline := time.Now().Add(systemctlActiveTimeout)
 	var lastOut string
 	for time.Now().Before(deadline) {
-		out, _ := utils.SudoCommand("systemctl", "is-active", "openvswitch-ipsec.service").CombinedOutput()
+		out, _ := utils.SudoCommand("systemctl", "is-active", ovsIPSecUnit).CombinedOutput()
 		lastOut = strings.TrimSpace(string(out))
 		if lastOut == "active" {
 			return nil

spinifex/network/host/ipsec_enable_test.go

@@ -25,22 +25,55 @@ func multiNodeClusterConfig() *config.ClusterConfig {
 }
 
 type recordingSudo struct {
-	runs         [][]string
-	activeOutput string
+	runs          [][]string
+	activeOutput  string
+	enabledOutput map[string]string
+	activePerUnit map[string]string
 }
 
 func (r *recordingSudo) stub(name string, args ...string) *exec.Cmd {
 	r.runs = append(r.runs, append([]string{name}, args...))
-	if name == "systemctl" && len(args) >= 1 && args[0] == "is-active" {
+	if name != "systemctl" || len(args) < 2 {
+		return exec.Command("true")
+	}
+	unit := args[len(args)-1]
+	switch args[0] {
+	case "is-active":
+		if out, ok := r.activePerUnit[unit]; ok {
+			return exec.Command("printf", "%s", out)
+		}
 		out := r.activeOutput
 		if out == "" {
 			out = "active\n"
 		}
 		return exec.Command("printf", "%s", out)
+	case "is-enabled":
+		if out, ok := r.enabledOutput[unit]; ok {
+			return exec.Command("printf", "%s", out)
+		}
+		return exec.Command("printf", "%s", "enabled\n")
 	}
 	return exec.Command("true")
 }
 
+// helperRuns returns the IPsec state-helper invocations, dropping the
+// is-enabled/is-active probes that precede them.
+func (r *recordingSudo) helperRuns() [][]string {
+	var out [][]string
+	for _, run := range r.runs {
+		if run[0] == ipsecStateHelper {
+			out = append(out, run)
+		}
+	}
+	return out
+}
+
+func multiNodeIPSecConfig(enabled bool) *config.ClusterConfig {
+	cfg := multiNodeClusterConfig()
+	cfg.Network.IPSecEnabled = enabled
+	return cfg
+}
+
 func TestEnableOVNIPSec(t *testing.T) {
 	recorder := &recordingSudo{}
 	t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
@@ -167,3 +200,126 @@ func TestEnableOVNIPSec_NoConfigPath(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "config path unset")
 }
+
+func TestReconcileOVNIPSec_DisabledStopsCharon(t *testing.T) {
+	recorder := &recordingSudo{}
+	t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
+
+	require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false)))
+
+	assert.Equal(t, [][]string{{ipsecStateHelper, "off"}}, recorder.helperRuns())
+
+	for _, run := range recorder.runs {
+		assert.NotEqual(t, "ovs-vsctl", run[0], "must not touch OVS when IPsec is off: %v", run)
+	}
+}
+
+// The daemon has no systemctl grant, so a unit change must never be attempted
+// directly: it would fail at the polkit prompt and leave charon listening.
+func TestReconcileOVNIPSec_NeverCallsSystemctlDirectly(t *testing.T) {
+	recorder := &recordingSudo{}
+	t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
+
+	require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false)))
+
+	for _, run := range recorder.runs {
+		if run[0] != "systemctl" {
+			continue
+		}
+		assert.Contains(t, []string{"is-active", "is-enabled"}, run[1],
+			"only read-only systemctl verbs may be run directly: %v", run)
+	}
+}
+
+// A single-node cluster has no tunnels to protect, so charon must not be left
+// listening even though ipsec_enabled defaults to true.
+func TestReconcileOVNIPSec_SingleNodeStopsCharon(t *testing.T) {
+	recorder := &recordingSudo{}
+	t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
+
+	cfg := &config.ClusterConfig{Node: "node1", Nodes: map[string]config.Config{"node1": {}}}
+	cfg.Network.IPSecEnabled = true
+
+	require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", cfg))
+
+	assert.Equal(t, [][]string{{ipsecStateHelper, "off"}}, recorder.helperRuns())
+}
+
+func TestReconcileOVNIPSec_EnabledTurnsTheServicesOn(t *testing.T) {
+	recorder := &recordingSudo{
+		enabledOutput: map[string]string{
+			ovsIPSecUnit:          "disabled\n",
+			strongswanStarterUnit: "disabled\n",
+		},
+		activePerUnit: map[string]string{
+			ovsIPSecUnit:          "active\n",
+			strongswanStarterUnit: "inactive\n",
+		},
+	}
+	t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
+
+	configDir := t.TempDir()
+	configPath := filepath.Join(configDir, "spinifex.toml")
+	require.NoError(t, os.WriteFile(configPath, []byte("placeholder"), 0600))
+	for _, rel := range []string{"ca.pem", "ipsec/peer.pem", "ipsec/peer.key"} {
+		full := filepath.Join(configDir, rel)
+		require.NoError(t, os.MkdirAll(filepath.Dir(full), 0750))
+		require.NoError(t, os.WriteFile(full, []byte("x"), 0600))
+	}
+
+	origNBSock := ovnNBSocketPath
+	ovnNBSocketPath = filepath.Join(configDir, "no-such-socket")
+	t.Cleanup(func() { ovnNBSocketPath = origNBSock })
+
+	require.NoError(t, ReconcileOVNIPSec(configPath, multiNodeIPSecConfig(true)))
+
+	assert.Equal(t, [][]string{{ipsecStateHelper, "on"}}, recorder.helperRuns())
+
+	// The enable path must still reach the OVS writes.
+	var sawEncapsulation bool
+	for _, run := range recorder.runs {
+		if run[0] == "ovs-vsctl" && strings.Contains(strings.Join(run, " "), "ipsec_encapsulation=true") {
+			sawEncapsulation = true
+		}
+	}
+	assert.True(t, sawEncapsulation, "ipsec_encapsulation was never set: %v", recorder.runs)
+}
+
+func TestReconcileOVNIPSec_AlreadyInStateIsNoOp(t *testing.T) {
+	recorder := &recordingSudo{
+		enabledOutput: map[string]string{strongswanStarterUnit: "disabled\n", ovsIPSecUnit: "disabled\n"},
+		activePerUnit: map[string]string{strongswanStarterUnit: "inactive\n", ovsIPSecUnit: "inactive\n"},
+	}
+	t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
+
+	require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false)))
+	assert.Empty(t, recorder.helperRuns())
+}
+
+func TestReconcileOVNIPSec_UnknownUnitIsNotAnError(t *testing.T) {
+	recorder := &recordingSudo{
+		enabledOutput: map[string]string{
+			strongswanStarterUnit: "Failed to get unit file state for strongswan-starter.service: No such file or directory\n",
+			ovsIPSecUnit:          "Failed to get unit file state for openvswitch-ipsec.service: No such file or directory\n",
+		},
+		activePerUnit: map[string]string{
+			strongswanStarterUnit: "inactive\n",
+			ovsIPSecUnit:          "inactive\n",
+		},
+	}
+	t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
+
+	require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false)))
+	assert.Empty(t, recorder.helperRuns())
+}
+
+// A nil cluster config means the intent is unknown; tearing down working
+// tunnels on a guess would be worse than leaving them.
+func TestReconcileOVNIPSec_NilConfigLeavesUnitsAlone(t *testing.T) {
+	t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd {
+		t.Fatalf("utils.SudoCommand must not run with a nil cluster config; got %s %v", name, args)
+		return exec.Command("true")
+	}))
+
+	require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", nil))
+}