From cc252472e26e24f265a00411b9a1a584a4b82dc9 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 26 Sep 2026 22:33:27 +0200 Subject: [PATCH] A taken tunnel brings its ListenPort, even on a node the hub cannot dial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A home node behind NAT (no Endpoint → not Reachable) that took over a tunnel must still listen on that tunnel's port: its LAN peers dial it there. ListenPort was gated on Reachable, which conflated 'a peer dials me here' with 'the hub can dial me' — so the takeover guard refused overlay-up, and the guard's suggested remedy (re-place with an endpoint) breaks a NAT'd node's path: it stops keepalive and hands the hub a private LAN address to dial. TakeOver now carries the found tunnel's port (already known to the controller), and the interface listens on it when the node is not otherwise reachable. Two tests; Endpoint-reachable nodes keep the old path unchanged. --- cmd/mesh-controller/network.go | 2 +- internal/overlay/declaration.go | 6 ++++++ internal/overlay/declaration_test.go | 26 ++++++++++++++++++++++++++ internal/overlay/graph.go | 5 +++++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/cmd/mesh-controller/network.go b/cmd/mesh-controller/network.go index 5bdf704..83243ee 100644 --- a/cmd/mesh-controller/network.go +++ b/cmd/mesh-controller/network.go @@ -256,7 +256,7 @@ func network(ctx context.Context, inv *inventory.Inventory, on map[string]bool, "Re-place it — `overlay place %s --hub --endpoint :%d …` — and push again; "+ "nothing was composed", p.Name, t.Interface, wrong, p.Name, t.Port) } - n.TakesOver = &overlay.TakeOver{Interface: t.Interface, Unit: t.Unit, Config: t.Config} + n.TakesOver = &overlay.TakeOver{Interface: t.Interface, Unit: t.Unit, Config: t.Config, Port: t.Port} } if p.Hub { for _, c := range carried { diff --git a/internal/overlay/declaration.go b/internal/overlay/declaration.go index 65b4e30..8950b69 100644 --- a/internal/overlay/declaration.go +++ b/internal/overlay/declaration.go @@ -123,6 +123,12 @@ func config(node Node, peers []Peer, keyPath string) string { if port := portOf(node.Endpoint); port != "" { fmt.Fprintf(&b, "ListenPort = %s\n", port) } + } else if node.TakesOver != nil && node.TakesOver.Port != 0 { + // Not dialable from the hub, but a LAN peer dials this node on the tunnel it took over, + // so the mesh's interface must listen on that same port (novox/hq: a taken tunnel brings + // its port). Without this the takeover guard refuses overlay-up, and re-placing the node + // with an endpoint — the guard's suggested remedy — breaks a NAT'd node's path. + fmt.Fprintf(&b, "ListenPort = %d\n", node.TakesOver.Port) } // The private key is set from a file the node wrote, so it never appears here and never // travelled. Everything else in this file came from the mesh; this one line is the node's. diff --git a/internal/overlay/declaration_test.go b/internal/overlay/declaration_test.go index 8a941ff..d763816 100644 --- a/internal/overlay/declaration_test.go +++ b/internal/overlay/declaration_test.go @@ -260,3 +260,29 @@ func TestTakingOverAFoundTunnelIsSaidOnTheInterfacesService(t *testing.T) { } } } + +func TestATakenTunnelBringsItsListenPortEvenWhenNotDialable(t *testing.T) { + // A home node behind NAT (no Endpoint, so not Reachable) that took over a tunnel must still + // listen on that tunnel's port, because its LAN peers dial it there (novox/hq: a taken tunnel + // brings its port). Without this the takeover guard refuses overlay-up. + config, _ := declarationFor(t, Node{ + Name: "shanks", Key: "SPOKE", Address: "10.10.0.3", + TakesOver: &TakeOver{Interface: "wg0", Unit: "wg-quick@wg0", Port: 51820}, + }, nil) + + if !strings.Contains(config, "ListenPort = 51820") { + t.Fatalf("a taken tunnel's port must be the mesh interface's ListenPort:\n%s", config) + } + if strings.Contains(config, "Endpoint =") { + t.Error("a node that only listens for LAN peers must not advertise an endpoint") + } +} + +func TestANodeWithNoTunnelAndNoEndpointStillListensOnNothing(t *testing.T) { + // The guard against over-emitting: a plain spoke with neither an endpoint nor a taken tunnel + // writes no ListenPort — it purely dials out. + config, _ := declarationFor(t, Node{Name: "laptop", Key: "K", Address: "10.10.0.9"}, nil) + if strings.Contains(config, "ListenPort") { + t.Fatalf("a dial-only node needs no ListenPort:\n%s", config) + } +} diff --git a/internal/overlay/graph.go b/internal/overlay/graph.go index c4f9c28..3a11bce 100644 --- a/internal/overlay/graph.go +++ b/internal/overlay/graph.go @@ -50,6 +50,11 @@ type TakeOver struct { Interface string Unit string Config string + // Port is the port the found tunnel listened on. The mesh's interface must listen on it too, + // even on a node that is not dialable from the hub: a home node's LAN peers dial it there + // (novox/hq: a taken tunnel brings its port). Listening is "a peer dials me here"; it is not + // "the hub can dial me", which is Reachable — the two were conflated. + Port int } // HostPrefix is one address as a route: /32 for IPv4, /128 for IPv6. -- 2.54.0