diff --git a/spinifex/handlers/ec2/vpc/eni.go b/spinifex/handlers/ec2/vpc/eni.go index d0f8a1d08..9ec3b887a 100644 --- a/spinifex/handlers/ec2/vpc/eni.go +++ b/spinifex/handlers/ec2/vpc/eni.go @@ -18,6 +18,7 @@ import ( "github.com/mulgadc/spinifex/spinifex/foundation/aws/paging" "github.com/mulgadc/spinifex/spinifex/foundation/aws/tags" "github.com/mulgadc/spinifex/spinifex/foundation/state/kvutil" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/network/topology" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go/jetstream" @@ -168,7 +169,7 @@ func (s *VPCServiceImpl) CreateNetworkInterface(ctx context.Context, input *ec2. } // Generate a deterministic MAC address - macAddr := utils.HashMAC(eniId) + macAddr := networkids.HashMAC(eniId) description := "" if input.Description != nil { diff --git a/spinifex/network/external/dhcp/nclient4.go b/spinifex/network/external/dhcp/nclient4.go index a1ef53d1e..126defbea 100644 --- a/spinifex/network/external/dhcp/nclient4.go +++ b/spinifex/network/external/dhcp/nclient4.go @@ -9,7 +9,7 @@ import ( "github.com/insomniacslk/dhcp/dhcpv4" "github.com/insomniacslk/dhcp/dhcpv4/nclient4" - "github.com/mulgadc/spinifex/spinifex/utils" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" ) // NClient4Client is the production DHCP client (AF_PACKET per call, no @@ -282,7 +282,7 @@ func (c *NClient4Client) Release(_ context.Context, lease *Lease) error { // using the same HashMAC scheme as OVN router/port MACs so leases are legible // in upstream dnsmasq logs. func DeriveMAC(clientID string) (net.HardwareAddr, error) { - hw, err := net.ParseMAC(utils.HashMAC(clientID)) + hw, err := net.ParseMAC(networkids.HashMAC(clientID)) if err != nil { return nil, fmt.Errorf("derive mac for client-id %q: %w", clientID, err) } diff --git a/spinifex/network/external/igw.go b/spinifex/network/external/igw.go index 0188f7f04..a9c6833ab 100644 --- a/spinifex/network/external/igw.go +++ b/spinifex/network/external/igw.go @@ -7,11 +7,11 @@ import ( "log/slog" "net/netip" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/network/ovn" "github.com/mulgadc/spinifex/spinifex/network/ovn/nbdb" "github.com/mulgadc/spinifex/spinifex/network/policy" "github.com/mulgadc/spinifex/spinifex/network/topology" - "github.com/mulgadc/spinifex/spinifex/utils" ) // FlowsBarrier blocks until ovn-northd compiles NB→SB and chassis install flows. @@ -195,7 +195,7 @@ func (m *igwManager) AttachIGW(ctx context.Context, spec IGWSpec) error { } if err := m.ovn.CreateLogicalRouterPort(ctx, routerName, &nbdb.LogicalRouterPort{ Name: gwPortName, - MAC: utils.HashMAC(gwPortName), + MAC: networkids.HashMAC(gwPortName), Networks: []string{gwNetwork}, ExternalIDs: lrpExtIDs, }); err != nil { diff --git a/spinifex/network/host/imds_attach.go b/spinifex/network/host/imds_attach.go index 25e84ad5a..14e65a1ef 100644 --- a/spinifex/network/host/imds_attach.go +++ b/spinifex/network/host/imds_attach.go @@ -5,7 +5,7 @@ import ( "errors" "fmt" - "github.com/mulgadc/spinifex/spinifex/utils" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" ) @@ -39,7 +39,7 @@ func imdsDatapathSpec(eniID, mac, subnetID string) IMDSTapDatapath { Endpoint: IMDSEndpointName(eniID), EndpointMAC: IMDSEndpointMAC(eniID), GuestMAC: mac, - GatewayMAC: utils.HashMAC(subnetID), + GatewayMAC: networkids.HashMAC(subnetID), PatchIMDS: IMDSPatchPort(eniID), PatchInt: IMDSIntPatchPort(eniID), IfaceID: vm.OVSIfaceID(eniID), diff --git a/spinifex/network/host/imds_attach_test.go b/spinifex/network/host/imds_attach_test.go index bb940ad49..e427aee33 100644 --- a/spinifex/network/host/imds_attach_test.go +++ b/spinifex/network/host/imds_attach_test.go @@ -7,8 +7,9 @@ import ( "strconv" "testing" - "github.com/mulgadc/spinifex/spinifex/utils" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" + "github.com/mulgadc/spinifex/spinifex/utils" ) func TestIMDSDatapathSpec(t *testing.T) { @@ -33,8 +34,8 @@ func TestIMDSDatapathSpec(t *testing.T) { } // GatewayMAC must match the subnet's OVN router-port MAC so the egress flow // restores the reply source to the gateway the guest expects. - if want := utils.HashMAC(subnetID); d.GatewayMAC != want { - t.Errorf("GatewayMAC = %q, want %q (utils.HashMAC(subnetID))", d.GatewayMAC, want) + if want := networkids.HashMAC(subnetID); d.GatewayMAC != want { + t.Errorf("GatewayMAC = %q, want %q (networkids.HashMAC(subnetID))", d.GatewayMAC, want) } if d.PatchIMDS != IMDSPatchPort(eniID) { t.Errorf("PatchIMDS = %q, want %q", d.PatchIMDS, IMDSPatchPort(eniID)) diff --git a/spinifex/network/host/imds_datapath.go b/spinifex/network/host/imds_datapath.go index f6e7b6024..edcf2b13f 100644 --- a/spinifex/network/host/imds_datapath.go +++ b/spinifex/network/host/imds_datapath.go @@ -9,6 +9,7 @@ import ( "log/slog" "net" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/utils" ) @@ -105,7 +106,7 @@ func IMDSIntPatchPort(eniID string) string { return imdsIntPatchPrefix + shortEN // endpoint owns this MAC so the ingress demux can rewrite the guest's gateway // dst MAC to it (the kernel drops the frame as OTHERHOST otherwise). func IMDSEndpointMAC(eniID string) string { - return utils.HashMAC("imds-ep:" + eniID) + return networkids.HashMAC("imds-ep:" + eniID) } // imdsFlowCookie returns the per-tap OpenFlow cookie tagging endpoint's flows. diff --git a/spinifex/network/identifiers/mac.go b/spinifex/network/identifiers/mac.go new file mode 100644 index 000000000..7245da989 --- /dev/null +++ b/spinifex/network/identifiers/mac.go @@ -0,0 +1,19 @@ +// Package identifiers derives stable network identifiers from resource IDs. +package identifiers + +import ( + "crypto/sha256" + "net" +) + +// HashMAC returns a deterministic locally-administered unicast MAC for id +// (SHA-256; first octet 0x02). id must be globally unique; callers sharing a +// base ID across resource classes must compose a class tag (for example, +// "dev:"+id). +func HashMAC(id string) string { + sum := sha256.Sum256([]byte(id)) + b := make([]byte, 6) + b[0] = 0x02 + copy(b[1:], sum[:5]) + return net.HardwareAddr(b).String() +} diff --git a/spinifex/network/identifiers/mac_test.go b/spinifex/network/identifiers/mac_test.go new file mode 100644 index 000000000..39ba12846 --- /dev/null +++ b/spinifex/network/identifiers/mac_test.go @@ -0,0 +1,49 @@ +package identifiers + +import ( + "fmt" + "net" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHashMAC_FirstOctetPinnedTo02(t *testing.T) { + // First octet must be 0x02 (not just any LAA value) to stay out of vendor OUI space. + for i := range 1000 { + hw, err := net.ParseMAC(HashMAC(fmt.Sprintf("id-%d", i))) + require.NoError(t, err) + assert.Equal(t, byte(0x02), hw[0], "first octet must be 0x02, got %#x", hw[0]) + } +} + +func TestHashMAC_Determinism(t *testing.T) { + // Same id must produce the same MAC across calls and across goroutines. + // Reconcilers across nodes rely on this — non-determinism would + // split-brain DHCP server_mac vs LRP MAC. + const id = "i-abc123" + want := HashMAC(id) + + for range 100 { + assert.Equal(t, want, HashMAC(id)) + } + + var wg sync.WaitGroup + for range 50 { + wg.Go(func() { + for range 100 { + assert.Equal(t, want, HashMAC(id)) + } + }) + } + wg.Wait() +} + +func TestHashMAC_IDSeparates(t *testing.T) { + // Different ids must yield different MACs. + a := HashMAC("subnet-aaaaaaaaaaaaaaaaa") + b := HashMAC("subnet-bbbbbbbbbbbbbbbbb") + assert.NotEqual(t, a, b) +} diff --git a/spinifex/network/reconcile/apply.go b/spinifex/network/reconcile/apply.go index de043ba48..0a20a3aa6 100644 --- a/spinifex/network/reconcile/apply.go +++ b/spinifex/network/reconcile/apply.go @@ -11,12 +11,12 @@ import ( "sync" "time" + "github.com/mulgadc/spinifex/spinifex/foundation/telemetry" "github.com/mulgadc/spinifex/spinifex/network/external" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/network/ovn/nbdb" "github.com/mulgadc/spinifex/spinifex/network/policy" "github.com/mulgadc/spinifex/spinifex/network/topology" - "github.com/mulgadc/spinifex/spinifex/foundation/telemetry" - "github.com/mulgadc/spinifex/spinifex/utils" ) // Bounds for the post-rebind SB chassis-claim wait. Package vars so tests can shorten. @@ -177,7 +177,7 @@ func (r *reconciler) applySubnets(ctx context.Context, intent IntentState, actua continue } gwCIDRString := fmt.Sprintf("%s/%d", gwIP, prefixBits) - routerMAC := utils.HashMAC(subnetID) + routerMAC := networkids.HashMAC(subnetID) if _, ok := actual.Switches[switchName]; !ok { ls := &nbdb.LogicalSwitch{ diff --git a/spinifex/network/topology/manager_live.go b/spinifex/network/topology/manager_live.go index 0453ce837..14d0c59af 100644 --- a/spinifex/network/topology/manager_live.go +++ b/spinifex/network/topology/manager_live.go @@ -9,9 +9,9 @@ import ( "strconv" "strings" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/network/ovn" "github.com/mulgadc/spinifex/spinifex/network/ovn/nbdb" - "github.com/mulgadc/spinifex/spinifex/utils" ) // Option configures a liveManager at construction. @@ -187,7 +187,7 @@ func (m *liveManager) EnsureSubnet(ctx context.Context, spec SubnetSpec) error { } cidr := spec.CIDR.String() gwCIDR := fmt.Sprintf("%s/%d", gwIP, prefixBits) - routerMAC := utils.HashMAC(spec.SubnetID) + routerMAC := networkids.HashMAC(spec.SubnetID) switchName := SubnetSwitch(spec.SubnetID) routerName := VPCRouter(spec.VPCID) routerPortName := SubnetRouterPort(spec.SubnetID) diff --git a/spinifex/runtime/compute/vm/network_helpers.go b/spinifex/runtime/compute/vm/network_helpers.go index 1197e0d2f..352afabd2 100644 --- a/spinifex/runtime/compute/vm/network_helpers.go +++ b/spinifex/runtime/compute/vm/network_helpers.go @@ -6,8 +6,8 @@ import ( "strings" "github.com/aws/aws-sdk-go/aws" + networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/network/topology" - "github.com/mulgadc/spinifex/spinifex/utils" ) // TapSpec parameterises a single tap-on-OVS-bridge plumbing operation. @@ -95,14 +95,14 @@ func IMDSPrimaryTapSpec(eniID string, queues, mtu int) TapSpec { // dev/hostfwd NIC. The "dev:" tag disambiguates from the mgmt NIC of the // same instance (which shares instanceID). func GenerateDevMAC(instanceID string) string { - return utils.HashMAC("dev:" + instanceID) + return networkids.HashMAC("dev:" + instanceID) } // GenerateMgmtMAC returns the locally-administered unicast MAC for the // management NIC. The "mgmt:" tag disambiguates from the dev NIC of the // same instance (which shares instanceID). func GenerateMgmtMAC(instanceID string) string { - return utils.HashMAC("mgmt:" + instanceID) + return networkids.HashMAC("mgmt:" + instanceID) } // attachPrimaryIMDSDatapath installs the per-tap IMDS datapath for the instance's diff --git a/spinifex/utils/utils.go b/spinifex/utils/utils.go index 55e5b43a4..b6ab5ee58 100644 --- a/spinifex/utils/utils.go +++ b/spinifex/utils/utils.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "crypto/rand" - "crypto/sha256" "encoding/hex" "encoding/json" "encoding/xml" @@ -12,7 +11,6 @@ import ( "fmt" "io" "log/slog" - "net" "net/http" "os" "os/signal" @@ -403,16 +401,6 @@ func HumanBytes(b uint64) string { return fmt.Sprintf("%.1f %ciB", float64(b)/float64(div), "KMGTPEZY"[exp]) } -// HashMAC returns a deterministic locally-administered unicast MAC for id (SHA-256; first octet 0x02). -// id must be globally unique; callers sharing a base id across resource classes must compose a class tag (e.g. "dev:"+id). -func HashMAC(id string) string { - sum := sha256.Sum256([]byte(id)) - b := make([]byte, 6) - b[0] = 0x02 - copy(b[1:], sum[:5]) - return net.HardwareAddr(b).String() -} - func dirExists(path string) bool { info, err := os.Stat(path) if os.IsNotExist(err) { diff --git a/spinifex/utils/utils_test.go b/spinifex/utils/utils_test.go index ccc973e17..d466d053b 100644 --- a/spinifex/utils/utils_test.go +++ b/spinifex/utils/utils_test.go @@ -1180,44 +1180,6 @@ func TestWaitForProcessExit_Timeout(t *testing.T) { assert.Contains(t, err.Error(), "timeout") } -func TestHashMAC_FirstOctetPinnedTo02(t *testing.T) { - // First octet must be 0x02 (not just any LAA value) to stay out of vendor OUI space. - for i := range 1000 { - hw, err := net.ParseMAC(HashMAC(fmt.Sprintf("id-%d", i))) - require.NoError(t, err) - assert.Equal(t, byte(0x02), hw[0], "first octet must be 0x02, got %#x", hw[0]) - } -} - -func TestHashMAC_Determinism(t *testing.T) { - // Same id must produce the same MAC across calls and across goroutines. - // Reconcilers across nodes rely on this — non-determinism would - // split-brain DHCP server_mac vs LRP MAC. - const id = "i-abc123" - want := HashMAC(id) - - for range 100 { - assert.Equal(t, want, HashMAC(id)) - } - - var wg sync.WaitGroup - for range 50 { - wg.Go(func() { - for range 100 { - assert.Equal(t, want, HashMAC(id)) - } - }) - } - wg.Wait() -} - -func TestHashMAC_IdSeparates(t *testing.T) { - // Different ids must yield different MACs. - a := HashMAC("subnet-aaaaaaaaaaaaaaaaa") - b := HashMAC("subnet-bbbbbbbbbbbbbbbbb") - assert.NotEqual(t, a, b) -} - // writeTempImage writes content to a file named name under t.TempDir() and // returns the absolute path. func writeTempImage(t *testing.T, name string, content []byte) string {