diff --git a/internal/catalogue/filtering.go b/internal/catalogue/filtering.go index 8824801..1e202f3 100644 --- a/internal/catalogue/filtering.go +++ b/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)) } diff --git a/internal/catalogue/filtering_test.go b/internal/catalogue/filtering_test.go index e326c40..0cd3bf4 100644 --- a/internal/catalogue/filtering_test.go +++ b/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 -// "from a container on the node itself"). +// chainBody is one chain's own lines, so an assertion cannot be satisfied by an identical line in +// another chain. // -// The addresses a mesh-scoped rule admits are the machines' own on the private network. A container -// reaching a port on the machine it runs on comes from a bridge, matching none of them — and where the -// runtime routes directly, that packet is delivered to this machine rather than forwarded, so the -// forward chain's allowance never sees it either. +// **Written because that happened.** The rule letting this machine's own callers through appears in the +// input chain and, in the same words, in the forward chain. A test asserting on the whole rendered file +// passed with the input chain's copy deleted — it was reading the forward chain's. ADR 0137's own tests +// 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 -// machine's own name timed out for eleven hours while the mesh reported the machine healthy. -func TestAMeshScopedPortAdmitsThisMachinesOwnGuests(t *testing.T) { +// Local is not a boundary this mesh draws, and whether a caller sits in a container was never meant to +// change the answer. It did, because the chain asked about addresses: a caller on the machine carries +// 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. - if !strings.Contains(nft, "ip saddr { 10.10.0.1, 10.10.0.2 } tcp dport 5432 accept") { - t.Fatalf("the private network no longer reaches a mesh-scoped port:\n%s", nft) + // In the INPUT chain, which is where a call to a service on this machine arrives. The forward + // chain carries the same line in the same words, so asserting on the whole file proves nothing. + 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) } - // 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) + // 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) + } } }