diff --git a/scripts/setup.sh b/scripts/setup.sh index 07d3453d2..886b924ea 100755 --- a/scripts/setup.sh +++ b/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 " >&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" diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 6009dbc4d..e7bafaf08 100644 --- a/spinifex/daemon/daemon.go +++ b/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 diff --git a/spinifex/network/host/ipsec_enable.go b/spinifex/network/host/ipsec_enable.go index 2e2c525a2..ad6cbce7f 100644 --- a/spinifex/network/host/ipsec_enable.go +++ b/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 diff --git a/spinifex/network/host/ipsec_enable_test.go b/spinifex/network/host/ipsec_enable_test.go index 80bb850d7..00be84469 100644 --- a/spinifex/network/host/ipsec_enable_test.go +++ b/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)) +}