From c041d43aa1ebd913be3ef93dfdfd760c64b408eb Mon Sep 17 00:00:00 2001 From: YACOVM Date: Wed, 19 Apr 2017 16:59:40 +0300 Subject: [PATCH] [FAB-3245] Use crypto rand in gossip In gossip the random uses math instead of crypto, and it is initialized with a static seed of 42 instead of something more random. This has an implication that the randomized selection is not so random. We should replace it with a crypto random for more security. Change-Id: Iaa28c14410a19d076b301d76b4539769c03f4764 Signed-off-by: Yacov Manevich --- gossip/comm/comm_impl.go | 9 +--- gossip/comm/comm_test.go | 2 +- gossip/gossip/algo/pull.go | 9 +--- gossip/gossip/msgstore/msgs_test.go | 2 +- gossip/state/state.go | 5 +- gossip/util/misc.go | 34 ++++++++++-- gossip/util/misc_test.go | 80 +++++++++++++++++++++++++++++ 7 files changed, 117 insertions(+), 24 deletions(-) create mode 100644 gossip/util/misc_test.go diff --git a/gossip/comm/comm_impl.go b/gossip/comm/comm_impl.go index b68e66a9473..38fab34b8af 100644 --- a/gossip/comm/comm_impl.go +++ b/gossip/comm/comm_impl.go @@ -21,7 +21,6 @@ import ( "crypto/tls" "errors" "fmt" - "math/rand" "net" "os" "reflect" @@ -52,10 +51,6 @@ const ( var errSendOverflow = errors.New(sendOverflowErr) -func init() { - rand.Seed(42) -} - // SetDialTimeout sets the dial timeout func SetDialTimeout(timeout time.Duration) { viper.Set("peer.gossip.dialTimeout", timeout) @@ -604,8 +599,8 @@ func createGRPCLayer(port int) (*grpc.Server, net.Listener, grpc.DialOption, []b var serverOpts []grpc.ServerOption var dialOpts grpc.DialOption - keyFileName := fmt.Sprintf("key.%d.pem", rand.Int63()) - certFileName := fmt.Sprintf("cert.%d.pem", rand.Int63()) + keyFileName := fmt.Sprintf("key.%d.pem", util.RandomUInt64()) + certFileName := fmt.Sprintf("cert.%d.pem", util.RandomUInt64()) defer os.Remove(keyFileName) defer os.Remove(certFileName) diff --git a/gossip/comm/comm_test.go b/gossip/comm/comm_test.go index d008c876c87..534d217a7df 100644 --- a/gossip/comm/comm_test.go +++ b/gossip/comm/comm_test.go @@ -43,7 +43,7 @@ import ( ) func init() { - rand.Seed(42) + rand.Seed(time.Now().UnixNano()) factory.InitFactories(nil) } diff --git a/gossip/gossip/algo/pull.go b/gossip/gossip/algo/pull.go index 3dee8b95cc1..0cdc4ee6364 100644 --- a/gossip/gossip/algo/pull.go +++ b/gossip/gossip/algo/pull.go @@ -17,7 +17,6 @@ limitations under the License. package algo import ( - "math/rand" "sync" "sync/atomic" "time" @@ -44,10 +43,6 @@ import ( */ -func init() { - rand.Seed(42) -} - const ( defDigestWaitTime = time.Duration(1) * time.Second defRequestWaitTime = time.Duration(1) * time.Second @@ -214,7 +209,7 @@ func (engine *PullEngine) processIncomingDigests() { requestMapping := make(map[string][]string) for n, sources := range engine.item2owners { // select a random source - source := sources[rand.Intn(len(sources))] + source := sources[util.RandomInt(len(sources))] if _, exists := requestMapping[source]; !exists { requestMapping[source] = make([]string, 0) } @@ -341,7 +336,7 @@ func (engine *PullEngine) OnRes(items []string, nonce uint64) { func (engine *PullEngine) newNONCE() uint64 { n := uint64(0) for { - n = uint64(rand.Int63()) + n = util.RandomUInt64() if !engine.outgoingNONCES.Exists(n) { return n } diff --git a/gossip/gossip/msgstore/msgs_test.go b/gossip/gossip/msgstore/msgs_test.go index c629f660d67..73da2d597db 100644 --- a/gossip/gossip/msgstore/msgs_test.go +++ b/gossip/gossip/msgstore/msgs_test.go @@ -29,7 +29,7 @@ import ( ) func init() { - rand.Seed(42) + rand.Seed(time.Now().UnixNano()) } func alwaysNoAction(this interface{}, that interface{}) common.InvalidationResult { diff --git a/gossip/state/state.go b/gossip/state/state.go index 01957607954..70e8802dcae 100644 --- a/gossip/state/state.go +++ b/gossip/state/state.go @@ -19,7 +19,6 @@ package state import ( "bytes" "errors" - "math/rand" "sync" "sync/atomic" "time" @@ -543,7 +542,7 @@ func (s *GossipStateProviderImpl) requestBlocksInRange(start uint64, end uint64) // Generate state request message for given blocks in range [beginSeq...endSeq] func (s *GossipStateProviderImpl) stateRequestMessage(beginSeq uint64, endSeq uint64) *proto.GossipMessage { return &proto.GossipMessage{ - Nonce: uint64(rand.Uint32())<<32 + uint64(rand.Uint32()), + Nonce: util.RandomUInt64(), Tag: proto.GossipMessage_CHAN_OR_ORG, Channel: []byte(s.chainID), Content: &proto.GossipMessage_StateRequest{ @@ -566,7 +565,7 @@ func (s *GossipStateProviderImpl) selectPeerToRequestFrom(height uint64) (*comm. } // Select peers to ask for blocks - return peers[rand.Intn(n)], nil + return peers[util.RandomInt(n)], nil } // filterPeers return list of peers which aligns the predicate provided diff --git a/gossip/util/misc.go b/gossip/util/misc.go index 9ee12092135..3eb766ec0f0 100644 --- a/gossip/util/misc.go +++ b/gossip/util/misc.go @@ -17,7 +17,10 @@ limitations under the License. package util import ( + cryptorand "crypto/rand" "fmt" + "io" + "math/big" "math/rand" "reflect" "runtime" @@ -30,10 +33,6 @@ import ( // Equals returns whether a and b are the same type Equals func(a interface{}, b interface{}) bool -func init() { - rand.Seed(42) -} - // IndexInSlice returns the index of given object o in array func IndexInSlice(array interface{}, o interface{}, equals Equals) int { arr := reflect.ValueOf(array) @@ -65,7 +64,7 @@ func GetRandomIndices(indiceCount, highestIndex int) []int { } for len(indices) < indiceCount { - n := rand.Intn(highestIndex + 1) + n := RandomInt(highestIndex + 1) if IndexInSlice(indices, n, numbericEqual) != -1 { continue } @@ -159,3 +158,28 @@ func GetDurationOrDefault(key string, defVal time.Duration) time.Duration { return defVal } + +// RandomInt returns, as an int, a non-negative pseudo-random integer in [0,n) +// It panics if n <= 0 +func RandomInt(n int) int { + if n <= 0 { + panic(fmt.Sprintf("Got invalid (non positive) value: %d", n)) + } + m := int(RandomUInt64()) % n + if m < 0 { + return n + m + } + return m +} + +// RandomUInt64 returns a random uint64 +func RandomUInt64() uint64 { + b := make([]byte, 8) + _, err := io.ReadFull(cryptorand.Reader, b) + if err == nil { + n := new(big.Int) + return n.SetBytes(b).Uint64() + } + rand.Seed(rand.Int63()) + return uint64(rand.Int63()) +} diff --git a/gossip/util/misc_test.go b/gossip/util/misc_test.go new file mode 100644 index 00000000000..828a5001923 --- /dev/null +++ b/gossip/util/misc_test.go @@ -0,0 +1,80 @@ +/* +Copyright IBM Corp. 2017 All Rights Reserved. + +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 util + +import ( + "crypto/rand" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func testHappyPath(t *testing.T) { + n1 := RandomInt(10000) + n2 := RandomInt(10000) + assert.NotEqual(t, n1, n2) + n3 := RandomUInt64() + n4 := RandomUInt64() + assert.NotEqual(t, n3, n4) +} + +func TestGetRandomInt(t *testing.T) { + testHappyPath(t) +} + +func TestNonNegativeValues(t *testing.T) { + assert.True(t, RandomInt(1000000) >= 0) +} + +func TestGetRandomIntBadInput(t *testing.T) { + f1 := func() { + RandomInt(0) + } + f2 := func() { + RandomInt(-500) + } + assert.Panics(t, f1) + assert.Panics(t, f2) +} + +type reader struct { + mock.Mock +} + +func (r *reader) Read(p []byte) (int, error) { + args := r.Mock.Called(p) + n := args.Get(0).(int) + err := args.Get(1) + if err == nil { + return n, nil + } + return n, err.(error) +} + +func TestGetRandomIntNoEntropy(t *testing.T) { + rr := rand.Reader + defer func() { + rand.Reader = rr + }() + r := &reader{} + r.On("Read", mock.Anything).Return(0, errors.New("Not enough entropy")) + rand.Reader = r + // Make sure randomness still works even when we have no entropy + testHappyPath(t) +}