From 864cdea4c656db5313983d751ded512847acc331 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 29 Sep 2026 13:30:32 +0200 Subject: [PATCH] Anything on this machine may call anything on this machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local is not a boundary this mesh draws. A service here is callable by everything else 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. One rule for every service here, replacing the line-per-port added an hour ago, which only ever covered the ports somebody remembered to think about. The three reaches are now three lines: on this machine, over the private network, from anywhere. The tests assert per chain body, because the forward chain carries the same line in the same words and an assertion on the whole file passed with the input chain's copy deleted — which is what ADR 0137's own tests say to do and this file was not doing. --- internal/catalogue/filtering.go | 39 +++++++------- internal/catalogue/filtering_test.go | 79 ++++++++++++++++++++++------ 2 files changed, 83 insertions(+), 35 deletions(-) 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) + } } } -- 2.54.0