a449db80

feat(agent): a host refuses a guest subnet that contains its own uplink

a73x 2026-08-06 09:12

Commit message

feat(agent): a host refuses a guest subnet that contains its own uplink

Before the bridge is created, the Linux platform checks whether the resolved
guest subnet already contains an address this host is configured with, on any
interface but the bridge itself, and refuses to start naming both sides and the
flag that fixes it.

The failure it prevents was reproduced in network namespaces rather than
argued, and the obvious prediction about it is wrong in a way worth recording.
An agent whose subnet overlaps its own uplink assigns that subnet's .1 to the
bridge. Outbound survives — the DHCP-installed default route names the uplink
device explicitly, so nexthop resolution stays pinned there even once the
bridge's connected route wins the FIB tie — which means a check that only asked
"can this host still reach the control plane" would call it healthy. Inbound is
what dies. The gateway's address is now a local address on this box, so every
reply it sends there resolves to lo and never leaves; the host stops answering
anything that dials it, including the tunnel its own fleet uses to reach its
guests. The one situation this arises in is a fresh agent inside a guest, which
is exactly where losing the network also loses the way in to fix it.

The bridge's own interface is skipped, because every agent that has run once
finds its bridge on its own subnet and that is the normal case.

An enumeration that fails outright allows and warns. This is a diagnostic, and
refusing to start because a diagnostic could not be run would turn a guard
against unreachability into a cause of it.

It lives in netenv with its interface reading injected, not inline in the
composition root, so the logic is exercised on any machine: net.Interface's own
Addrs method reaches the kernel, so a fabricated one answers nothing, and the
check takes a name-and-addresses pair a test can describe freely. One of those
tests puts a bare-metal host, a guest host and the nested wedge side by side,
because the risk in adding a refusal is refusing something that already works.

internal/agent/netenv/netenv.go

Old		New
@@ -31,7 +31,11 @@ type Net struct {
31	// for tests; production checks /sys/class/net/<name>/tun_flags, which	31	// for tests; production checks /sys/class/net/<name>/tun_flags, which
32	// exists only for tun/tap links — no error-string parsing.	32	// exists only for tun/tap links — no error-string parsing.
33	isTap func(name string) bool	33	isTap func(name string) bool
34	dhcp *dhcp.Server	34	// ifaces enumerates this host's interfaces for the uplink-collision check.
		35	// Injectable for the same reason isTap is: the production reader touches
		36	// the kernel, and the logic above it should be testable without one.
		37	ifaces func() ([]Iface, error)
		38	dhcp *dhcp.Server
35	}	39	}
36		40
37	// guestDNS is handed to guests as their DHCP DNS servers (option 6). Public	41	// guestDNS is handed to guests as their DHCP DNS servers (option 6). Public
@@ -48,7 +52,7 @@ func New(run exec.Runner, cidr string) (*Net, error) {
48	if !p.Addr().Is4() {	52	if !p.Addr().Is4() {
49	return nil, fmt.Errorf("bridge CIDR must be IPv4, got %s", cidr)	53	return nil, fmt.Errorf("bridge CIDR must be IPv4, got %s", cidr)
50	}	54	}
51	n := &Net{run: run, cidr: p, isTap: sysfsIsTap}	55	n := &Net{run: run, cidr: p, isTap: sysfsIsTap, ifaces: hostIfaces}
52	gw := net.ParseIP(n.Gateway())	56	gw := net.ParseIP(n.Gateway())
53	mask := net.CIDRMask(p.Bits(), 32)	57	mask := net.CIDRMask(p.Bits(), 32)
54	n.dhcp = dhcp.NewServer(Bridge, n.cidr.String(), gw, mask, guestDNS, 12*time.Hour)	58	n.dhcp = dhcp.NewServer(Bridge, n.cidr.String(), gw, mask, guestDNS, 12*time.Hour)

internal/agent/netenv/uplink.go

@@ @@ -0,0 +1,108 @@ @@
+package netenv
+import (
+	"fmt"
+	"log/slog"
+	"net"
+	"net/netip"
+)
+// Iface is one host interface as the uplink check sees it: a name, and the
+// addresses configured on it.
+//
+// It exists because net.Interface's own Addrs method reaches the kernel, so a
+// fabricated net.Interface answers nothing — the check takes this instead, and
+// a test can describe any host it likes.
+type Iface struct {
+	Name  string
+	Addrs []net.Addr
+}
+// hostIfaces enumerates this host's interfaces and the addresses on each. An
+// interface whose addresses cannot be read is reported without them rather than
+// failing the enumeration: one unreadable interface should not blind the check
+// to every other.
+func hostIfaces() ([]Iface, error) {
+	ifaces, err := net.Interfaces()
+	if err != nil {
+		return nil, err
+	}
+	out := make([]Iface, 0, len(ifaces))
+	for _, iface := range ifaces {
+		addrs, err := iface.Addrs()
+		if err != nil {
+			addrs = nil
+		}
+		out = append(out, Iface{Name: iface.Name, Addrs: addrs})
+	}
+	return out, nil
+}
+// CheckUplinkCollision refuses a guest subnet that already contains an address
+// this host is configured with, on any interface but the bridge itself. It is
+// called before EnsureBridge, because by the time the bridge exists the damage
+// is done and the way in to undo it is gone.
+//
+// The failure it prevents was reproduced in network namespaces rather than
+// argued, and the obvious prediction about it is wrong in an instructive way.
+// An agent whose resolved subnet overlaps its own uplink assigns the gateway
+// address — .1 of that subnet — to the bridge. Outbound survives, because the
+// DHCP-installed default route names the uplink device explicitly, so a check
+// that only asked "can this host still reach the control plane" would call it
+// healthy. Inbound is what dies: the gateway's address is now a LOCAL address
+// on this box, so every reply it sends there resolves to lo and never leaves.
+// The host stops answering anything that dials it, including the tunnel its
+// own fleet uses to reach its guests — and the machine best placed to fix it is
+// the one that just lost the route.
+//
+// The one situation this arises in is the one where it hurts most: a fresh
+// agent inside a guest, where losing the network also loses the way in.
+//
+// The bridge's own interface is skipped. Finding the bridge on the bridge's
+// subnet is the normal case on every restart, not a collision.
+//
+// An enumeration that fails outright ALLOWS and warns. This is a diagnostic;
+// refusing to start because a diagnostic could not be run would turn a check
+// against unreachability into a cause of it.
+func (n *Net) CheckUplinkCollision() error {
+	ifaces, err := n.ifaces()
+	if err != nil {
+		slog.Warn("cannot read this host's interfaces; starting without the uplink-collision check",
+			"guest_cidr", n.cidr.Masked(), "err", err)
+		return nil
+	}
+	for _, iface := range ifaces {
+		if iface.Name == Bridge {
+			continue
+		}
+		for _, a := range iface.Addrs {
+			addr, ok := ifaceAddr(a)
+			if !ok || !n.cidr.Contains(addr) {
+				continue
+			}
+			return fmt.Errorf(
+				"guest subnet %s already contains %s, configured on %s: "+
+					"putting the bridge there would make this host's own address a local one, "+
+					"and it would stop answering anything that dials it — "+
+					"give this host a subnet it is not already on (--bridge-cidr)",
+				n.cidr.Masked(), addr, iface.Name)
+		}
+	}
+	return nil
+}
+// ifaceAddr reduces one interface address to the IPv4 address it carries.
+// Anything else — an IPv6 address, or an address shape that is not an IPNet —
+// cannot collide with a guest subnet, which New has already established is
+// IPv4.
+func ifaceAddr(a net.Addr) (netip.Addr, bool) {
+	ipnet, ok := a.(*net.IPNet)
+	if !ok {
+		return netip.Addr{}, false
+	}
+	v4 := ipnet.IP.To4()
+	if v4 == nil {
+		return netip.Addr{}, false
+	}
+	return netip.AddrFromSlice(v4)
+}

internal/agent/netenv/uplink_test.go

@@ @@ -0,0 +1,220 @@ @@
+package netenv
+import (
+	"errors"
+	"net"
+	"strings"
+	"testing"
+)
+// ifaceFrom describes one interface the way a host would present it. A CIDR
+// per address, because a bare address tells the check nothing it needs.
+func ifaceFrom(t *testing.T, name string, cidrs ...string) Iface {
+	t.Helper()
+	iface := Iface{Name: name}
+	for _, c := range cidrs {
+		ip, ipnet, err := net.ParseCIDR(c)
+		if err != nil {
+			t.Fatalf("bad test address %q: %v", c, err)
+		}
+		iface.Addrs = append(iface.Addrs, &net.IPNet{IP: ip, Mask: ipnet.Mask})
+	}
+	return iface
+}
+// newNetFor builds a Net on cidr whose interface enumeration answers ifaces.
+// The command runner is nil: this check reads interfaces and runs nothing.
+func newNetFor(t *testing.T, cidr string, ifaces ...Iface) *Net {
+	t.Helper()
+	n, err := New(nil, cidr)
+	if err != nil {
+		t.Fatalf("New(%q): %v", cidr, err)
+	}
+	n.ifaces = func() ([]Iface, error) { return ifaces, nil }
+	return n
+}
+// TestCheckUplinkCollisionRefusesASubnetContainingThisHost pins the refusal
+// that keeps a nested agent reachable. An agent whose guest subnet overlaps its
+// own uplink assigns that subnet's .1 to the bridge, its own address becomes
+// local, and it stops answering anything that dials it — including the fleet
+// that would have to fix it.
+func TestCheckUplinkCollisionRefusesASubnetContainingThisHost(t *testing.T) {
+	n := newNetFor(t, "10.78.1.0/24",
+		ifaceFrom(t, "lo", "127.0.0.1/8"),
+		ifaceFrom(t, "eth0", "10.78.1.3/24"),
+	)
+	err := n.CheckUplinkCollision()
+	if err == nil {
+		t.Fatal("a subnet containing this host's own address must be refused")
+	}
+	// Actionable without further digging: both sides named, and the way out.
+	for _, want := range []string{"10.78.1.0/24", "10.78.1.3", "eth0", "--bridge-cidr"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("error must name %q, got: %v", want, err)
+		}
+	}
+}
+// TestCheckUplinkCollisionAllowsADisjointSubnet pins the ordinary case: a host
+// whose guests live somewhere it does not.
+func TestCheckUplinkCollisionAllowsADisjointSubnet(t *testing.T) {
+	n := newNetFor(t, "10.77.1.0/24",
+		ifaceFrom(t, "lo", "127.0.0.1/8"),
+		ifaceFrom(t, "eth0", "192.168.1.50/24"),
+		ifaceFrom(t, "docker0", "172.17.0.1/16"),
+	)
+	if err := n.CheckUplinkCollision(); err != nil {
+		t.Errorf("a disjoint subnet must be allowed, got: %v", err)
+	}
+}
+// TestCheckUplinkCollisionIgnoresTheBridgeItself pins the exemption that makes
+// the check survive a restart. Every agent that has run once finds its own
+// bridge sitting on its own subnet; that is the normal case, not a collision.
+func TestCheckUplinkCollisionIgnoresTheBridgeItself(t *testing.T) {
+	n := newNetFor(t, "10.77.1.0/24",
+		ifaceFrom(t, "eth0", "192.168.1.50/24"),
+		ifaceFrom(t, Bridge, "10.77.1.1/24"),
+	)
+	if err := n.CheckUplinkCollision(); err != nil {
+		t.Errorf("the bridge's own address must not read as a collision, got: %v", err)
+	}
+}
+// TestCheckUplinkCollisionSkipsAddressesItCannotCompare pins that nothing a
+// guest subnet could not contain is treated as a collision. The subnet is IPv4
+// by construction (New enforces it), so an IPv6 address or an address shape
+// that is not a network is simply not a candidate.
+func TestCheckUplinkCollisionSkipsAddressesItCannotCompare(t *testing.T) {
+	v6 := ifaceFrom(t, "eth0", "192.168.1.50/24")
+	_, v6net, _ := net.ParseCIDR("fd00::/64")
+	v6.Addrs = append(v6.Addrs, &net.IPNet{IP: net.ParseIP("fd00::1"), Mask: v6net.Mask})
+	// A non-IPNet address: interfaces do not normally present these, but the
+	// type assertion has to have an answer.
+	v6.Addrs = append(v6.Addrs, &net.TCPAddr{IP: net.ParseIP("10.77.1.9")})
+	n := newNetFor(t, "10.77.1.0/24", v6)
+	if err := n.CheckUplinkCollision(); err != nil {
+		t.Errorf("uncomparable addresses must be skipped, got: %v", err)
+	}
+}
+// TestCheckUplinkCollisionAllowsWhenItCannotLook pins the deliberate
+// asymmetry: this is a diagnostic, and refusing to start because a diagnostic
+// could not be run would turn a guard against unreachability into a cause of
+// it.
+func TestCheckUplinkCollisionAllowsWhenItCannotLook(t *testing.T) {
+	n, err := New(nil, "10.77.1.0/24")
+	if err != nil {
+		t.Fatal(err)
+	}
+	n.ifaces = func() ([]Iface, error) { return nil, errors.New("netlink is not available") }
+	if err := n.CheckUplinkCollision(); err != nil {
+		t.Errorf("an unreadable interface list must allow, got: %v", err)
+	}
+}
+// TestHostIfacesReadsThisMachine is a smoke test over the real enumeration. It
+// cannot assert an interface list — it runs on whatever CI is — but it does
+// prove the reading works unprivileged and returns something usable, which is
+// the half a fabricated Iface can never cover.
+func TestHostIfacesReadsThisMachine(t *testing.T) {
+	ifaces, err := hostIfaces()
+	if err != nil {
+		t.Skipf("interfaces unreadable here: %v", err)
+	}
+	if len(ifaces) == 0 {
+		t.Skip("no interfaces on this machine")
+	}
+	var named int
+	for _, iface := range ifaces {
+		if iface.Name != "" {
+			named++
+		}
+	}
+	if named == 0 {
+		t.Error("hostIfaces returned only unnamed interfaces")
+	}
+}
+// TestCheckUplinkCollisionAgainstRealHostShapes puts the three arrangements
+// side by side, because the risk in adding a refusal is refusing something that
+// works. A bare-metal host finds only its own bridge on the guest subnet; a
+// guest host with a disjoint subnet finds nothing; and the nested agent handed
+// the subnet its own uplink sits in is the one that has to be stopped.
+func TestCheckUplinkCollisionAgainstRealHostShapes(t *testing.T) {
+	for _, tc := range []struct {
+		name, guestCIDR string
+		ifaces          []Iface
+		wantRefused     bool
+	}{
+		{
+			name: "bare metal, bridge already up", guestCIDR: "10.78.3.0/24",
+			ifaces: []Iface{
+				ifaceFrom(t, "lo", "127.0.0.1/8"),
+				ifaceFrom(t, "enp4s0", "192.168.0.251/24"),
+				ifaceFrom(t, Bridge, "10.78.3.1/24"),
+			},
+		},
+		{
+			name: "a guest host whose guests live elsewhere", guestCIDR: "10.101.1.0/24",
+			ifaces: []Iface{
+				ifaceFrom(t, "lo", "127.0.0.1/8"),
+				ifaceFrom(t, "ens3", "10.78.3.4/24"),
+			},
+		},
+		{
+			name: "nested, and handed the subnet it is already on", guestCIDR: "10.78.3.0/24",
+			ifaces: []Iface{
+				ifaceFrom(t, "lo", "127.0.0.1/8"),
+				ifaceFrom(t, "ens3", "10.78.3.4/24"),
+			},
+			wantRefused: true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			err := newNetFor(t, tc.guestCIDR, tc.ifaces...).CheckUplinkCollision()
+			switch {
+			case tc.wantRefused && err == nil:
+				t.Error("this host would make itself unreachable and must be refused")
+			case !tc.wantRefused && err != nil:
+				t.Errorf("a working host must not be refused: %v", err)
+			}
+		})
+	}
+}
+// TestIfaceAddr pins the reduction of one interface address to a comparable
+// IPv4 address.
+func TestIfaceAddr(t *testing.T) {
+	ipnet := func(ip, mask string) net.Addr {
+		_, n, err := net.ParseCIDR(ip + "/" + mask)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return &net.IPNet{IP: net.ParseIP(ip), Mask: n.Mask}
+	}
+	for _, tc := range []struct {
+		name string
+		addr net.Addr
+		want string // "" = not comparable
+	}{
+		{"an IPv4 address", ipnet("10.0.0.5", "24"), "10.0.0.5"},
+		{"IPv6", &net.IPNet{IP: net.ParseIP("fd00::1"), Mask: net.CIDRMask(64, 128)}, ""},
+		{"not an IPNet", &net.TCPAddr{IP: net.ParseIP("10.0.0.5")}, ""},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			got, ok := ifaceAddr(tc.addr)
+			if tc.want == "" {
+				if ok {
+					t.Errorf("ifaceAddr accepted %v as %s", tc.addr, got)
+				}
+				return
+			}
+			if !ok || got.String() != tc.want {
+				t.Errorf("ifaceAddr() = %s, %v; want %s", got, ok, tc.want)
+			}
+		})
+	}
+}

internal/agent/run/wire_linux.go

Old		New
@@ -91,6 +91,13 @@ func newPlatform(ctx context.Context, cfg Config, st *state.Store) (platform, er
91	if err != nil {	91	if err != nil {
92	return platform{}, fmt.Errorf("netenv init: %w", err)	92	return platform{}, fmt.Errorf("netenv init: %w", err)
93	}	93	}
		94	// Before anything is created: a subnet overlapping this host's own uplink
		95	// would make its address local and stop it answering, and the only machine
		96	// positioned to fix that is the one that just lost the route. Refusing here
		97	// costs a startup error; not refusing costs the host.
		98	if err := net.CheckUplinkCollision(); err != nil {
		99	return platform{}, err
		100	}
94	if err := net.EnsureBridge(ctx); err != nil {	101	if err := net.EnsureBridge(ctx); err != nil {
95	return platform{}, fmt.Errorf("ensure bridge: %w", err)	102	return platform{}, fmt.Errorf("ensure bridge: %w", err)
96	}	103	}