diff --git a/backend/union/policy/eprand.go b/backend/union/policy/eprand.go index ef223e7be..dea64b76e 100644 --- a/backend/union/policy/eprand.go +++ b/backend/union/policy/eprand.go @@ -3,7 +3,6 @@ package policy import ( "context" "math/rand" - "time" "github.com/rclone/rclone/backend/union/upstream" "github.com/rclone/rclone/fs" @@ -20,12 +19,10 @@ type EpRand struct { } func (p *EpRand) rand(upstreams []*upstream.Fs) *upstream.Fs { - rand.Seed(time.Now().Unix()) return upstreams[rand.Intn(len(upstreams))] } func (p *EpRand) randEntries(entries []upstream.Entry) upstream.Entry { - rand.Seed(time.Now().Unix()) return entries[rand.Intn(len(entries))] } diff --git a/cmd/cmd.go b/cmd/cmd.go index 85a361a74..1fc98ac9e 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -10,7 +10,6 @@ import ( "context" "fmt" "log" - "math/rand" "os" "os/exec" "path" @@ -37,6 +36,7 @@ import ( "github.com/rclone/rclone/fs/rc/rcflags" "github.com/rclone/rclone/fs/rc/rcserver" "github.com/rclone/rclone/lib/atexit" + "github.com/rclone/rclone/lib/random" "github.com/rclone/rclone/lib/terminal" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -533,7 +533,9 @@ func AddBackendFlags() { // Main runs rclone interpreting flags and commands out of os.Args func Main() { - rand.Seed(time.Now().Unix()) + if err := random.Seed(); err != nil { + log.Fatalf("Fatal error: %v", err) + } setupRootCommand(Root) AddBackendFlags() if err := Root.Execute(); err != nil { diff --git a/lib/random/random.go b/lib/random/random.go index 5d6f4d6b0..57a858beb 100644 --- a/lib/random/random.go +++ b/lib/random/random.go @@ -4,6 +4,7 @@ package random import ( cryptorand "crypto/rand" "encoding/base64" + "encoding/binary" mathrand "math/rand" "github.com/pkg/errors" @@ -52,3 +53,19 @@ func Password(bits int) (password string, err error) { password = base64.RawURLEncoding.EncodeToString(pw) return password, nil } + +// Seed the global math/rand with crypto strong data +// +// This doesn't make it OK to use math/rand in crypto sensitive +// environments - don't do that! However it does help to mitigate the +// problem if that happens accidentally. This would have helped with +// CVE-2020-28924 - #4783 +func Seed() error { + var seed int64 + err := binary.Read(cryptorand.Reader, binary.LittleEndian, &seed) + if err != nil { + return errors.Wrap(err, "failed to read random seed") + } + mathrand.Seed(seed) + return nil +} diff --git a/lib/random/random_test.go b/lib/random/random_test.go index 1be57dda6..a710833d3 100644 --- a/lib/random/random_test.go +++ b/lib/random/random_test.go @@ -1,6 +1,7 @@ package random import ( + "math/rand" "testing" "github.com/stretchr/testify/assert" @@ -48,3 +49,16 @@ func TestPasswordDuplicates(t *testing.T) { seen[s] = true } } + +func TestSeed(t *testing.T) { + // seed 100 times and check the first random number doesn't repeat + // This test could fail with a probability of ~ 10**-15 + const n = 100 + var seen = map[int64]bool{} + for i := 0; i < n; i++ { + assert.NoError(t, Seed()) + first := rand.Int63() + assert.False(t, seen[first]) + seen[first] = true + } +}