From da5e38cf18847c90260742da2b9b7b650e507afa Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Mon, 3 Aug 2026 17:26:11 +1000 Subject: [PATCH] fix(imds): answer ARP for the captured link-local addresses (#678) The per-tap datapath only matched IP, on the assumption that a guest reaches 169.254.169.254 through its default gateway. A guest that treats 169.254.0.0/16 as on-link resolves the address itself instead, and nothing on br-imds, br-int or in OVN answered that ARP, so it could not reach IMDS at all. Windows does exactly this, which left cloudbase-init timing out on every metadata request and skipping the rest of its plugin stage. Install a per-tap ARP responder for each captured address under the existing cookie, replying with the gateway MAC the egress flow already presents. --- spinifex/network/host/imds_datapath.go | 77 +++++++++++++++++++-- spinifex/network/host/imds_datapath_test.go | 64 +++++++++++++++++ spinifex/network/topology/dhcp_options.go | 4 +- 3 files changed, 138 insertions(+), 7 deletions(-) diff --git a/spinifex/network/host/imds_datapath.go b/spinifex/network/host/imds_datapath.go index 463e7cabd..7b952dd38 100644 --- a/spinifex/network/host/imds_datapath.go +++ b/spinifex/network/host/imds_datapath.go @@ -2,10 +2,12 @@ package host import ( "context" + "encoding/hex" "errors" "fmt" "hash/fnv" "log/slog" + "net" "github.com/mulgadc/spinifex/spinifex/utils" ) @@ -22,10 +24,11 @@ const ( // Kept in one place so the endpoint addresses and the ingress flows stay in sync. var imdsCaptureAddrs = []string{imdsMetaAddr, imdsDNSAddr} -// Per-tap OpenFlow priorities on IMDSBridge. Demux (.254/.253 interception) and -// egress sit above the forward flows so IMDS traffic is captured and everything -// else is bridged tap<->patch to br-int. +// Per-tap OpenFlow priorities on IMDSBridge. The ARP responder, demux +// (.254/.253 interception) and egress sit above the forward flows so IMDS +// traffic is captured and everything else is bridged tap<->patch to br-int. const ( + imdsARPPriority = 250 imdsDemuxPriority = 200 imdsForwardPriority = 100 ) @@ -220,9 +223,23 @@ func setEndpointSysctl(ctx context.Context, r Runner, endpoint, suffix, val stri // here would wipe the forward flows the patch installer added under the same cookie. func installIMDSTapFlows(ctx context.Context, r Runner, d IMDSTapDatapath) error { cookie := imdsFlowCookie(d.Endpoint) - // Ingress: guest -> endpoint. The guest addresses the frame to its gateway - // MAC (.254/.253 are off-link, routed via the default gateway), so rewrite - // the dst MAC to the endpoint or the kernel drops it as OTHERHOST. + // ARP: a guest that treats 169.254.0.0/16 as on-link (RFC 3927, which is what + // Windows does) resolves the captured addresses itself instead of routing via + // the gateway. Nothing on br-int or in OVN owns them, so answer here or that + // guest never reaches IMDS at all. + for _, addr := range imdsCaptureAddrs { + spec, err := imdsARPResponderFlow(d, addr) + if err != nil { + return err + } + if err := installIMDSFlow(ctx, r, cookie, spec); err != nil { + return err + } + } + // Ingress: guest -> endpoint. A guest that routed the frame addressed it to + // its gateway MAC, and one that resolved it on-link used the MAC the ARP + // responder gave out; either way rewrite the dst MAC to the endpoint or the + // kernel drops it as OTHERHOST. for _, addr := range imdsCaptureAddrs { spec := fmt.Sprintf("table=0,priority=%d,in_port=%s,ip,nw_dst=%s,actions=mod_dl_dst:%s,output:%s", imdsDemuxPriority, d.Tap, addr, d.EndpointMAC, d.Endpoint) @@ -236,3 +253,51 @@ func installIMDSTapFlows(ctx context.Context, r Runner, d IMDSTapDatapath) error imdsDemuxPriority, d.Endpoint, d.GatewayMAC, d.GuestMAC, d.Tap) return installIMDSFlow(ctx, r, cookie, egress) } + +// imdsARPResponderFlow builds the flow that turns a guest's ARP request for addr +// into a reply reflected back out the tap. The reply advertises GatewayMAC — the +// same L2 identity the egress flow already presents — so the guest sees one MAC +// for the address however it resolved it, and the ingress demux (which matches +// in_port and nw_dst, not dst MAC) still catches what it sends. +func imdsARPResponderFlow(d IMDSTapDatapath, addr string) (string, error) { + mac, err := macToHex(d.GatewayMAC) + if err != nil { + return "", err + } + ip, err := ipv4ToHex(addr) + if err != nil { + return "", err + } + return fmt.Sprintf("table=0,priority=%d,in_port=%s,arp,arp_tpa=%s,arp_op=1,"+ + "actions=move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],mod_dl_src:%s,"+ + "load:0x2->NXM_OF_ARP_OP[],"+ + "move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],"+ + "move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[],"+ + "load:0x%s->NXM_NX_ARP_SHA[],load:0x%s->NXM_OF_ARP_SPA[],IN_PORT", + imdsARPPriority, d.Tap, addr, d.GatewayMAC, mac, ip), nil +} + +// macToHex renders a MAC as the bare hex OpenFlow's load: action expects. +func macToHex(mac string) (string, error) { + hw, err := net.ParseMAC(mac) + if err != nil { + return "", fmt.Errorf("parse MAC %q: %w", mac, err) + } + if len(hw) != 6 { + return "", fmt.Errorf("MAC %q is not 48-bit", mac) + } + return hex.EncodeToString(hw), nil +} + +// ipv4ToHex renders an IPv4 address as the bare hex OpenFlow's load: action expects. +func ipv4ToHex(addr string) (string, error) { + ip := net.ParseIP(addr) + if ip == nil { + return "", fmt.Errorf("parse IP %q", addr) + } + v4 := ip.To4() + if v4 == nil { + return "", fmt.Errorf("IP %q is not IPv4", addr) + } + return hex.EncodeToString(v4), nil +} diff --git a/spinifex/network/host/imds_datapath_test.go b/spinifex/network/host/imds_datapath_test.go index d8e3b49b6..d676fabf2 100644 --- a/spinifex/network/host/imds_datapath_test.go +++ b/spinifex/network/host/imds_datapath_test.go @@ -136,6 +136,10 @@ func TestInstallTapDatapath(t *testing.T) { utils.EndpointSysctlHelper + " " + d.Endpoint + " accept_local 1", // Flows are not cleared here: installIMDSDatapath clears the shared cookie // once up front so this install does not wipe the patch's forward flows. + // ARP responder, one per captured addr, for guests that resolve the + // link-local addresses on-link instead of routing via the gateway. + "ovs-ofctl add-flow " + IMDSBridge + " cookie=" + cookie + ",table=0,priority=250,in_port=" + d.Tap + ",arp,arp_tpa=" + imdsMetaAddr + ",arp_op=1,actions=move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],mod_dl_src:" + d.GatewayMAC + ",load:0x2->NXM_OF_ARP_OP[],move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[],load:0x02aaaaaaaaaa->NXM_NX_ARP_SHA[],load:0xa9fea9fe->NXM_OF_ARP_SPA[],IN_PORT", + "ovs-ofctl add-flow " + IMDSBridge + " cookie=" + cookie + ",table=0,priority=250,in_port=" + d.Tap + ",arp,arp_tpa=" + imdsDNSAddr + ",arp_op=1,actions=move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],mod_dl_src:" + d.GatewayMAC + ",load:0x2->NXM_OF_ARP_OP[],move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[],load:0x02aaaaaaaaaa->NXM_NX_ARP_SHA[],load:0xa9fea9fd->NXM_OF_ARP_SPA[],IN_PORT", // Ingress demux (gateway dst MAC -> endpoint MAC), one per captured addr. "ovs-ofctl add-flow " + IMDSBridge + " cookie=" + cookie + ",table=0,priority=200,in_port=" + d.Tap + ",ip,nw_dst=" + imdsMetaAddr + ",actions=mod_dl_dst:" + d.EndpointMAC + ",output:" + d.Endpoint, "ovs-ofctl add-flow " + IMDSBridge + " cookie=" + cookie + ",table=0,priority=200,in_port=" + d.Tap + ",ip,nw_dst=" + imdsDNSAddr + ",actions=mod_dl_dst:" + d.EndpointMAC + ",output:" + d.Endpoint, @@ -221,3 +225,63 @@ func TestRemoveTapDatapathDeletesEndpointDespitePatchError(t *testing.T) { t.Errorf("endpoint must be deleted even after a patch delete failure; calls: %v", s.calls) } } + +// A guest that treats 169.254.0.0/16 as on-link ARPs for the captured addresses +// rather than routing via the gateway. The reply must advertise GatewayMAC, the +// same L2 identity the egress flow presents, so the guest sees one MAC for the +// address whichever way it resolved it. +func TestIMDSARPResponderFlow(t *testing.T) { + d := testDatapath() + spec, err := imdsARPResponderFlow(d, imdsMetaAddr) + if err != nil { + t.Fatalf("imdsARPResponderFlow: %v", err) + } + + for _, want := range []string{ + "priority=250", + "in_port=" + d.Tap, + "arp_tpa=" + imdsMetaAddr, + "arp_op=1", + "mod_dl_src:" + d.GatewayMAC, + "load:0x2->NXM_OF_ARP_OP[]", + "load:0x02aaaaaaaaaa->NXM_NX_ARP_SHA[]", + "load:0xa9fea9fe->NXM_OF_ARP_SPA[]", + "IN_PORT", + } { + if !strings.Contains(spec, want) { + t.Errorf("flow missing %q:\n %s", want, spec) + } + } +} + +// Every captured address gets a responder, or the one left out is unreachable +// from an on-link guest. +func TestIMDSARPResponderCoversEveryCapturedAddr(t *testing.T) { + d := testDatapath() + for _, addr := range imdsCaptureAddrs { + spec, err := imdsARPResponderFlow(d, addr) + if err != nil { + t.Fatalf("imdsARPResponderFlow(%s): %v", addr, err) + } + if !strings.Contains(spec, "arp_tpa="+addr) { + t.Errorf("flow for %s does not match it:\n %s", addr, spec) + } + } +} + +// A malformed MAC or address must fail loudly rather than install a flow that +// silently answers ARP with nonsense. +func TestIMDSARPResponderFlowRejectsBadInput(t *testing.T) { + d := testDatapath() + d.GatewayMAC = "not-a-mac" + if _, err := imdsARPResponderFlow(d, imdsMetaAddr); err == nil { + t.Error("expected an error for a malformed gateway MAC") + } + + if _, err := imdsARPResponderFlow(testDatapath(), "fe80::1"); err == nil { + t.Error("expected an error for a non-IPv4 captured address") + } + if _, err := imdsARPResponderFlow(testDatapath(), "not-an-ip"); err == nil { + t.Error("expected an error for a malformed captured address") + } +} diff --git a/spinifex/network/topology/dhcp_options.go b/spinifex/network/topology/dhcp_options.go index bbc292d13..ae81c318b 100644 --- a/spinifex/network/topology/dhcp_options.go +++ b/spinifex/network/topology/dhcp_options.go @@ -2,7 +2,9 @@ package topology // BuildSubnetDHCPOptions returns the OVN DHCPOptions map for a subnet. Shared // by the live manager and reconciler to prevent dns_server drift. IMDS is not -// steered via option 121; the subnet-switch localport answers L2 ARP directly. +// steered via option 121: a guest either routes to it via the gateway, where the +// br-imds ingress demux catches it, or resolves it on-link, where the per-tap ARP +// responder answers. Both live in network/host/imds_datapath.go. func BuildSubnetDHCPOptions(gwIP, routerMAC, dnsServer string) map[string]string { return map[string]string{ "server_id": gwIP,