Skip to content

Commit

Permalink
Fix use of rand + improve entropy-exhaustion
Browse files Browse the repository at this point in the history
Possible fix for containers/podman#12155

The golang docs for `math/rand` specifically mention that `rand.Seed()`
should never ever be used in conjunction with any other rand method.
Fix this with a simple/local mutex to protect critical code sections.
This could be made more safe by exposing the mutex to downstream callers.
This is left up to a future commit as/if needed.

Also, in entropy-exhaustion situations it's possible for multiple
concurrent *processes* to obtain the same fallback seed value, where the
lock will not provide any protection.  Clashes here are especially bad
given the large number of downstream users of `GenerateNonCryptoID()`.

Since the Linux kernel guarantees process ID uniqueness within a
reasonable timespan, include this value into the fallback seed (along
with the time).

This is *not* a perfect solution, it is still possible for two processes
to generate the same fallback seed value, given extremely unlucky
timing.  However, this is an improvement versus simplistic reliance
on the clock.

Signed-off-by: Chris Evich <[email protected]>
  • Loading branch information
cevich committed Nov 2, 2021
1 parent 77999de commit 8f515b4
Showing 1 changed file with 14 additions and 4 deletions.
18 changes: 14 additions & 4 deletions pkg/stringid/stringid.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ import (
"regexp"
"strconv"
"strings"
"sync"
"syscall"
"time"
)

const shortLen = 12

var (
randLock sync.Mutex // No math/rand method should ever be called concurrently
validShortID = regexp.MustCompile("^[a-f0-9]{12}$")
validHex = regexp.MustCompile(`^[a-f0-9]{64}$`)
)
Expand Down Expand Up @@ -67,7 +70,10 @@ func GenerateRandomID() string {
// secure sources of random.
// It helps you to save entropy.
func GenerateNonCryptoID() string {
return generateID(readerFunc(rand.Read))
randLock.Lock()
id := generateID(readerFunc(rand.Read))
randLock.Unlock()
return id
}

// ValidateID checks whether an ID string is a valid image ID.
Expand All @@ -80,16 +86,20 @@ func ValidateID(id string) error {

func init() {
// safely set the seed globally so we generate random ids. Tries to use a
// crypto seed before falling back to time.
// crypto seed (concurrent-safe) before falling back to time (concurrent-unsafe).
var seed int64
if cryptoseed, err := cryptorand.Int(cryptorand.Reader, big.NewInt(math.MaxInt64)); err != nil {
// This should not happen, but worst-case fallback to time-based seed.
seed = time.Now().UnixNano()
// Attempt to mitigate clashes slightly by including the current process ID.
// This is slow and imperfect, but preferable to hanging (indefinately)
// for the entropy pool to fill.
seed = time.Now().UnixNano() + int64(syscall.Getid())
} else {
seed = cryptoseed.Int64()
}

randLock.Lock()
rand.Seed(seed)
randLock.Unlock()
}

type readerFunc func(p []byte) (int, error)
Expand Down

0 comments on commit 8f515b4

Please sign in to comment.