From 78465e25492795121855e758ee420e6334f22c22 Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Fri, 26 Jun 2020 17:58:47 +0300 Subject: [PATCH] QSP-6: Enforces crypto-secure PRNGs (#6401) * adds cryptorand analyzer * better naming * rely on suffix * sync/pending_* use crypto/rand * define shared/rand * updates fetcher * fixes rand issue in sync package * gofmt * shared/rand: more docs + add exclusion nogo_config.json * updates validator/assignments * updates comment * fixes remaning cases * re-arranges comments * fixes tests * renames in shared/rand API * adds simple no-panic test * gazelle Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com> --- BUILD.bazel | 1 + beacon-chain/db/testing/BUILD.bazel | 1 + beacon-chain/db/testing/setup_db.go | 8 +- beacon-chain/rpc/service.go | 3 - beacon-chain/rpc/validator/BUILD.bazel | 1 + beacon-chain/rpc/validator/assignments.go | 35 +++----- .../rpc/validator/assignments_test.go | 4 +- beacon-chain/rpc/validator/proposer.go | 17 +--- beacon-chain/sync/BUILD.bazel | 2 +- beacon-chain/sync/initial-sync/BUILD.bazel | 1 + .../sync/initial-sync/blocks_fetcher.go | 26 ++---- .../sync/pending_attestations_queue.go | 5 +- beacon-chain/sync/pending_blocks_queue.go | 5 +- nogo_config.json | 22 +++-- shared/rand/BUILD.bazel | 15 ++++ shared/rand/rand.go | 80 ++++++++++++++++++ shared/rand/rand_test.go | 24 ++++++ shared/testutil/BUILD.bazel | 1 + shared/testutil/block.go | 12 +-- shared/testutil/helpers.go | 4 +- tools/analyzers/cryptorand/BUILD.bazel | 28 +++++++ tools/analyzers/cryptorand/analyzer.go | 82 +++++++++++++++++++ tools/analyzers/cryptorand/analyzer_test.go | 11 +++ .../analyzers/cryptorand/testdata/BUILD.bazel | 11 +++ .../cryptorand/testdata/custom_import.go | 23 ++++++ .../analyzers/cryptorand/testdata/rand_new.go | 20 +++++ validator/db/BUILD.bazel | 1 + validator/db/setup_db.go | 9 +- 28 files changed, 360 insertions(+), 92 deletions(-) create mode 100644 shared/rand/BUILD.bazel create mode 100644 shared/rand/rand.go create mode 100644 shared/rand/rand_test.go create mode 100644 tools/analyzers/cryptorand/BUILD.bazel create mode 100644 tools/analyzers/cryptorand/analyzer.go create mode 100644 tools/analyzers/cryptorand/analyzer_test.go create mode 100644 tools/analyzers/cryptorand/testdata/BUILD.bazel create mode 100644 tools/analyzers/cryptorand/testdata/custom_import.go create mode 100644 tools/analyzers/cryptorand/testdata/rand_new.go diff --git a/BUILD.bazel b/BUILD.bazel index 1f9712ebf..da095f7ba 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -105,6 +105,7 @@ nogo( "@org_golang_x_tools//go/analysis/passes/asmdecl:go_tool_library", "//tools/analyzers/maligned:go_tool_library", "//tools/analyzers/roughtime:go_tool_library", + "//tools/analyzers/cryptorand:go_tool_library", "//tools/analyzers/errcheck:go_tool_library", "//tools/analyzers/featureconfig:go_tool_library", ] + select({ diff --git a/beacon-chain/db/testing/BUILD.bazel b/beacon-chain/db/testing/BUILD.bazel index c5d90826b..7ecee6779 100644 --- a/beacon-chain/db/testing/BUILD.bazel +++ b/beacon-chain/db/testing/BUILD.bazel @@ -10,6 +10,7 @@ go_library( "//beacon-chain/cache:go_default_library", "//beacon-chain/db:go_default_library", "//beacon-chain/db/kv:go_default_library", + "//shared/rand:go_default_library", "//shared/testutil:go_default_library", ], ) diff --git a/beacon-chain/db/testing/setup_db.go b/beacon-chain/db/testing/setup_db.go index 5377252c2..15b181dbf 100644 --- a/beacon-chain/db/testing/setup_db.go +++ b/beacon-chain/db/testing/setup_db.go @@ -3,9 +3,7 @@ package testing import ( - "crypto/rand" "fmt" - "math/big" "os" "path" "testing" @@ -13,15 +11,13 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/cache" "github.com/prysmaticlabs/prysm/beacon-chain/db" "github.com/prysmaticlabs/prysm/beacon-chain/db/kv" + "github.com/prysmaticlabs/prysm/shared/rand" "github.com/prysmaticlabs/prysm/shared/testutil" ) // SetupDB instantiates and returns database backed by key value store. func SetupDB(t testing.TB) (db.Database, *cache.StateSummaryCache) { - randPath, err := rand.Int(rand.Reader, big.NewInt(1000000)) - if err != nil { - t.Fatalf("could not generate random file path: %v", err) - } + randPath := rand.NewDeterministicGenerator().Int() p := path.Join(testutil.TempDir(), fmt.Sprintf("/%d", randPath)) if err := os.RemoveAll(p); err != nil { t.Fatalf("failed to remove directory: %v", err) diff --git a/beacon-chain/rpc/service.go b/beacon-chain/rpc/service.go index b5ec263ae..8b9df15c3 100644 --- a/beacon-chain/rpc/service.go +++ b/beacon-chain/rpc/service.go @@ -5,9 +5,7 @@ package rpc import ( "context" "fmt" - "math/rand" "net" - "os" middleware "github.com/grpc-ecosystem/go-grpc-middleware" recovery "github.com/grpc-ecosystem/go-grpc-middleware/recovery" @@ -50,7 +48,6 @@ var log logrus.FieldLogger func init() { log = logrus.WithField("prefix", "rpc") - rand.Seed(int64(os.Getpid())) } // Service defining an RPC server for a beacon node. diff --git a/beacon-chain/rpc/validator/BUILD.bazel b/beacon-chain/rpc/validator/BUILD.bazel index 8dc37d795..f8e0ee03a 100644 --- a/beacon-chain/rpc/validator/BUILD.bazel +++ b/beacon-chain/rpc/validator/BUILD.bazel @@ -43,6 +43,7 @@ go_library( "//shared/featureconfig:go_default_library", "//shared/hashutil:go_default_library", "//shared/params:go_default_library", + "//shared/rand:go_default_library", "//shared/roughtime:go_default_library", "//shared/slotutil:go_default_library", "//shared/traceutil:go_default_library", diff --git a/beacon-chain/rpc/validator/assignments.go b/beacon-chain/rpc/validator/assignments.go index 4d62756ac..7b3a2cd2b 100644 --- a/beacon-chain/rpc/validator/assignments.go +++ b/beacon-chain/rpc/validator/assignments.go @@ -2,11 +2,8 @@ package validator import ( "context" - "crypto/rand" - "math/big" "time" - "github.com/pkg/errors" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" "github.com/prysmaticlabs/prysm/beacon-chain/cache" "github.com/prysmaticlabs/prysm/beacon-chain/core/feed" @@ -15,6 +12,7 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/core/state" "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/rand" "github.com/prysmaticlabs/prysm/shared/roughtime" "github.com/prysmaticlabs/prysm/shared/slotutil" "google.golang.org/grpc/codes" @@ -178,12 +176,8 @@ func (vs *Server) duties(ctx context.Context, req *ethpb.DutiesRequest) (*ethpb. validatorAssignments = append(validatorAssignments, assignment) nextValidatorAssignments = append(nextValidatorAssignments, nextAssignment) // Assign relevant validator to subnet. - if err := assignValidatorToSubnet(pubKey, assignment.Status); err != nil { - return nil, errors.Wrap(err, "could not assign validator to subnet") - } - if err := assignValidatorToSubnet(pubKey, nextAssignment.Status); err != nil { - return nil, errors.Wrap(err, "could not assign validator to subnet") - } + assignValidatorToSubnet(pubKey, assignment.Status) + assignValidatorToSubnet(pubKey, nextAssignment.Status) } return ðpb.DutiesResponse{ @@ -195,33 +189,26 @@ func (vs *Server) duties(ctx context.Context, req *ethpb.DutiesRequest) (*ethpb. // assignValidatorToSubnet checks the status and pubkey of a particular validator // to discern whether persistent subnets need to be registered for them. -func assignValidatorToSubnet(pubkey []byte, status ethpb.ValidatorStatus) error { +func assignValidatorToSubnet(pubkey []byte, status ethpb.ValidatorStatus) { if status != ethpb.ValidatorStatus_ACTIVE && status != ethpb.ValidatorStatus_EXITING { - return nil + return } _, ok, expTime := cache.SubnetIDs.GetPersistentSubnets(pubkey) if ok && expTime.After(roughtime.Now()) { - return nil + return } epochDuration := time.Duration(params.BeaconConfig().SlotsPerEpoch * params.BeaconConfig().SecondsPerSlot) assignedIdxs := []uint64{} + randGen := rand.NewGenerator() for i := uint64(0); i < params.BeaconNetworkConfig().RandomSubnetsPerValidator; i++ { - randInt, err := rand.Int(rand.Reader, big.NewInt(int64(params.BeaconNetworkConfig().AttestationSubnetCount))) - if err != nil { - return errors.Wrap(err, "could not get random subnet index") - } - assignedIdxs = append(assignedIdxs, randInt.Uint64()) + assignedIdx := randGen.Intn(int(params.BeaconNetworkConfig().AttestationSubnetCount)) + assignedIdxs = append(assignedIdxs, uint64(assignedIdx)) } - randInt, err := rand.Int(rand.Reader, big.NewInt(int64(params.BeaconNetworkConfig().EpochsPerRandomSubnetSubscription))) - if err != nil { - return errors.Wrap(err, "could not get random subnet duration") - } - assignedDuration := randInt.Int64() - assignedDuration += int64(params.BeaconNetworkConfig().EpochsPerRandomSubnetSubscription) + assignedDuration := randGen.Intn(int(params.BeaconNetworkConfig().EpochsPerRandomSubnetSubscription)) + assignedDuration += int(params.BeaconNetworkConfig().EpochsPerRandomSubnetSubscription) totalDuration := epochDuration * time.Duration(assignedDuration) cache.SubnetIDs.AddPersistentCommittee(pubkey, assignedIdxs, totalDuration*time.Second) - return nil } diff --git a/beacon-chain/rpc/validator/assignments_test.go b/beacon-chain/rpc/validator/assignments_test.go index 611fe9b1a..59d9f38e7 100644 --- a/beacon-chain/rpc/validator/assignments_test.go +++ b/beacon-chain/rpc/validator/assignments_test.go @@ -467,9 +467,7 @@ func TestStreamDuties_OK_ChainReorg(t *testing.T) { func TestAssignValidatorToSubnet(t *testing.T) { k := pubKey(3) - if err := assignValidatorToSubnet(k, ethpb.ValidatorStatus_ACTIVE); err != nil { - t.Fatal(err) - } + assignValidatorToSubnet(k, ethpb.ValidatorStatus_ACTIVE) coms, ok, exp := cache.SubnetIDs.GetPersistentSubnets(k) if !ok { t.Fatal("No cache entry found for validator") diff --git a/beacon-chain/rpc/validator/proposer.go b/beacon-chain/rpc/validator/proposer.go index 39e478a1a..abbd81f43 100644 --- a/beacon-chain/rpc/validator/proposer.go +++ b/beacon-chain/rpc/validator/proposer.go @@ -2,7 +2,6 @@ package validator import ( "context" - "crypto/rand" "fmt" "math/big" "time" @@ -23,6 +22,7 @@ import ( "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/prysmaticlabs/prysm/shared/hashutil" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/rand" "github.com/prysmaticlabs/prysm/shared/trieutil" "go.opencensus.io/trace" "google.golang.org/grpc/codes" @@ -223,18 +223,9 @@ func (vs *Server) randomETH1DataVote(ctx context.Context) (*ethpb.Eth1Data, erro } // set random roots and block hashes to prevent a majority from being // built if the eth1 node is offline - randomDepBytes := make([]byte, 32) - randomBlkBytes := make([]byte, 32) - _, err = rand.Read(randomDepBytes) - if err != nil { - return nil, err - } - _, err = rand.Read(randomBlkBytes) - if err != nil { - return nil, err - } - depRoot := hashutil.Hash(randomDepBytes) - blockHash := hashutil.Hash(randomBlkBytes) + randGen := rand.NewGenerator() + depRoot := hashutil.Hash(bytesutil.Bytes32(randGen.Uint64())) + blockHash := hashutil.Hash(bytesutil.Bytes32(randGen.Uint64())) return ðpb.Eth1Data{ DepositRoot: depRoot[:], DepositCount: headState.Eth1DepositIndex(), diff --git a/beacon-chain/sync/BUILD.bazel b/beacon-chain/sync/BUILD.bazel index 95f566fe2..a882da303 100644 --- a/beacon-chain/sync/BUILD.bazel +++ b/beacon-chain/sync/BUILD.bazel @@ -71,6 +71,7 @@ go_library( "//shared/messagehandler:go_default_library", "//shared/p2putils:go_default_library", "//shared/params:go_default_library", + "//shared/rand:go_default_library", "//shared/roughtime:go_default_library", "//shared/runutil:go_default_library", "//shared/sliceutil:go_default_library", @@ -89,7 +90,6 @@ go_library( "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", "@io_opencensus_go//trace:go_default_library", - "@org_golang_x_exp//rand:go_default_library", ], ) diff --git a/beacon-chain/sync/initial-sync/BUILD.bazel b/beacon-chain/sync/initial-sync/BUILD.bazel index 391aee7c0..8ed8dfb84 100644 --- a/beacon-chain/sync/initial-sync/BUILD.bazel +++ b/beacon-chain/sync/initial-sync/BUILD.bazel @@ -30,6 +30,7 @@ go_library( "//shared/bytesutil:go_default_library", "//shared/mathutil:go_default_library", "//shared/params:go_default_library", + "//shared/rand:go_default_library", "//shared/roughtime:go_default_library", "@com_github_kevinms_leakybucket_go//:go_default_library", "@com_github_libp2p_go_libp2p_core//peer:go_default_library", diff --git a/beacon-chain/sync/initial-sync/blocks_fetcher.go b/beacon-chain/sync/initial-sync/blocks_fetcher.go index ff060961b..9130b2d33 100644 --- a/beacon-chain/sync/initial-sync/blocks_fetcher.go +++ b/beacon-chain/sync/initial-sync/blocks_fetcher.go @@ -2,12 +2,9 @@ package initialsync import ( "context" - "crypto/rand" "fmt" "io" "math" - "math/big" - mathRand "math/rand" "sort" "sync" "time" @@ -24,6 +21,7 @@ import ( p2ppb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" "github.com/prysmaticlabs/prysm/shared/mathutil" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/rand" "github.com/prysmaticlabs/prysm/shared/roughtime" "github.com/sirupsen/logrus" "go.opencensus.io/trace" @@ -64,6 +62,7 @@ type blocksFetcher struct { sync.Mutex ctx context.Context cancel context.CancelFunc + rand *rand.Rand headFetcher blockchain.HeadFetcher p2p p2p.P2P blocksPerSecond uint64 @@ -108,6 +107,7 @@ func newBlocksFetcher(ctx context.Context, cfg *blocksFetcherConfig) *blocksFetc return &blocksFetcher{ ctx: ctx, cancel: cancel, + rand: rand.NewGenerator(), headFetcher: cfg.headFetcher, p2p: cfg.p2p, blocksPerSecond: uint64(blocksPerSecond), @@ -387,11 +387,7 @@ func (f *blocksFetcher) selectFailOverPeer(excludedPID peer.ID, peers []peer.ID) return "", errNoPeersAvailable } - randInt, err := rand.Int(rand.Reader, big.NewInt(int64(len(peers)))) - if err != nil { - return "", err - } - return peers[randInt.Int64()], nil + return peers[f.rand.Int()%len(peers)], nil } // waitForMinimumPeers spins and waits up until enough peers are available. @@ -425,12 +421,7 @@ func (f *blocksFetcher) filterPeers(peers []peer.ID, peersPercentage float64) ([ // Shuffle peers to prevent a bad peer from // stalling sync with invalid blocks. - randSource, err := rand.Int(rand.Reader, big.NewInt(roughtime.Now().Unix())) - if err != nil { - return nil, errors.Wrap(err, "could not generate random int") - } - randGenerator := mathRand.New(mathRand.NewSource(randSource.Int64())) - randGenerator.Shuffle(len(peers), func(i, j int) { + f.rand.Shuffle(len(peers), func(i, j int) { peers[i], peers[j] = peers[j], peers[i] }) @@ -478,11 +469,6 @@ func (f *blocksFetcher) nonSkippedSlotAfter(ctx context.Context, slot uint64) (u return 0, errNoPeersAvailable } - randSource, err := rand.Int(rand.Reader, big.NewInt(roughtime.Now().Unix())) - if err != nil { - return 0, errors.Wrap(err, "could not generate random int") - } - randGenerator := mathRand.New(mathRand.NewSource(randSource.Int64())) slotsPerEpoch := params.BeaconConfig().SlotsPerEpoch pidInd := 0 @@ -525,7 +511,7 @@ func (f *blocksFetcher) nonSkippedSlotAfter(ctx context.Context, slot uint64) (u slot = slot + nonSkippedSlotsFullSearchEpochs*slotsPerEpoch upperBoundSlot := helpers.StartSlot(epoch + 1) for ind := slot + 1; ind < upperBoundSlot; ind += (slotsPerEpoch * slotsPerEpoch) / 2 { - start := ind + uint64(randGenerator.Intn(int(slotsPerEpoch))) + start := ind + uint64(f.rand.Intn(int(slotsPerEpoch))) nextSlot, err := fetch(peers[pidInd%len(peers)], start, slotsPerEpoch/2, slotsPerEpoch) if err != nil { return 0, err diff --git a/beacon-chain/sync/pending_attestations_queue.go b/beacon-chain/sync/pending_attestations_queue.go index 725cf0741..1a6808a79 100644 --- a/beacon-chain/sync/pending_attestations_queue.go +++ b/beacon-chain/sync/pending_attestations_queue.go @@ -13,12 +13,12 @@ import ( "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/rand" "github.com/prysmaticlabs/prysm/shared/runutil" "github.com/prysmaticlabs/prysm/shared/slotutil" "github.com/prysmaticlabs/prysm/shared/traceutil" "github.com/sirupsen/logrus" "go.opencensus.io/trace" - "golang.org/x/exp/rand" ) // This defines how often a node cleans up and processes pending attestations in the queue. @@ -60,6 +60,7 @@ func (s *Service) processPendingAtts(ctx context.Context) error { } s.pendingAttsLock.RUnlock() + randGen := rand.NewGenerator() for _, bRoot := range roots { s.pendingAttsLock.RLock() attestations := s.blkRootToPendingAtts[bRoot] @@ -130,7 +131,7 @@ func (s *Service) processPendingAtts(ctx context.Context) error { log.Debug("No peer IDs available to request missing block from for pending attestation") return nil } - pid := pids[rand.Int()%len(pids)] + pid := pids[randGen.Int()%len(pids)] targetSlot := helpers.SlotToEpoch(attestations[0].Message.Aggregate.Data.Target.Epoch) for _, p := range pids { cs, err := s.p2p.Peers().ChainState(p) diff --git a/beacon-chain/sync/pending_blocks_queue.go b/beacon-chain/sync/pending_blocks_queue.go index 9e498ea29..b88e8f3e6 100644 --- a/beacon-chain/sync/pending_blocks_queue.go +++ b/beacon-chain/sync/pending_blocks_queue.go @@ -11,12 +11,12 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil" "github.com/prysmaticlabs/prysm/shared/bytesutil" + "github.com/prysmaticlabs/prysm/shared/rand" "github.com/prysmaticlabs/prysm/shared/runutil" "github.com/prysmaticlabs/prysm/shared/slotutil" "github.com/prysmaticlabs/prysm/shared/traceutil" "github.com/sirupsen/logrus" "go.opencensus.io/trace" - "golang.org/x/exp/rand" ) var processPendingBlocksPeriod = slotutil.DivideSlotBy(3 /* times per slot */) @@ -51,6 +51,7 @@ func (s *Service) processPendingBlocks(ctx context.Context) error { trace.Int64Attribute("numPeers", int64(len(pids))), ) + randGen := rand.NewGenerator() for _, slot := range slots { ctx, span := trace.StartSpan(ctx, "processPendingBlocks.InnerLoop") span.AddAttributes(trace.Int64Attribute("slot", int64(slot))) @@ -80,7 +81,7 @@ func (s *Service) processPendingBlocks(ctx context.Context) error { // Start with a random peer to query, but choose the first peer in our unsorted list that claims to // have a head slot newer than the block slot we are requesting. - pid := pids[rand.Int()%len(pids)] + pid := pids[randGen.Int()%len(pids)] for _, p := range pids { cs, err := s.p2p.Peers().ChainState(p) if err != nil { diff --git a/nogo_config.json b/nogo_config.json index e421293f1..0693639b3 100644 --- a/nogo_config.json +++ b/nogo_config.json @@ -34,7 +34,7 @@ "exclude_files": { "external/.*": "Third party code" } - }, + }, "copylocks": { "exclude_files": { "external/.*": "Third party code" @@ -44,7 +44,7 @@ "exclude_files": { "external/.*": "Third party code" } - }, + }, "cgocall": { "exclude_files": { "external/.*": "Third party code" @@ -70,8 +70,8 @@ }, "roughtime": { "only_files": { - "beacon-chain/.*": "", - "shared/.*": "", + "beacon-chain/.*": "", + "shared/.*": "", "validator/.*": "" }, "exclude_files": { @@ -96,7 +96,19 @@ }, "featureconfig": { "only_files": { - ".*_test\\.go": "Only tests" + ".*_test\\.go": "Only tests" + } + }, + "cryptorand": { + "only_files": { + "beacon-chain/.*": "", + "shared/.*": "", + "validator/.*": "" + }, + "exclude_files": { + ".*/.*_test\\.go": "Tests are OK to use weak crypto", + "shared/rand/rand\\.go": "Abstracts CSPRNGs for common use", + "shared/aggregation/testing/bitlistutils.go": "Test-only package" } } } diff --git a/shared/rand/BUILD.bazel b/shared/rand/BUILD.bazel new file mode 100644 index 000000000..5c8281403 --- /dev/null +++ b/shared/rand/BUILD.bazel @@ -0,0 +1,15 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") +load("@prysm//tools/go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["rand.go"], + importpath = "github.com/prysmaticlabs/prysm/shared/rand", + visibility = ["//visibility:public"], +) + +go_test( + name = "go_default_test", + srcs = ["rand_test.go"], + embed = [":go_default_library"], +) diff --git a/shared/rand/rand.go b/shared/rand/rand.go new file mode 100644 index 000000000..4a2bca5ff --- /dev/null +++ b/shared/rand/rand.go @@ -0,0 +1,80 @@ +/* +Package rand defines methods of obtaining cryptographically secure random number generators. + +One is expected to use randomness from this package only, without introducing any other packages. +This limits the scope of code that needs to be hardened. + +There are two modes, one for deterministic and another non-deterministic randomness: +1. If deterministic pseudo-random generator is enough, use: + + import "github.com/prysmaticlabs/prysm/shared/rand" + randGen := rand.NewDeterministicGenerator() + randGen.Intn(32) // or any other func defined in math.rand API + + In this mode, only seed is generated using cryptographically secure source (crypto/rand). So, + once seed is obtained, and generator is seeded, the next generations are deterministic, thus fast. + This method is still better than using unix time for source of randomness - since time is not a + good source of seed randomness, when you have many concurrent servers using it (and they have + coinciding random generators' start times). + +2. For cryptographically secure non-deterministic mode (CSPRNG), use: + + import "github.com/prysmaticlabs/prysm/shared/rand" + randGen := rand.NewGenerator() + randGen.Intn(32) // or any other func defined in math.rand API + + Again, any of the functions from `math/rand` can be used, however, they all use custom source + of randomness (crypto/rand), on every step. This makes randomness non-deterministic. However, + you take a performance hit -- as it is an order of magnitude slower. +*/ +package rand + +import ( + "crypto/rand" + "encoding/binary" + mrand "math/rand" +) + +type source struct{} + +var _ = mrand.Source64(&source{}) + +// Seed does nothing when crypto/rand is used as source. +func (s *source) Seed(seed int64) {} + +// Int63 returns uniformly-distributed random (as in CSPRNG) int64 value within [0, 1<<63) range. +// Panics if random generator reader cannot return data. +func (s *source) Int63() int64 { + return int64(s.Uint64() & ^uint64(1<<63)) +} + +// Uint64 returns uniformly-distributed random (as in CSPRNG) uint64 value within [0, 1<<64) range. +// Panics if random generator reader cannot return data. +func (s *source) Uint64() (val uint64) { + if err := binary.Read(rand.Reader, binary.BigEndian, &val); err != nil { + panic(err) + } + return +} + +// Rand is alias for underlying random generator. +type Rand = mrand.Rand + +// NewGenerator returns a new generator that uses random values from crypto/rand as a source +// (cryptographically secure random number generator). +// Panics if crypto/rand input cannot be read. +// Use it for everything where crypto secure non-deterministic randomness is required. Performance +// takes a hit, so use sparingly. +func NewGenerator() *Rand { + return mrand.New(&source{}) +} + +// NewDeterministicGenerator returns a random generator which is only seeded with crypto/rand, +// but is deterministic otherwise (given seed, produces given results, deterministically). +// Panics if crypto/rand input cannot be read. +// Use this method for performance, where deterministic pseudo-random behaviour is enough. +// Otherwise, rely on NewGenerator(). +func NewDeterministicGenerator() *Rand { + randGen := NewGenerator() + return mrand.New(mrand.NewSource(randGen.Int63())) +} diff --git a/shared/rand/rand_test.go b/shared/rand/rand_test.go new file mode 100644 index 000000000..b027b1155 --- /dev/null +++ b/shared/rand/rand_test.go @@ -0,0 +1,24 @@ +package rand + +import ( + "math/rand" + "testing" +) + +func TestNewGenerator(t *testing.T) { + // Make sure that generation works, no panics. + randGen := NewGenerator() + _ = randGen.Int63() + _ = randGen.Uint64() + _ = randGen.Intn(32) + var _ = rand.Source64(randGen) +} + +func TestNewDeterministicGenerator(t *testing.T) { + // Make sure that generation works, no panics. + randGen := NewDeterministicGenerator() + _ = randGen.Int63() + _ = randGen.Uint64() + _ = randGen.Intn(32) + var _ = rand.Source64(randGen) +} diff --git a/shared/testutil/BUILD.bazel b/shared/testutil/BUILD.bazel index 7933e35a4..999f57b7f 100644 --- a/shared/testutil/BUILD.bazel +++ b/shared/testutil/BUILD.bazel @@ -27,6 +27,7 @@ go_library( "//shared/hashutil:go_default_library", "//shared/interop:go_default_library", "//shared/params:go_default_library", + "//shared/rand:go_default_library", "//shared/trieutil:go_default_library", "@com_github_ghodss_yaml//:go_default_library", "@com_github_gogo_protobuf//proto:go_default_library", diff --git a/shared/testutil/block.go b/shared/testutil/block.go index ec1787dc9..583fea3b0 100644 --- a/shared/testutil/block.go +++ b/shared/testutil/block.go @@ -5,7 +5,6 @@ import ( "fmt" "log" "math" - "math/rand" "github.com/pkg/errors" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" @@ -17,6 +16,7 @@ import ( "github.com/prysmaticlabs/prysm/shared/bls" "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/rand" ) // BlockGenConfig is used to define the requested conditions @@ -316,13 +316,14 @@ func generateAttesterSlashings( numSlashings uint64, ) ([]*ethpb.AttesterSlashing, error) { attesterSlashings := make([]*ethpb.AttesterSlashing, numSlashings) + randGen := rand.NewDeterministicGenerator() for i := uint64(0); i < numSlashings; i++ { - committeeIndex := rand.Uint64() % params.BeaconConfig().MaxCommitteesPerSlot + committeeIndex := randGen.Uint64() % params.BeaconConfig().MaxCommitteesPerSlot committee, err := helpers.BeaconCommitteeFromState(bState, bState.Slot(), committeeIndex) if err != nil { return nil, err } - randIndex := rand.Uint64() % uint64(len(committee)) + randIndex := randGen.Uint64() % uint64(len(committee)) valIndex := committee[randIndex] slashing, err := GenerateAttesterSlashingForValidator(bState, privs[valIndex], valIndex) if err != nil { @@ -379,8 +380,9 @@ func GenerateAttestations(bState *stateTrie.BeaconState, privs []bls.SecretKey, } } if randomRoot { + randGen := rand.NewDeterministicGenerator() b := make([]byte, 32) - _, err := rand.Read(b) + _, err := randGen.Read(b) if err != nil { return nil, err } @@ -527,5 +529,5 @@ func randValIndex(bState *stateTrie.BeaconState) (uint64, error) { if err != nil { return 0, err } - return rand.Uint64() % activeCount, nil + return rand.NewGenerator().Uint64() % activeCount, nil } diff --git a/shared/testutil/helpers.go b/shared/testutil/helpers.go index 0d8777cb5..afc675ad6 100644 --- a/shared/testutil/helpers.go +++ b/shared/testutil/helpers.go @@ -3,7 +3,6 @@ package testutil import ( "context" "encoding/binary" - "math/rand" "testing" "github.com/pkg/errors" @@ -13,6 +12,7 @@ import ( stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state" "github.com/prysmaticlabs/prysm/shared/bls" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/rand" ) // RandaoReveal returns a signature of the requested epoch using the beacon proposer private key. @@ -76,7 +76,7 @@ func BlockSignature( // Random32Bytes generates a random 32 byte slice. func Random32Bytes(t *testing.T) []byte { b := make([]byte, 32) - _, err := rand.Read(b) + _, err := rand.NewDeterministicGenerator().Read(b) if err != nil { t.Fatal(err) } diff --git a/tools/analyzers/cryptorand/BUILD.bazel b/tools/analyzers/cryptorand/BUILD.bazel new file mode 100644 index 000000000..d10a1dd6c --- /dev/null +++ b/tools/analyzers/cryptorand/BUILD.bazel @@ -0,0 +1,28 @@ +load("@prysm//tools/go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_tool_library") + +go_library( + name = "go_default_library", + srcs = ["analyzer.go"], + importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/cryptorand", + visibility = ["//visibility:public"], + deps = [ + "@org_golang_x_tools//go/analysis:go_default_library", + "@org_golang_x_tools//go/analysis/passes/inspect:go_default_library", + "@org_golang_x_tools//go/ast/inspector:go_default_library", + ], +) + +go_tool_library( + name = "go_tool_library", + srcs = ["analyzer.go"], + importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/cryptorand", + visibility = ["//visibility:public"], + deps = [ + "@org_golang_x_tools//go/analysis:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/inspect:go_tool_library", + "@org_golang_x_tools//go/ast/inspector:go_tool_library", + ], +) + +# gazelle:exclude analyzer_test.go diff --git a/tools/analyzers/cryptorand/analyzer.go b/tools/analyzers/cryptorand/analyzer.go new file mode 100644 index 000000000..5c6552e04 --- /dev/null +++ b/tools/analyzers/cryptorand/analyzer.go @@ -0,0 +1,82 @@ +package cryptorand + +import ( + "errors" + "fmt" + "go/ast" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +// Doc explaining the tool. +const Doc = "Tool to enforce the use of stronger crypto: crypto/rand instead of math/rand" + +var errWeakCrypto = errors.New("crypto-secure RNGs are required, use CSPRNG or PRNG defined in github.com/prysmaticlabs/prysm/shared/rand") + +// Analyzer runs static analysis. +var Analyzer = &analysis.Analyzer{ + Name: "cryptorand", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, errors.New("analyzer is not type *inspector.Inspector") + } + + nodeFilter := []ast.Node{ + (*ast.File)(nil), + (*ast.ImportSpec)(nil), + (*ast.CallExpr)(nil), + } + + aliases := make(map[string]string) + disallowedFns := []string{"NewSource", "New", "Seed", "Int63", "Uint32", "Uint64", "Int31", "Int", + "Int63n", "Int31n", "Intn", "Float64", "Float32", "Perm", "Shuffle", "Read"} + + inspect.Preorder(nodeFilter, func(node ast.Node) { + switch stmt := node.(type) { + case *ast.File: + // Reset aliases (per file). + aliases = make(map[string]string) + case *ast.ImportSpec: + // Collect aliases to rand packages. + pkg := stmt.Path.Value + if strings.HasSuffix(pkg, "/rand\"") && !strings.Contains(pkg, "/prysm/shared/rand") { + if stmt.Name != nil { + aliases[stmt.Name.Name] = stmt.Path.Value + } else { + aliases["rand"] = stmt.Path.Value + } + } + case *ast.CallExpr: + // Check if any of disallowed functions have been used. + for pkg, path := range aliases { + for _, fn := range disallowedFns { + if isPkgDot(stmt.Fun, pkg, fn) { + pass.Reportf(node.Pos(), fmt.Sprintf( + "%s: %s.%s() (from %s)", errWeakCrypto.Error(), pkg, fn, path)) + } + } + } + } + }) + + return nil, nil +} + +func isPkgDot(expr ast.Expr, pkg, name string) bool { + sel, ok := expr.(*ast.SelectorExpr) + return ok && isIdent(sel.X, pkg) && isIdent(sel.Sel, name) +} + +func isIdent(expr ast.Expr, ident string) bool { + id, ok := expr.(*ast.Ident) + return ok && id.Name == ident +} diff --git a/tools/analyzers/cryptorand/analyzer_test.go b/tools/analyzers/cryptorand/analyzer_test.go new file mode 100644 index 000000000..545adf8f6 --- /dev/null +++ b/tools/analyzers/cryptorand/analyzer_test.go @@ -0,0 +1,11 @@ +package cryptorand + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), Analyzer) +} diff --git a/tools/analyzers/cryptorand/testdata/BUILD.bazel b/tools/analyzers/cryptorand/testdata/BUILD.bazel new file mode 100644 index 000000000..49b7794a3 --- /dev/null +++ b/tools/analyzers/cryptorand/testdata/BUILD.bazel @@ -0,0 +1,11 @@ +load("@prysm//tools/go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = [ + "custom_import.go", + "rand_new.go", + ], + importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/cryptorand/testdata", + visibility = ["//visibility:public"], +) diff --git a/tools/analyzers/cryptorand/testdata/custom_import.go b/tools/analyzers/cryptorand/testdata/custom_import.go new file mode 100644 index 000000000..5991244ef --- /dev/null +++ b/tools/analyzers/cryptorand/testdata/custom_import.go @@ -0,0 +1,23 @@ +package testdata + +import ( + foobar "math/rand" + mathRand "math/rand" + "time" +) + +func UseRandNewCustomImport() { + randGenerator := mathRand.New(mathRand.NewSource(time.Now().UnixNano())) + start := uint64(randGenerator.Intn(32)) + _ = start + + randGenerator = mathRand.New(mathRand.NewSource(time.Now().UnixNano())) +} + +func UseWithoutSeeCustomImportd() { + assignedIndex := mathRand.Intn(128) + _ = assignedIndex + foobar.Shuffle(10, func(i, j int) { + + }) +} diff --git a/tools/analyzers/cryptorand/testdata/rand_new.go b/tools/analyzers/cryptorand/testdata/rand_new.go new file mode 100644 index 000000000..aa82713f7 --- /dev/null +++ b/tools/analyzers/cryptorand/testdata/rand_new.go @@ -0,0 +1,20 @@ +package testdata + +import ( + "math/rand" + mathRand "math/rand" + "time" +) + +func UseRandNew() { + randGenerator := mathRand.New(rand.NewSource(time.Now().UnixNano())) + start := uint64(randGenerator.Intn(32)) + _ = start + + randGenerator = rand.New(rand.NewSource(time.Now().UnixNano())) +} + +func UseWithoutSeed() { + assignedIndex := rand.Intn(int(128)) + _ = assignedIndex +} diff --git a/validator/db/BUILD.bazel b/validator/db/BUILD.bazel index 85b5b3a95..3ecf4df7c 100644 --- a/validator/db/BUILD.bazel +++ b/validator/db/BUILD.bazel @@ -16,6 +16,7 @@ go_library( deps = [ "//proto/slashing:go_default_library", "//shared/params:go_default_library", + "//shared/rand:go_default_library", "//validator/db/iface:go_default_library", "@com_github_gogo_protobuf//proto:go_default_library", "@com_github_pkg_errors//:go_default_library", diff --git a/validator/db/setup_db.go b/validator/db/setup_db.go index 8bc075ec5..4548e3b27 100644 --- a/validator/db/setup_db.go +++ b/validator/db/setup_db.go @@ -1,20 +1,17 @@ package db import ( - "crypto/rand" "fmt" - "math/big" "os" "path/filepath" "testing" + + "github.com/prysmaticlabs/prysm/shared/rand" ) // SetupDB instantiates and returns a DB instance for the validator client. func SetupDB(t testing.TB, pubkeys [][48]byte) *Store { - randPath, err := rand.Int(rand.Reader, big.NewInt(1000000)) - if err != nil { - t.Fatalf("Could not generate random file path: %v", err) - } + randPath := rand.NewDeterministicGenerator().Int() p := filepath.Join(TempDir(), fmt.Sprintf("/%d", randPath)) if err := os.RemoveAll(p); err != nil { t.Fatalf("Failed to remove directory: %v", err)