From 57bfbc20df525af7c35ad5cbcde05ecd6dac07a0 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Thu, 13 Aug 2026 12:29:26 +1000 Subject: [PATCH] test: listener invariant static guard + e2e runtime check (#782) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: add the listener invariant static guard and e2e runtime check Adds both halves the plan called for: a static test in spinifex/network/invariants that scans install scripts and config templates for a bare OVSDB ptcp remote, a wildcard bind on a Cluster/Encap port, or a bind port absent from the inventory, and an e2e test in tests/e2e/multinode that asserts the same on live ss -tulnp output. Both read docs/security/network-connections/README.md "## 1. Inbound Listeners" as their fixture through a shared, non-test parser package (spinifex/network/listenerinventory), so a wildcard exception has to be declared in the doc's own prose rather than hardcoded in either test. * fix: make the listener wildcard exception fail closed on an explicit marker The prior heuristic granted an exception for any unnegated "wildcard" or "0.0.0.0" mention within 40 characters of the port's row. That fails open: a future row phrased as "avoids the wildcard" or with a negation further upstream than the window would silently exempt itself. Replace it with a single exact literal phrase, "binds the wildcard by design", that a row must contain to grant the exception — no other wording counts, so an unrecognized mention denies rather than grants. Update the two rows that need it (5300, 500/4500) and document the marker's role in the inventory's own prose. * docs: record that the Geneve tunnel socket binds the wildcard by design --- docs/security/network-connections/README.md | 14 +- .../network/invariants/listener_bind_test.go | 234 ++++++++++++++++ spinifex/network/listenerinventory/doc.go | 14 + spinifex/network/listenerinventory/table.go | 254 ++++++++++++++++++ .../network/listenerinventory/table_test.go | 177 ++++++++++++ .../e2e/multinode/listener_invariant_test.go | 139 ++++++++++ tests/e2e/multinode/tests_test.go | 7 + 7 files changed, 836 insertions(+), 3 deletions(-) create mode 100644 spinifex/network/invariants/listener_bind_test.go create mode 100644 spinifex/network/listenerinventory/doc.go create mode 100644 spinifex/network/listenerinventory/table.go create mode 100644 spinifex/network/listenerinventory/table_test.go create mode 100644 tests/e2e/multinode/listener_invariant_test.go diff --git a/docs/security/network-connections/README.md b/docs/security/network-connections/README.md index 2476f08fa..74dab41bf 100644 --- a/docs/security/network-connections/README.md +++ b/docs/security/network-connections/README.md @@ -80,6 +80,14 @@ The inventory in [§1](#1-inbound-listeners)–[§2](#2-outbound-connections) sa - **Guest** — bound to a per-instance interface and reachable only by that instance's VM. - **Localhost** — bound to `127.0.0.1`; not reachable off-node. +The listener invariant tests (`spinifex/network/invariants` and the multinode e2e suite) read +this table and fail any Cluster- or Encap-scope port found bound to the wildcard address, +unless that row's Purpose or Auth text contains the exact phrase **"binds the wildcard by +design"**. That phrase is load-bearing, not incidental wording — a row that merely mentions +"wildcard" or "0.0.0.0", negated or not, grants no exception. Adding a new wildcard-bound +Cluster/Encap listener means adding that literal phrase to its row, not just describing the +behavior in other words. + | Port | Service | Protocol | Scope | Purpose | Auth / Verification | |------|---------|----------|-------|---------|--------------------| | 9999 | spinifex-awsgw | HTTPS | External | AWS-compatible API (EC2, S3, ELBv2, IAM) — customer endpoint | AWS SigV4 + TLS (cluster CA) | @@ -90,15 +98,15 @@ The inventory in [§1](#1-inbound-listeners)–[§2](#2-outbound-connections) sa | 4432 | Formation server | HTTPS | Cluster (bootstrap only) | Cluster join coordination; active only while a join token is valid. Binds the node's `--bind` (lan) address. See *Formation port lifecycle* below. | Short-lived bearer token + TLS¹ | | 4222 | spinifex-nats (client) | NATS + TLS | Cluster | Internal service bus for EC2/EBS/VPC/S3 handlers | Token + mutual TLS (cluster CA) | | 4248 | spinifex-nats (cluster) | NATS + TLS | Cluster | Inter-node NATS federation | Token + mutual TLS (cluster CA) | -| 5300 | northstar | DNS (UDP + TCP) | Cluster | Forward target for every node's per-instance DNS shim, dialled cross-node. Binds the wildcard address. | None — restrict by firewall | +| 5300 | northstar | DNS (UDP + TCP) | Cluster | Forward target for every node's per-instance DNS shim, dialled cross-node. Binds the wildcard by design. | None — restrict by firewall | | 6660 | predastore (blob node) | QUIC / UDP | Cluster | Erasure-coded object shard transport between hosts. Multi-node clusters only — see *Predastore ports* below. | TLS 1.3, server certificate verified against the cluster CA | | 7660 | predastore (meta node) | QUIC / UDP | Cluster | Raft consensus over global state — buckets and the object index — between hosts. Multi-node clusters only. | TLS 1.3, server certificate verified against the cluster CA | | 6641 | OVN Northbound DB (client) | OVSDB/TCP | Cluster | Logical network topology consumed by vpcd. Binds `127.0.0.1` plus the node's lan-plane address (`--lan-addr`), never the wildcard address. On a node with no separate lan plane that address is the public one, and a host firewall is the only remaining control. | Cluster network only; TLS planned | | 6642 | OVN Southbound DB (client) | OVSDB/TCP | Cluster | Chassis / port / MAC binding state. Binds `127.0.0.1` plus the node's lan-plane address (`--lan-addr`), never the wildcard address. On a node with no separate lan plane that address is the public one, and a host firewall is the only remaining control. | Cluster network only; TLS planned | | 6643 | OVN Northbound DB (RAFT) | OVSDB/TCP | Cluster | NB database RAFT replication between the 3 quorum nodes | Cluster network only; TLS planned | | 6644 | OVN Southbound DB (RAFT) | OVSDB/TCP | Cluster | SB database RAFT replication between the 3 quorum nodes | Cluster network only; TLS planned | -| 6081 | OVN (Geneve) | UDP | Encap | Tenant traffic overlay between chassis. A kernel UDP-tunnel socket, so packets are delivered locally and traverse the host's netfilter input hook before OVS sees them — a host firewall must accept them explicitly. | None — see 500/4500 | -| 500, 4500 | strongSwan `charon` | IKEv2 / UDP | Encap | IKE and NAT-T for the IPsec protecting Geneve, managed entirely by `ovs-monitor-ipsec`. Binds the wildcard address (upstream strongSwan default), so reach must be restricted by firewall. | Certificate-based, against the cluster CA | +| 6081 | OVN (Geneve) | UDP | Encap | Tenant traffic overlay between chassis. A kernel UDP-tunnel socket, so packets are delivered locally and traverse the host's netfilter input hook before OVS sees them — a host firewall must accept them explicitly. The socket is opened by the kernel tunnel driver and takes no bind address, so it binds the wildcard by design and reach must be restricted by firewall. | None — see 500/4500 | +| 500, 4500 | strongSwan `charon` | IKEv2 / UDP | Encap | IKE and NAT-T for the IPsec protecting Geneve, managed entirely by `ovs-monitor-ipsec`. Binds the wildcard by design (the upstream strongSwan default, accepted rather than overridden), so reach must be restricted by firewall. | Certificate-based, against the cluster CA | | — | ESP | IP proto 50 | Encap | The IPsec payload itself, once IKE has negotiated an SA | Cluster CA | | 8222 | spinifex-nats (monitoring) | HTTP | Localhost | `varz`/`subsz` metrics consumed by the daemon | Loopback only | | 323 | chronyd | NTP | Localhost | Time sync client control socket | Loopback only | diff --git a/spinifex/network/invariants/listener_bind_test.go b/spinifex/network/invariants/listener_bind_test.go new file mode 100644 index 000000000..52f18d8bc --- /dev/null +++ b/spinifex/network/invariants/listener_bind_test.go @@ -0,0 +1,234 @@ +package invariants + +//test:in-package — this file shares repoRoot, relTo and itoa with the rest +// of this package's test files (layers_test.go), all of which are +// unexported by design so an external caller cannot depend on them. Every +// other file here is in-package for the same reason; splitting this one +// file out would mean duplicating those helpers rather than reusing them. + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "regexp" + "sort" + "strconv" + "strings" + "testing" + + "github.com/mulgadc/spinifex/spinifex/network/listenerinventory" +) + +// TestListenerBindSitesMatchInventory is the static half of the listener +// invariant: docs/security/network-connections/README.md "## 1. Inbound +// Listeners" classifies every port's intended reach (External/Cluster/ +// Encap/Guest/Localhost). This test scans the install scripts and config +// templates for the two bind patterns known to put a listener on the wrong +// plane — a bare OVSDB `ptcp:PORT` remote, which binds the wildcard address +// by default, and a literal wildcard `0.0.0.0:PORT` / `:PORT` bind — and +// fails if either lands on a Cluster/Encap port the inventory has not +// explicitly excused, or on a port the inventory does not document at all. +// +// Unlike every other test in this package, it reads text, not Go: the +// defect it targets (spinifex #765, the OVN NB/SB wildcard bind) left no +// trace in an import graph or an AST, only in a shell script's set-connection +// call. It was found by hand on a live node; this is what would have caught +// it at review time. +func TestListenerBindSitesMatchInventory(t *testing.T) { + root := repoRoot(t) + docPath := listenerinventory.DocPath(root) + table, err := listenerinventory.ParseFile(docPath) + if err != nil { + t.Fatalf("parse %s: %v", relTo(docPath, root), err) + } + + var bad []string + for _, f := range listenerBindScanFiles(t, root) { + sites, serr := scanBindSites(f) + if serr != nil { + t.Fatalf("scan %s: %v", relTo(f, root), serr) + } + for _, s := range sites { + if msg := checkBindSite(table, s, root); msg != "" { + bad = append(bad, msg) + } + } + } + + if len(bad) == 0 { + return + } + sort.Strings(bad) + + var b strings.Builder + b.WriteString(`docs/security/network-connections/README.md "## 1. Inbound ` + + `Listeners" is the fixture for every bind site below; add or correct a ` + + "row there rather than widening this test.\n") + limit := 10 + for i, m := range bad { + if i >= limit { + b.WriteString(" …\n") + break + } + b.WriteString(" ") + b.WriteString(m) + b.WriteString("\n") + } + if len(bad) > limit { + b.WriteString(" ") + b.WriteString(itoa(len(bad) - limit)) + b.WriteString(" further violations suppressed.\n") + } + t.Fatalf("%s", b.String()) +} + +// listenerBindScanFiles returns the install scripts and config templates +// known to write listener bind addresses. Each is required to exist so a +// rename silently drops coverage instead of narrowing it. +func listenerBindScanFiles(t *testing.T, root string) []string { + t.Helper() + files := []string{ + filepath.Join(root, "scripts", "setup-ovn.sh"), + filepath.Join(root, "scripts", "install-node.sh"), + filepath.Join(root, "scripts", "setup.sh"), + } + tomls, err := filepath.Glob(filepath.Join(root, "cmd", "spinifex", "cmd", "templates", "*.toml")) + if err != nil { + t.Fatalf("glob templates: %v", err) + } + sort.Strings(tomls) + files = append(files, tomls...) + for _, f := range files { + if _, statErr := os.Stat(f); statErr != nil { + t.Fatalf("listener bind scan target missing: %s: %v", f, statErr) + } + } + return files +} + +// bindSite is one candidate listener bind found by scanning a script or +// template: an OVSDB ptcp remote, or a literal wildcard address:port pair. +type bindSite struct { + file string + line int + port int + wildcard bool + barePtcp bool + raw string +} + +var ( + // ptcpRe matches an OVSDB ptcp remote: `ptcp:PORT` alone, or + // `ptcp:PORT:ADDR` with an explicit bind address. + ptcpRe = regexp.MustCompile(`\bptcp:(\d+)(?::(\S+))?`) + // wildcardExplicitRe matches a literal `0.0.0.0:PORT`, bounded so it + // does not match the tail of an unrelated dotted-quad like + // `127.0.0.1:6641` or a template placeholder's trailing `:53`. + wildcardExplicitRe = regexp.MustCompile(`(?:^|[\s"'=,])(0\.0\.0\.0):(\d{2,5})(?:[\s"'),]|$)`) + // bareColonRe matches a value that is exactly `:PORT` with no address — + // Go's "listen on every interface" shorthand. + bareColonRe = regexp.MustCompile(`"(:(\d{2,5}))"`) +) + +// scanBindSites reads path line by line and extracts every ptcp remote and +// literal wildcard bind. Comment lines are skipped: this package's own +// prose (e.g. "not a wildcard ptcp:6641") would otherwise read as a bind. +func scanBindSites(path string) ([]bindSite, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + var sites []bindSite + sc := bufio.NewScanner(f) + lineNo := 0 + for sc.Scan() { + lineNo++ + line := sc.Text() + if strings.HasPrefix(strings.TrimSpace(line), "#") { + continue + } + + for _, m := range ptcpRe.FindAllStringSubmatch(line, -1) { + port, perr := strconv.Atoi(m[1]) + if perr != nil { + continue + } + bare := m[2] == "" + wc := bare || listenerinventory.IsWildcardAddr(m[2]) + sites = append(sites, bindSite{ + file: path, line: lineNo, port: port, + wildcard: wc, barePtcp: bare, + raw: strings.TrimSpace(m[0]), + }) + } + for _, m := range wildcardExplicitRe.FindAllStringSubmatch(line, -1) { + port, perr := strconv.Atoi(m[2]) + if perr != nil { + continue + } + sites = append(sites, bindSite{ + file: path, line: lineNo, port: port, + wildcard: true, + raw: m[1] + ":" + m[2], + }) + } + for _, m := range bareColonRe.FindAllStringSubmatch(line, -1) { + port, perr := strconv.Atoi(m[2]) + if perr != nil { + continue + } + sites = append(sites, bindSite{ + file: path, line: lineNo, port: port, + wildcard: true, + raw: m[1], + }) + } + } + return sites, sc.Err() +} + +// checkBindSite applies the three listener-invariant rules to one bind site +// and returns a failure message naming the file, the line and the fix, or "" +// if the site is clean. +func checkBindSite(table *listenerinventory.Table, s bindSite, root string) string { + rel := relTo(s.file, root) + + if s.barePtcp { + return fmt.Sprintf( + "%s:%d: bare `ptcp:%d` has no explicit bind address, so OVSDB "+ + "binds the wildcard by default — this is the F1 defect (OVN "+ + "NB/SB on the WAN). Fix: pin it, e.g. `ptcp:%d:127.0.0.1`.", + rel, s.line, s.port, s.port) + } + + if !table.KnownPort(s.port) { + return fmt.Sprintf( + "%s:%d: binds port %d (%s), which has no row in "+ + "docs/security/network-connections/README.md \"## 1. Inbound "+ + "Listeners\". Fix: add a row classifying its Scope, or remove "+ + "the listener.", + rel, s.line, s.port, s.raw) + } + + if !s.wildcard { + return "" + } + for _, r := range table.RowsForPort(s.port) { + if r.Scope != listenerinventory.ScopeCluster && r.Scope != listenerinventory.ScopeEncap { + continue + } + if r.WildcardOK() { + continue + } + return fmt.Sprintf( + "%s:%d: binds port %d to the wildcard address (%s), but the "+ + "inventory classifies it %s and its row does not say it binds "+ + "the wildcard by design. Fix: bind the lan/vpc-plane address "+ + "instead, or add that justification to the inventory row.", + rel, s.line, s.port, s.raw, r.Scope) + } + return "" +} diff --git a/spinifex/network/listenerinventory/doc.go b/spinifex/network/listenerinventory/doc.go new file mode 100644 index 000000000..98107dd3d --- /dev/null +++ b/spinifex/network/listenerinventory/doc.go @@ -0,0 +1,14 @@ +// Package listenerinventory parses the "## 1. Inbound Listeners" table in +// docs/security/network-connections/README.md into a queryable Table of +// port -> scope -> exception-declaring prose. +// +// It exists so the inventory doc is the single fixture two otherwise +// unrelated checks read: the static bind-site scan in +// spinifex/network/invariants, which reads install scripts and config +// templates, and the runtime e2e check in tests/e2e/multinode, which reads +// `ss -tulnp` on live nodes. Neither belongs under spinifex/network/'s +// ADR-0006 layer tree — this package is not part of that stack, and lives +// beside invariants for the same reason invariants does: it is read by both +// a go-test-only consumer and an e2e-tagged one, so it cannot itself carry +// a build tag or live under either caller. +package listenerinventory diff --git a/spinifex/network/listenerinventory/table.go b/spinifex/network/listenerinventory/table.go new file mode 100644 index 000000000..f5a51fcc4 --- /dev/null +++ b/spinifex/network/listenerinventory/table.go @@ -0,0 +1,254 @@ +package listenerinventory + +import ( + "fmt" + "os" + "os/exec" + "regexp" + "slices" + "strconv" + "strings" +) + +// Scope classifies a listener's intended reach, matching the "Scope" column +// of the inventory table. +type Scope int + +const ( + ScopeUnknown Scope = iota + ScopeExternal + ScopeCluster + ScopeEncap + ScopeGuest + ScopeLocalhost +) + +func (s Scope) String() string { + switch s { + case ScopeExternal: + return "External" + case ScopeCluster: + return "Cluster" + case ScopeEncap: + return "Encap" + case ScopeGuest: + return "Guest" + case ScopeLocalhost: + return "Localhost" + default: + return "Unknown" + } +} + +// Row is one parsed data row of the inbound-listeners table. +type Row struct { + RawPort string // the Port cell verbatim, e.g. "500, 4500" or "169.254.169.254:80" + Ports []int // numeric ports parsed out of RawPort; nil for non-numeric cells (ESP, dynamic) + Addr string // literal address prefix when RawPort is "ip:port"; empty otherwise + Service string + Protocol string + Scope Scope + Purpose string + Auth string +} + +// wildcardExceptionMarker is the exact phrase a row's Purpose or Auth column +// must contain to declare a wildcard-bind exception. This check is +// deliberately fail-closed: no other wording counts, including a mention of +// "wildcard" or "0.0.0.0" with no marker, and including a negated mention +// like "never the wildcard address". Both resolve to no exception, for the +// same reason — the marker is absent — rather than one relying on the +// absence of a negation word an author didn't think to use. +const wildcardExceptionMarker = "binds the wildcard by design" + +// WildcardOK reports whether this row's own Purpose/Auth prose declares, +// via the exact marker above, that the listener binds the wildcard address +// by design. This is the doc-declared exception a bind-site or runtime +// check must honor rather than a hardcoded port list. The comparison folds +// case only — sentence-initial "Binds the wildcard by design." still +// counts — nothing else about the marker is fuzzy. +func (r Row) WildcardOK() bool { + return strings.Contains(strings.ToLower(r.Purpose), wildcardExceptionMarker) || + strings.Contains(strings.ToLower(r.Auth), wildcardExceptionMarker) +} + +// Table is the parsed inbound-listeners inventory. +type Table struct { + Rows []Row +} + +// ParseFile reads and parses the inventory table from the security doc at path. +func ParseFile(path string) (*Table, error) { + b, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("listenerinventory: read %s: %w", path, err) + } + return Parse(string(b)) +} + +const inboundListenersHeading = "## 1. Inbound Listeners" + +// Parse extracts the inbound-listeners markdown table from content. +func Parse(content string) (*Table, error) { + lines := strings.Split(content, "\n") + start := -1 + for i, l := range lines { + if strings.HasPrefix(strings.TrimSpace(l), inboundListenersHeading) { + start = i + break + } + } + if start < 0 { + return nil, fmt.Errorf("listenerinventory: no %q section found", inboundListenersHeading) + } + + var headerSeen, sepSeen bool + var t Table + for _, l := range lines[start+1:] { + trimmed := strings.TrimSpace(l) + if !strings.HasPrefix(trimmed, "|") { + if headerSeen { + break // prose or a blank line ends the table + } + continue + } + if !headerSeen { + headerSeen = true // the header row itself + continue + } + if !sepSeen { + sepSeen = true // the "|---|---|" separator row + continue + } + if row, ok := parseRow(trimmed); ok { + t.Rows = append(t.Rows, row) + } + } + if len(t.Rows) == 0 { + return nil, fmt.Errorf("listenerinventory: parsed zero rows from %q", inboundListenersHeading) + } + return &t, nil +} + +func parseRow(line string) (Row, bool) { + line = strings.TrimPrefix(line, "|") + line = strings.TrimSuffix(line, "|") + cells := strings.Split(line, "|") + if len(cells) < 6 { + return Row{}, false + } + for i := range cells { + cells[i] = strings.TrimSpace(cells[i]) + } + ports, addr := parsePortCell(cells[0]) + return Row{ + RawPort: cells[0], + Ports: ports, + Addr: addr, + Service: cells[1], + Protocol: cells[2], + Scope: parseScope(cells[3]), + Purpose: cells[4], + Auth: cells[5], + }, true +} + +var ipPortCellRe = regexp.MustCompile(`^(\d{1,3}(?:\.\d{1,3}){3}):(\d+)$`) + +// parsePortCell handles every shape the Port column takes: a bare number, a +// comma-separated list ("500, 4500"), an address:port pair +// ("169.254.169.254:80"), an em-dash for a portless protocol (ESP), and +// free text ("socket / dynamic TCP") for a listener with no fixed port. +func parsePortCell(cell string) (ports []int, addr string) { + cell = strings.TrimSpace(cell) + if cell == "" || cell == "—" || cell == "-" { + return nil, "" + } + if m := ipPortCellRe.FindStringSubmatch(cell); m != nil { + if p, err := strconv.Atoi(m[2]); err == nil { + return []int{p}, m[1] + } + return nil, "" + } + if !strings.ContainsAny(cell, "0123456789") { + return nil, "" + } + for tok := range strings.SplitSeq(cell, ",") { + tok = strings.TrimSpace(tok) + p, err := strconv.Atoi(tok) + if err != nil { + continue // prose token in an otherwise numeric cell; skip rather than fail + } + ports = append(ports, p) + } + return ports, "" +} + +func parseScope(cell string) Scope { + lower := strings.ToLower(cell) + switch { + case strings.Contains(lower, "encap"): + return ScopeEncap + case strings.Contains(lower, "external"): + return ScopeExternal + case strings.Contains(lower, "guest"): + return ScopeGuest + case strings.Contains(lower, "localhost"): + return ScopeLocalhost + case strings.Contains(lower, "cluster"): + return ScopeCluster + default: + return ScopeUnknown + } +} + +// RowsForPort returns every row whose Ports list contains port. More than +// one row can share a port — e.g. 53 is both northstar's External listener +// on the advertise address and vpcd's Guest-scope VPC DNS on a per-instance +// address — so callers must check Scope on each returned row rather than +// assume a 1:1 mapping. +func (t *Table) RowsForPort(port int) []Row { + var out []Row + for _, r := range t.Rows { + if slices.Contains(r.Ports, port) { + out = append(out, r) + } + } + return out +} + +// KnownPort reports whether port appears in any row of the table. +func (t *Table) KnownPort(port int) bool { + return len(t.RowsForPort(port)) > 0 +} + +// IsWildcardAddr reports whether addr denotes "every interface", in either +// ss(8) or config-file notation: "0.0.0.0", "::", "*", "[::]", or the empty +// string (Go's bare ":PORT" listen form, address already stripped). +func IsWildcardAddr(addr string) bool { + switch addr { + case "0.0.0.0", "::", "*", "[::]", "": + return true + default: + return false + } +} + +// RepoRoot returns the spinifex module root via `go env GOMOD`. +func RepoRoot() (string, error) { + out, err := exec.Command("go", "env", "GOMOD").Output() + if err != nil { + return "", fmt.Errorf("listenerinventory: go env GOMOD: %w", err) + } + gomod := strings.TrimSpace(string(out)) + if gomod == "" || gomod == "/dev/null" { + return "", fmt.Errorf("listenerinventory: not in a go module (GOMOD=%q)", gomod) + } + return strings.TrimSuffix(gomod, "/go.mod"), nil +} + +// DocPath returns the standard location of the inbound-listener inventory +// relative to root. +func DocPath(root string) string { + return root + "/docs/security/network-connections/README.md" +} diff --git a/spinifex/network/listenerinventory/table_test.go b/spinifex/network/listenerinventory/table_test.go new file mode 100644 index 000000000..85b29d22b --- /dev/null +++ b/spinifex/network/listenerinventory/table_test.go @@ -0,0 +1,177 @@ +package listenerinventory_test + +import ( + "testing" + + "github.com/mulgadc/spinifex/spinifex/network/listenerinventory" +) + +func TestParse_Synthetic(t *testing.T) { + const doc = `# doc + +## 1. Inbound Listeners + +Some prose before the table. + +| Port | Service | Protocol | Scope | Purpose | Auth / Verification | +|------|---------|----------|-------|---------|--------------------| +| 9999 | gw | HTTPS | External | public API | TLS | +| 5300 | ns | DNS | Cluster | Forward target. Binds the wildcard by design. | none | +| 6641 | ovn-nb | OVSDB | Cluster | Binds 127.0.0.1 plus lan, never the wildcard address. | TLS | +| 7777 | mystery | UDP | Cluster | Mentions the wildcard address here, unrelated to any exception. | none | +| 500, 4500 | charon | IKEv2 | Encap | Binds the wildcard by design (upstream default). | cert | +| — | ESP | IP proto 50 | Encap | payload | cert | +| 169.254.169.254:80 | vpcd | HTTP | Guest | imds | tokens | +| socket / dynamic TCP | nbdkit | NBD | Host-local / cluster | block transport | unix | + +## 2. Outbound Connections + +not a table row +` + table, err := listenerinventory.Parse(doc) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if len(table.Rows) != 8 { + t.Fatalf("got %d rows, want 8", len(table.Rows)) + } + + if !table.KnownPort(9999) { + t.Fatalf("expected 9999 known") + } + if table.KnownPort(1234) { + t.Fatalf("expected 1234 unknown") + } + + for _, r := range table.RowsForPort(5300) { + if r.Scope != listenerinventory.ScopeCluster { + t.Fatalf("5300 scope = %v, want Cluster", r.Scope) + } + if !r.WildcardOK() { + t.Fatalf("5300 should declare a wildcard exception") + } + } + + // 6641's mention is negated ("never the wildcard") and 7777's is plain + // prose with no negation at all — both must resolve to no exception, for + // the same structural reason: neither row contains the exact marker + // phrase. Fail-closed means a bare mention never grants an exception on + // its own, negated or not. + for _, r := range table.RowsForPort(6641) { + if r.WildcardOK() { + t.Fatalf("6641 says \"never the wildcard\" and has no marker phrase; should not read as an exception") + } + } + for _, r := range table.RowsForPort(7777) { + if r.WildcardOK() { + t.Fatalf("7777 mentions \"wildcard\" with no marker phrase and no negation; should still not read as an exception") + } + } + + for _, port := range []int{500, 4500} { + found := false + for _, r := range table.RowsForPort(port) { + found = true + if r.Scope != listenerinventory.ScopeEncap { + t.Fatalf("%d scope = %v, want Encap", port, r.Scope) + } + if !r.WildcardOK() { + t.Fatalf("%d should declare a wildcard exception", port) + } + } + if !found { + t.Fatalf("expected %d to parse out of the \"500, 4500\" cell", port) + } + } + + espRows := table.Rows + sawESP := false + for _, r := range espRows { + if r.Service == "ESP" { + sawESP = true + if len(r.Ports) != 0 { + t.Fatalf("ESP row should have no numeric ports, got %v", r.Ports) + } + } + } + if !sawESP { + t.Fatalf("expected an ESP row") + } + + imds := table.RowsForPort(80) + if len(imds) != 1 || imds[0].Addr != "169.254.169.254" || imds[0].Scope != listenerinventory.ScopeGuest { + t.Fatalf("imds row parsed as %+v", imds) + } + + for _, r := range table.Rows { + if r.Service == "nbdkit" && len(r.Ports) != 0 { + t.Fatalf("dynamic-port row should have no numeric ports, got %v", r.Ports) + } + } +} + +func TestIsWildcardAddr(t *testing.T) { + cases := map[string]bool{ + "0.0.0.0": true, + "::": true, + "*": true, + "[::]": true, + "": true, + "10.0.1.2": false, + "127.0.0.1": false, + } + for addr, want := range cases { + if got := listenerinventory.IsWildcardAddr(addr); got != want { + t.Errorf("IsWildcardAddr(%q) = %v, want %v", addr, got, want) + } + } +} + +func TestParse_RealDoc(t *testing.T) { + root, err := listenerinventory.RepoRoot() + if err != nil { + t.Fatalf("RepoRoot: %v", err) + } + table, err := listenerinventory.ParseFile(listenerinventory.DocPath(root)) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + if len(table.Rows) < 15 { + t.Fatalf("got %d rows from the real inventory, expected considerably more", len(table.Rows)) + } + + wantWildcardOK := map[int]listenerinventory.Scope{ + 5300: listenerinventory.ScopeCluster, + 500: listenerinventory.ScopeEncap, + 4500: listenerinventory.ScopeEncap, + } + for port, scope := range wantWildcardOK { + rows := table.RowsForPort(port) + if len(rows) == 0 { + t.Fatalf("port %d not found in the real inventory", port) + } + found := false + for _, r := range rows { + if r.Scope == scope { + found = true + if !r.WildcardOK() { + t.Errorf("port %d (%v) should declare a wildcard exception in the real doc", port, scope) + } + } + } + if !found { + t.Errorf("port %d has no %v row in the real inventory", port, scope) + } + } + + // 6641/6642 say "never the wildcard address" — read as no exception + // because the marker phrase is absent, not because "never" was spotted. + wantNoException := []int{6641, 6642} + for _, port := range wantNoException { + for _, r := range table.RowsForPort(port) { + if r.Scope == listenerinventory.ScopeCluster && r.WildcardOK() { + t.Errorf("port %d should NOT declare a wildcard exception (no marker phrase present)", port) + } + } + } +} diff --git a/tests/e2e/multinode/listener_invariant_test.go b/tests/e2e/multinode/listener_invariant_test.go new file mode 100644 index 000000000..4eb4a249d --- /dev/null +++ b/tests/e2e/multinode/listener_invariant_test.go @@ -0,0 +1,139 @@ +//go:build e2e + +package multinode + +import ( + "context" + "fmt" + "strconv" + "strings" + "testing" + "time" + + "github.com/mulgadc/spinifex/spinifex/network/listenerinventory" + "github.com/mulgadc/spinifex/tests/e2e/harness" +) + +// runListenerInvariant is the runtime half of the listener invariant. The +// static half in spinifex/network/invariants reads install scripts and +// config templates; only this half reads what each node's kernel actually +// bound, which is what caught the original OVN NB/SB wildcard defect (spinifex +// #765) — that bug was found by hand on a live node with `ss -tulnp`, not by +// any static test. +// +// docs/security/network-connections/README.md "## 1. Inbound Listeners" is +// the same fixture the static half reads: it classifies each port's intended +// reach, and a row's own prose is the only thing allowed to excuse a +// wildcard bind. No port list is hardcoded here. +func runListenerInvariant(t *testing.T, fix *Fixture) { + harness.Phase(t, "Multinode — Listener Invariant") + + root, err := listenerinventory.RepoRoot() + if err != nil { + t.Fatalf("resolve module root: %v", err) + } + table, err := listenerinventory.ParseFile(listenerinventory.DocPath(root)) + if err != nil { + t.Fatalf("parse inbound listener inventory: %v", err) + } + + ssh := harness.NewPeerSSH() + var violations []string + for _, node := range fix.Cluster.Nodes { + harness.Step(t, "ss -tulnp on %s", node.Name) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + out, runErr := ssh.Run(ctx, node.Addr, "sudo ss -tulnp") + cancel() + if runErr != nil { + t.Fatalf("ss -tulnp on %s: %v", node.Name, runErr) + } + + for _, sock := range parseSSOutput(string(out)) { + for _, r := range table.RowsForPort(sock.port) { + if r.Scope != listenerinventory.ScopeCluster && r.Scope != listenerinventory.ScopeEncap { + continue + } + if v := checkSocketScope(node, sock, r); v != "" { + violations = append(violations, v) + } + } + } + } + + if len(violations) > 0 { + t.Fatalf("listener invariant violated (docs/security/network-connections/README.md"+ + " \"## 1. Inbound Listeners\" is the fixture; add or correct a row rather than"+ + " widening this test):\n %s", strings.Join(violations, "\n ")) + } + harness.Detail(t, "nodes_checked", len(fix.Cluster.Nodes)) +} + +// checkSocketScope reports a violation message if sock is bound somewhere +// row's Scope does not permit, honoring row's own doc-declared wildcard +// exception. Returns "" if the socket is clean for this row. +func checkSocketScope(node harness.Node, sock ssSocket, row listenerinventory.Row) string { + wildcard := listenerinventory.IsWildcardAddr(sock.addr) + switch { + case wildcard && row.WildcardOK(): + return "" + case wildcard: + return fmt.Sprintf( + "%s: port %d (%s, %s scope) bound to the wildcard address %s; its inventory row does not declare a wildcard exception", + node.Name, sock.port, sock.proto, row.Scope, sock.addr) + case sock.addr == node.Addr: + return fmt.Sprintf( + "%s: port %d (%s, %s scope) bound to %s, the node's WAN address", + node.Name, sock.port, sock.proto, row.Scope, sock.addr) + } + return "" +} + +// ssSocket is one row of `ss -tulnp` output: a listening TCP socket or an +// unconnected (server) UDP socket. +type ssSocket struct { + proto string + addr string + port int +} + +// parseSSOutput extracts every tcp/udp local bind from `ss -tulnp` output. +// Columns are whitespace-separated except the Process field, which is a +// single comma-joined token with no embedded space. +func parseSSOutput(out string) []ssSocket { + var socks []ssSocket + for _, line := range strings.Split(out, "\n") { + fields := strings.Fields(line) + if len(fields) < 5 { + continue + } + proto := fields[0] + if proto != "tcp" && proto != "udp" { + continue // header row, or a protocol ss -tulnp does not report + } + addr, portStr := splitAddrPort(fields[4]) + port, err := strconv.Atoi(portStr) + if err != nil { + continue + } + socks = append(socks, ssSocket{proto: proto, addr: addr, port: port}) + } + return socks +} + +// splitAddrPort splits an ss(8) "Local Address:Port" token into address and +// port, handling the bracketed IPv6 form ("[::]:500") separately since an +// IPv6 address contains colons of its own. +func splitAddrPort(tok string) (addr, port string) { + if strings.HasPrefix(tok, "[") { + end := strings.Index(tok, "]") + if end < 0 { + return tok, "" + } + return tok[1:end], strings.TrimPrefix(tok[end+1:], ":") + } + i := strings.LastIndex(tok, ":") + if i < 0 { + return tok, "" + } + return tok[:i], tok[i+1:] +} diff --git a/tests/e2e/multinode/tests_test.go b/tests/e2e/multinode/tests_test.go index 3d1bbf393..215c08ff6 100644 --- a/tests/e2e/multinode/tests_test.go +++ b/tests/e2e/multinode/tests_test.go @@ -101,3 +101,10 @@ func TestMultinodeIPSec(t *testing.T) { t.Parallel() runIPSec(t, requireMultiNodeFixture(t)) } + +// TestMultinodeListenerInvariant is read-only over SSH; launches no AWS +// resources and mutates no cluster state. +func TestMultinodeListenerInvariant(t *testing.T) { + t.Parallel() + runListenerInvariant(t, requireMultiNodeFixture(t)) +}