diff --git a/build/systemd/spinifex-daemon.service b/build/systemd/spinifex-daemon.service index 3e0715024..a61d9329b 100644 --- a/build/systemd/spinifex-daemon.service +++ b/build/systemd/spinifex-daemon.service @@ -42,9 +42,9 @@ DeviceAllow=/dev/net/tun rw DeviceAllow=char-vfio rw DeviceAllow=/dev/vfio/vfio rw ReadOnlyPaths=/etc/spinifex/spinifex.toml /etc/spinifex/server.pem /etc/spinifex/server.key /etc/spinifex/ca.pem -# /proc/sys/net/ipv4/conf: per-tap IMDS endpoints set rp_filter/accept_local, which -# ProtectKernelTunables=yes would otherwise keep read-only. -ReadWritePaths=/var/lib/spinifex/spinifex /var/log/spinifex /run/spinifex /run/spinifex/nbd /proc/sys/net/ipv4/conf +# /proc/sys/net/ipv4/conf: ProtectKernelTunables would keep the per-tap IMDS +# rp_filter/accept_local read-only; /etc/spinifex/firewall: the root helper writes peers.nft. +ReadWritePaths=/var/lib/spinifex/spinifex /var/log/spinifex /run/spinifex /run/spinifex/nbd /proc/sys/net/ipv4/conf -/etc/spinifex/firewall # RG-9: hardening baseline ProtectHome=yes diff --git a/build/systemd/spinifex-firewall.service b/build/systemd/spinifex-firewall.service new file mode 100644 index 000000000..9d1a0be6a --- /dev/null +++ b/build/systemd/spinifex-firewall.service @@ -0,0 +1,24 @@ +[Unit] +Description=Spinifex host firewall (nftables input policy) +Documentation=file:/etc/spinifex/firewall/spinifex.nft +Wants=network-pre.target +Before=network-pre.target shutdown.target +DefaultDependencies=no +ConditionPathExists=/etc/spinifex/firewall/peers.nft + +[Service] +Type=oneshot +RemainAfterExit=yes +StandardInput=null +ProtectSystem=full +ProtectHome=true +ExecStart=/usr/local/lib/spinifex/spinifex-firewall-apply +ExecReload=/usr/local/lib/spinifex/spinifex-firewall-apply +TimeoutStopSec=10s + +# No ExecStop: flushing on stop would take vpcd's MASQUERADE and per-EIP FORWARD +# rules with it. Remove the policy deliberately with +# `nft delete table inet spinifex_filter`. + +[Install] +WantedBy=sysinit.target diff --git a/scripts/install-node.sh b/scripts/install-node.sh index c51f22366..42f56c3c0 100755 --- a/scripts/install-node.sh +++ b/scripts/install-node.sh @@ -101,8 +101,8 @@ while [[ $# -gt 0 ]]; do done <"$2" shift 2 ;; - --smoke) RUN_SMOKE=true; shift ;; - --wipe) WIPE=true; shift ;; + --smoke) RUN_SMOKE=true; shift ;; + --wipe) WIPE=true; shift ;; --yes|-y) ASSUME_YES=true; shift ;; --dry-run) DRY_RUN=true; shift ;; -h|--help) sed -n '2,49p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//'; exit 0 ;; @@ -596,6 +596,17 @@ for name in "${NODE_NAMES[@]}"; do Check 'sudo ovs-vsctl get Open_vSwitch . external_ids:system-id' on that host." done +# --- Host firewall --------------------------------------------------------- +# +# Not applied here. spinifex-daemon reconciles the peer sets from cluster +# membership on every startup, so it also picks up nodes added or replaced after +# formation, which a one-shot write from this script would not. + +echo "" +log "host firewall: reconciled by spinifex-daemon from cluster membership." +log " Check with 'sudo nft list table inet spinifex_filter' on a node," +log " or disable it cluster-wide with network.firewall_enabled = false." + # The pool is the one thing the operator supplied by hand and the one thing # nothing else validates, so print what actually landed. dns_servers in # particular is detected rather than passed, and is wrong often enough to be diff --git a/scripts/setup.sh b/scripts/setup.sh index 886b924ea..ceeab2f9c 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -14,9 +14,9 @@ # skip systemctl daemon-reload + enable, short-circuit setup_sudo. # VERBOSE Set to 1 to echo "[setup] " before each top-level step. # SETUP_STAGES Comma-separated subset of stages to run: -# deps, aws, users, sudoers, files, directories, -# env, systemd, logrotate, udev, fixown, -# migrations +# deps, aws, users, sudoers, firewall, files, +# directories, env, systemd, logrotate, udev, +# fixown, migrations # Unset = run every stage appropriate for the current mode. set -e @@ -28,6 +28,14 @@ INSTALL_BASE_URL="${INSTALL_BASE_URL:-https://install.mulgadc.com}" ENDPOINT_SYSCTL_HELPER="/usr/local/lib/spinifex/spinifex-set-endpoint-sysctl" IPSEC_STATE_HELPER="/usr/local/lib/spinifex/spinifex-set-ipsec-state" +# The firewall policy and the peer addresses it scopes cluster ports to. The +# policy ships with the node; the peers are written by install-node.sh, which is +# the only component that knows every node's resolved planes. +FIREWALL_DIR="/etc/spinifex/firewall" +FIREWALL_RULES="${FIREWALL_DIR}/spinifex.nft" +FIREWALL_PEERS="${FIREWALL_DIR}/peers.nft" +FIREWALL_APPLY="/usr/local/lib/spinifex/spinifex-firewall-apply" + # --- Colors --- RED='\033[0;31m' GREEN='\033[0;32m' @@ -279,6 +287,175 @@ HELPER $SUDO chmod 0755 "${IPSEC_STATE_HELPER}" } +# The policy is written here rather than shipped in the tarball so the ISO chroot +# and the ansible path get an identical file without a packaging step. Installed +# but NOT enabled: the peer set is empty until install-node.sh resolves the +# cluster's planes, and a drop policy with no peers would break formation. +install_firewall() { + stage "installing host firewall policy" + info "Installing host firewall policy..." + + $SUDO install -d -m 0755 "$FIREWALL_DIR" /usr/local/lib/spinifex + $SUDO tee "$FIREWALL_RULES" > /dev/null << 'RULES' +#!/usr/sbin/nft -f +# Spinifex host firewall. Managed by scripts/setup.sh — edits are overwritten. +# +# Only `table inet spinifex_filter` is created, deleted and replaced. vpcd writes +# MASQUERADE and per-EIP FORWARD ACCEPT rules into the `ip nat` and `ip filter` +# tables and reinstalls them only when the service starts, so anything that +# flushes the whole ruleset breaks elastic IPs silently until the next restart. +# +# INPUT only. nftables runs every table registered on a hook and any drop is +# final, so a forward-hook policy here could not be rescued by vpcd's ACCEPTs in +# the other table. OUTPUT is untouched: the IMDS reply path egresses under +# per-tap policy routing. + +include "/etc/spinifex/firewall/peers.nft" + +# Create-then-delete so the replace is atomic and the delete cannot fail on a +# first run. nft applies the whole file as one transaction or none of it. +table inet spinifex_filter +delete table inet spinifex_filter + +table inet spinifex_filter { + chain input { + type filter hook input priority filter; policy drop; + + # First, or every reply to a connection this node opened is dropped. + ct state established,related accept + ct state invalid drop + iif lo accept + + # Guest metadata and VPC DNS terminate on per-ENI OVS internal ports in + # the host netns, so guest traffic to 169.254.169.254 and .253 arrives + # here. Without this, cloud-init, instance-role credentials and all + # guest DNS break. The ports exist only while instances run, so this + # matches the prefix rather than an enumerated list. + iifname "ime-*" accept + + # PMTUD is not optional under a Geneve overlay. + icmp type { echo-request, destination-unreachable, time-exceeded, parameter-problem } accept + icmpv6 type { echo-request, destination-unreachable, packet-too-big, time-exceeded, parameter-problem, nd-neighbor-solicit, nd-neighbor-advert, nd-router-solicit, nd-router-advert } accept + + # A broadcast DHCP reply does not reliably match an established + # conntrack entry, so the uplink and external-pool leases need this. + udp sport 67 udp dport 68 accept + + # Public plane. 9999 is also how every guest agent (lb, eks, ecs, rds) + # reaches its control plane over br-mgmt: they speak SigV4 to the AWS + # gateway, never NATS, so no cluster port needs to face the guests. + tcp dport { 22, 3000, 8443, 9999 } accept + tcp dport 53 accept + udp dport 53 accept + + # Cluster plane, peer-scoped. On a single-NIC node the planes collapse + # onto the public address, which is why these are peer addresses rather + # than an interface or a CIDR. + ip saddr $spinifex_peers tcp dport { 4222, 4248, 4432, 5300, 6641, 6642, 6643, 6644 } accept + ip saddr $spinifex_peers udp dport { 5300, 6660, 7660 } accept + + # Encap plane, peer-scoped: Geneve, IKE, NAT-T and ESP. Geneve is a + # kernel UDP-tunnel socket, so encapsulated packets are delivered + # locally and pass this hook before the OVS vport sees them. + ip saddr $spinifex_encap_peers udp dport { 6081, 500, 4500 } accept + ip saddr $spinifex_encap_peers meta l4proto esp accept + + # Rate-limited so a scan cannot fill the journal. This is the only way + # to tell a policy gap from an application fault after the fact. + limit rate 5/minute burst 10 packets log prefix "spinifex-fw drop: " level info + } +} +RULES + $SUDO chmod 0644 "$FIREWALL_RULES" + info " $FIREWALL_RULES" + + $SUDO tee "$FIREWALL_APPLY" > /dev/null << 'APPLY' +#!/bin/sh +# Applies the spinifex host firewall. Fails without changing the ruleset when +# the peer file is missing, so a node whose daemon has not yet resolved cluster +# membership is left reachable rather than cut off from a cluster it cannot name. +set -eu + +RULES="/etc/spinifex/firewall/spinifex.nft" +PEERS="/etc/spinifex/firewall/peers.nft" + +[ -r "$RULES" ] || { echo "no firewall policy at $RULES" >&2; exit 1; } + +# set-peers reads two comma-separated address lists on stdin (cluster peers, +# then encap peers) and rewrites the peer file before applying. Every address is +# re-validated here as a bare dotted quad: this runs as root under a NOPASSWD +# grant, so a caller must not be able to inject nft syntax through it. +# disable removes spinifex's own table and the peer file, leaving every other +# table alone. Idempotent, so it is safe on a node that never had the policy. +if [ "${1:-}" = "disable" ]; then + nft delete table inet spinifex_filter 2>/dev/null || true + rm -f "$PEERS" + systemctl disable --now spinifex-firewall.service >/dev/null 2>&1 || true + exit 0 +fi + +if [ "${1:-}" = "set-peers" ]; then + read -r peers_in || peers_in="" + read -r encap_in || encap_in="" + + # Octet ranges are checked, not just the shape: nft rejects 10.0.0.999 and + # the whole transaction with it, which would leave a peer file that fails on + # every boot. A loop, not a pipeline — a rejection in a `while read` subshell + # cannot fail the function. + OCTET='(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9]?[0-9])' + validate() { + _out="" + for a in $(echo "$1" | tr ',' ' '); do + echo "$a" | grep -Eq "^${OCTET}(\.${OCTET}){3}$" || { + echo "address not permitted: $a" >&2 + return 2 + } + if [ -z "$_out" ]; then _out="$a"; else _out="$_out, $a"; fi + done + printf '%s' "$_out" + } + + peers=$(validate "$peers_in") || exit 2 + encap=$(validate "$encap_in") || exit 2 + [ -n "$peers" ] && [ -n "$encap" ] || { + echo "set-peers needs a non-empty peer and encap list" >&2 + exit 2 + } + + tmp=$(mktemp) + bak=$(mktemp) + printf '%s\n' \ + '# Managed by spinifex-daemon. Regenerated from cluster membership.' \ + "define spinifex_peers = { $peers }" \ + "define spinifex_encap_peers = { $encap }" > "$tmp" + if [ -r "$PEERS" ]; then cp "$PEERS" "$bak"; fi + install -m 0644 "$tmp" "$PEERS" + + # Roll the peer file back if the ruleset will not load, so the boot-time + # unit never inherits a file already known to fail. + if ! nft -f "$RULES"; then + if [ -s "$bak" ]; then install -m 0644 "$bak" "$PEERS"; else rm -f "$PEERS"; fi + rm -f "$tmp" "$bak" + exit 1 + fi + rm -f "$tmp" "$bak" + exit 0 +fi + +if [ ! -r "$PEERS" ]; then + echo "no peer file at $PEERS: the daemon writes it once cluster membership" >&2 + echo "resolves. Ruleset left unchanged." >&2 + exit 1 +fi + +exec nft -f "$RULES" +APPLY + $SUDO chown root:root "$FIREWALL_APPLY" + $SUDO chmod 0755 "$FIREWALL_APPLY" + info " $FIREWALL_APPLY" + info "Host firewall installed (applied by spinifex-daemon once peers resolve)" +} + install_sudoers() { stage "installing scoped sudoers rules" install_endpoint_sysctl_helper @@ -329,6 +506,7 @@ SUDOERS $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} +spinifex-daemon ALL=(root) NOPASSWD: ${FIREWALL_APPLY} SUDOERS $SUDO chmod 0440 /etc/sudoers.d/spinifex-network $SUDO visudo -cf /etc/sudoers.d/spinifex-network || fatal "Invalid sudoers syntax in spinifex-network" @@ -350,7 +528,7 @@ libvirt-daemon-system libvirt-clients pciutils jq curl iproute2 netcat-openbsd wget unzip xz-utils file ovn-central ovn-host openvswitch-switch openvswitch-ipsec strongswan-charon dhcpcd-base -chrony" +chrony nftables" install_apt_deps() { stage "installing apt dependencies" @@ -810,6 +988,10 @@ EOF # takes effect once enabled) — without this the JetStream ENOSPC-latch # watchdog is inert and a full disk requires a manual restart forever. $SUDO systemctl enable --now spinifex-nats-watchdog.timer + # Enabled, not started: its ConditionPathExists holds it inert until the + # daemon writes a peer file, so this only decides that a node which has one + # gets the policy back at boot, before any service opens a socket. + $SUDO systemctl enable spinifex-firewall.service info "Systemd units installed and enabled (per-service users)" } @@ -977,6 +1159,7 @@ main() { stage_enabled aws && install_aws_cli stage_enabled users && create_service_users stage_enabled sudoers && install_sudoers + stage_enabled firewall && install_firewall stage_enabled files && install_files stage_enabled directories && create_directories stage_enabled env && install_systemd_env diff --git a/spinifex/config/config.go b/spinifex/config/config.go index 0031c2c2d..6f17ce97c 100644 --- a/spinifex/config/config.go +++ b/spinifex/config/config.go @@ -68,6 +68,9 @@ type NetworkConfig struct { ExternalPools []ExternalPool `mapstructure:"external_pools"` // One or more IP pools // IPSecEnabled toggles OVN native IPsec (AES-256-GCM) on every node. Default true; disable only for trusted lab topologies. IPSecEnabled bool `mapstructure:"ipsec_enabled"` + // FirewallEnabled toggles the host firewall that scopes cluster ports to cluster + // members. Default true; disable only where a site firewall already scopes them. + FirewallEnabled bool `mapstructure:"firewall_enabled"` // NATExemptCIDRs are extra destinations that skip routed-mode SNAT (added // to the transit /24 in the spinifex_nat_exempt set). nat mode only. NATExemptCIDRs []string `mapstructure:"nat_exempt_cidrs"` @@ -390,6 +393,9 @@ func LoadConfig(configPath string) (*ClusterConfig, error) { // Default ipsec_enabled to true; operators must explicitly set false to disable. viper.SetDefault("network.ipsec_enabled", true) + // Default firewall_enabled to true; operators must explicitly set false to disable. + viper.SetDefault("network.firewall_enabled", true) + // Cluster-wide AWS-parity defaults so existing deployments keep working. viper.SetDefault("aws.region", DefaultAWSRegion) viper.SetDefault("aws.internal_suffix", DefaultAWSInternalSuffix) diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 4ece5cc7f..ca36cea85 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -1402,6 +1402,13 @@ func (d *Daemon) startCluster() error { slog.Warn("Failed to reconcile OVN native IPsec", "err", err) } + // Keep the host firewall's peer sets in step with cluster membership. Every + // formation path reaches this, which none of the installers do. Runs in the + // background because the first attempt routinely loses the race with + // ovn-controller registering its chassis, and blocking startup on that would + // hold up every service below. + go host.MaintainFirewall(d.ctx, d.configPath, d.clusterConfig) + // Write service manifest so other nodes know what this node runs if d.jsManager != nil { if err := d.jsManager.WriteServiceManifest( diff --git a/spinifex/network/host/firewall.go b/spinifex/network/host/firewall.go new file mode 100644 index 000000000..fbee2781a --- /dev/null +++ b/spinifex/network/host/firewall.go @@ -0,0 +1,278 @@ +package host + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/netip" + "os" + "os/exec" + "path/filepath" + "regexp" + "sort" + "strings" + "time" + + "github.com/mulgadc/spinifex/spinifex/config" + "github.com/mulgadc/spinifex/spinifex/utils" +) + +// firewallApplyHelper is the fixed-verb root helper installed by setup.sh. The +// daemon's unit sets ProtectSystem=full, so it cannot write /etc itself, and a +// NOPASSWD grant for nft would be root-equivalent. +var firewallApplyHelper = "/usr/local/lib/spinifex/spinifex-firewall-apply" + +// firewallPeersPath is read (not written) here to skip a no-op reconcile. +var firewallPeersPath = "/etc/spinifex/firewall/peers.nft" + +// natsRoutePattern pulls the address out of a nats-route:// URL, whose userinfo +// carries the cluster token and must not be mistaken for the host. +var natsRoutePattern = regexp.MustCompile(`nats-route://(?:[^@/]*@)?([0-9.]+):\d+`) + +// ovnEncapCommand is a var so tests can stand in for the Southbound query. +// Unprivileged: the remote is TCP, so this needs no local socket and no sudo +// grant. setup.sh deliberately grants ovn-sbctl to nobody, because its +// arguments are unrestricted enough to be root-equivalent. +var ovnEncapCommand = func(remote string) *exec.Cmd { + return exec.Command("ovn-sbctl", "--db="+remote, "--timeout=10", + "--bare", "--columns=ip", "list", "Encap") +} + +// firewallRetryDelay is the first retry gap after a failed reconcile; it doubles +// up to firewallReconcileInterval. On a fresh bootstrap the daemon routinely +// starts before OVN's Southbound DB accepts connections, so the first attempt +// failing is expected rather than exceptional. +var firewallRetryDelay = 15 * time.Second + +// firewallReconcileInterval re-runs the reconcile so a node added or replaced +// after bootstrap is picked up without a daemon restart. Cheap: an unchanged +// peer set short-circuits before the helper is invoked. +var firewallReconcileInterval = 5 * time.Minute + +// MaintainFirewall schedules ReconcileFirewall until it succeeds, then re-runs +// it at firewallReconcileInterval, for the lifetime of ctx. A scheduler only — +// all the work stays in the one entrypoint. +// +// Both halves matter. A single attempt at startup loses the race with +// ovn-controller registering its chassis, which leaves the node with no policy +// until something happens to restart the daemon; and without the periodic +// re-run a membership change is never picked up at all. +func MaintainFirewall(ctx context.Context, configPath string, clusterConfig *config.ClusterConfig) { + delay := firewallRetryDelay + for { + if err := ReconcileFirewall(configPath, clusterConfig); err != nil { + slog.Warn("Failed to reconcile host firewall, will retry", "err", err, "retry_in", delay) + delay = min(delay*2, firewallReconcileInterval) + } else { + delay = firewallReconcileInterval + } + + select { + case <-ctx.Done(): + return + case <-time.After(delay): + } + } +} + +// ReconcileFirewall keeps the host firewall's peer sets in step with cluster +// membership. Runs on every startup, so a node added or replaced after +// bootstrap is picked up without re-running an installer. This is the only +// place that enables the policy: the four formation paths (install-node.sh, +// ansible, bm-bootstrap.sh, ISO/single-node) all reach it, and none of them +// would otherwise. +func ReconcileFirewall(configPath string, clusterConfig *config.ClusterConfig) error { + // A nil config means membership is unknown. Leaving the existing policy in + // place beats replacing it with a set that would lock this node out. + if clusterConfig == nil { + return nil + } + + // Off means off on a node that was previously on, not merely "skip": leaving + // a stale drop policy loaded after an operator disables it would be the kind + // of surprise the toggle exists to avoid. + if !clusterConfig.Network.FirewallEnabled { + return disableFirewall() + } + + // Encap addresses are not in the cluster config, and guessing them from the + // lan plane would drop Geneve on any node whose vpc bridge is genuinely + // separate. The Southbound Encap table is authoritative, so when it cannot + // be read the reconcile is skipped rather than narrowed onto a guess. + encap, err := ovnEncapAddrs(clusterConfig) + if err != nil { + return fmt.Errorf("read OVN encap addresses: %w", err) + } + + // Every chassis, or none. Chassis register as each node's ovn-controller + // starts, so a partial answer is the normal state early in a bootstrap, and + // writing it would drop Geneve from every node not yet in it. More than + // expected is fine — a decommissioned chassis lingering in the DB only makes + // the set wider. Waiting costs nothing: the node stays open until the set is + // known good, which is the same fail-open posture as the rest of this. + if expected := len(clusterConfig.Nodes); len(encap) < expected { + return fmt.Errorf("OVN reports %d of %d chassis encap addresses; not writing a partial peer set", + len(encap), expected) + } + + peers := clusterPeerAddrs(configPath, clusterConfig, encap) + if len(peers) == 0 { + return nil + } + + desired := renderPeersFile(peers, encap) + if current, err := os.ReadFile(firewallPeersPath); err == nil && string(current) == desired { + return nil + } + + cmd := utils.SudoCommand(firewallApplyHelper, "set-peers") + cmd.Stdin = strings.NewReader(strings.Join(peers, ",") + "\n" + strings.Join(encap, ",") + "\n") + if out, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("set firewall peers: %w: %s", err, strings.TrimSpace(string(out))) + } + + slog.Info("firewall: peer sets reconciled", "peers", len(peers), "encap_peers", len(encap)) + return nil +} + +// disableFirewall removes only spinifex's own table. A no-op when it was never +// loaded, so this is safe to run on every startup of a node that has the +// firewall switched off. +func disableFirewall() error { + if _, statErr := os.Stat(firewallPeersPath); errors.Is(statErr, os.ErrNotExist) { + return nil + } + out, err := utils.SudoCommand(firewallApplyHelper, "disable").CombinedOutput() + if err != nil { + return fmt.Errorf("disable firewall: %w: %s", err, strings.TrimSpace(string(out))) + } + slog.Info("firewall: disabled by config, policy removed") + return nil +} + +// clusterPeerAddrs is every address a cluster member might send from, across +// all three planes. No single source has them all: a node's own config names +// its peers only by their wan address, so the lan plane comes from the NATS +// cluster routes and the vpc plane from the encap list already read from OVN. +// A missing address here silently drops working cluster traffic, so this errs +// wide rather than narrow. +func clusterPeerAddrs(configPath string, clusterConfig *config.ClusterConfig, encap []string) []string { + seen := make(map[string]struct{}) + add := func(addr string) { + if addr = strings.TrimSpace(addr); isPlainIPv4(addr) { + seen[addr] = struct{}{} + } + } + + for _, node := range clusterConfig.Nodes { + add(node.Host) + add(node.AdvertiseIP) + // The OVN remotes name the database nodes on the lan plane, which is + // where 6641-6644 are dialled from. + for remote := range strings.SplitSeq(node.VPCD.OVNNBAddr+","+node.VPCD.OVNSBAddr, ",") { + add(hostFromRemote(remote)) + } + } + + for _, addr := range natsRouteAddrs(configPath) { + add(addr) + } + for _, addr := range encap { + add(addr) + } + + return sortedKeys(seen) +} + +// natsRouteAddrs reads the peer addresses out of the generated NATS config. +// Formation writes every member's cluster-plane address there, on every node, +// which is the only complete record of the lan plane a node holds locally. +// Absent on a single-node cluster, where there are no routes to list. +func natsRouteAddrs(configPath string) []string { + if configPath == "" { + return nil + } + data, err := os.ReadFile(filepath.Join(filepath.Dir(configPath), "nats", "nats.conf")) + if err != nil { + return nil + } + + var out []string + for _, match := range natsRoutePattern.FindAllStringSubmatch(string(data), -1) { + out = append(out, match[1]) + } + return out +} + +// hostFromRemote takes the address out of an OVSDB remote such as +// "tcp:10.9.7.2:6642", ignoring anything that is not host:port shaped. +func hostFromRemote(remote string) string { + parts := strings.Split(strings.TrimSpace(remote), ":") + if len(parts) != 3 { + return "" + } + return parts[1] +} + +// ovnEncapAddrs lists every chassis's tunnel endpoint from the Southbound DB. +// Queried over the configured remote rather than a local socket, because only +// the database nodes have one. +func ovnEncapAddrs(clusterConfig *config.ClusterConfig) ([]string, error) { + remote := ovnSBRemote(clusterConfig) + if remote == "" { + return nil, fmt.Errorf("no OVN Southbound address in config") + } + + out, err := ovnEncapCommand(remote).CombinedOutput() + if err != nil { + return nil, fmt.Errorf("%w: %s", err, strings.TrimSpace(string(out))) + } + + seen := make(map[string]struct{}) + for line := range strings.SplitSeq(string(out), "\n") { + addr := strings.Trim(strings.TrimSpace(line), `"`) + if isPlainIPv4(addr) { + seen[addr] = struct{}{} + } + } + return sortedKeys(seen), nil +} + +// ovnSBRemote takes the Southbound address off any node's vpcd section. Every +// node is given the same remote list, so the first one populated will do. +func ovnSBRemote(clusterConfig *config.ClusterConfig) string { + for _, node := range clusterConfig.Nodes { + if addr := strings.TrimSpace(node.VPCD.OVNSBAddr); addr != "" { + return addr + } + } + return "" +} + +func renderPeersFile(peers, encap []string) string { + return "# Managed by spinifex-daemon. Regenerated from cluster membership.\n" + + "define spinifex_peers = { " + strings.Join(peers, ", ") + " }\n" + + "define spinifex_encap_peers = { " + strings.Join(encap, ", ") + " }\n" +} + +// isPlainIPv4 rejects anything that is not a bare dotted quad, including the +// host:port and wildcard forms that appear elsewhere in the config. The helper +// validates again as root; this keeps obvious junk out of the request. +func isPlainIPv4(addr string) bool { + // netip.Is4 is false for the 4-in-6 form, which nft will not take in an + // ipv4_addr set. net.IP.To4 would accept it. Loopback is dropped because a + // single-node cluster names itself that way in the OVN remotes, and `iif lo + // accept` already covers it — in a source set it could never match. + ip, err := netip.ParseAddr(addr) + return err == nil && ip.Is4() && !ip.IsUnspecified() && !ip.IsLoopback() +} + +func sortedKeys(m map[string]struct{}) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} diff --git a/spinifex/network/host/firewall_test.go b/spinifex/network/host/firewall_test.go new file mode 100644 index 000000000..bd91331e4 --- /dev/null +++ b/spinifex/network/host/firewall_test.go @@ -0,0 +1,317 @@ +package host + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/mulgadc/spinifex/spinifex/config" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// firewallTestEnv points the package vars at a temp dir and stubs both the +// Southbound query and the root helper. stdinPath receives whatever the helper +// was fed, which is the payload the peer sets are built from. +type firewallTestEnv struct { + configPath string + peersPath string + stdinPath string + + mu sync.Mutex + runs [][]string +} + +func (e *firewallTestEnv) record(name string, args ...string) { + e.mu.Lock() + defer e.mu.Unlock() + e.runs = append(e.runs, append([]string{name}, args...)) +} + +// helperRuns copies under the lock: MaintainFirewall records from its own +// goroutine while the test reads. +func (e *firewallTestEnv) helperRuns() [][]string { + e.mu.Lock() + defer e.mu.Unlock() + return append([][]string(nil), e.runs...) +} + +func newFirewallTestEnv(t *testing.T, encap string) *firewallTestEnv { + t.Helper() + dir := t.TempDir() + env := &firewallTestEnv{ + configPath: filepath.Join(dir, "spinifex.toml"), + peersPath: filepath.Join(dir, "peers.nft"), + stdinPath: filepath.Join(dir, "stdin"), + } + + origPeers, origHelper, origEncap := firewallPeersPath, firewallApplyHelper, ovnEncapCommand + firewallPeersPath = env.peersPath + firewallApplyHelper = "/usr/local/lib/spinifex/spinifex-firewall-apply" + ovnEncapCommand = func(string) *exec.Cmd { return exec.Command("printf", "%s", encap) } + t.Cleanup(func() { + firewallPeersPath, firewallApplyHelper, ovnEncapCommand = origPeers, origHelper, origEncap + }) + + t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + env.record(name, args...) + if len(args) > 0 && args[0] == "set-peers" { + return exec.Command("sh", "-c", env.setPeersScript()) + } + return exec.Command("true") + })) + + return env +} + +// setPeersScript stands in for the root helper: it keeps the payload for +// assertions and renders the same peer file. Rendering matters — code that skips +// an unchanged set reads that file back, so a stub that only captured stdin +// would make every reconcile look like a change. +func (e *firewallTestEnv) setPeersScript() string { + return "cat > " + e.stdinPath + "\n" + + "{ printf '%s\\n' '# Managed by spinifex-daemon. Regenerated from cluster membership.'\n" + + " printf 'define spinifex_peers = { %s }\\n' \"$(sed -n 1p " + e.stdinPath + " | sed 's/,/, /g')\"\n" + + " printf 'define spinifex_encap_peers = { %s }\\n' \"$(sed -n 2p " + e.stdinPath + " | sed 's/,/, /g')\"\n" + + "} > " + e.peersPath + "\n" +} + +func (e *firewallTestEnv) helperStdin(t *testing.T) string { + t.Helper() + data, err := os.ReadFile(e.stdinPath) + require.NoError(t, err) + return string(data) +} + +func firewallClusterConfig(enabled bool) *config.ClusterConfig { + cfg := &config.ClusterConfig{ + Node: "node1", + Nodes: map[string]config.Config{ + "node1": {Host: "10.9.7.21", AdvertiseIP: "192.168.1.21"}, + "node2": {Host: "10.9.7.22", AdvertiseIP: "192.168.1.22"}, + }, + } + cfg.Network.FirewallEnabled = enabled + for name, node := range cfg.Nodes { + node.VPCD.OVNSBAddr = "tcp:10.9.7.21:6642" + cfg.Nodes[name] = node + } + return cfg +} + +func TestReconcileFirewall(t *testing.T) { + env := newFirewallTestEnv(t, "10.9.8.21\n10.9.8.22\n") + + require.NoError(t, ReconcileFirewall(env.configPath, firewallClusterConfig(true))) + + runs := env.helperRuns() + require.Len(t, runs, 1) + assert.Equal(t, []string{firewallApplyHelper, "set-peers"}, runs[0]) + + // Both planes in the peer set, encap kept separate and narrower. + lines := strings.Split(strings.TrimSpace(env.helperStdin(t)), "\n") + require.Len(t, lines, 2) + assert.Equal(t, "10.9.7.21,10.9.7.22,10.9.8.21,10.9.8.22,192.168.1.21,192.168.1.22", lines[0]) + assert.Equal(t, "10.9.8.21,10.9.8.22", lines[1]) +} + +func TestReconcileFirewall_NoOpWhenPeersUnchanged(t *testing.T) { + env := newFirewallTestEnv(t, "10.9.8.21\n10.9.8.22\n") + desired := renderPeersFile( + []string{"10.9.7.21", "10.9.7.22", "10.9.8.21", "10.9.8.22", "192.168.1.21", "192.168.1.22"}, + []string{"10.9.8.21", "10.9.8.22"}) + require.NoError(t, os.WriteFile(env.peersPath, []byte(desired), 0644)) + + require.NoError(t, ReconcileFirewall(env.configPath, firewallClusterConfig(true))) + assert.Empty(t, env.helperRuns(), "an unchanged peer set must not re-apply the policy") +} + +// A missing Southbound answer must not narrow the set onto a guess: a peer file +// without every chassis's encap address drops Geneve between the nodes it omits. +func TestReconcileFirewall_EmptyEncapDoesNotWrite(t *testing.T) { + env := newFirewallTestEnv(t, "") + + err := ReconcileFirewall(env.configPath, firewallClusterConfig(true)) + require.Error(t, err) + assert.Contains(t, err.Error(), "0 of 2 chassis") + assert.Empty(t, env.helperRuns()) +} + +// The case observed on a fresh 5-node bootstrap: one node's ovn-controller had +// not registered yet. Writing the partial answer would drop Geneve from every +// chassis missing from it. +func TestReconcileFirewall_PartialEncapDoesNotWrite(t *testing.T) { + env := newFirewallTestEnv(t, "10.9.8.21\n") + + err := ReconcileFirewall(env.configPath, firewallClusterConfig(true)) + require.Error(t, err) + assert.Contains(t, err.Error(), "1 of 2 chassis") + assert.Empty(t, env.helperRuns(), "a partial chassis list must not reach the helper") +} + +// A chassis left behind by a decommissioned node only widens the set, so it must +// not block the reconcile. +func TestReconcileFirewall_ExtraEncapStillWrites(t *testing.T) { + env := newFirewallTestEnv(t, "10.9.8.21\n10.9.8.22\n10.9.8.23\n") + + require.NoError(t, ReconcileFirewall(env.configPath, firewallClusterConfig(true))) + require.Len(t, env.helperRuns(), 1) +} + +// The reconcile must recover on its own. Before this, a node that lost the race +// with ovn-controller stayed unprotected until something restarted the daemon. +func TestMaintainFirewall_RetriesUntilChassisRegister(t *testing.T) { + env := newFirewallTestEnv(t, "") + + var attempts atomic.Int32 + ovnEncapCommand = func(string) *exec.Cmd { + // Two partial answers, then the full set — a bootstrap in miniature. + switch attempts.Add(1) { + case 1: + return exec.Command("printf", "%s", "") + case 2: + return exec.Command("printf", "%s", "10.9.8.21\n") + default: + return exec.Command("printf", "%s", "10.9.8.21\n10.9.8.22\n") + } + } + + origRetry, origInterval := firewallRetryDelay, firewallReconcileInterval + firewallRetryDelay = time.Millisecond + firewallReconcileInterval = 50 * time.Millisecond + t.Cleanup(func() { firewallRetryDelay, firewallReconcileInterval = origRetry, origInterval }) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := make(chan struct{}) + go func() { MaintainFirewall(ctx, env.configPath, firewallClusterConfig(true)); close(done) }() + + require.Eventually(t, func() bool { return len(env.helperRuns()) > 0 }, 5*time.Second, 5*time.Millisecond, + "loop must keep retrying until every chassis has registered") + cancel() + + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("MaintainFirewall did not return when its context was cancelled") + } + assert.GreaterOrEqual(t, attempts.Load(), int32(3), "should have retried past the partial answers") +} + +// An unchanged peer set must not re-apply the policy on every tick. +func TestMaintainFirewall_SteadyStateIsQuiet(t *testing.T) { + env := newFirewallTestEnv(t, "10.9.8.21\n10.9.8.22\n") + + origRetry, origInterval := firewallRetryDelay, firewallReconcileInterval + firewallRetryDelay = time.Millisecond + firewallReconcileInterval = time.Millisecond + t.Cleanup(func() { firewallRetryDelay, firewallReconcileInterval = origRetry, origInterval }) + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + MaintainFirewall(ctx, env.configPath, firewallClusterConfig(true)) + + // Many ticks, one apply: the rendered-file comparison short-circuits the rest. + assert.Len(t, env.helperRuns(), 1) +} + +func TestReconcileFirewall_DisabledRemovesThePolicy(t *testing.T) { + env := newFirewallTestEnv(t, "10.9.8.21\n") + require.NoError(t, os.WriteFile(env.peersPath, []byte("define x = { }\n"), 0644)) + + require.NoError(t, ReconcileFirewall(env.configPath, firewallClusterConfig(false))) + + runs := env.helperRuns() + require.Len(t, runs, 1) + assert.Equal(t, []string{firewallApplyHelper, "disable"}, runs[0]) +} + +// Disabled on a node that never had the policy is a no-op, not an error: this +// runs on every daemon start. +func TestReconcileFirewall_DisabledWithoutPolicyIsSilent(t *testing.T) { + env := newFirewallTestEnv(t, "10.9.8.21\n") + + require.NoError(t, ReconcileFirewall(env.configPath, firewallClusterConfig(false))) + assert.Empty(t, env.helperRuns()) +} + +func TestReconcileFirewall_NilConfigLeavesPolicyAlone(t *testing.T) { + env := newFirewallTestEnv(t, "10.9.8.21\n") + + require.NoError(t, ReconcileFirewall(env.configPath, nil)) + assert.Empty(t, env.helperRuns()) +} + +func TestClusterPeerAddrsRejectsNonIPv4(t *testing.T) { + cfg := &config.ClusterConfig{ + Nodes: map[string]config.Config{ + "a": {Host: "10.9.7.21", AdvertiseIP: "node1.example.com"}, + "b": {Host: "10.9.7.22:4432", AdvertiseIP: "0.0.0.0"}, + "c": {Host: "", AdvertiseIP: "::1"}, + }, + } + assert.Equal(t, []string{"10.9.7.21"}, clusterPeerAddrs("", cfg, nil)) +} + +// The lan plane is the case a node's own config cannot supply: peers appear +// there by their wan address only, so a set built from config alone drops the +// cluster traffic they actually send. +func TestClusterPeerAddrsUnionsAllThreePlanes(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "spinifex.toml") + require.NoError(t, os.MkdirAll(filepath.Join(dir, "nats"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "nats", "nats.conf"), []byte(` +cluster { + listen: 10.9.7.5:4248 + routes = [ + "nats-route://tok_secret@10.9.7.2:4248", + "nats-route://tok_secret@10.9.7.4:4248", + ] +} +`), 0644)) + + cfg := &config.ClusterConfig{ + Nodes: map[string]config.Config{ + "local": {Host: "10.9.7.5", AdvertiseIP: "192.168.1.25"}, + "peer": {Host: "192.168.1.21"}, + }, + } + local := cfg.Nodes["local"] + local.VPCD.OVNSBAddr = "tcp:10.9.7.2:6642,tcp:10.9.7.3:6642" + local.VPCD.OVNNBAddr = "tcp:10.9.7.2:6641,tcp:10.9.7.3:6641" + cfg.Nodes["local"] = local + + assert.Equal(t, []string{ + "10.9.7.2", "10.9.7.3", "10.9.7.4", "10.9.7.5", "10.9.8.1", + "192.168.1.21", "192.168.1.25", + }, clusterPeerAddrs(configPath, cfg, []string{"10.9.8.1"})) +} + +// The route token must not be mistaken for the host. +func TestNATSRouteAddrsIgnoresTheToken(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "nats"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "nats", "nats.conf"), + []byte(`routes = [ "nats-route://nats_1.2.3.4tok@10.9.7.2:4248" ]`), 0644)) + + assert.Equal(t, []string{"10.9.7.2"}, natsRouteAddrs(filepath.Join(dir, "spinifex.toml"))) + assert.Nil(t, natsRouteAddrs("")) + assert.Nil(t, natsRouteAddrs(filepath.Join(t.TempDir(), "spinifex.toml"))) +} + +func TestIsPlainIPv4(t *testing.T) { + for _, addr := range []string{"10.9.7.21", "192.168.1.1", "255.255.255.255"} { + assert.True(t, isPlainIPv4(addr), addr) + } + for _, addr := range []string{"", "0.0.0.0", "10.9.7", "10.9.7.21:4432", + "10.9.7.1234", "10.9.7.a", "::1", "10.9.7.21/24", "10.0.0.999", "::ffff:10.0.0.1", "127.0.0.1"} { + assert.False(t, isPlainIPv4(addr), addr) + } +} diff --git a/tests/e2e/multinode/firewall_quorum_test.go b/tests/e2e/multinode/firewall_quorum_test.go new file mode 100644 index 000000000..4eceaa721 --- /dev/null +++ b/tests/e2e/multinode/firewall_quorum_test.go @@ -0,0 +1,125 @@ +//go:build e2e + +package multinode + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/mulgadc/spinifex/tests/e2e/harness" + "github.com/stretchr/testify/require" +) + +const ( + firewallPeersFile = "/etc/spinifex/firewall/peers.nft" + firewallConfigFile = "/etc/spinifex/spinifex.toml" + firewallConfigBak = "/var/tmp/spinifex.toml.e2e-firewall-quorum" + + // The phantom entry only has to raise the expected node count. It must carry + // no OVN remote: the Southbound address is taken from the first node that has + // one and map order is random, so a bogus remote would fail the query and + // stop the reconcile at the unreachable path instead of the quorum gate. + firewallPhantomNode = `printf '\n[nodes.e2ephantom]\nhost = "10.99.99.99"\n'` + + firewallQuorumLog = "chassis encap addresses; not writing a partial peer set" +) + +// runFirewallChassisQuorum proves the daemon refuses to write a peer set built +// from a partial chassis list. That list is the normal state early in a +// bootstrap, and writing it produces a firewall that drops Geneve from every +// node missing from it — a policy that looks applied and silently breaks the +// overlay. +// +// The count is inflated rather than OVN broken. Deleting a chassis would +// reproduce the race more literally at the cost of disrupting a node's overlay +// mid-suite; adding a node the cluster does not have reaches the same branch +// with nothing in OVN disturbed. +func runFirewallChassisQuorum(t *testing.T, fix *Fixture) { + harness.Phase(t, "Multinode — Firewall Chassis Quorum") + + // Last node, not the first: this restarts the node's daemon, and the trio and + // the operator gateway both lean on the first. + node := fix.Cluster.Nodes[len(fix.Cluster.Nodes)-1] + + baseline := string(harness.PeerFileContents(t, node, firewallPeersFile)) + require.Containsf(t, baseline, "spinifex_encap_peers", + "%s has no encap peer set to start from, so there is nothing to protect", node.Name) + + harness.Step(t, "add a phantom node to %s, raising the expected chassis count by one", node.Name) + firewallRun(t, node, "sudo cp -a "+firewallConfigFile+" "+firewallConfigBak) + + // Safety net only — the body restores and asserts recovery, then drops the + // backup, so this fires just when the test fails partway. + t.Cleanup(func() { + if _, err := firewallRunErr(node, "test -e "+firewallConfigBak); err != nil { + return + } + firewallRestore(t, node) + }) + + firewallRun(t, node, firewallPhantomNode+" | sudo tee -a "+firewallConfigFile+" >/dev/null") + + since := strings.TrimSpace(firewallRun(t, node, `date -u '+%Y-%m-%d %H:%M:%S'`)) + harness.Step(t, "drop the peer file and restart spinifex-daemon on %s", node.Name) + firewallRun(t, node, "sudo rm -f "+firewallPeersFile+" && sudo systemctl restart spinifex-daemon") + + // Backoff starts at 15s, so the first attempt lands well inside this window. + journal := "sudo journalctl -u spinifex-daemon --since '" + since + "' --no-pager" + require.Eventuallyf(t, func() bool { + out, err := firewallRunErr(node, journal) + return err == nil && strings.Contains(out, firewallQuorumLog) + }, 90*time.Second, 3*time.Second, + "%s never logged the chassis quorum gate; it either wrote a partial peer set or failed somewhere earlier", node.Name) + + harness.Step(t, "assert no peer file was written while the chassis list is short") + present := strings.TrimSpace(firewallRun(t, + node, "sudo test -e "+firewallPeersFile+" && echo present || echo absent")) + require.Equalf(t, "absent", present, + "%s wrote %s from a partial chassis list", node.Name, firewallPeersFile) + + // The loaded ruleset is runtime state and outlives the peer file, which is + // what keeps the node protected while the reconcile is refusing to write. + _, err := firewallRunErr(node, "sudo nft list table inet spinifex_filter") + require.NoErrorf(t, err, "%s lost its firewall table while the reconcile was blocked", node.Name) + + harness.Step(t, "restore the config and confirm the reconcile recovers on its own") + firewallRestore(t, node) + harness.WaitNodeServiceReady(t, node, harness.WithTimeout(90*time.Second)) + + var restored string + require.Eventuallyf(t, func() bool { + out, err := firewallRunErr(node, "sudo cat "+firewallPeersFile) + restored = out + return err == nil && strings.Contains(out, "spinifex_encap_peers") + }, 90*time.Second, 3*time.Second, "%s never rewrote %s after the phantom was removed", node.Name, firewallPeersFile) + + require.Equalf(t, baseline, restored, + "%s recovered to a different peer set than it started with", node.Name) + harness.Detail(t, "peer_set_restored", "byte-identical") + + firewallRun(t, node, "sudo rm -f "+firewallConfigBak) +} + +// firewallRestore puts the saved config back and restarts the daemon so the +// next reconcile sees the real node count. +func firewallRestore(t *testing.T, node harness.Node) { + t.Helper() + firewallRun(t, node, "sudo cp -a "+firewallConfigBak+" "+firewallConfigFile+ + " && sudo systemctl restart spinifex-daemon") +} + +func firewallRun(t *testing.T, node harness.Node, cmd string) string { + t.Helper() + out, err := firewallRunErr(node, cmd) + require.NoErrorf(t, err, "%s on %s: %s", cmd, node.Name, out) + return out +} + +func firewallRunErr(node harness.Node, cmd string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + out, err := harness.NewPeerSSH().Run(ctx, node.Addr, cmd) + return string(out), err +} diff --git a/tests/e2e/multinode/tests_test.go b/tests/e2e/multinode/tests_test.go index 5d7edf090..3d1bbf393 100644 --- a/tests/e2e/multinode/tests_test.go +++ b/tests/e2e/multinode/tests_test.go @@ -79,6 +79,12 @@ func TestMultinodeOVNRaft(t *testing.T) { runOVNRaft(t, requireMultiNodeFixture(t)) } +// TestMultinodeFirewallChassisQuorum is sequential: it edits one node's config +// and restarts that node's daemon, restoring both before it returns. +func TestMultinodeFirewallChassisQuorum(t *testing.T) { + runFirewallChassisQuorum(t, requireMultiNodeFixture(t)) +} + // TestMultinodeSpread is sequential after NodeRecovery; owns 10.100.0.0/16 + EIP pool. func TestMultinodeSpread(t *testing.T) { runSpread(t, requireMultiNodeFixture(t))