Skip to content
This repository has been archived by the owner on Jul 15, 2018. It is now read-only.

rand: perhaps make an internal rand.RandSource that's concurrent safe to avoid nefarious rand.Seed(1) resets #99

Closed
odeke-em opened this issue Dec 13, 2017 · 0 comments

Comments

@odeke-em
Copy link
Contributor

Problem Statement

As a followup to tendermint/tendermint#973, inside some Tendermint repos, we'll see direct invocations of math/rand.R* such as here in:

randElemStart := rand.Intn(length)

https://github.com/tendermint/tmlibs/blob/master/common/bit_array.go#L220
https://github.com/tendermint/tmlibs/blob/master/common/bit_array.go#L235

where we trustingly assumed that at init we properly seeded the random source with

tmlibs/common/random.go

Lines 13 to 21 in bfcc021

func init() {
b := cRandBytes(8)
var seed uint64
for i := 0; i < 8; i++ {
seed |= uint64(b[i])
seed <<= 8
}
rand.Seed(int64(seed))
}

but as shown in issue tendermint/tendermint#973, a couple of our dependencies also set rand.Seed hence if our init gets invoked before theirs, all our initial seeding is no longer useful

Analysis

I think our usages of math/rand.R* are predicated on the ease of use of the stdlib and the fact that the default source guarantees to be concurrency safe but others don't https://golang.org/pkg/math/rand/
screen shot 2017-12-13 at 11 20 02 am

Proposed fix

Mirror the functionality of the default source with our own internal RNG seeded with crypto/rand's true randomness and then gut/replace every usage of math/rand in all Tendermint's code

odeke-em added a commit that referenced this issue Dec 14, 2017
Fixes #99
Updates tendermint/tendermint#973

Removed usages of math/rand.DefaultSource in favour of our
own source that's seeded with a completely random source
and is safe for use in concurrent in multiple goroutines.
Also extend some functionality that the stdlib exposes such as
* Perm
* Intn
* Int31
* Int63

Also added an integration test whose purpose is to be run as
a consistency check to ensure that our results never repeat
hence that our internal PRNG is uniquely seeded each time.
odeke-em added a commit that referenced this issue Dec 14, 2017
Fixes #99
Updates tendermint/tendermint#973

Removed usages of math/rand.DefaultSource in favour of our
own source that's seeded with a completely random source
and is safe for use in concurrent in multiple goroutines.
Also extend some functionality that the stdlib exposes such as
* Perm
* Intn
* Int31
* Int63

Also added an integration test whose purpose is to be run as
a consistency check to ensure that our results never repeat
hence that our internal PRNG is uniquely seeded each time.
odeke-em added a commit that referenced this issue Dec 14, 2017
Fixes #99
Updates tendermint/tendermint#973

Removed usages of math/rand.DefaultSource in favour of our
own source that's seeded with a completely random source
and is safe for use in concurrent in multiple goroutines.
Also extend some functionality that the stdlib exposes such as
* Perm
* Intn
* Int31
* Int63

Also added an integration test whose purpose is to be run as
a consistency check to ensure that our results never repeat
hence that our internal PRNG is uniquely seeded each time.
odeke-em added a commit that referenced this issue Dec 14, 2017
Fixes #99
Updates tendermint/tendermint#973

Removed usages of math/rand.DefaultSource in favour of our
own source that's seeded with a completely random source
and is safe for use in concurrent in multiple goroutines.
Also extend some functionality that the stdlib exposes such as
* RandPerm
* RandIntn
* RandInt31
* RandInt63

Also added an integration test whose purpose is to be run as
a consistency check to ensure that our results never repeat
hence that our internal PRNG is uniquely seeded each time.
odeke-em added a commit that referenced this issue Dec 14, 2017
Fixes #99
Updates tendermint/tendermint#973

Removed usages of math/rand.DefaultSource in favour of our
own source that's seeded with a completely random source
and is safe for use in concurrent in multiple goroutines.
Also extend some functionality that the stdlib exposes such as
* RandPerm
* RandIntn
* RandInt31
* RandInt63

Also added an integration test whose purpose is to be run as
a consistency check to ensure that our results never repeat
hence that our internal PRNG is uniquely seeded each time.
odeke-em added a commit that referenced this issue Dec 14, 2017
Fixes #99
Updates tendermint/tendermint#973

Removed usages of math/rand.DefaultSource in favour of our
own source that's seeded with a completely random source
and is safe for use in concurrent in multiple goroutines.
Also extend some functionality that the stdlib exposes such as
* RandPerm
* RandIntn
* RandInt31
* RandInt63

Also added an integration test whose purpose is to be run as
a consistency check to ensure that our results never repeat
hence that our internal PRNG is uniquely seeded each time.
This integration test can be triggered by setting environment variable:
`TENDERMINT_INTEGRATION_TESTS=true`
for example
```shell
TENDERMINT_INTEGRATION_TESTS=true go test
```
@melekes melekes closed this as completed Dec 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants