From 83f75082e5b03fafca9201d9d9939028f712b0b2 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Sat, 28 Oct 2023 16:11:16 -0700 Subject: [PATCH] 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 | 1 + pkg/rand/rand.go | 2 -- pkg/rand/rand_linux.go | 2 -- pkg/rand/rng.go | 44 ++++++++++++++++++++++++++ pkg/tcpip/network/arp/arp.go | 2 +- pkg/tcpip/network/ipv4/igmp.go | 2 +- pkg/tcpip/network/ipv6/ipv6.go | 2 +- pkg/tcpip/network/ipv6/mld.go | 2 +- pkg/tcpip/network/ipv6/mld_test.go | 17 ---------- pkg/tcpip/network/ipv6/ndp.go | 4 +-- pkg/tcpip/network/ipv6/ndp_test.go | 14 -------- pkg/tcpip/ports/BUILD | 2 ++ pkg/tcpip/ports/ports.go | 9 +++--- pkg/tcpip/ports/ports_test.go | 6 ++-- pkg/tcpip/stack/ndp_test.go | 16 ---------- pkg/tcpip/stack/neighbor_cache.go | 2 +- pkg/tcpip/stack/neighbor_cache_test.go | 10 +++--- pkg/tcpip/stack/neighbor_entry_test.go | 8 ++--- pkg/tcpip/stack/stack.go | 31 ++++++++++-------- pkg/tcpip/transport/icmp/endpoint.go | 2 +- pkg/tcpip/transport/tcp/accept.go | 2 +- pkg/tcpip/transport/tcp/endpoint.go | 16 ++++++---- pkg/tcpip/transport/tcp/protocol.go | 9 +++--- pkg/tcpip/transport/udp/endpoint.go | 2 +- 24 files changed, 104 insertions(+), 103 deletions(-) create mode 100644 pkg/rand/rng.go diff --git a/pkg/rand/BUILD b/pkg/rand/BUILD index 7389054ea..46ebdaf4c 100644 --- a/pkg/rand/BUILD +++ b/pkg/rand/BUILD @@ -10,6 +10,7 @@ go_library( srcs = [ "rand.go", "rand_linux.go", + "rng.go", ], visibility = ["//:sandbox"], deps = [ diff --git a/pkg/rand/rand.go b/pkg/rand/rand.go index be0e85fdb..94d2764d6 100644 --- a/pkg/rand/rand.go +++ b/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" diff --git a/pkg/rand/rand_linux.go b/pkg/rand/rand_linux.go index fd5fa5d6a..0913e8b00 100644 --- a/pkg/rand/rand_linux.go +++ b/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 ( diff --git a/pkg/rand/rng.go b/pkg/rand/rng.go new file mode 100644 index 000000000..f35944662 --- /dev/null +++ b/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[:]) +} diff --git a/pkg/tcpip/network/arp/arp.go b/pkg/tcpip/network/arp/arp.go index 68786e108..ebcdf92df 100644 --- a/pkg/tcpip/network/arp/arp.go +++ b/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, diff --git a/pkg/tcpip/network/ipv4/igmp.go b/pkg/tcpip/network/ipv4/igmp.go index 68c36316a..664f6e2f3 100644 --- a/pkg/tcpip/network/ipv4/igmp.go +++ b/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, diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go index fc8e56871..21eaec105 100644 --- a/pkg/tcpip/network/ipv6/ipv6.go +++ b/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, diff --git a/pkg/tcpip/network/ipv6/mld.go b/pkg/tcpip/network/ipv6/mld.go index a9736581e..7feb52239 100644 --- a/pkg/tcpip/network/ipv6/mld.go +++ b/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, diff --git a/pkg/tcpip/network/ipv6/mld_test.go b/pkg/tcpip/network/ipv6/mld_test.go index 2da420e9c..bc2cc418f 100644 --- a/pkg/tcpip/network/ipv6/mld_test.go +++ b/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() } diff --git a/pkg/tcpip/network/ipv6/ndp.go b/pkg/tcpip/network/ipv6/ndp.go index 67f1fe249..274ba5057 100644 --- a/pkg/tcpip/network/ipv6/ndp.go +++ b/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 { diff --git a/pkg/tcpip/network/ipv6/ndp_test.go b/pkg/tcpip/network/ipv6/ndp_test.go index e0db9aed8..f45048bd2 100644 --- a/pkg/tcpip/network/ipv6/ndp_test.go +++ b/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{ diff --git a/pkg/tcpip/ports/BUILD b/pkg/tcpip/ports/BUILD index 97be46d90..b3f752062 100644 --- a/pkg/tcpip/ports/BUILD +++ b/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", diff --git a/pkg/tcpip/ports/ports.go b/pkg/tcpip/ports/ports.go index 11a9dc0b1..1642e142f 100644 --- a/pkg/tcpip/ports/ports.go +++ b/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() diff --git a/pkg/tcpip/ports/ports_test.go b/pkg/tcpip/ports/ports_test.go index a91b130df..c688bfd8f 100644 --- a/pkg/tcpip/ports/ports_test.go +++ b/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) } diff --git a/pkg/tcpip/stack/ndp_test.go b/pkg/tcpip/stack/ndp_test.go index 5cf129891..75f3ae5f5 100644 --- a/pkg/tcpip/stack/ndp_test.go +++ b/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 { diff --git a/pkg/tcpip/stack/neighbor_cache.go b/pkg/tcpip/stack/neighbor_cache.go index b38bef4e2..c08073679 100644 --- a/pkg/tcpip/stack/neighbor_cache.go +++ b/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() diff --git a/pkg/tcpip/stack/neighbor_cache_test.go b/pkg/tcpip/stack/neighbor_cache_test.go index cb556f6b4..d409e632b 100644 --- a/pkg/tcpip/stack/neighbor_cache_test.go +++ b/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{ diff --git a/pkg/tcpip/stack/neighbor_entry_test.go b/pkg/tcpip/stack/neighbor_entry_test.go index 682f74154..8117b1b88 100644 --- a/pkg/tcpip/stack/neighbor_entry_test.go +++ b/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()), } diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 1abdb2303..532d24fd8 100644 --- a/pkg/tcpip/stack/stack.go +++ b/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 } diff --git a/pkg/tcpip/transport/icmp/endpoint.go b/pkg/tcpip/transport/icmp/endpoint.go index 3bebf8d56..d3e72e37b 100644 --- a/pkg/tcpip/transport/icmp/endpoint.go +++ b/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) { diff --git a/pkg/tcpip/transport/tcp/accept.go b/pkg/tcpip/transport/tcp/accept.go index 0006f2503..1caa7cdf2 100644 --- a/pkg/tcpip/transport/tcp/accept.go +++ b/pkg/tcpip/transport/tcp/accept.go @@ -119,7 +119,7 @@ func newListenContext(stk *stack.Stack, protocol *protocol, listenEP *endpoint, } for i := range l.nonce { - if _, err := io.ReadFull(stk.SecureRNG(), l.nonce[i][:]); err != nil { + if _, err := io.ReadFull(stk.SecureRNG().Reader, l.nonce[i][:]); err != nil { panic(err) } } diff --git a/pkg/tcpip/transport/tcp/endpoint.go b/pkg/tcpip/transport/tcp/endpoint.go index 464afb6d7..5ce0ab9ab 100644 --- a/pkg/tcpip/transport/tcp/endpoint.go +++ b/pkg/tcpip/transport/tcp/endpoint.go @@ -858,10 +858,12 @@ func newEndpoint(s *stack.Stack, protocol *protocol, netProto tcpip.NetworkProto interval: DefaultKeepaliveInterval, count: DefaultKeepaliveCount, }, - uniqueID: s.UniqueID(), - ipv4TTL: tcpip.UseDefaultIPv4TTL, - ipv6HopLimit: tcpip.UseDefaultIPv6HopLimit, - txHash: s.Rand().Uint32(), + uniqueID: s.UniqueID(), + ipv4TTL: tcpip.UseDefaultIPv4TTL, + ipv6HopLimit: tcpip.UseDefaultIPv6HopLimit, + // txHash only determines which outgoing queue to use, so + // InsecureRNG is fine. + txHash: s.InsecureRNG().Uint32(), windowClamp: DefaultReceiveBufferSize, maxSynRetries: DefaultSynRetries, } @@ -2295,7 +2297,7 @@ func (e *endpoint) registerEndpoint(addr tcpip.FullAddress, netProto tcpip.Netwo BindToDevice: bindToDevice, Dest: addr, } - if _, err := e.stack.ReservePort(e.stack.Rand(), portRes, nil /* testPort */); err != nil { + if _, err := e.stack.ReservePort(e.stack.SecureRNG(), portRes, nil /* testPort */); err != nil { if _, ok := err.(*tcpip.ErrPortInUse); !ok || !reuse { return false, nil } @@ -2342,7 +2344,7 @@ func (e *endpoint) registerEndpoint(addr tcpip.FullAddress, netProto tcpip.Netwo BindToDevice: bindToDevice, Dest: addr, } - if _, err := e.stack.ReservePort(e.stack.Rand(), portRes, nil /* testPort */); err != nil { + if _, err := e.stack.ReservePort(e.stack.SecureRNG(), portRes, nil /* testPort */); err != nil { return false, nil } } @@ -2778,7 +2780,7 @@ func (e *endpoint) bindLocked(addr tcpip.FullAddress) (err tcpip.Error) { BindToDevice: bindToDevice, Dest: tcpip.FullAddress{}, } - port, err := e.stack.ReservePort(e.stack.Rand(), portRes, func(p uint16) (bool, tcpip.Error) { + port, err := e.stack.ReservePort(e.stack.SecureRNG(), portRes, func(p uint16) (bool, tcpip.Error) { id := e.TransportEndpointInfo.ID id.LocalPort = p // CheckRegisterTransportEndpoint should only return an error if there is a diff --git a/pkg/tcpip/transport/tcp/protocol.go b/pkg/tcpip/transport/tcp/protocol.go index 81059d6a3..555951488 100644 --- a/pkg/tcpip/transport/tcp/protocol.go +++ b/pkg/tcpip/transport/tcp/protocol.go @@ -507,6 +507,7 @@ func (*protocol) Parse(pkt stack.PacketBufferPtr) bool { // NewProtocol returns a TCP transport protocol. func NewProtocol(s *stack.Stack) stack.TransportProtocol { + rng := s.SecureRNG() p := protocol{ stack: s, sendBufferSize: tcpip.TCPSendBufferSizeRangeOption{ @@ -530,11 +531,11 @@ func NewProtocol(s *stack.Stack) stack.TransportProtocol { maxRTO: MaxRTO, maxRetries: MaxRetries, recovery: tcpip.TCPRACKLossDetection, - seqnumSecret: s.Rand().Uint32(), - portOffsetSecret: s.Rand().Uint32(), - tsOffsetSecret: s.Rand().Uint32(), + seqnumSecret: rng.Uint32(), + portOffsetSecret: rng.Uint32(), + tsOffsetSecret: rng.Uint32(), } - p.dispatcher.init(s.Rand(), runtime.GOMAXPROCS(0)) + p.dispatcher.init(s.InsecureRNG(), runtime.GOMAXPROCS(0)) return &p } diff --git a/pkg/tcpip/transport/udp/endpoint.go b/pkg/tcpip/transport/udp/endpoint.go index eab6c95f4..e873e31df 100644 --- a/pkg/tcpip/transport/udp/endpoint.go +++ b/pkg/tcpip/transport/udp/endpoint.go @@ -773,7 +773,7 @@ func (e *endpoint) registerWithStack(netProtos []tcpip.NetworkProtocolNumber, id BindToDevice: bindToDevice, Dest: tcpip.FullAddress{}, } - port, err := e.stack.ReservePort(e.stack.Rand(), portRes, nil /* testPort */) + port, err := e.stack.ReservePort(e.stack.SecureRNG(), portRes, nil /* testPort */) if err != nil { return id, bindToDevice, err }