From b973bf5655638f268c8b75a6292914f93513ac53 Mon Sep 17 00:00:00 2001
From: Tobias Klauser <tobias@cilium.io>
Date: Fri, 8 Dec 2023 12:31:17 +0100
Subject: [PATCH] pkg/rand: replace SafeRand by global source in math/rand

Since Go 1.20 the top-level math/rand functions are auto-seeded by
default, see https://go.dev/doc/go1.20#library and
https://github.com/golang/go/issues/54880. They also use the runtime's
lock-free fastrand64 function when 1. Seed has not been called and 2.
GODEBUG=randautoseed=0 is not used, see
https://github.com/golang/go/issues/49892.

This allows to drop SafeRand and use the global source again. SafeRand
was originally introduced in commit fac5ddea5007 ("rand: add and use
concurrency-safe PRNG source") to fix #10988 by providing a
concurrency-safe PRNG source other than the global math/rand source
which could be used at the time because it can't be interfered with from
unrelated packages by means of calling rand.Seed, see #10575. rand.Seed
is deprecated since Go 1.20 and per the paragraph above the global
source is seeded by default. This makes it unlikely that packages would
interfere with the global PRNG state anymore and the top-level math/rand
functions are safe to use again.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
---
 contrib/scripts/rand-check.sh        | 24 --------
 daemon/cmd/status.go                 |  4 +-
 daemon/cmd/status_test.go            |  7 ++-
 pkg/backoff/backoff.go               | 10 +---
 pkg/ipam/service/allocator/bitmap.go | 13 ++---
 pkg/kvstore/etcd.go                  | 16 ++---
 pkg/lock/stoppable_waitgroup_test.go |  4 --
 pkg/node/manager/manager.go          |  5 +-
 pkg/proxy/proxy.go                   | 10 +---
 pkg/rand/safe_rand.go                | 87 ----------------------------
 pkg/rand/safe_rand_test.go           | 57 ------------------
 test/helpers/utils.go                |  7 +--
 12 files changed, 26 insertions(+), 218 deletions(-)
 delete mode 100755 contrib/scripts/rand-check.sh
 delete mode 100644 pkg/rand/safe_rand.go
 delete mode 100644 pkg/rand/safe_rand_test.go

diff --git a/contrib/scripts/rand-check.sh b/contrib/scripts/rand-check.sh
deleted file mode 100755
index 564b8254d4763..0000000000000
--- a/contrib/scripts/rand-check.sh
+++ /dev/null
@@ -1,24 +0,0 @@
-#!/usr/bin/env bash
-# SPDX-License-Identifier: Apache-2.0
-# Copyright Authors of Cilium
-
-# Make sure pkg/rand is used instead of math/rand, see
-# https://github.com/cilium/cilium/issues/10988. It's fine to use math/rand in
-# tests though.
-for l in rand\.NewSource; do
-  m=$(find . -name "*.go" \
-	  -not -name "*_test.go" \
-	  -not -path "./.git/*" \
-	  -not -path "./_build/*" \
-	  -not -path "./contrib/*" \
-	  -not -path "./pkg/rand/*" \
-	  -not -regex ".*/vendor/.*" \
-	  -not -path "./test/*" \
-	  -print0 \
-	  | xargs -0 grep --exclude NewSafeSource "$l")
-  if [[ ! -z "$m" ]] ; then
-    echo "Found $l usage. Please use pkg/rand instead for a concurrency-safe implementation:"
-    echo $m
-    exit 1
-  fi
-done
diff --git a/daemon/cmd/status.go b/daemon/cmd/status.go
index a7609c8309d09..ae80677803485 100644
--- a/daemon/cmd/status.go
+++ b/daemon/cmd/status.go
@@ -6,6 +6,7 @@ package cmd
 import (
 	"context"
 	"fmt"
+	"math/rand"
 	"net/http"
 	"strings"
 
@@ -37,7 +38,6 @@ import (
 	nodeTypes "github.com/cilium/cilium/pkg/node/types"
 	"github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/promise"
-	"github.com/cilium/cilium/pkg/rand"
 	"github.com/cilium/cilium/pkg/status"
 	"github.com/cilium/cilium/pkg/time"
 	"github.com/cilium/cilium/pkg/version"
@@ -54,7 +54,7 @@ const (
 	k8sMinimumEventHearbeat = time.Minute
 )
 
-var randGen = rand.NewSafeRand(time.Now().UnixNano())
+var randGen = rand.New(rand.NewSource(time.Now().UnixNano()))
 
 type k8sVersion struct {
 	version          string
diff --git a/daemon/cmd/status_test.go b/daemon/cmd/status_test.go
index abe1bf92c84e6..68b8ffe94fd00 100644
--- a/daemon/cmd/status_test.go
+++ b/daemon/cmd/status_test.go
@@ -4,6 +4,7 @@
 package cmd
 
 import (
+	"math/rand"
 	"time"
 
 	. "github.com/cilium/checkmate"
@@ -44,9 +45,9 @@ func (g *GetNodesSuite) SetUpSuite(c *C) {
 }
 
 func (g *GetNodesSuite) Test_getNodesHandle(c *C) {
-	// Set seed so we can have the same pseudorandom client IDs.
-	// The seed is set to 0 for each unit test.
-	randGen.Seed(0)
+	// Create a PRNG source with a fixed seed so we can have the same pseudo-random client IDs
+	// for each test run.
+	randGen = rand.New(rand.NewSource(0))
 	const numberOfClients = 10
 
 	clientIDs := make([]int64, 0, numberOfClients)
diff --git a/pkg/backoff/backoff.go b/pkg/backoff/backoff.go
index 2cfbde3dcad8c..e08dc2b23cb0a 100644
--- a/pkg/backoff/backoff.go
+++ b/pkg/backoff/backoff.go
@@ -7,21 +7,17 @@ import (
 	"context"
 	"fmt"
 	"math"
+	"math/rand"
 
 	"github.com/google/uuid"
 	"github.com/sirupsen/logrus"
 
 	"github.com/cilium/cilium/pkg/logging"
 	"github.com/cilium/cilium/pkg/logging/logfields"
-	"github.com/cilium/cilium/pkg/rand"
 	"github.com/cilium/cilium/pkg/time"
 )
 
-var (
-	log = logging.DefaultLogger.WithField(logfields.LogSubsys, "backoff")
-
-	randGen = rand.NewSafeRand(time.Now().UnixNano())
-)
+var log = logging.DefaultLogger.WithField(logfields.LogSubsys, "backoff")
 
 // NodeManager is the interface required to implement cluster size dependent
 // intervals
@@ -98,7 +94,7 @@ func CalculateDuration(min, max time.Duration, factor float64, jitter bool, fail
 	}
 
 	if jitter {
-		t = randGen.Float64()*(t-minFloat) + minFloat
+		t = rand.Float64()*(t-minFloat) + minFloat
 	}
 
 	return time.Duration(t)
diff --git a/pkg/ipam/service/allocator/bitmap.go b/pkg/ipam/service/allocator/bitmap.go
index 96b2ceba4897e..1d89ba8398cee 100644
--- a/pkg/ipam/service/allocator/bitmap.go
+++ b/pkg/ipam/service/allocator/bitmap.go
@@ -7,10 +7,9 @@ package allocator
 import (
 	"errors"
 	"math/big"
+	"math/rand"
 
 	"github.com/cilium/cilium/pkg/lock"
-	"github.com/cilium/cilium/pkg/rand"
-	"github.com/cilium/cilium/pkg/time"
 )
 
 // AllocationBitmap is a contiguous block of resources that can be allocated atomically.
@@ -50,9 +49,7 @@ type bitAllocator interface {
 // NewAllocationMap creates an allocation bitmap using the random scan strategy.
 func NewAllocationMap(max int, rangeSpec string) *AllocationBitmap {
 	a := AllocationBitmap{
-		strategy: randomScanStrategy{
-			rand: rand.NewSafeRand(time.Now().UnixNano()),
-		},
+		strategy:  randomScanStrategy{},
 		allocated: big.NewInt(0),
 		count:     0,
 		max:       max,
@@ -186,15 +183,13 @@ func (r *AllocationBitmap) Restore(rangeSpec string, data []byte) error {
 // randomScanStrategy chooses a random address from the provided big.Int, and then
 // scans forward looking for the next available address (it will wrap the range if
 // necessary).
-type randomScanStrategy struct {
-	rand *rand.SafeRand
-}
+type randomScanStrategy struct{}
 
 func (rss randomScanStrategy) AllocateBit(allocated *big.Int, max, count int) (int, bool) {
 	if count >= max {
 		return 0, false
 	}
-	offset := rss.rand.Intn(max)
+	offset := rand.Intn(max)
 	for i := 0; i < max; i++ {
 		at := (offset + i) % max
 		if allocated.Bit(at) == 0 {
diff --git a/pkg/kvstore/etcd.go b/pkg/kvstore/etcd.go
index 78c68293da2f6..e8cf83c206cac 100644
--- a/pkg/kvstore/etcd.go
+++ b/pkg/kvstore/etcd.go
@@ -9,6 +9,7 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
+	"math/rand"
 	"net/url"
 	"os"
 	"strconv"
@@ -32,7 +33,6 @@ import (
 	"github.com/cilium/cilium/pkg/inctimer"
 	"github.com/cilium/cilium/pkg/lock"
 	"github.com/cilium/cilium/pkg/option"
-	"github.com/cilium/cilium/pkg/rand"
 	ciliumrate "github.com/cilium/cilium/pkg/rate"
 	ciliumratemetrics "github.com/cilium/cilium/pkg/rate/metrics"
 	"github.com/cilium/cilium/pkg/spanstat"
@@ -63,13 +63,9 @@ const (
 	etcdMaxKeysPerLease = 1000
 )
 
-var (
-	// ErrLockLeaseExpired is an error whenever the lease of the lock does not
-	// exist or it was expired.
-	ErrLockLeaseExpired = errors.New("transaction did not succeed: lock lease expired")
-
-	randGen = rand.NewSafeRand(time.Now().UnixNano())
-)
+// ErrLockLeaseExpired is an error whenever the lease of the lock does not
+// exist or it was expired.
+var ErrLockLeaseExpired = errors.New("transaction did not succeed: lock lease expired")
 
 type etcdModule struct {
 	opts   backendOptions
@@ -181,7 +177,7 @@ func (e *etcdModule) getConfig() map[string]string {
 }
 
 func shuffleEndpoints(endpoints []string) {
-	randGen.Shuffle(len(endpoints), func(i, j int) {
+	rand.Shuffle(len(endpoints), func(i, j int) {
 		endpoints[i], endpoints[j] = endpoints[j], endpoints[i]
 	})
 }
@@ -425,7 +421,7 @@ func (e *etcdClient) waitForInitLock(ctx context.Context) <-chan error {
 
 			// Generate a random number so that we can acquire a lock even
 			// if other agents are killed while locking this path.
-			randNumber := strconv.FormatUint(randGen.Uint64(), 16)
+			randNumber := strconv.FormatUint(rand.Uint64(), 16)
 			locker, err := e.LockPath(ctx, InitLockPath+"/"+randNumber)
 			if err == nil {
 				locker.Unlock(context.Background())
diff --git a/pkg/lock/stoppable_waitgroup_test.go b/pkg/lock/stoppable_waitgroup_test.go
index b4eb555fe67c3..dbac07757b4be 100644
--- a/pkg/lock/stoppable_waitgroup_test.go
+++ b/pkg/lock/stoppable_waitgroup_test.go
@@ -154,10 +154,6 @@ func (s *SemaphoredMutexSuite) TestWaitChannel(c *C) {
 func (s *SemaphoredMutexSuite) TestParallelism(c *C) {
 	l := NewStoppableWaitGroup()
 
-	// Use math/rand instead of pkg/rand to avoid a test import cycle which
-	// go vet would complain about. Use the global default entropy source
-	// rather than creating a new source to avoid concurrency issues.
-	rand.Seed(time.Now().UnixNano())
 	in := make(chan int)
 	stop := make(chan struct{})
 	go func() {
diff --git a/pkg/node/manager/manager.go b/pkg/node/manager/manager.go
index df3f2996ef7f0..9006e89485be8 100644
--- a/pkg/node/manager/manager.go
+++ b/pkg/node/manager/manager.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math/rand"
 	"net"
 	"net/netip"
 
@@ -35,13 +36,11 @@ import (
 	"github.com/cilium/cilium/pkg/node/addressing"
 	nodeTypes "github.com/cilium/cilium/pkg/node/types"
 	"github.com/cilium/cilium/pkg/option"
-	"github.com/cilium/cilium/pkg/rand"
 	"github.com/cilium/cilium/pkg/source"
 	"github.com/cilium/cilium/pkg/time"
 )
 
 var (
-	randGen                    = rand.NewSafeRand(time.Now().UnixNano())
 	baseBackgroundSyncInterval = time.Minute
 
 	neighborTableRefreshControllerGroup = controller.NewGroup("neighbor-table-refresh")
@@ -836,7 +835,7 @@ func (m *manager) StartNeighborRefresh(nh datapath.NodeNeighbors) {
 						// To avoid flooding network with arping requests
 						// at the same time, spread them over the
 						// [0; ARPPingRefreshPeriod/2) period.
-						n := randGen.Int63n(int64(option.Config.ARPPingRefreshPeriod / 2))
+						n := rand.Int63n(int64(option.Config.ARPPingRefreshPeriod / 2))
 						time.Sleep(time.Duration(n))
 						nh.NodeNeighborRefresh(c, e)
 					}(ctx, entryNode)
diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go
index 2d3f03ced81ac..cfed812a72cbf 100644
--- a/pkg/proxy/proxy.go
+++ b/pkg/proxy/proxy.go
@@ -6,6 +6,7 @@ package proxy
 import (
 	"context"
 	"fmt"
+	"math/rand"
 	"net"
 
 	"github.com/sirupsen/logrus"
@@ -25,16 +26,11 @@ import (
 	"github.com/cilium/cilium/pkg/policy"
 	"github.com/cilium/cilium/pkg/proxy/endpoint"
 	"github.com/cilium/cilium/pkg/proxy/types"
-	"github.com/cilium/cilium/pkg/rand"
 	"github.com/cilium/cilium/pkg/revert"
 	"github.com/cilium/cilium/pkg/time"
 )
 
-var (
-	log = logging.DefaultLogger.WithField(logfields.LogSubsys, "proxy")
-
-	portRandomizer = rand.NewSafeRand(time.Now().UnixNano())
-)
+var log = logging.DefaultLogger.WithField(logfields.LogSubsys, "proxy")
 
 // field names used while logging
 const (
@@ -201,7 +197,7 @@ func (p *Proxy) allocatePort(port, min, max uint16) (uint16, error) {
 	}
 
 	// TODO: Maybe not create a large permutation each time?
-	portRange := portRandomizer.Perm(int(max - min + 1))
+	portRange := rand.Perm(int(max - min + 1))
 
 	// Allow reuse of previously used ports only if no ports are otherwise availeble.
 	// This allows the same port to be used again by a listener being reconfigured
diff --git a/pkg/rand/safe_rand.go b/pkg/rand/safe_rand.go
deleted file mode 100644
index 9829484a18159..0000000000000
--- a/pkg/rand/safe_rand.go
+++ /dev/null
@@ -1,87 +0,0 @@
-// SPDX-License-Identifier: Apache-2.0
-// Copyright Authors of Cilium
-
-package rand
-
-import (
-	"math/rand"
-
-	"github.com/cilium/cilium/pkg/lock"
-)
-
-// SafeRand is a concurrency-safe source of pseudo-random numbers. The Go
-// stdlib's math/rand.Source is not concurrency-safe. The global source in
-// math/rand would be concurrency safe (due to its internal use of
-// lockedSource), but it is prone to inter-package interference with the PRNG
-// state.
-// Also see https://github.com/cilium/cilium/issues/10988
-type SafeRand struct {
-	mu lock.Mutex
-	r  *rand.Rand
-}
-
-func NewSafeRand(seed int64) *SafeRand {
-	return &SafeRand{r: rand.New(rand.NewSource(seed))}
-}
-
-func (sr *SafeRand) Seed(seed int64) {
-	sr.mu.Lock()
-	sr.r.Seed(seed)
-	sr.mu.Unlock()
-}
-
-func (sr *SafeRand) Int63() int64 {
-	sr.mu.Lock()
-	v := sr.r.Int63()
-	sr.mu.Unlock()
-	return v
-}
-
-func (sr *SafeRand) Int63n(n int64) int64 {
-	sr.mu.Lock()
-	v := sr.r.Int63n(n)
-	sr.mu.Unlock()
-	return v
-}
-
-func (sr *SafeRand) Uint32() uint32 {
-	sr.mu.Lock()
-	v := sr.r.Uint32()
-	sr.mu.Unlock()
-	return v
-}
-
-func (sr *SafeRand) Uint64() uint64 {
-	sr.mu.Lock()
-	v := sr.r.Uint64()
-	sr.mu.Unlock()
-	return v
-}
-
-func (sr *SafeRand) Intn(n int) int {
-	sr.mu.Lock()
-	v := sr.r.Intn(n)
-	sr.mu.Unlock()
-	return v
-}
-
-func (sr *SafeRand) Float64() float64 {
-	sr.mu.Lock()
-	v := sr.r.Float64()
-	sr.mu.Unlock()
-	return v
-}
-
-func (sr *SafeRand) Perm(n int) []int {
-	sr.mu.Lock()
-	v := sr.r.Perm(n)
-	sr.mu.Unlock()
-	return v
-
-}
-
-func (sr *SafeRand) Shuffle(n int, swap func(i, j int)) {
-	sr.mu.Lock()
-	sr.r.Shuffle(n, swap)
-	sr.mu.Unlock()
-}
diff --git a/pkg/rand/safe_rand_test.go b/pkg/rand/safe_rand_test.go
deleted file mode 100644
index 4d3b80b6af1d1..0000000000000
--- a/pkg/rand/safe_rand_test.go
+++ /dev/null
@@ -1,57 +0,0 @@
-// SPDX-License-Identifier: Apache-2.0
-// Copyright Authors of Cilium
-
-package rand
-
-import (
-	"testing"
-	"time"
-)
-
-func TestIntn(t *testing.T) {
-	r0 := NewSafeRand(1)
-	r1 := NewSafeRand(1)
-
-	n := 10
-	for i := 0; i < 100; i++ {
-		v0 := r0.Intn(n)
-		v1 := r1.Intn(n)
-
-		if v0 != v1 {
-			t.Errorf("Intn() returned different values at iteration %d: %d != %d", i, v0, v1)
-		}
-	}
-}
-
-func TestShuffle(t *testing.T) {
-	r0 := NewSafeRand(time.Now().UnixNano())
-
-	s1 := []string{"1", "2", "3", "4", "5"}
-	s2 := make([]string, len(s1))
-	copy(s2, s1)
-
-	var same int
-	for retry := 0; retry < 10; retry++ {
-		same = 0
-
-		r0.Shuffle(len(s2), func(i, j int) {
-			s2[i], s2[j] = s2[j], s2[i]
-		})
-
-		if len(s1) != len(s2) {
-			t.Errorf("Shuffle() resulted in slices of inequal length")
-		}
-
-		for i := range s1 {
-			if s1[i] == s2[i] {
-				same++
-			}
-		}
-		if same != len(s1) {
-			break
-		}
-	}
-	if same == len(s1) {
-		t.Errorf("Shuffle() did not modify s2 in 10 retries")
-	}
-}
diff --git a/test/helpers/utils.go b/test/helpers/utils.go
index 399faaa1c4aed..349e818b1d4c4 100644
--- a/test/helpers/utils.go
+++ b/test/helpers/utils.go
@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"html/template"
 	"io"
+	"math/rand"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -20,15 +21,11 @@ import (
 	. "github.com/onsi/gomega"
 	"golang.org/x/sys/unix"
 
-	"github.com/cilium/cilium/pkg/rand"
 	"github.com/cilium/cilium/pkg/versioncheck"
 	"github.com/cilium/cilium/test/config"
 	ginkgoext "github.com/cilium/cilium/test/ginkgo-ext"
 )
 
-// ensure that our random numbers are seeded differently on each run
-var randGen = rand.NewSafeRand(time.Now().UnixNano())
-
 // IsRunningOnJenkins detects if the currently running Ginkgo application is
 // most likely running in a Jenkins environment. Returns true if certain
 // environment variables that are present in Jenkins jobs are set, false
@@ -67,7 +64,7 @@ func CountValues(key string, data []string) (int, int) {
 
 // MakeUID returns a randomly generated string.
 func MakeUID() string {
-	return fmt.Sprintf("%08x", randGen.Uint32())
+	return fmt.Sprintf("%08x", rand.Uint32())
 }
 
 // RenderTemplate renders a text/template string into a buffer.