diff --git a/hscontrol/derp/derp.go b/hscontrol/derp/derp.go index 5d8a5c12f..5b2153ae8 100644 --- a/hscontrol/derp/derp.go +++ b/hscontrol/derp/derp.go @@ -147,38 +147,45 @@ func shuffleDERPMap(dm *tailcfg.DERPMap) { slices.Sort(ids) + // A new generator per call gives the same input the same order. Clients + // close their home relay when its node order changes, so a refresh that + // finds no change must not reorder anything. + rng := rand.New(rand.NewSource(derpShuffleSeed())) //nolint:gosec // weak random is fine for DERP scrambling + for _, id := range ids { region := dm.Regions[id] if len(region.Nodes) == 0 { continue } - derpRandom().Shuffle(len(region.Nodes), reflect.Swapper(region.Nodes)) + rng.Shuffle(len(region.Nodes), reflect.Swapper(region.Nodes)) } } var crc64Table = crc64.MakeTable(crc64.ISO) var ( - derpRandomInst *rand.Rand - derpRandomMu sync.Mutex + derpSeed int64 + derpSeedSet bool + derpSeedMu sync.Mutex ) -func derpRandom() *rand.Rand { - derpRandomMu.Lock() - defer derpRandomMu.Unlock() +func derpShuffleSeed() int64 { + derpSeedMu.Lock() + defer derpSeedMu.Unlock() - if derpRandomInst == nil { + if !derpSeedSet { seed := cmp.Or(viper.GetString("dns.base_domain"), time.Now().String()) - derpRandomInst = rand.New(rand.NewSource(int64(crc64.Checksum([]byte(seed), crc64Table)))) //nolint:gosec // weak random is fine for DERP scrambling + derpSeed = int64(crc64.Checksum([]byte(seed), crc64Table)) //nolint:gosec // only a seed, overflow is harmless + derpSeedSet = true } - return derpRandomInst + return derpSeed } func resetDerpRandomForTesting() { - derpRandomMu.Lock() - defer derpRandomMu.Unlock() + derpSeedMu.Lock() + defer derpSeedMu.Unlock() - derpRandomInst = nil + derpSeedSet = false } diff --git a/hscontrol/derp/derp_test.go b/hscontrol/derp/derp_test.go index 79c160cdd..83803443e 100644 --- a/hscontrol/derp/derp_test.go +++ b/hscontrol/derp/derp_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/juanfont/headscale/hscontrol/types" "github.com/spf13/viper" "github.com/stretchr/testify/require" "tailscale.com/tailcfg" @@ -262,6 +263,39 @@ func TestShuffleDERPMapDeterministic(t *testing.T) { } } +// TestGetDERPMapKeepsOrderAcrossRefreshes: clients close their home relay when +// its node order changes, so a refresh of the same input must keep the order. +func TestGetDERPMapKeepsOrderAcrossRefreshes(t *testing.T) { + viper.Set("dns.base_domain", "stable.example.com") + + defer viper.Reset() + + resetDerpRandomForTesting() + + source := &tailcfg.DERPMap{Regions: map[tailcfg.DERPRegionID]*tailcfg.DERPRegion{}} + + for id := range tailcfg.DERPRegionID(5) { + region := &tailcfg.DERPRegion{RegionID: id + 1} + for _, name := range []string{"a", "b", "c", "d", "e"} { + region.Nodes = append(region.Nodes, &tailcfg.DERPNode{Name: name, RegionID: id + 1}) + } + + source.Regions[id+1] = region + } + + first, err := GetDERPMap(types.DERPConfig{DERPMap: source}) + require.NoError(t, err) + + for range 5 { + next, err := GetDERPMap(types.DERPConfig{DERPMap: source}) + require.NoError(t, err) + + if diff := cmp.Diff(first, next); diff != "" { + t.Fatalf("refresh changed the DERP map (-first +next):\n%s", diff) + } + } +} + func TestShuffleDERPMapEdgeCases(t *testing.T) { tests := []struct { name string