From 9488cfcf0ba20db9f4412640ff43e4e8578ec037 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Tue, 24 Sep 2024 10:18:26 -0700 Subject: [PATCH] nftables: use a secure RNG Linux implements the random operation with a cryptographically secure RNG. We must do the same. PiperOrigin-RevId: 678301194 --- pkg/tcpip/nftables/BUILD | 2 ++ pkg/tcpip/nftables/nftables.go | 9 ++++----- pkg/tcpip/nftables/nftables_test.go | 30 +++++++++++++++++++++-------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/pkg/tcpip/nftables/BUILD b/pkg/tcpip/nftables/BUILD index 8fb035da1..5524ec122 100644 --- a/pkg/tcpip/nftables/BUILD +++ b/pkg/tcpip/nftables/BUILD @@ -13,6 +13,7 @@ go_library( ], deps = [ "//pkg/abi/linux", + "//pkg/rand", "//pkg/tcpip", "//pkg/tcpip/checksum", "//pkg/tcpip/header", @@ -30,6 +31,7 @@ go_test( deps = [ "//pkg/abi/linux", "//pkg/buffer", + "//pkg/rand", "//pkg/sync", "//pkg/tcpip", "//pkg/tcpip/faketime", diff --git a/pkg/tcpip/nftables/nftables.go b/pkg/tcpip/nftables/nftables.go index 52177a46f..f88e08820 100644 --- a/pkg/tcpip/nftables/nftables.go +++ b/pkg/tcpip/nftables/nftables.go @@ -44,12 +44,12 @@ import ( "bytes" "encoding/binary" "fmt" - "math/rand" "slices" "sync/atomic" "time" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/checksum" "gvisor.dev/gvisor/pkg/tcpip/header" @@ -229,7 +229,7 @@ type NFTables struct { filters [NumAFs]*addressFamilyFilter // Filters for each address family. clock tcpip.Clock // Clock for timing evaluations. startTime time.Time // Time NFTables object was created. - rng *rand.Rand // Random number generator. + rng rand.RNG // Random number generator. } // addressFamilyFilter represents the nftables state for a specific address @@ -2199,12 +2199,11 @@ func (r *Rule) evaluate(regs *registerSet, pkt *stack.PacketBuffer) error { // NewNFTables creates a new NFTables state object using the given clock for // timing operations. // Note: Expects random number generator to be initialized with a seed. -// TODO(b/345684870): Use a secure RNG. -func NewNFTables(clock tcpip.Clock, rng *rand.Rand) *NFTables { +func NewNFTables(clock tcpip.Clock, rng rand.RNG) *NFTables { if clock == nil { panic("nftables state must be initialized with a non-nil clock") } - if rng == nil { + if rng.Reader == nil { panic("nftables state must be initialized with a non-nil random number generator") } return &NFTables{clock: clock, startTime: clock.Now(), rng: rng} diff --git a/pkg/tcpip/nftables/nftables_test.go b/pkg/tcpip/nftables/nftables_test.go index 23ebc8eb6..6777aee25 100644 --- a/pkg/tcpip/nftables/nftables_test.go +++ b/pkg/tcpip/nftables/nftables_test.go @@ -17,7 +17,6 @@ package nftables import ( "encoding/binary" "fmt" - "math/rand" "reflect" "slices" "testing" @@ -25,6 +24,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/buffer" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/faketime" @@ -2370,8 +2370,8 @@ func TestEvaluateLast(t *testing.T) { // Sets up an NFTables object with a base chain and fake manual clock. fakeClock := faketime.NewManualClock() - fixedRng := rand.New(rand.NewSource(0)) - nf := NewNFTables(fakeClock, fixedRng) + fixedRNG := rand.RNGFrom(&fixedReader{}) + nf := NewNFTables(fakeClock, fixedRNG) tab, err := nf.AddTable(arbitraryFamily, "test", "test table", false) if err != nil { t.Fatalf("unexpected error for AddTable: %v", err) @@ -2822,8 +2822,8 @@ func TestEvaluateMetaLoad(t *testing.T) { timeNS := now.UnixNano() timeDay := now.Weekday() timeHour := now.Hour()*3600 + now.Minute()*60 + now.Second() - fixedRng := rand.New(rand.NewSource(0)) - seededRandUint32 := fixedRng.Uint32() // fixes rng + fixedRNG := rand.RNGFrom(&fixedReader{}) + seededRandUint32 := fixedRNG.Uint32() // fixes rng for _, test := range []struct { tname string @@ -2920,7 +2920,7 @@ func TestEvaluateMetaLoad(t *testing.T) { t.Run(test.tname, func(t *testing.T) { // Sets up an NFTables object with a base chain and fake manual clock. // Using Manual Clock sets time.Now to Unix Epoch which fixes rng seed! - nf := NewNFTables(fakeClock, rand.New(rand.NewSource(0))) + nf := NewNFTables(fakeClock, rand.RNGFrom(&fixedReader{})) tab, err := nf.AddTable(arbitraryFamily, "test", "test table", false) if err != nil { @@ -3713,8 +3713,8 @@ func packetResultString(initial, final *stack.PacketBuffer) string { // newNFTablesStd creates a new NFTables object w/ a standard clock for testing. func newNFTablesStd() *NFTables { stdClock := tcpip.NewStdClock() - fixedRng := rand.New(rand.NewSource(0)) - return NewNFTables(stdClock, fixedRng) + fixedRNG := rand.RNGFrom(&fixedReader{}) + return NewNFTables(stdClock, fixedRNG) } // mustCreateImmediate wraps the newImmediate function for brevity. @@ -3815,3 +3815,17 @@ func mustCreateMetaSet(t *testing.T, key metaKey, sreg uint8) *metaSet { } return mtset } + +// A fixedReader sets all bytes to the same value (1) when Read is called. +// +// It is used to make the RNG deterministic for testing, i.e. it's really +// really bad at being an RNG. +type fixedReader struct{} + +// Read implements io.Reader.Read. +func (*fixedReader) Read(buf []byte) (int, error) { + for i := range len(buf) { + buf[i] = 1 + } + return len(buf), nil +}