A computed module says what its machine opens, so a hub can be filtered

The machine that most needed a firewall was the one that could not have one. A
hub is dialled by every node at other sites and needs its port open; a machine
that is not a hub dials out and needs nothing open. They are the same module,
and `listens` in a manifest is one answer for every machine that runs it — so
the machine a static answer gets wrong is the one facing the public internet.

A generator can now say what it opens, in a second interface rather than a
method on every generator: most have nothing to say here, and requiring an
empty method of each would be a cost paid everywhere for one caller.

The port is the one in the endpoint, which is where the interface takes its
ListenPort from. One source, so a rule set cannot open a port the interface is
not on. Open to everywhere and deliberately: a node at another site is not on
the private network until this port lets it on, so restricting it to the mesh
would be a rule that can never be satisfied by the thing it exists for.

And a generator that cannot say is refused rather than read as silence. Closing
a port on the evidence of a failure to look is how a machine is severed by a
fault somewhere else — and the machine it would sever is the hub, whose only
route to being fixed is the network it just closed.

internal/catalogue/declaration.go

@@ -26,6 +26,22 @@ type Generator interface {
 	Resources(node string) ([]map[string]any, bool, error)
 }
 
+// OpensPorts is a generator that also says what its resources accept connections on.
+//
+// **Separate from Generator, because most generators have nothing to say here** and requiring an
+// empty method of each would be a cost paid everywhere for one caller.
+//
+// It exists because a static field cannot express this. A hub accepts connections from every node
+// at other sites; a machine that is not a hub dials out and needs nothing open — and they are the
+// same module. `listens` in a manifest is one answer for every machine that runs it, so the
+// machine that most needs filtering, the one facing the public internet, was the one whose rule
+// set would have closed its own overlay.
+type OpensPorts interface {
+	// Listens is what this node accepts on because of what was computed for it. Nothing is the
+	// ordinary answer: most machines running a computed module open no port at all.
+	Listens(node string) ([]Listening, error)
+}
+
 // Grant is one consumer's credential, on the machine that must create it.
 type Grant struct {
 	// Provision is what was required.
@@ -93,7 +109,11 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) {
 
 	// Once, from every module's listens -- not per module. A module receiving only its own ports
 	// would write a rule set that closed every other module on the machine.
-	filtering := AsNftables(r.Filtering(), with.Mesh)
+	rules, err := r.Filtering(with.Generators)
+	if err != nil {
+		return nil, err
+	}
+	filtering := AsNftables(rules, with.Mesh)
 
 	var out []map[string]any
 	for _, m := range r.Modules {

internal/catalogue/filtering.go

@@ -31,7 +31,7 @@ type Rule struct {
 // a consequence of what runs on it, not a second list kept in step by hand. Nothing else opens a
 // port: **what is not declared is closed**, which is the property that makes the derivation worth
 // having rather than merely tidy.
-func (r Resolution) Filtering() []Rule {
+func (r Resolution) Filtering(computed map[string]Generator) ([]Rule, error) {
 	// Keyed by what actually distinguishes an opening. Two modules wanting :443 from the mesh is
 	// one rule with two sources; one wanting it from the mesh and another from anywhere is two,
 	// and they are collapsed below -- deliberately, and only in the widening direction.
@@ -42,7 +42,26 @@ func (r Resolution) Filtering() []Rule {
 	}
 	found := map[opening]*Rule{}
 	for _, m := range r.Modules {
-		for _, l := range m.Listens {
+		// What a module says, and what was computed for it on this machine. The second is how a
+		// hub's own port is derived: it is a fact about this machine's place in the mesh, which
+		// the module cannot know and the mesh cannot avoid knowing.
+		opens := m.Listens
+		if m.Computed != "" {
+			if generator, known := computed[m.Computed].(OpensPorts); known {
+				also, err := generator.Listens(r.Node)
+				if err != nil {
+					// **Refused, not treated as nothing.** A generator that cannot say what a
+					// machine opens is not one that says it opens nothing, and closing a port on
+					// the evidence of a failure to look is how a machine is severed by a fault
+					// somewhere else entirely.
+					return nil, fmt.Errorf(
+						"%s could not say what %s opens, so no rule set can be computed for it: %w",
+						m.Module, r.Node, err)
+				}
+				opens = append(append([]Listening{}, opens...), also...)
+			}
+		}
+		for _, l := range opens {
 			at := opening{port: l.Port, protocol: l.At(), from: l.From}
 			rule, seen := found[at]
 			if !seen {
@@ -70,7 +89,7 @@ func (r Resolution) Filtering() []Rule {
 		}
 		return out[a].From < out[b].From
 	})
-	return widest(out)
+	return widest(out), nil
 }
 
 // widest drops a rule that another already covers.

internal/catalogue/filtering_test.go

@@ -1,6 +1,7 @@
 package catalogue
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"testing"
@@ -55,7 +56,7 @@ func TestTheRuleSetIsEveryAssignedModulesPorts(t *testing.T) {
 		{Module: "web", Listens: []Listening{{Port: 443, From: FromEverywhere}}},
 		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
 	}}
-	rules := r.Filtering()
+	rules := mustFilter(t, r, nil)
 	if len(rules) != 2 {
 		t.Fatalf("a node's rule set lost a module's ports: %+v", rules)
 	}
@@ -70,7 +71,7 @@ func TestTwoModulesWantingOnePortAreBothNamed(t *testing.T) {
 		{Module: "web", Listens: []Listening{{Port: 443, From: FromEverywhere, Why: "the site"}}},
 		{Module: "board", Listens: []Listening{{Port: 443, From: FromEverywhere, Why: "the board"}}},
 	}}
-	rules := r.Filtering()
+	rules := mustFilter(t, r, nil)
 	if len(rules) != 1 {
 		t.Fatalf("one port became %d rules", len(rules))
 	}
@@ -90,7 +91,7 @@ func TestAPortOpenToEveryoneIsNotAlsoRestrictedToTheMesh(t *testing.T) {
 		{Module: "web", Listens: []Listening{{Port: 443, From: FromEverywhere}}},
 		{Module: "board", Listens: []Listening{{Port: 443, From: FromMesh}}},
 	}}
-	rules := r.Filtering()
+	rules := mustFilter(t, r, nil)
 	if len(rules) != 1 {
 		t.Fatalf("the same port was rendered twice, once restricting nothing: %+v", rules)
 	}
@@ -104,9 +105,9 @@ func TestAPortOpenToEveryoneIsNotAlsoRestrictedToTheMesh(t *testing.T) {
 
 // What is not declared is closed.
 func TestWhatNoModuleDeclaredIsClosed(t *testing.T) {
-	nft := AsNftables((Resolution{Modules: []Manifest{
+	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "web", Listens: []Listening{{Port: 443, From: FromEverywhere}}},
-	}}).Filtering(), []string{"198.51.100.2"})
+	}}, nil), []string{"198.51.100.2"})
 	// Naming the chain, not just the policy: the forward chain drops too, and an assertion on
 	// "policy drop" alone passes while the input chain accepts everything. It did, once, here.
 	if !strings.Contains(nft, "type filter hook input priority filter; policy drop;") {
@@ -159,9 +160,9 @@ func TestTheRuleSetDoesNotDecideWhatTheMachineForwards(t *testing.T) {
 
 // "From the mesh" is the addresses the mesh actually has.
 func TestFromTheMeshIsTheNodesTheMeshKnows(t *testing.T) {
-	nft := AsNftables((Resolution{Modules: []Manifest{
+	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
-	}}).Filtering(), []string{"198.51.100.2", "198.51.100.3"})
+	}}, nil), []string{"198.51.100.2", "198.51.100.3"})
 	if !strings.Contains(nft, "ip saddr { 198.51.100.2, 198.51.100.3 } tcp dport 5432 accept") {
 		t.Fatalf("a mesh-scoped port was not restricted to the mesh's addresses:\n%s", nft)
 	}
@@ -169,9 +170,9 @@ func TestFromTheMeshIsTheNodesTheMeshKnows(t *testing.T) {
 
 // The case that must not be widened silently.
 func TestAMeshPortOnANodeWithNoMeshIsClosedAndSaysSo(t *testing.T) {
-	nft := AsNftables((Resolution{Modules: []Manifest{
+	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
-	}}).Filtering(), nil)
+	}}, nil), nil)
 	if strings.Contains(nft, "dport 5432 accept") {
 		t.Fatalf("a port meant for the mesh was opened to everything:\n%s", nft)
 	}
@@ -182,9 +183,9 @@ func TestAMeshPortOnANodeWithNoMeshIsClosedAndSaysSo(t *testing.T) {
 
 // A port bound for something else on the same machine must not reach the network.
 func TestAMachineScopedPortIsNotOpened(t *testing.T) {
-	nft := AsNftables((Resolution{Modules: []Manifest{
+	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "cache", Listens: []Listening{{Port: 6379, From: FromMachine}}},
-	}}).Filtering(), []string{"198.51.100.2"})
+	}}, nil), []string{"198.51.100.2"})
 	if strings.Contains(nft, "dport 6379 accept") {
 		t.Fatalf("a port for this machine only was opened to the network:\n%s", nft)
 	}
@@ -224,9 +225,9 @@ func TestAskingForTheRuleSetWithNowhereToPutItIsRefused(t *testing.T) {
 // nftables matches the two address families separately, and one set holding both is a syntax
 // error — so the file fails to load, the service reports a fault, and the machine filters nothing.
 func TestAMeshOnBothAddressFamiliesRendersBoth(t *testing.T) {
-	nft := AsNftables((Resolution{Modules: []Manifest{
+	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
-	}}).Filtering(), []string{"198.51.100.2", "2001:db8::2"})
+	}}, nil), []string{"198.51.100.2", "2001:db8::2"})
 	if !strings.Contains(nft, "ip saddr { 198.51.100.2 } tcp dport 5432 accept") {
 		t.Fatalf("the machines with v4 addresses were dropped:\n%s", nft)
 	}
@@ -364,3 +365,96 @@ func TestAMachineOffTheNetworkIsBoundToItselfByAnAddressThatWorks(t *testing.T)
 	}
 	t.Fatal("nothing was written")
 }
+
+// mustFilter is the rule set, refusing to continue if it could not be computed.
+func mustFilter(t *testing.T, r Resolution, computed map[string]Generator) []Rule {
+	t.Helper()
+	rules, err := r.Filtering(computed)
+	if err != nil {
+		t.Fatalf("no rule set could be computed: %v", err)
+	}
+	return rules
+}
+
+// opensOnHub is a generator that says a machine opens a port because of where it sits.
+type opensOnHub struct{ hub string }
+
+func (opensOnHub) Resources(string) ([]map[string]any, bool, error) { return nil, true, nil }
+
+func (o opensOnHub) Listens(node string) ([]Listening, error) {
+	if node != o.hub {
+		return nil, nil
+	}
+	return []Listening{{Port: 51820, Protocol: "udp", From: FromEverywhere,
+		Why: "the private network"}}, nil
+}
+
+// cannotSay is a generator that does not know what a machine opens.
+type cannotSay struct{}
+
+func (cannotSay) Resources(string) ([]map[string]any, bool, error) { return nil, true, nil }
+func (cannotSay) Listens(string) ([]Listening, error) {
+	return nil, errors.New("this machine's endpoint has no port in it")
+}
+
+// A computed module contributes listens the way it contributes resources.
+//
+// A static field is one answer for every machine that runs the module, and a hub's own port is
+// not one of those: the machine that most needs filtering — the one facing the public internet —
+// is exactly the one a static answer gets wrong.
+func TestAComputedModuleOpensThePortItsMachineNeeds(t *testing.T) {
+	network := Manifest{Module: "networking", Computed: "mesh-network"}
+	gens := map[string]Generator{"mesh-network": opensOnHub{hub: "anchor"}}
+
+	onHub := mustFilter(t, Resolution{Node: "anchor", Modules: []Manifest{network}}, gens)
+	if len(onHub) != 1 || onHub[0].Port != 51820 {
+		t.Fatalf("the hub's own way onto the private network was not opened: %+v", onHub)
+	}
+	if onHub[0].Because[0] != "networking" {
+		t.Fatalf("the rule does not name what caused it: %+v", onHub[0])
+	}
+
+	// And the machine that dials out opens nothing, from the same module.
+	elsewhere := mustFilter(t, Resolution{Node: "laptop", Modules: []Manifest{network}}, gens)
+	if len(elsewhere) != 0 {
+		t.Fatalf("a machine nothing dials opened a port because another machine needed one: %+v",
+			elsewhere)
+	}
+}
+
+// What a module says and what was computed for it are both kept.
+func TestAComputedModulesOwnListensAreNotLost(t *testing.T) {
+	network := Manifest{Module: "networking", Computed: "mesh-network",
+		Listens: []Listening{{Port: 9, From: FromMesh, Why: "something the module always wants"}}}
+	rules := mustFilter(t, Resolution{Node: "anchor", Modules: []Manifest{network}},
+		map[string]Generator{"mesh-network": opensOnHub{hub: "anchor"}})
+	if len(rules) != 2 {
+		t.Fatalf("one of the two sources was dropped: %+v", rules)
+	}
+}
+
+// A generator that cannot say is refused, not read as silence.
+//
+// Closing a port on the evidence of a failure to look is how a machine is severed by a fault
+// somewhere else entirely — and the machine it would sever is the hub.
+func TestAGeneratorThatCannotSayRefusesTheRuleSet(t *testing.T) {
+	_, err := Resolution{Node: "anchor",
+		Modules: []Manifest{{Module: "networking", Computed: "mesh-network"}},
+	}.Filtering(map[string]Generator{"mesh-network": cannotSay{}})
+	if err == nil {
+		t.Fatal("a machine whose open ports could not be computed was given a rule set anyway")
+	}
+	if !strings.Contains(err.Error(), "anchor") {
+		t.Fatalf("the refusal does not name the machine: %v", err)
+	}
+}
+
+// A generator with nothing to say about ports is ordinary and must not be required to say so.
+func TestAGeneratorThatOpensNothingNeedsNoMethod(t *testing.T) {
+	rules := mustFilter(t, Resolution{Node: "anchor",
+		Modules: []Manifest{{Module: "names", Computed: "mesh-names"}}},
+		map[string]Generator{"mesh-names": computedOnce{}})
+	if len(rules) != 0 {
+		t.Fatalf("a generator that says nothing about ports opened one: %+v", rules)
+	}
+}

internal/overlay/generator.go

@@ -1,6 +1,12 @@
 package overlay
 
-import "encoding/json"
+import (
+	"encoding/json"
+	"fmt"
+	"strconv"
+
+	"github.com/novox/mesh-control/internal/catalogue"
+)
 
 // The private network as a module rather than as code beside the module system.
 //
@@ -195,3 +201,46 @@ func DomainManifest() map[string]any {
 // reaches the broker, which is what being in the mesh is — so it answers "not part of this" for
 // everyone instead of refusing for want of a hub.
 func Empty() *Generator { return &Generator{graph: Graph{}} }
+
+// Listens is the port this node accepts the private network on, which only a hub has.
+//
+// **A fact about this machine's place in the mesh, not about the module.** Every machine on the
+// network runs the same module; a hub is dialled by every node at other sites and needs its port
+// open, and a machine that is not a hub dials out and needs nothing open at all. A static field in
+// a manifest is one answer for every machine that runs it, so it cannot say this — and the machine
+// it would get wrong is the one facing the public internet, which is the machine that most needs
+// filtering.
+//
+// The port is the one in the endpoint, which is also where the interface takes its ListenPort
+// from. One source, so a rule set cannot open a port the interface is not on.
+func (g *Generator) Listens(node string) ([]catalogue.Listening, error) {
+	for _, n := range g.nodes {
+		if n.Name != node {
+			continue
+		}
+		if !n.Reachable() {
+			// It dials out and nothing dials it. Opening a port here would be opening one on a
+			// machine nothing connects to, which is not harmless — it is a rule with no source
+			// that somebody later has to work out the reason for.
+			return nil, nil
+		}
+		port := portOf(n.Endpoint)
+		if port == "" {
+			return nil, fmt.Errorf(
+				"%s is reachable at %q and no port can be read from it, so what it must accept "+
+					"the private network on is unknown", node, n.Endpoint)
+		}
+		number, err := strconv.Atoi(port)
+		if err != nil {
+			return nil, fmt.Errorf("%s is reachable at %q, and %q is not a port", node, n.Endpoint, port)
+		}
+		return []catalogue.Listening{{
+			Port: number, Protocol: "udp", From: catalogue.FromEverywhere,
+			// From everywhere, and deliberately: a node at another site is not on the private
+			// network until this port lets it on, so restricting this to the mesh would be a
+			// rule that can never be satisfied by the thing it exists for.
+			Why: "the private network — a node at another site has no other way in",
+		}}, nil
+	}
+	return nil, nil
+}

internal/overlay/opens_test.go

@@ -0,0 +1,113 @@
+package overlay
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-control/internal/catalogue"
+)
+
+func networkOf(t *testing.T, nodes []Node) *Generator {
+	t.Helper()
+	made, err := From(nodes, "10.42.0.0/16", "/var/lib/mesh-host/overlay.key")
+	if err != nil {
+		t.Fatal(err)
+	}
+	return made
+}
+
+// A hub is dialled by every node at other sites, and needs its port open. A machine that is not a
+// hub dials out and needs nothing open at all.
+//
+// They are the same module, which is why a static field in a manifest cannot say it — and the
+// machine it gets wrong is the one facing the public internet, which is the machine that most
+// needs filtering.
+func TestOnlyAMachineThatCanBeDialledOpensTheOverlaysPort(t *testing.T) {
+	network := networkOf(t, []Node{
+		{Name: "anchor", Key: "a", Endpoint: "198.51.100.10:51820", Site: "one", Hub: true},
+		{Name: "laptop", Key: "b", Site: "one"},
+	})
+
+	opens, err := network.Listens("anchor")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(opens) != 1 {
+		t.Fatalf("the machine every other one dials opens %d ports", len(opens))
+	}
+	if opens[0].Port != 51820 || opens[0].At() != "udp" {
+		t.Fatalf("the port is not the one the interface listens on: %+v", opens[0])
+	}
+	// From everywhere, and deliberately: a node at another site is not on the private network
+	// until this port lets it on, so restricting it to the mesh would be a rule that can never be
+	// satisfied by the thing it exists for.
+	if opens[0].From != catalogue.FromEverywhere {
+		t.Fatalf("the way onto the private network is restricted to the private network: %+v", opens[0])
+	}
+	if opens[0].Why == "" {
+		t.Fatal("a port is opened and nothing says why, which is what makes a rule set unreadable")
+	}
+
+	// And the machine nothing dials opens nothing. Not harmless to get wrong: a rule with no
+	// reason is one somebody later has to work out the reason for.
+	quiet, err := network.Listens("laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(quiet) != 0 {
+		t.Fatalf("a machine nothing dials opened %d port(s)", len(quiet))
+	}
+}
+
+// The port comes from the endpoint, which is where the interface takes its ListenPort from. One
+// source, so a rule set cannot open a port the interface is not on.
+func TestTheOpenedPortIsTheOneTheInterfaceListensOn(t *testing.T) {
+	network := networkOf(t, []Node{
+		{Name: "anchor", Key: "a", Endpoint: "198.51.100.10:60000", Site: "one", Hub: true},
+	})
+	opens, err := network.Listens("anchor")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(opens) != 1 || opens[0].Port != 60000 {
+		t.Fatalf("the rule set would open a port the interface is not on: %+v", opens)
+	}
+
+	written, err := Declaration(Node{Name: "anchor", Key: "a", Address: "10.42.0.1",
+		Endpoint: "198.51.100.10:60000", Hub: true}, nil, "/k")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(written), "ListenPort = 60000") {
+		t.Fatalf("the interface and the rule set disagree about the port:\n%s", written)
+	}
+}
+
+// An endpoint with no port is refused rather than treated as a machine that opens nothing.
+//
+// A generator that cannot say what a machine opens is not one that says it opens nothing, and
+// closing a port on the evidence of a failure to look is how a machine is severed by a fault
+// somewhere else entirely.
+func TestAnEndpointWithNoPortIsRefusedRatherThanTakenAsSilence(t *testing.T) {
+	network := networkOf(t, []Node{
+		{Name: "anchor", Key: "a", Endpoint: "198.51.100.10", Site: "one", Hub: true},
+	})
+	if _, err := network.Listens("anchor"); err == nil {
+		t.Fatal("a machine whose port could not be read was treated as opening nothing")
+	}
+}
+
+// A machine the network has never heard of opens nothing, and that is an answer rather than an
+// error: a node assigned the module before it is placed is in exactly that state.
+func TestAMachineNotOnTheNetworkOpensNothing(t *testing.T) {
+	network := networkOf(t, []Node{
+		{Name: "anchor", Key: "a", Endpoint: "198.51.100.10:51820", Site: "one", Hub: true},
+	})
+	opens, err := network.Listens("a-stranger")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(opens) != 0 {
+		t.Fatalf("a machine not on the network opened %d port(s)", len(opens))
+	}
+}