novox/mesh-controller
3
0
Fork 0
Code Issues Pull Requests 1 Releases Wiki Activity

The firewall governs what is forwarded, and never closes ssh #23

Merged
jschoubben merged 1 commits from feat/the-firewall-covers-forwarded-traffic into main 2026-09-14 13:27:35 +00:00
Conversation 0 Commits 1 Files Changed 3
+193 -26
3 changed files with 193 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/catalogue/declaration.go
+1 -1
View File
@@ -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
+107 -8
View File
@@ -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
+85 -17
View File
@@ -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)
}
}