From 7e52be6883c19e1fbcf306d381e18e41af71e050 Mon Sep 17 00:00:00 2001 From: ulziibay-kernel <253135130+ulziibay-kernel@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:50:02 +0000 Subject: [PATCH 1/2] Harden guest TAP bridge ports against cross-guest frame leakage Port isolation only blocks guest-to-guest forwarding. Unknown-unicast frames arriving from the uplink for a MAC the bridge has forgotten (aged out, or a guest that was just torn down) are still flooded to every guest port, so a guest with a raw socket can passively capture traffic addressed to other guests on the same bridge. On isolated networks, for each guest TAP: - disable unicast flooding so unknown-destination frames are never delivered to the port - pin the guest MAC to its port with a permanent FDB entry so inbound delivery never depends on flooding - disable MAC learning on the port so a guest cannot relocate FDB entries by spoofing a source MAC Co-Authored-By: Claude Opus 5 --- lib/network/allocate.go | 4 +-- lib/network/bridge_darwin.go | 2 +- lib/network/bridge_linux.go | 61 +++++++++++++++++++++++++++++++- lib/network/bridge_linux_test.go | 20 +++++++++++ 4 files changed, 83 insertions(+), 4 deletions(-) diff --git a/lib/network/allocate.go b/lib/network/allocate.go index 112ee151b..3fe42ea85 100644 --- a/lib/network/allocate.go +++ b/lib/network/allocate.go @@ -59,7 +59,7 @@ func (m *manager) CreateAllocation(ctx context.Context, req AllocateRequest) (*N attribute.Bool("download_rate_limit", req.DownloadBps > 0), attribute.Bool("upload_rate_limit", req.UploadBps > 0), ) - err = m.createTAPDevice(tapCtx, netConfig.TAPDevice, network.Bridge, network.Isolated) + err = m.createTAPDevice(tapCtx, netConfig.TAPDevice, network.Bridge, netConfig.MAC, network.Isolated) tapSpanEnd(err) if err != nil { cleanupErr := m.deleteTAPDeviceForInstanceSerialized(ctx, req.InstanceID, netConfig.TAPDevice) @@ -143,7 +143,7 @@ func (m *manager) RecreateAllocation(ctx context.Context, instanceID string, dow attribute.Bool("download_rate_limit", downloadBps > 0), attribute.Bool("upload_rate_limit", uploadBps > 0), ) - err = m.createTAPDevice(tapCtx, alloc.TAPDevice, network.Bridge, network.Isolated) + err = m.createTAPDevice(tapCtx, alloc.TAPDevice, network.Bridge, alloc.MAC, network.Isolated) tapSpanEnd(err) if err != nil { _ = m.deleteTAPDeviceForInstanceSerialized(ctx, instanceID, alloc.TAPDevice) diff --git a/lib/network/bridge_darwin.go b/lib/network/bridge_darwin.go index 50799f43c..3ad9fa6fa 100644 --- a/lib/network/bridge_darwin.go +++ b/lib/network/bridge_darwin.go @@ -36,7 +36,7 @@ func (m *manager) setupBridgeHTB(ctx context.Context, bridgeName string, capacit // createTAPDevice is a no-op on macOS as we use NAT networking. // Virtualization.framework creates virtual network interfaces internally. -func (m *manager) createTAPDevice(ctx context.Context, tapName, bridgeName string, isolated bool) error { +func (m *manager) createTAPDevice(ctx context.Context, tapName, bridgeName, mac string, isolated bool) error { // On macOS with vz, network devices are created by the VMM itself return nil } diff --git a/lib/network/bridge_linux.go b/lib/network/bridge_linux.go index 2e5bfc986..e57523427 100644 --- a/lib/network/bridge_linux.go +++ b/lib/network/bridge_linux.go @@ -521,7 +521,7 @@ func (m *manager) lastHypemanForwardRulePosition() int { } // createTAPDevice creates TAP device and attaches it to the bridge. -func (m *manager) createTAPDevice(ctx context.Context, tapName, bridgeName string, isolated bool) error { +func (m *manager) createTAPDevice(ctx context.Context, tapName, bridgeName, mac string, isolated bool) error { // 1. Check if TAP already exists _, linkLookupEnd := startNetworkStep(ctx, "network.create_tap.link_lookup_existing", attribute.String("operation", "link_lookup_existing"), @@ -612,11 +612,70 @@ func (m *manager) createTAPDevice(ctx context.Context, tapName, bridgeName strin if err != nil { return fmt.Errorf("set isolation mode: %w", err) } + + // Isolation only stops guest-to-guest forwarding. Frames arriving from + // the uplink for a MAC the bridge has forgotten (aged out, or a guest + // that was just torn down) are still flooded to every port, so one + // guest can sniff traffic addressed to another. Pin the guest MAC to + // its port and turn off flooding and learning on the port so delivery + // never depends on flooding and a guest can't move FDB entries by + // spoofing a source MAC. + _, floodEnd := startNetworkStep(ctx, "network.create_tap.set_flood_off", + attribute.String("operation", "set_flood_off"), + attribute.String("tap", tapName), + ) + err = netlink.LinkSetFlood(tapLink, false) + floodEnd(err) + if err != nil { + return fmt.Errorf("disable unicast flooding: %w", err) + } + + _, learningEnd := startNetworkStep(ctx, "network.create_tap.set_learning_off", + attribute.String("operation", "set_learning_off"), + attribute.String("tap", tapName), + ) + err = netlink.LinkSetLearning(tapLink, false) + learningEnd(err) + if err != nil { + return fmt.Errorf("disable MAC learning: %w", err) + } + + var fdbEntry *netlink.Neigh + fdbEntry, err = guestFDBEntry(tapLink.Attrs().Index, mac) + if err != nil { + return err + } + _, fdbEnd := startNetworkStep(ctx, "network.create_tap.add_fdb_entry", + attribute.String("operation", "add_fdb_entry"), + attribute.String("tap", tapName), + attribute.String("mac", mac), + ) + err = netlink.NeighAppend(fdbEntry) + fdbEnd(err) + if err != nil { + return fmt.Errorf("add static FDB entry: %w", err) + } } return nil } +// guestFDBEntry builds the permanent bridge FDB entry that pins a guest MAC to +// its TAP port. Equivalent to `bridge fdb add dev master permanent`. +func guestFDBEntry(tapIndex int, mac string) (*netlink.Neigh, error) { + hwAddr, err := net.ParseMAC(mac) + if err != nil { + return nil, fmt.Errorf("parse guest MAC %q: %w", mac, err) + } + return &netlink.Neigh{ + LinkIndex: tapIndex, + Family: unix.AF_BRIDGE, + State: netlink.NUD_PERMANENT, + Flags: netlink.NTF_MASTER, + HardwareAddr: hwAddr, + }, nil +} + // applyDownloadRateLimit applies download (external→VM) rate limiting using TBF on TAP egress. func (m *manager) applyDownloadRateLimit(ctx context.Context, tapName string, rateLimitBps int64) error { rateStr := formatTcRate(rateLimitBps) diff --git a/lib/network/bridge_linux_test.go b/lib/network/bridge_linux_test.go index 3c86fd1b3..3a1b65f96 100644 --- a/lib/network/bridge_linux_test.go +++ b/lib/network/bridge_linux_test.go @@ -6,6 +6,9 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" ) func TestParseBridgeFilters(t *testing.T) { @@ -66,3 +69,20 @@ func TestPlanOrphanedBridgeTCBailsWhenNoRTIIFParses(t *testing.T) { assert.Nil(t, staleFilters) assert.Nil(t, staleClasses) } + +func TestGuestFDBEntry(t *testing.T) { + entry, err := guestFDBEntry(42, "02:00:00:aa:bb:cc") + require.NoError(t, err) + + assert.Equal(t, 42, entry.LinkIndex) + assert.Equal(t, unix.AF_BRIDGE, entry.Family) + assert.Equal(t, netlink.NUD_PERMANENT, entry.State) + assert.Equal(t, netlink.NTF_MASTER, entry.Flags) + assert.Equal(t, "02:00:00:aa:bb:cc", entry.HardwareAddr.String()) + assert.Nil(t, entry.IP) +} + +func TestGuestFDBEntryRejectsBadMAC(t *testing.T) { + _, err := guestFDBEntry(42, "not-a-mac") + assert.Error(t, err) +} From 8dc007adf79314d9c037429944cf9f3f4b84fc0e Mon Sep 17 00:00:00 2001 From: ulziibay-kernel <253135130+ulziibay-kernel@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:07:06 +0000 Subject: [PATCH 2/2] Verify isolated-port hardening against a real bridge Move the flood/learning/FDB steps into hardenIsolatedPort and cover them with root-gated tests that build an actual bridge and TAP, so the netlink requests are exercised rather than only the entry builder. Build the FDB entry before touching the port so an unparseable MAC leaves the port at its defaults instead of flooding off with nothing pinned, and use NeighSet rather than NeighAppend: NLM_F_APPEND carries multi-destination semantics, while replace is the idempotent primitive for pinning a single unicast MAC. Co-Authored-By: Claude Opus 5 --- lib/network/bridge_linux.go | 90 ++++++++++++++++++-------------- lib/network/bridge_linux_test.go | 62 ++++++++++++++++++++++ 2 files changed, 112 insertions(+), 40 deletions(-) diff --git a/lib/network/bridge_linux.go b/lib/network/bridge_linux.go index e57523427..118353012 100644 --- a/lib/network/bridge_linux.go +++ b/lib/network/bridge_linux.go @@ -613,55 +613,65 @@ func (m *manager) createTAPDevice(ctx context.Context, tapName, bridgeName, mac return fmt.Errorf("set isolation mode: %w", err) } - // Isolation only stops guest-to-guest forwarding. Frames arriving from - // the uplink for a MAC the bridge has forgotten (aged out, or a guest - // that was just torn down) are still flooded to every port, so one - // guest can sniff traffic addressed to another. Pin the guest MAC to - // its port and turn off flooding and learning on the port so delivery - // never depends on flooding and a guest can't move FDB entries by - // spoofing a source MAC. - _, floodEnd := startNetworkStep(ctx, "network.create_tap.set_flood_off", - attribute.String("operation", "set_flood_off"), - attribute.String("tap", tapName), - ) - err = netlink.LinkSetFlood(tapLink, false) - floodEnd(err) - if err != nil { - return fmt.Errorf("disable unicast flooding: %w", err) + if err := hardenIsolatedPort(ctx, tapLink, tapName, mac); err != nil { + return err } + } - _, learningEnd := startNetworkStep(ctx, "network.create_tap.set_learning_off", - attribute.String("operation", "set_learning_off"), - attribute.String("tap", tapName), - ) - err = netlink.LinkSetLearning(tapLink, false) - learningEnd(err) - if err != nil { - return fmt.Errorf("disable MAC learning: %w", err) - } + return nil +} - var fdbEntry *netlink.Neigh - fdbEntry, err = guestFDBEntry(tapLink.Attrs().Index, mac) - if err != nil { - return err - } - _, fdbEnd := startNetworkStep(ctx, "network.create_tap.add_fdb_entry", - attribute.String("operation", "add_fdb_entry"), - attribute.String("tap", tapName), - attribute.String("mac", mac), - ) - err = netlink.NeighAppend(fdbEntry) - fdbEnd(err) - if err != nil { - return fmt.Errorf("add static FDB entry: %w", err) - } +// hardenIsolatedPort keeps an isolated guest from seeing its neighbours' +// traffic. Isolation only stops guest-to-guest forwarding: frames arriving +// from the uplink for a MAC the bridge has forgotten (aged out, or a guest +// that was just torn down) are still flooded to every port. Pinning the guest +// MAC to its own port makes inbound delivery independent of flooding, so +// flooding can be turned off; turning learning off then stops a guest from +// relocating FDB entries by spoofing a source MAC. +func hardenIsolatedPort(ctx context.Context, tapLink netlink.Link, tapName, mac string) error { + // Built before the port is touched so a malformed MAC can't leave the port + // with flooding off and nothing pinned to it. + fdbEntry, err := guestFDBEntry(tapLink.Attrs().Index, mac) + if err != nil { + return err + } + + _, fdbEnd := startNetworkStep(ctx, "network.create_tap.add_fdb_entry", + attribute.String("operation", "add_fdb_entry"), + attribute.String("tap", tapName), + attribute.String("mac", mac), + ) + err = netlink.NeighSet(fdbEntry) + fdbEnd(err) + if err != nil { + return fmt.Errorf("add static FDB entry: %w", err) + } + + _, floodEnd := startNetworkStep(ctx, "network.create_tap.set_flood_off", + attribute.String("operation", "set_flood_off"), + attribute.String("tap", tapName), + ) + err = netlink.LinkSetFlood(tapLink, false) + floodEnd(err) + if err != nil { + return fmt.Errorf("disable unicast flooding: %w", err) + } + + _, learningEnd := startNetworkStep(ctx, "network.create_tap.set_learning_off", + attribute.String("operation", "set_learning_off"), + attribute.String("tap", tapName), + ) + err = netlink.LinkSetLearning(tapLink, false) + learningEnd(err) + if err != nil { + return fmt.Errorf("disable MAC learning: %w", err) } return nil } // guestFDBEntry builds the permanent bridge FDB entry that pins a guest MAC to -// its TAP port. Equivalent to `bridge fdb add dev master permanent`. +// its TAP port. Equivalent to `bridge fdb replace dev master permanent`. func guestFDBEntry(tapIndex int, mac string) (*netlink.Neigh, error) { hwAddr, err := net.ParseMAC(mac) if err != nil { diff --git a/lib/network/bridge_linux_test.go b/lib/network/bridge_linux_test.go index 3a1b65f96..21e1733f6 100644 --- a/lib/network/bridge_linux_test.go +++ b/lib/network/bridge_linux_test.go @@ -3,6 +3,9 @@ package network import ( + "context" + "os" + "slices" "testing" "github.com/stretchr/testify/assert" @@ -86,3 +89,62 @@ func TestGuestFDBEntryRejectsBadMAC(t *testing.T) { _, err := guestFDBEntry(42, "not-a-mac") assert.Error(t, err) } + +// TestHardenIsolatedPortOnRealBridge exercises the netlink calls against a real +// bridge, which is the only way to catch a request the kernel rejects. +func TestHardenIsolatedPortOnRealBridge(t *testing.T) { + const mac = "02:00:00:aa:bb:cc" + tap := bridgedTAPForTest(t, "brhardn0", "taphardn0") + + require.NoError(t, hardenIsolatedPort(context.Background(), tap, tap.Name, mac)) + + protinfo, err := netlink.LinkGetProtinfo(tap) + require.NoError(t, err) + assert.False(t, protinfo.Flood, "unicast flooding should be off") + assert.False(t, protinfo.Learning, "MAC learning should be off") + + entries, err := netlink.NeighList(tap.Attrs().Index, unix.AF_BRIDGE) + require.NoError(t, err) + pinned := slices.IndexFunc(entries, func(n netlink.Neigh) bool { + return n.HardwareAddr.String() == mac + }) + require.NotEqual(t, -1, pinned, "guest MAC should be pinned to the TAP port") + assert.Equal(t, netlink.NUD_PERMANENT, entries[pinned].State) +} + +// TestHardenIsolatedPortLeavesPortAloneOnBadMAC covers the case where the +// allocation carries a MAC we can't parse: the port keeps its defaults rather +// than ending up with flooding off and nothing pinned to it. +func TestHardenIsolatedPortLeavesPortAloneOnBadMAC(t *testing.T) { + tap := bridgedTAPForTest(t, "brhardn1", "taphardn1") + + require.Error(t, hardenIsolatedPort(context.Background(), tap, tap.Name, "")) + + protinfo, err := netlink.LinkGetProtinfo(tap) + require.NoError(t, err) + assert.True(t, protinfo.Flood, "flooding should be untouched") + assert.True(t, protinfo.Learning, "learning should be untouched") +} + +// bridgedTAPForTest creates a throwaway bridge with one TAP enslaved to it, and +// skips the test when it can't (creating links needs root). +func bridgedTAPForTest(t *testing.T, bridgeName, tapName string) *netlink.Tuntap { + t.Helper() + if os.Getuid() != 0 { + t.Skip("Skipping test that requires root") + } + + bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}} + require.NoError(t, netlink.LinkAdd(bridge)) + t.Cleanup(func() { _ = netlink.LinkDel(bridge) }) + + tap := &netlink.Tuntap{ + LinkAttrs: netlink.LinkAttrs{Name: tapName}, + Mode: netlink.TUNTAP_MODE_TAP, + } + require.NoError(t, netlink.LinkAdd(tap)) + t.Cleanup(func() { _ = netlink.LinkDel(tap) }) + require.NoError(t, netlink.LinkSetMaster(tap, bridge)) + + return tap +}