Merge pull request 'Anything on this machine may call anything on this machine' (#143) from fix/local-calls-are-not-filtered into main

internal/catalogue/filtering.go

@@ -265,6 +265,26 @@ func AsNftables(rules []Rule, mesh []string, outward bool, foundation []int,
 	b.WriteString("\t\tct state established,related accept\n")
 	b.WriteString("\t\tct state invalid drop\n")
 	b.WriteString("\t\tiif lo accept\n")
+	// **Anything on this machine may call anything on this machine.**
+	//
+	// Local is not a boundary this mesh draws. A service running here is callable by everything else
+	// running here, whatever form either takes — a package with a unit, a binary, a container. Whether
+	// a caller sits in a container was never meant to change the answer, and the only reason it did was
+	// that this chain asked about addresses: a caller on the machine carries the machine's address, a
+	// caller in one of its containers carries a bridge address, and a rule naming the former silently
+	// refused the latter.
+	//
+	// Measured: a module reaching its database on this machine's own name timed out for eleven hours
+	// while the machine itself could reach it, and the mesh called the machine healthy throughout
+	// (novox/hq 04-ISSUES/145).
+	//
+	// Asked by the link it arrives on rather than the address it comes from: anything that did not
+	// arrive from outside this machine, and did not arrive over the private network, is this machine's
+	// own. One rule for every service here, in place of a line per port that only ever covered the
+	// ports somebody remembered to think about.
+	if inward != "" {
+		b.WriteString(fmt.Sprintf("\t\tiifname != { %s } accept\n", inward))
+	}
 	b.WriteString("\t\ticmp type echo-request accept\n")
 	b.WriteString("\t\ticmpv6 type { echo-request, nd-neighbor-solicit, nd-neighbor-advert, nd-router-advert } accept\n")
 
@@ -374,25 +394,6 @@ func AsNftables(rules []Rule, mesh []string, outward bool, foundation []int,
 				b.WriteString(fmt.Sprintf("\t\tip6 saddr { %s } %s dport %d accept\n",
 					strings.Join(six, ", "), rule.Protocol, rule.Port))
 			}
-			// **And this machine's own guests, which are part of what it hosts** (novox/hq ADR 0100:
-			// the store is reachable "from a container on the node itself").
-			//
-			// The addresses above are the machines' own on the private network. A container reaching a
-			// port on the machine it runs on comes from a bridge, so it matches none of them — and with
-			// the runtime routing directly, that packet is delivered to this machine rather than
-			// forwarded, so the forward chain's allowance never sees it either.
-			//
-			// Measured, and it was an outage: after a machine was converged, every module that reached
-			// another by the machine's own name timed out. A web application logged
-			// "connection to server at novox.internal (10.10.0.1), port 6852 failed: timeout expired"
-			// for eleven hours while the mesh reported the machine healthy.
-			//
-			// Asked for by the link it arrives on, for the reason §4 no longer names an address: a
-			// range describes one machine and goes stale in silence.
-			if inward != "" {
-				b.WriteString(fmt.Sprintf("\t\tiifname != { %s } %s dport %d accept\n",
-					inward, rule.Protocol, rule.Port))
-			}
 		case FromEverywhere:
 			b.WriteString(fmt.Sprintf("\t\t%s dport %d accept\n", rule.Protocol, rule.Port))
 		}

internal/catalogue/filtering_test.go

@@ -805,29 +805,76 @@ func TestSSHIsNeverLeftWithoutARule(t *testing.T) {
 	}
 }
 
-// **This machine's own guests are part of what it hosts** (novox/hq ADR 0100: the store is reachable
+// chainBody is one chain's own lines, so an assertion cannot be satisfied by an identical line in
-// "from a container on the node itself").
+// another chain.
 //
-// The addresses a mesh-scoped rule admits are the machines' own on the private network. A container
+// **Written because that happened.** The rule letting this machine's own callers through appears in the
-// reaching a port on the machine it runs on comes from a bridge, matching none of them — and where the
+// input chain and, in the same words, in the forward chain. A test asserting on the whole rendered file
-// runtime routes directly, that packet is delivered to this machine rather than forwarded, so the
+// passed with the input chain's copy deleted — it was reading the forward chain's. ADR 0137's own tests
-// forward chain's allowance never sees it either.
+// say to assert per chain body for exactly this reason, and this file was not doing it.
+func chainBody(t *testing.T, nft, chain string) string {
+	t.Helper()
+	open := "\tchain " + chain + " {"
+	i := strings.Index(nft, open)
+	if i < 0 {
+		t.Fatalf("no chain %q in:\n%s", chain, nft)
+	}
+	rest := nft[i+len(open):]
+	j := strings.Index(rest, "\n\t}")
+	if j < 0 {
+		t.Fatalf("chain %q does not close in:\n%s", chain, nft)
+	}
+	return rest[:j]
+}
+
+// **Anything on this machine may call anything on this machine.**
 //
-// Measured, and it was an outage: after a machine was converged, every module reaching another by the
+// Local is not a boundary this mesh draws, and whether a caller sits in a container was never meant to
-// machine's own name timed out for eleven hours while the mesh reported the machine healthy.
+// change the answer. It did, because the chain asked about addresses: a caller on the machine carries
-func TestAMeshScopedPortAdmitsThisMachinesOwnGuests(t *testing.T) {
+// the machine's address and a caller in one of its containers carries a bridge address, so a rule
+// naming the machines' own addresses silently refused every container on them.
+//
+// Measured: a module reaching its database on its own machine's name timed out for eleven hours while
+// the machine itself could reach it (novox/hq 04-ISSUES/145).
+func TestAnythingOnThisMachineMayCallAnythingOnIt(t *testing.T) {
 	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
+		{Module: "private", Listens: []Listening{{Port: 9999, From: FromMachine}}},
 	}}, nil), []string{"10.10.0.1", "10.10.0.2"}, false, nil, []string{"eth0"}, "mesh0")
 
-	// The private network's own addresses, as before.
+	// In the INPUT chain, which is where a call to a service on this machine arrives. The forward
-	if !strings.Contains(nft, "ip saddr { 10.10.0.1, 10.10.0.2 } tcp dport 5432 accept") {
+	// chain carries the same line in the same words, so asserting on the whole file proves nothing.
-		t.Fatalf("the private network no longer reaches a mesh-scoped port:\n%s", nft)
+	input := chainBody(t, nft, "input")
+	if !strings.Contains(input, `iifname != { "eth0", "mesh0" } accept`) {
+		t.Fatalf("a caller on this machine cannot reach a service on it:\n%s", input)
+	}
+	// One rule, for every service here — not a line per port that only covers the ports somebody
+	// remembered to think about.
+	if strings.Contains(input, `iifname != { "eth0", "mesh0" } tcp dport 5432`) {
+		t.Fatalf("the local allowance is still written per port:\n%s", input)
+	}
+	// And the private network still reaches what is exposed to it, which is a different question.
+	if !strings.Contains(input, "ip saddr { 10.10.0.1, 10.10.0.2 } tcp dport 5432 accept") {
+		t.Fatalf("the private network no longer reaches a service exposed to it:\n%s", input)
+	}
+}
+
+// The three reaches, as three lines. This is the whole of what the filter says about who may call what.
+func TestTheThreeReachesAreThreeLines(t *testing.T) {
+	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
+		{Module: "internal-only", Listens: []Listening{{Port: 5432, From: FromMesh}}},
+		{Module: "public", Listens: []Listening{{Port: 443, From: FromEverywhere}}},
+	}}, nil), []string{"10.10.0.1"}, false, nil, []string{"eth0"}, "mesh0")
+
+	input := chainBody(t, nft, "input")
+	for what, want := range map[string]string{
+		"on this machine":          `iifname != { "eth0", "mesh0" } accept`,
+		"over the private network": "ip saddr { 10.10.0.1 } tcp dport 5432 accept",
+		"from anywhere":            "tcp dport 443 accept",
+	} {
+		if !strings.Contains(input, want) {
+			t.Fatalf("a caller %s cannot reach what is exposed to it (%q):\n%s", what, want, input)
 		}
-	// And this machine's guests, by the link they arrive on.
-	if !strings.Contains(nft, `iifname != { "eth0", "mesh0" } tcp dport 5432 accept`) {
-		t.Fatalf("a container on this machine cannot reach a mesh-scoped port on it, which is the "+
-			"outage this test exists for:\n%s", nft)
 	}
 }