Skip to content

Commit

Permalink
[FAB-3245] Use crypto rand in gossip
Browse files Browse the repository at this point in the history
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 <yacovm@il.ibm.com>
  • Loading branch information
yacovm committed Apr 23, 2017
1 parent d7af6e8 commit c041d43
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 24 deletions.
9 changes: 2 additions & 7 deletions gossip/comm/comm_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"crypto/tls"
"errors"
"fmt"
"math/rand"
"net"
"os"
"reflect"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion gossip/comm/comm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import (
)

func init() {
rand.Seed(42)
rand.Seed(time.Now().UnixNano())
factory.InitFactories(nil)
}

Expand Down
9 changes: 2 additions & 7 deletions gossip/gossip/algo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package algo

import (
"math/rand"
"sync"
"sync/atomic"
"time"
Expand All @@ -44,10 +43,6 @@ import (
*/

func init() {
rand.Seed(42)
}

const (
defDigestWaitTime = time.Duration(1) * time.Second
defRequestWaitTime = time.Duration(1) * time.Second
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion gossip/gossip/msgstore/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
)

func init() {
rand.Seed(42)
rand.Seed(time.Now().UnixNano())
}

func alwaysNoAction(this interface{}, that interface{}) common.InvalidationResult {
Expand Down
5 changes: 2 additions & 3 deletions gossip/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package state
import (
"bytes"
"errors"
"math/rand"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand Down
34 changes: 29 additions & 5 deletions gossip/util/misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ limitations under the License.
package util

import (
cryptorand "crypto/rand"
"fmt"
"io"
"math/big"
"math/rand"
"reflect"
"runtime"
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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())
}
80 changes: 80 additions & 0 deletions gossip/util/misc_test.go
Original file line number Diff line number Diff line change
@@ -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)
}

0 comments on commit c041d43

Please sign in to comment.