internal/catalogue/declaration.go

@@ -207,7 +207,7 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) {
 	if err != nil {
 		return nil, err
 	}
-	filtering := AsNftables(rules, with.Mesh)
+	filtering := AsNftables(rules, with.Mesh, r.PublicDomain != "")
 
 	var out []map[string]any
 	for _, m := range r.Modules {

internal/catalogue/filtering.go

@@ -209,7 +209,14 @@ func widest(rules []Rule) []Rule {
 // `mesh` is rendered as the addresses of the nodes that are actually on the private network, not
 // as a subnet. The mesh knows every address; a subnet is a guess that stays wrong quietly, and
 // the set shrinks when a node leaves without anybody editing anything.
-func AsNftables(rules []Rule, mesh []string) string {
+// SSHPort is the one port a machine may never lose, whatever else it is told.
+const SSHPort = 22
+
+// AsNftables renders a rule set as a complete nftables configuration.
+//
+// `outward` says this machine is reachable from outside the mesh, which is the only thing that
+// decides whether ssh is answered there as well as on the private network.
+func AsNftables(rules []Rule, mesh []string, outward bool) string {
 	var b strings.Builder
 	b.WriteString("# Computed by the mesh from what is assigned to this node.\n")
 	b.WriteString("# Edits are lost on the next declaration; change a module's listens instead.\n\n")
@@ -231,6 +238,34 @@ func AsNftables(rules []Rule, mesh []string) string {
 	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")
 
+	// **ssh, always, and not because a module asked.**
+	//
+	// Every other line in this chain is derived from what is assigned here, which is the whole
+	// point of the mechanism. This one is not, and the exception is deliberate: the rules are
+	// computed from what the mesh knows, a mesh part-way through adopting a machine knows almost
+	// nothing, and the first port to close is the one used to fix it. The session that loads these
+	// rules survives on conntrack until it drops, and then the machine is reached from a rescue
+	// console (novox/hq issue 047).
+	//
+	// From the private network always, because that is how machines reach each other. From
+	// everywhere when this machine faces outward, because that is the only way in on the day the
+	// private network is what broke.
+	if four, six := byFamily(mesh); len(four) > 0 || len(six) > 0 {
+		b.WriteString("\n\t\t# ssh, from the mesh — never derived, never closed\n")
+		if len(four) > 0 {
+			b.WriteString(fmt.Sprintf("\t\tip saddr { %s } tcp dport %d accept\n",
+				strings.Join(four, ", "), SSHPort))
+		}
+		if len(six) > 0 {
+			b.WriteString(fmt.Sprintf("\t\tip6 saddr { %s } tcp dport %d accept\n",
+				strings.Join(six, ", "), SSHPort))
+		}
+	}
+	if outward {
+		b.WriteString("\t\t# and from outside, because this machine faces it\n")
+		b.WriteString(fmt.Sprintf("\t\ttcp dport %d accept\n", SSHPort))
+	}
+
 	if len(rules) > 0 {
 		b.WriteString("\n")
 	}
@@ -277,18 +312,82 @@ func AsNftables(rules []Rule, mesh []string) string {
 	// Nothing this node sends is filtered, and it is stated rather than left to nftables' default
 	// so that reading this file answers the question instead of requiring the reader to know it.
 	b.WriteString("\tchain output {\n\t\ttype filter hook output priority filter; policy accept;\n\t}\n")
-	// **No forward chain, deliberately.** What this machine forwards is the container runtime's
-	// business — docker writes its own rules for the bridges it creates, and a second table
-	// hooking forward is consulted as well as those, so a drop here drops container traffic that
-	// docker explicitly allowed. Every container on the node stops, including the control plane.
+
+	// **A forward chain, because without one this firewall does not cover container ports.**
 	//
-	// The mesh has no knowledge with which to compute a forwarding policy: nothing in a manifest
-	// says what a machine routes. Writing one anyway would be a rule nothing derives, which is the
-	// fault this whole mechanism exists to remove.
+	// It used to have none, on the reasoning that a drop here would drop container traffic the
+	// runtime explicitly allowed and stop every container on the node. The first half of that is
+	// true. The conclusion was not: a published port is redirected and then *forwarded*, so it
+	// never reaches the input chain above, and a firewall with no forward chain says nothing at all
+	// about the ports most worth protecting. Rehearsed on three machines: loading these rules
+	// refused a port on the host and left a published container port reachable (novox/hq issue 047).
+	//
+	// The way through is the one the system being replaced already used: deny by default here, and
+	// then explicitly allow the runtime's own networks, so containers keep working while everything
+	// else has to be asked for.
+	b.WriteString("\tchain forward {\n")
+	b.WriteString("\t\ttype filter hook forward priority filter; policy drop;\n")
+	b.WriteString("\t\tct state established,related accept\n")
+	b.WriteString("\t\tct state invalid drop\n")
+	b.WriteString("\n")
+	// What the container runtime created. Without these, denying by default stops every container
+	// on the machine — which is exactly the failure the absent chain was avoiding, avoided properly.
+	for _, network := range runtimeNetworks {
+		b.WriteString(fmt.Sprintf("\t\t# %s\n", network.why))
+		b.WriteString(fmt.Sprintf("\t\tip saddr %s accept\n", network.cidr))
+	}
+
+	if len(rules) > 0 {
+		b.WriteString("\n")
+	}
+	for _, rule := range rules {
+		for i, module := range rule.Because {
+			why := ""
+			if i < len(rule.Why) && rule.Why[i] != "" {
+				why = " — " + rule.Why[i]
+			}
+			b.WriteString(fmt.Sprintf("\t\t# %s%s\n", module, why))
+		}
+		// **Matched on where the packet was originally addressed, not where it is going now.**
+		// By the time a packet reaches this chain the runtime has already rewritten its
+		// destination to the container's own port, so matching the port a rule names would match
+		// nothing. Conntrack remembers what was asked for, which is the port the machine
+		// publishes and the only one a client ever knew.
+		switch rule.From {
+		case FromMachine:
+			b.WriteString(fmt.Sprintf("\t\t# %s/%d is this machine only, and is not forwarded\n",
+				rule.Protocol, rule.Port))
+		case FromMesh:
+			four, six := byFamily(mesh)
+			if len(four) > 0 {
+				b.WriteString(fmt.Sprintf("\t\tip saddr { %s } ct original proto-dst %d accept\n",
+					strings.Join(four, ", "), rule.Port))
+			}
+			if len(six) > 0 {
+				b.WriteString(fmt.Sprintf("\t\tip6 saddr { %s } ct original proto-dst %d accept\n",
+					strings.Join(six, ", "), rule.Port))
+			}
+		case FromEverywhere:
+			b.WriteString(fmt.Sprintf("\t\tct original proto-dst %d accept\n", rule.Port))
+		}
+	}
+	b.WriteString("\t}\n")
 	b.WriteString("}\n")
 	return b.String()
 }
 
+// runtimeNetworks are the container runtime's own networks, which must keep working when the
+// forward chain denies by default.
+//
+// Taken from what the system being replaced allows, which has been carrying this machine's traffic
+// for months: the runtime's bridge range and the range its compose files are given. A machine whose
+// runtime is configured with something else needs this to say so — which is a thing the mesh cannot
+// derive and a reason this list is named here rather than computed.
+var runtimeNetworks = []struct{ cidr, why string }{
+	{"172.16.0.0/12", "the container runtime's bridge networks"},
+	{"192.168.128.0/17", "the networks its compose files are given"},
+}
+
 // byFamily splits addresses into the two nftables understands separately.
 //
 // `ip saddr` and `ip6 saddr` are different matches, and one set holding both families is a syntax

internal/catalogue/filtering_test.go

@@ -79,7 +79,7 @@ func TestTwoModulesWantingOnePortAreBothNamed(t *testing.T) {
 		t.Fatalf("a module that wanted this port open is not named: %+v", rules[0])
 	}
 	// The consequence, which is the reason this matters: removing web must not read as closing 443.
-	nft := AsNftables(rules, nil)
+	nft := AsNftables(rules, nil, false)
 	if !strings.Contains(nft, "web") || !strings.Contains(nft, "board") {
 		t.Fatalf("the rendered rule set does not name both sources:\n%s", nft)
 	}
@@ -107,13 +107,16 @@ func TestAPortOpenToEveryoneIsNotAlsoRestrictedToTheMesh(t *testing.T) {
 func TestWhatNoModuleDeclaredIsClosed(t *testing.T) {
 	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "web", Listens: []Listening{{Port: 443, From: FromEverywhere}}},
-	}}, nil), []string{"198.51.100.2"})
+	}}, nil), []string{"198.51.100.2"}, false)
 	// 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;") {
 		t.Fatalf("the input chain does not drop by default, so nothing is filtered:\n%s", nft)
 	}
-	if strings.Contains(nft, "dport 22") {
+	// Some port nothing asked for. **Not 22**, which this used to use: ssh is now the one line in
+	// the chain that nothing derives, because a mesh that has been assigned almost nothing computes
+	// a firewall that shuts the port used to fix it (novox/hq issue 047).
+	if strings.Contains(nft, "dport 9999") {
 		t.Fatalf("a port no module declared was opened:\n%s", nft)
 	}
 	// Dropping by default must not mean unreachable: the node's own outbound link to the broker
@@ -131,7 +134,7 @@ func TestWhatNoModuleDeclaredIsClosed(t *testing.T) {
 // `flush ruleset` would do the first and not the second: it empties every table on the machine,
 // including the ones the container runtime writes for its bridges.
 func TestReloadingReplacesOnlyTheMeshsOwnRules(t *testing.T) {
-	nft := AsNftables(nil, nil)
+	nft := AsNftables(nil, nil, false)
 	if strings.Contains(nft, "flush ruleset") {
 		t.Fatalf("loading the rule set empties every table on the machine:\n%s", nft)
 	}
@@ -145,16 +148,54 @@ func TestReloadingReplacesOnlyTheMeshsOwnRules(t *testing.T) {
 	}
 }
 
-// The rule set governs what reaches this machine, and nothing else.
+// The rule set governs what is forwarded too, or it does not govern container ports at all.
 //
-// A forward policy would be consulted alongside the container runtime's own rules, so dropping
-// there stops container traffic the runtime allowed -- every container on the node, control plane
-// included. And nothing in a manifest says what a machine routes, so there is nothing to derive it
-// from: a forwarding rule here would be exactly the undeclared rule this mechanism removes.
-func TestTheRuleSetDoesNotDecideWhatTheMachineForwards(t *testing.T) {
-	nft := AsNftables(nil, []string{"198.51.100.2"})
-	if strings.Contains(nft, "hook forward") {
-		t.Fatalf("the rule set filters forwarding, which stops every container on the node:\n%s", nft)
+// **This reverses an earlier decision, and keeps the thing that decision was protecting.** There was
+// no forward chain, on the reasoning that dropping there stops container traffic the runtime
+// allowed — every container on the node, control plane included. That much is true. What it missed
+// is that a published port is redirected and then forwarded, so it never reaches the input chain,
+// and a firewall without a forward chain is silent about exactly the ports worth protecting
+// (novox/hq issue 047).
+//
+// So the chain exists and denies by default, and the runtime's own networks are allowed explicitly
+// — which is how the system being replaced has been doing it on these machines for months.
+func TestWhatIsForwardedIsGovernedToo(t *testing.T) {
+	nft := AsNftables(nil, []string{"198.51.100.2"}, false)
+	if !strings.Contains(nft, "hook forward priority filter; policy drop") {
+		t.Fatalf("forwarded traffic is not governed, so container ports are open:\n%s", nft)
+	}
+}
+
+// And containers keep working, which is the whole reason the chain was left out before.
+func TestTheRuntimesOwnNetworksKeepWorking(t *testing.T) {
+	nft := AsNftables(nil, []string{"198.51.100.2"}, false)
+	for _, network := range []string{"172.16.0.0/12", "192.168.128.0/17"} {
+		if !strings.Contains(nft, "ip saddr "+network+" accept") {
+			t.Fatalf("%s is not allowed, so denying by default stops every container:\n%s", network, nft)
+		}
+	}
+}
+
+// A published port is matched by what the client asked for, not by where the packet ends up.
+//
+// The runtime rewrites the destination before this chain sees it, so a rule naming the published
+// port would match nothing and the port would stay shut while reporting itself open.
+func TestAPublishedPortIsMatchedByWhatWasAskedFor(t *testing.T) {
+	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
+		{Module: "web", Listens: []Listening{{Port: 8080, From: FromEverywhere}}},
+	}}, nil), []string{"198.51.100.2"}, false)
+	if !strings.Contains(nft, "ct original proto-dst 8080 accept") {
+		t.Fatalf("the forwarded rule does not match the port a client asked for:\n%s", nft)
+	}
+}
+
+// And a mesh-scoped port stays mesh-scoped when it is reached through the runtime.
+func TestAMeshScopedPortIsMeshScopedWhenForwarded(t *testing.T) {
+	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
+		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
+	}}, nil), []string{"198.51.100.2"}, false)
+	if !strings.Contains(nft, "ip saddr { 198.51.100.2 } ct original proto-dst 5432 accept") {
+		t.Fatalf("a mesh-only port is reachable from anywhere once forwarded:\n%s", nft)
 	}
 }
 
@@ -162,7 +203,7 @@ func TestTheRuleSetDoesNotDecideWhatTheMachineForwards(t *testing.T) {
 func TestFromTheMeshIsTheNodesTheMeshKnows(t *testing.T) {
 	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
-	}}, nil), []string{"198.51.100.2", "198.51.100.3"})
+	}}, nil), []string{"198.51.100.2", "198.51.100.3"}, false)
 	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)
 	}
@@ -172,7 +213,7 @@ func TestFromTheMeshIsTheNodesTheMeshKnows(t *testing.T) {
 func TestAMeshPortOnANodeWithNoMeshIsClosedAndSaysSo(t *testing.T) {
 	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
-	}}, nil), nil)
+	}}, nil), nil, false)
 	if strings.Contains(nft, "dport 5432 accept") {
 		t.Fatalf("a port meant for the mesh was opened to everything:\n%s", nft)
 	}
@@ -185,7 +226,7 @@ func TestAMeshPortOnANodeWithNoMeshIsClosedAndSaysSo(t *testing.T) {
 func TestAMachineScopedPortIsNotOpened(t *testing.T) {
 	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "cache", Listens: []Listening{{Port: 6379, From: FromMachine}}},
-	}}, nil), []string{"198.51.100.2"})
+	}}, nil), []string{"198.51.100.2"}, false)
 	if strings.Contains(nft, "dport 6379 accept") {
 		t.Fatalf("a port for this machine only was opened to the network:\n%s", nft)
 	}
@@ -227,7 +268,7 @@ func TestAskingForTheRuleSetWithNowhereToPutItIsRefused(t *testing.T) {
 func TestAMeshOnBothAddressFamiliesRendersBoth(t *testing.T) {
 	nft := AsNftables(mustFilter(t, Resolution{Modules: []Manifest{
 		{Module: "store", Listens: []Listening{{Port: 5432, From: FromMesh}}},
-	}}, nil), []string{"198.51.100.2", "2001:db8::2"})
+	}}, nil), []string{"198.51.100.2", "2001:db8::2"}, false)
 	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)
 	}
@@ -622,3 +663,30 @@ func TestExposureRefusesAPortNotListenedOnAndABadSource(t *testing.T) {
 		t.Error("a source that is not mesh/anywhere/machine was accepted")
 	}
 }
+
+// ssh survives whatever the mesh does or does not know about this machine.
+//
+// **The one rule here that nothing derives.** Every other line comes from what is assigned, which
+// is the point of the mechanism — and a mesh part-way through adopting a machine has been assigned
+// almost nothing, so what it computes is a chain that drops the port used to fix it. The session
+// loading the rules lives on conntrack until it drops, and then the machine is reached from a
+// rescue console (novox/hq issue 047).
+func TestSSHIsOpenFromTheMeshEvenWhenNothingIsAssigned(t *testing.T) {
+	nft := AsNftables(nil, []string{"198.51.100.2", "198.51.100.3"}, false)
+	if !strings.Contains(nft, "ip saddr { 198.51.100.2, 198.51.100.3 } tcp dport 22 accept") {
+		t.Fatalf("ssh is not open to the mesh, so a machine can lock everyone out:\n%s", nft)
+	}
+	// And not to the world, on a machine that does not face it.
+	if strings.Contains(nft, "\t\ttcp dport 22 accept") {
+		t.Fatalf("ssh is open to everywhere on a machine that faces inward only:\n%s", nft)
+	}
+}
+
+// And from outside as well, on a machine that faces outward — because that is the way in when the
+// private network is the thing that broke.
+func TestSSHIsOpenFromOutsideOnAMachineThatFacesIt(t *testing.T) {
+	nft := AsNftables(nil, []string{"198.51.100.2"}, true)
+	if !strings.Contains(nft, "\t\ttcp dport 22 accept") {
+		t.Fatalf("a machine reachable from outside does not answer ssh there:\n%s", nft)
+	}
+}