From cbdbf6b7d3487b2ac3f9b3389181426016bed9ac Mon Sep 17 00:00:00 2001 From: jochens Date: Fri, 2 Oct 2026 11:53:53 +0200 Subject: [PATCH] Every physical link faces outside, up or down The filter accepts what does not arrive on a link the machine names as outward, and the host named only links carrying a default route. An unplugged wired port was left unfiltered for whenever it was plugged in (novox/hq issue 197). A link backed by a physical device is now named whether or not it is up. --- cmd/mesh-host/main.go | 2 +- internal/outward/links.go | 46 +++++++++++++++++++++++++++++--- internal/outward/links_test.go | 48 +++++++++++++++++++++++++++++----- 3 files changed, 85 insertions(+), 11 deletions(-) diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index fa43156..6b157b0 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -1421,7 +1421,7 @@ func applyAndKeep(ctx context.Context, opts options, raw []byte, signed *store.D // (novox/hq ADR 0140). Reported whatever the node's mode: a converged node's filter needs it, // and an adopted one becomes converged without a further round trip. A machine that cannot read // its own routing table says nothing rather than guessing, and is sent no filter. - if links, err := outward.Links(""); err != nil { + if links, err := outward.Links("", ""); err != nil { fmt.Fprintf(os.Stderr, "mesh-host: applied, and could not read which links face outside: %v\n", err) } else { report.Outward = links diff --git a/internal/outward/links.go b/internal/outward/links.go index f15edd7..689a19f 100644 --- a/internal/outward/links.go +++ b/internal/outward/links.go @@ -29,17 +29,32 @@ import ( // routing table without one. const ProcNet = "/proc/net" -// Links are the interfaces carrying a default route, for both address families, sorted and without -// repeats. +// SysClassNet is where the kernel lists the machine's network interfaces, one directory each. A +// parameter for the same reason. +const SysClassNet = "/sys/class/net" + +// Links are the interfaces carrying a default route, for both address families, and every interface +// backed by a physical device, sorted and without repeats. +// +// **A physical link faces outside whether or not it is up** (novox/hq issue 197). The filter accepts +// whatever did not arrive on a link named here, so a link left out of this list is not filtered at +// all. A cable unplugged when the machine last reported carries no default route, and was left out: +// plugged in, everything arriving on it was accepted until the next report and the next push — and a +// second physical link that never carries the default route was never filtered. A physical device is +// read from the kernel's own list, where it has a `device` entry; a bridge, a veth, the tunnel and the +// loopback have none, and stay what they are, this machine's own. // // A machine may have more than one: a laptop with a cable and a radio has two, and both face // outside. A machine with none — no route off itself — returns nothing, and the mesh refuses to // compose a filter for it rather than writing a rule around a link with no name, which would be a // rule set that does not load and a machine filtering nothing while its unit reports success. -func Links(procNet string) ([]string, error) { +func Links(procNet, sysClassNet string) ([]string, error) { if procNet == "" { procNet = ProcNet } + if sysClassNet == "" { + sysClassNet = SysClassNet + } seen := map[string]bool{} four, err := defaultsV4(filepath.Join(procNet, "route")) @@ -50,7 +65,11 @@ func Links(procNet string) ([]string, error) { if err != nil { return nil, err } - for _, name := range append(four, six...) { + devices, err := physical(sysClassNet) + if err != nil { + return nil, err + } + for _, name := range append(append(four, six...), devices...) { if name != "" && name != "lo" { seen[name] = true } @@ -64,6 +83,25 @@ func Links(procNet string) ([]string, error) { return out, nil } +// physical is every interface the kernel lists with a device behind it. A list that is not there is +// not an error — a machine without sysfs mounted reports what its routing table says, as before. +func physical(sysClassNet string) ([]string, error) { + entries, err := os.ReadDir(sysClassNet) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, err + } + var out []string + for _, e := range entries { + if _, err := os.Stat(filepath.Join(sysClassNet, e.Name(), "device")); err == nil { + out = append(out, e.Name()) + } + } + return out, nil +} + // defaultsV4 reads /proc/net/route, whose columns are // // Iface Destination Gateway Flags RefCnt Use Metric Mask ... diff --git a/internal/outward/links_test.go b/internal/outward/links_test.go index f1a2e6a..893a45a 100644 --- a/internal/outward/links_test.go +++ b/internal/outward/links_test.go @@ -32,7 +32,7 @@ func TestLinksAreTheOnesCarryingADefaultRoute(t *testing.T) { write(t, dir, "route", routeV4) write(t, dir, "ipv6_route", routeV6) - got, err := Links(dir) + got, err := Links(dir, t.TempDir()) if err != nil { t.Fatal(err) } @@ -52,7 +52,7 @@ func TestAZeroDestinationWithAMaskIsNotADefaultRoute(t *testing.T) { write(t, dir, "route", `Iface Destination Gateway Flags RefCnt Use Metric Mask MTU Window IRTT br-abc 00000000 00000000 0001 0 0 0 00FFFFFF 0 0 0 `) - got, err := Links(dir) + got, err := Links(dir, t.TempDir()) if err != nil { t.Fatal(err) } @@ -67,7 +67,7 @@ br-abc 00000000 00000000 0001 0 0 0 00FFFFFF 0 0 0 func TestNoDefaultRouteIsNoLinks(t *testing.T) { dir := t.TempDir() write(t, dir, "route", "Iface\tDestination\tGateway \tFlags\tRefCnt\tUse\tMetric\tMask\t\tMTU\tWindow\tIRTT\n") - got, err := Links(dir) + got, err := Links(dir, t.TempDir()) if err != nil { t.Fatal(err) } @@ -81,7 +81,7 @@ func TestNoDefaultRouteIsNoLinks(t *testing.T) { func TestAMissingTableIsNotAFailure(t *testing.T) { dir := t.TempDir() write(t, dir, "route", routeV4) - got, err := Links(dir) + got, err := Links(dir, t.TempDir()) if err != nil { t.Fatalf("a missing v6 table should not fail: %v", err) } @@ -97,7 +97,7 @@ func TestALinkIsReportedOnce(t *testing.T) { write(t, dir, "ipv6_route", "00000000000000000000000000000000 00 00000000000000000000000000000000 00 "+ "fe800000000000000000000000000001 00000400 00000001 00000000 00000003 enp9s0\n") - got, err := Links(dir) + got, err := Links(dir, t.TempDir()) if err != nil { t.Fatal(err) } @@ -109,7 +109,7 @@ func TestALinkIsReportedOnce(t *testing.T) { // Against this machine's own routing table, so the parse is held to what the kernel actually writes // and not only to a fixture written to agree with it. func TestAgainstThisMachinesOwnTable(t *testing.T) { - got, err := Links("") + got, err := Links("", "") if err != nil { t.Fatal(err) } @@ -118,3 +118,39 @@ func TestAgainstThisMachinesOwnTable(t *testing.T) { } t.Logf("this machine's outward links: %v", got) } + +// sysNet is a /sys/class/net: each name a directory, with a `device` entry when a device backs it. +func sysNet(t *testing.T, physical []string, virtual []string) string { + t.Helper() + dir := t.TempDir() + for _, name := range physical { + if err := os.MkdirAll(filepath.Join(dir, name, "device"), 0o755); err != nil { + t.Fatal(err) + } + } + for _, name := range virtual { + if err := os.MkdirAll(filepath.Join(dir, name), 0o755); err != nil { + t.Fatal(err) + } + } + return dir +} + +// **A physical link faces outside whether or not it carries the default route** (novox/hq issue +// 197). A machine on its radio with its cable unplugged reported only the radio, and the filter then +// accepted everything arriving on the cable the moment it was plugged in. Bridges, veths, the tunnel +// and the loopback have no device behind them and stay this machine's own. +func TestEveryPhysicalLinkFacesOutsideUpOrDown(t *testing.T) { + proc := t.TempDir() + write(t, proc, "route", `Iface Destination Gateway Flags RefCnt Use Metric Mask MTU Window IRTT +wlp5s0 00000000 01FEA8C0 0003 0 0 600 00000000 0 0 0 +`) + sys := sysNet(t, []string{"wlp5s0", "enp6s0"}, []string{"lo", "docker0", "br-0123456789ab", "veth1", "mesh0"}) + got, err := Links(proc, sys) + if err != nil { + t.Fatal(err) + } + if want := []string{"enp6s0", "wlp5s0"}; !reflect.DeepEqual(got, want) { + t.Fatalf("outward links are %v, want %v", got, want) + } +}