nestack: use cryptographically secure RNG when appropriate

This addresses an issue discovered by Inon Kaplan (PhD candidate in the
Hebrew University School of Computer Science and Engineering), Ron Even (BSc
graduate of Bar Ilan University) and Amit Klein (faculty member in the Hebrew
University School of Computer Science and Engineering). Details will be
provided in their paper, to be presented in a forthcoming academic conference.

Also:
- Add a secure RNG type to prevent mixing up with the default PRNG
- Give the PRNG the name `InsecureRNG` to make it more obvious to future
  contributors that some RNGs are inappropriate in certain instances.
- Some tests were injecting fake RNGs and had to be relaxed: they relied on the
  stack calling the RNG a specific number of times and in a specific order.
  That order is now changed, and is too brittle to unit test.
- Remove the double package comment in pkg/rand. The linter complains.
PiperOrigin-RevId: 577513723

pkg/rand/BUILD

@@ -10,6 +10,7 @@ go_library(
     srcs = [
         "rand.go",
         "rand_linux.go",
+        "rng.go",
     ],
     visibility = ["//:sandbox"],
     deps = [

pkg/rand/rand.go

@@ -15,8 +15,6 @@
 //go:build !linux
 // +build !linux
 
-// Package rand implements a cryptographically secure pseudorandom number
-// generator.
 package rand
 
 import "crypto/rand"

pkg/rand/rand_linux.go

@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package rand implements a cryptographically secure pseudorandom number
-// generator.
 package rand
 
 import (

pkg/rand/rng.go

@@ -0,0 +1,44 @@
+// Copyright 2023 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package rand implements a cryptographically secure pseudorandom number
+// generator.
+package rand
+
+import (
+	"encoding/binary"
+	"fmt"
+	"io"
+)
+
+// RNG exposes convenience functions based on a cryptographically secure
+// io.Reader.
+type RNG struct {
+	Reader io.Reader
+}
+
+// RNGFrom returns a new RNG. r must be a cryptographically secure io.Reader.
+func RNGFrom(r io.Reader) RNG {
+	return RNG{Reader: r}
+}
+
+// Uint32 is analogous to the standard library's math/rand.Uint32.
+func (rg *RNG) Uint32() uint32 {
+	var data [4]byte
+	if _, err := rg.Reader.Read(data[:]); err != nil {
+		panic(fmt.Sprintf("Read() failed: %v", err))
+	}
+	// The endianness doesn't matter here as it's random bytes either way.
+	return binary.LittleEndian.Uint32(data[:])
+}

pkg/tcpip/network/arp/arp.go

@@ -278,7 +278,7 @@ func (p *protocol) NewEndpoint(nic stack.NetworkInterface, _ stack.TransportDisp
 	e.mu.Lock()
 	e.dad.Init(&e.mu, p.options.DADConfigs, ip.DADOptions{
 		Clock:     p.stack.Clock(),
-		SecureRNG: p.stack.SecureRNG(),
+		SecureRNG: p.stack.SecureRNG().Reader,
 		// ARP does not support sending nonce values.
 		NonceSize: 0,
 		Protocol:  e,

pkg/tcpip/network/ipv4/igmp.go

@@ -283,7 +283,7 @@ func (*igmpState) V2QueryMaxRespCodeToV1Delay(code uint16) time.Duration {
 func (igmp *igmpState) init(ep *endpoint) {
 	igmp.ep = ep
 	igmp.genericMulticastProtocol.Init(&ep.mu, ip.GenericMulticastProtocolOptions{
-		Rand:                      ep.protocol.stack.Rand(),
+		Rand:                      ep.protocol.stack.InsecureRNG(),
 		Clock:                     ep.protocol.stack.Clock(),
 		Protocol:                  igmp,
 		MaxUnsolicitedReportDelay: UnsolicitedReportIntervalMax,

pkg/tcpip/network/ipv6/ipv6.go

@@ -2341,7 +2341,7 @@ func (p *protocol) NewEndpoint(nic stack.NetworkInterface, dispatcher stack.Tran
 	const maxMulticastSolicit = 3
 	dadOptions := ip.DADOptions{
 		Clock:              p.stack.Clock(),
-		SecureRNG:          p.stack.SecureRNG(),
+		SecureRNG:          p.stack.SecureRNG().Reader,
 		NonceSize:          nonceSize,
 		ExtendDADTransmits: maxMulticastSolicit,
 		Protocol:           &e.mu.ndp,

pkg/tcpip/network/ipv6/mld.go

@@ -230,7 +230,7 @@ func (*mldState) V2QueryMaxRespCodeToV1Delay(code uint16) time.Duration {
 func (mld *mldState) init(ep *endpoint) {
 	mld.ep = ep
 	mld.genericMulticastProtocol.Init(&ep.mu.RWMutex, ip.GenericMulticastProtocolOptions{
-		Rand:                      ep.protocol.stack.Rand(),
+		Rand:                      ep.protocol.stack.InsecureRNG(),
 		Clock:                     ep.protocol.stack.Clock(),
 		Protocol:                  mld,
 		MaxUnsolicitedReportDelay: UnsolicitedReportIntervalMax,

pkg/tcpip/network/ipv6/mld_test.go

@@ -15,7 +15,6 @@
 package ipv6_test
 
 import (
-	"bytes"
 	"math/rand"
 	"os"
 	"testing"
@@ -246,28 +245,13 @@ func TestSendQueuedMLDReports(t *testing.T) {
 		},
 	}
 
-	nonce := [...]byte{
-		1, 2, 3, 4, 5, 6,
-	}
-
-	const maxNSMessages = 2
-	secureRNGBytes := make([]byte, len(nonce)*maxNSMessages)
-	for b := secureRNGBytes[:]; len(b) > 0; b = b[len(nonce):] {
-		if n := copy(b, nonce[:]); n != len(nonce) {
-			t.Fatalf("got copy(...) = %d, want = %d", n, len(nonce))
-		}
-	}
-
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			for _, subTest := range subTests {
 				t.Run(subTest.name, func(t *testing.T) {
 					dadResolutionTime := test.retransmitTimer * time.Duration(test.dadTransmits)
 					clock := faketime.NewManualClock()
-					var secureRNG bytes.Reader
-					secureRNG.Reset(secureRNGBytes[:])
 					s := stack.New(stack.Options{
-						SecureRNG:  &secureRNG,
 						RandSource: rand.NewSource(time.Now().UnixNano()),
 						NetworkProtocols: []stack.NetworkProtocolFactory{ipv6.NewProtocolWithOptions(ipv6.Options{
 							DADConfigs: stack.DADConfigurations{
@@ -308,7 +292,6 @@ func TestSendQueuedMLDReports(t *testing.T) {
 								checker.TTL(header.NDPHopLimit),
 								checker.NDPNS(
 									checker.NDPNSTargetAddress(addr),
-									checker.NDPNSOptions([]header.NDPOption{header.NDPNonceOption(nonce[:])}),
 								))
 							p.DecRef()
 						}

pkg/tcpip/network/ipv6/ndp.go

@@ -1820,7 +1820,7 @@ func (ndp *ndpState) startSolicitingRouters() {
 	// 4861 section 6.3.7.
 	var delay time.Duration
 	if ndp.configs.MaxRtrSolicitationDelay > 0 {
-		delay = time.Duration(ndp.ep.protocol.stack.Rand().Int63n(int64(ndp.configs.MaxRtrSolicitationDelay)))
+		delay = time.Duration(ndp.ep.protocol.stack.InsecureRNG().Int63n(int64(ndp.configs.MaxRtrSolicitationDelay)))
 	}
 
 	// Protected by ndp.ep.mu.
@@ -1965,7 +1965,7 @@ func (ndp *ndpState) init(ep *endpoint, dadOptions ip.DADOptions) {
 	ndp.slaacPrefixes = make(map[tcpip.Subnet]slaacPrefixState)
 
 	header.InitialTempIID(ndp.temporaryIIDHistory[:], ndp.ep.protocol.options.TempIIDSeed, ndp.ep.nic.ID())
-	ndp.temporaryAddressDesyncFactor = time.Duration(ep.protocol.stack.Rand().Int63n(int64(MaxDesyncFactor)))
+	ndp.temporaryAddressDesyncFactor = time.Duration(ep.protocol.stack.InsecureRNG().Int63n(int64(MaxDesyncFactor)))
 }
 
 func (ndp *ndpState) SendDADMessage(addr tcpip.Address, nonce []byte) tcpip.Error {

pkg/tcpip/network/ipv6/ndp_test.go

@@ -15,7 +15,6 @@
 package ipv6
 
 import (
-	"bytes"
 	"math/rand"
 	"strings"
 	"testing"
@@ -1286,21 +1285,9 @@ func TestCheckDuplicateAddress(t *testing.T) {
 		RetransmitTimer:        time.Second,
 	}
 
-	nonces := [...][]byte{
-		{1, 2, 3, 4, 5, 6},
-		{7, 8, 9, 10, 11, 12},
-	}
-
-	var secureRNGBytes []byte
-	for _, n := range nonces {
-		secureRNGBytes = append(secureRNGBytes, n...)
-	}
-	var secureRNG bytes.Reader
-	secureRNG.Reset(secureRNGBytes[:])
 	s := stack.New(stack.Options{
 		Clock:      clock,
 		RandSource: rand.NewSource(time.Now().UnixNano()),
-		SecureRNG:  &secureRNG,
 		NetworkProtocols: []stack.NetworkProtocolFactory{NewProtocolWithOptions(Options{
 			DADConfigs: dadConfigs,
 		})},
@@ -1346,7 +1333,6 @@ func TestCheckDuplicateAddress(t *testing.T) {
 			checker.TTL(header.NDPHopLimit),
 			checker.NDPNS(
 				checker.NDPNSTargetAddress(lladdr0),
-				checker.NDPNSOptions([]header.NDPOption{header.NDPNonceOption(nonces[dadPacketsSent])}),
 			))
 	}
 	protocolAddr := tcpip.ProtocolAddress{

pkg/tcpip/ports/BUILD

@@ -14,6 +14,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/atomicbitops",
+        "//pkg/rand",
         "//pkg/sync",
         "//pkg/tcpip",
         "//pkg/tcpip/header",
@@ -25,6 +26,7 @@ go_test(
     srcs = ["ports_test.go"],
     library = ":ports",
     deps = [
+        "//pkg/rand",
         "//pkg/tcpip",
         "//pkg/tcpip/testutil",
         "@com_github_google_go_cmp//cmp:go_default_library",

pkg/tcpip/ports/ports.go

@@ -18,9 +18,9 @@ package ports
 
 import (
 	"math"
-	"math/rand"
 
 	"gvisor.dev/gvisor/pkg/atomicbitops"
+	"gvisor.dev/gvisor/pkg/rand"
 	"gvisor.dev/gvisor/pkg/sync"
 	"gvisor.dev/gvisor/pkg/tcpip"
 	"gvisor.dev/gvisor/pkg/tcpip/header"
@@ -255,14 +255,13 @@ type PortTester func(port uint16) (good bool, err tcpip.Error)
 // possible ephemeral ports, allowing the caller to decide whether a given port
 // is suitable for its needs, and stopping when a port is found or an error
 // occurs.
-func (pm *PortManager) PickEphemeralPort(rng *rand.Rand, testPort PortTester) (port uint16, err tcpip.Error) {
+func (pm *PortManager) PickEphemeralPort(rng rand.RNG, testPort PortTester) (port uint16, err tcpip.Error) {
 	pm.ephemeralMu.RLock()
 	firstEphemeral := pm.firstEphemeral
 	numEphemeral := pm.numEphemeral
 	pm.ephemeralMu.RUnlock()
 
-	offset := uint32(rng.Int31n(int32(numEphemeral)))
-	return pickEphemeralPort(offset, firstEphemeral, numEphemeral, testPort)
+	return pickEphemeralPort(rng.Uint32(), firstEphemeral, numEphemeral, testPort)
 }
 
 // portHint atomically reads and returns the pm.hint value.
@@ -320,7 +319,7 @@ func pickEphemeralPort(offset uint32, first, count uint16, testPort PortTester)
 // An optional PortTester can be passed in which if provided will be used to
 // test if the picked port can be used. The function should return true if the
 // port is safe to use, false otherwise.
-func (pm *PortManager) ReservePort(rng *rand.Rand, res Reservation, testPort PortTester) (reservedPort uint16, err tcpip.Error) {
+func (pm *PortManager) ReservePort(rng rand.RNG, res Reservation, testPort PortTester) (reservedPort uint16, err tcpip.Error) {
 	pm.mu.Lock()
 	defer pm.mu.Unlock()
 

pkg/tcpip/ports/ports_test.go

@@ -18,9 +18,9 @@ import (
 	"math"
 	"math/rand"
 	"testing"
-	"time"
 
 	"github.com/google/go-cmp/cmp"
+	cryptorand "gvisor.dev/gvisor/pkg/rand"
 	"gvisor.dev/gvisor/pkg/tcpip"
 	"gvisor.dev/gvisor/pkg/tcpip/testutil"
 )
@@ -332,7 +332,7 @@ func TestPortReservation(t *testing.T) {
 		t.Run(test.tname, func(t *testing.T) {
 			pm := NewPortManager()
 			net := []tcpip.NetworkProtocolNumber{fakeNetworkNumber}
-			rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+			rng := cryptorand.RNGFrom(cryptorand.Reader)
 
 			for _, test := range test.actions {
 				first, _ := pm.PortRange()
@@ -419,7 +419,7 @@ func TestPickEphemeralPort(t *testing.T) {
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			pm := NewPortManager()
-			rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+			rng := cryptorand.RNGFrom(cryptorand.Reader)
 			if err := pm.SetPortRange(firstEphemeral, firstEphemeral+numEphemeralPorts); err != nil {
 				t.Fatalf("failed to set ephemeral port range: %s", err)
 			}

pkg/tcpip/stack/ndp_test.go

@@ -15,7 +15,6 @@
 package stack_test
 
 import (
-	"bytes"
 	"encoding/binary"
 	"fmt"
 	"math"
@@ -547,16 +546,6 @@ func TestDADResolve(t *testing.T) {
 		},
 	}
 
-	nonces := [][]byte{
-		{1, 2, 3, 4, 5, 6},
-		{7, 8, 9, 10, 11, 12},
-	}
-
-	var secureRNGBytes []byte
-	for _, n := range nonces {
-		secureRNGBytes = append(secureRNGBytes, n...)
-	}
-
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			ndpDisp := ndpDispatcher{
@@ -568,14 +557,10 @@ func TestDADResolve(t *testing.T) {
 			}
 			e.Endpoint.LinkEPCapabilities |= stack.CapabilityResolutionRequired
 
-			var secureRNG bytes.Reader
-			secureRNG.Reset(secureRNGBytes)
-
 			clock := faketime.NewManualClock()
 			s := stack.New(stack.Options{
 				Clock:      clock,
 				RandSource: rand.NewSource(time.Now().UnixNano()),
-				SecureRNG:  &secureRNG,
 				NetworkProtocols: []stack.NetworkProtocolFactory{ipv6.NewProtocolWithOptions(ipv6.Options{
 					NDPDisp: &ndpDisp,
 					DADConfigs: stack.DADConfigurations{
@@ -726,7 +711,6 @@ func TestDADResolve(t *testing.T) {
 					checker.TTL(header.NDPHopLimit),
 					checker.NDPNS(
 						checker.NDPNSTargetAddress(addr1),
-						checker.NDPNSOptions([]header.NDPOption{header.NDPNonceOption(nonces[i])}),
 					))
 
 				if l, want := p.AvailableHeaderBytes(), int(test.linkHeaderLen); l != want {

pkg/tcpip/stack/neighbor_cache.go

@@ -298,7 +298,7 @@ func (n *neighborCache) handleConfirmation(addr tcpip.Address, linkAddr tcpip.Li
 func (n *neighborCache) init(nic *nic, r LinkAddressResolver) {
 	*n = neighborCache{
 		nic:     nic,
-		state:   NewNUDState(nic.stack.nudConfigs, nic.stack.clock, nic.stack.randomGenerator),
+		state:   NewNUDState(nic.stack.nudConfigs, nic.stack.clock, nic.stack.insecureRNG),
 		linkRes: r,
 	}
 	n.mu.Lock()

pkg/tcpip/stack/neighbor_cache_test.go

@@ -87,11 +87,11 @@ func newTestNeighborResolver(nudDisp NUDDispatcher, config NUDConfigurations, cl
 		delay:   typicalLatency,
 	}
 	stack := &Stack{
-		clock:           clock,
-		nudDisp:         nudDisp,
-		nudConfigs:      config,
-		randomGenerator: rng,
-		stats:           tcpip.Stats{}.FillIn(),
+		clock:       clock,
+		nudDisp:     nudDisp,
+		nudConfigs:  config,
+		insecureRNG: rng,
+		stats:       tcpip.Stats{}.FillIn(),
 	}
 
 	linkRes.neigh.init(&nic{

pkg/tcpip/stack/neighbor_entry_test.go

@@ -209,10 +209,10 @@ func entryTestSetup(c NUDConfigurations) (*neighborEntry, *testNUDDispatcher, *e
 
 		id: entryTestNICID,
 		stack: &Stack{
-			clock:           clock,
-			nudDisp:         &disp,
-			nudConfigs:      c,
-			randomGenerator: rand.New(rand.NewSource(time.Now().UnixNano())),
+			clock:       clock,
+			nudDisp:     &disp,
+			nudConfigs:  c,
+			insecureRNG: rand.New(rand.NewSource(time.Now().UnixNano())),
 		},
 		stats: makeNICStats(tcpip.NICStats{}.FillIn()),
 	}

pkg/tcpip/stack/stack.go

@@ -139,11 +139,12 @@ type Stack struct {
 	uniqueIDGenerator UniqueID
 
 	// randomGenerator is an injectable pseudo random generator that can be
-	// used when a random number is required.
-	randomGenerator *rand.Rand
+	// used when a random number is required. It must not be used in
+	// security-sensitive contexts.
+	insecureRNG *rand.Rand
 
 	// secureRNG is a cryptographically secure random number generator.
-	secureRNG io.Reader
+	secureRNG cryptorand.RNG
 
 	// sendBufferSize holds the min/default/max send buffer sizes for
 	// endpoints other than TCP.
@@ -343,6 +344,7 @@ func New(opts Options) *Stack {
 	if opts.SecureRNG == nil {
 		opts.SecureRNG = cryptorand.Reader
 	}
+	secureRNG := cryptorand.RNGFrom(opts.SecureRNG)
 
 	randSrc := opts.RandSource
 	if randSrc == nil {
@@ -354,13 +356,13 @@ func New(opts Options) *Stack {
 		// we wrap it in a simple thread-safe version.
 		randSrc = &lockedRandomSource{src: rand.NewSource(v)}
 	}
-	randomGenerator := rand.New(randSrc)
+	insecureRNG := rand.New(randSrc)
 
 	if opts.IPTables == nil {
 		if opts.DefaultIPTables == nil {
 			opts.DefaultIPTables = DefaultTables
 		}
-		opts.IPTables = opts.DefaultIPTables(clock, randomGenerator)
+		opts.IPTables = opts.DefaultIPTables(clock, insecureRNG)
 	}
 
 	opts.NUDConfigs.resetInvalidFields()
@@ -378,12 +380,12 @@ func New(opts Options) *Stack {
 		handleLocal:                  opts.HandleLocal,
 		tables:                       opts.IPTables,
 		icmpRateLimiter:              NewICMPRateLimiter(clock),
-		seed:                         randomGenerator.Uint32(),
+		seed:                         secureRNG.Uint32(),
 		nudConfigs:                   opts.NUDConfigs,
 		uniqueIDGenerator:            opts.UniqueID,
 		nudDisp:                      opts.NUDDisp,
-		randomGenerator:              randomGenerator,
-		secureRNG:                    opts.SecureRNG,
+		insecureRNG:                  insecureRNG,
+		secureRNG:                    secureRNG,
 		sendBufferSize: tcpip.SendBufferSizeOption{
 			Min:     MinBufferSize,
 			Default: DefaultBufferSize,
@@ -395,7 +397,7 @@ func New(opts Options) *Stack {
 			Max:     DefaultMaxBufferSize,
 		},
 		tcpInvalidRateLimit: defaultTCPInvalidRateLimit,
-		tsOffsetSecret:      randomGenerator.Uint32(),
+		tsOffsetSecret:      secureRNG.Uint32(),
 	}
 
 	// Add specified network protocols.
@@ -2096,15 +2098,16 @@ func (s *Stack) Seed() uint32 {
 	return s.seed
 }
 
-// Rand returns a reference to a pseudo random generator that can be used
-// to generate random numbers as required.
-func (s *Stack) Rand() *rand.Rand {
-	return s.randomGenerator
+// InsecureRNG returns a reference to a pseudo random generator that can be used
+// to generate random numbers as required. It is not cryptographically secure
+// and should not be used for security sensitive work.
+func (s *Stack) InsecureRNG() *rand.Rand {
+	return s.insecureRNG
 }
 
 // SecureRNG returns the stack's cryptographically secure random number
 // generator.
-func (s *Stack) SecureRNG() io.Reader {
+func (s *Stack) SecureRNG() cryptorand.RNG {
 	return s.secureRNG
 }
 

pkg/tcpip/transport/icmp/endpoint.go

@@ -584,7 +584,7 @@ func (e *endpoint) registerWithStack(netProto tcpip.NetworkProtocolNumber, id st
 	}
 
 	// We need to find a port for the endpoint.
-	_, err := e.stack.PickEphemeralPort(e.stack.Rand(), func(p uint16) (bool, tcpip.Error) {
+	_, err := e.stack.PickEphemeralPort(e.stack.SecureRNG(), func(p uint16) (bool, tcpip.Error) {
 		id.LocalPort = p
 		err := e.stack.RegisterTransportEndpoint([]tcpip.NetworkProtocolNumber{netProto}, e.transProto, id, e, ports.Flags{}, bindToDevice)
 		switch err.(type) {