From 510e79ebe9b575141643b8db94177d5586c2117d Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Fri, 7 Jun 2019 11:21:18 +0100 Subject: [PATCH] Entropy cannot be saved Remove non cryptographic randomness. Signed-off-by: Justin Cormack (cherry picked from commit 2df693e533e904f432c59279c07b2b8cbeece4f0) Signed-off-by: Sebastiaan van Stijn --- container/container_unix.go | 2 +- daemon/create_unix.go | 2 +- daemon/create_windows.go | 2 +- daemon/exec/exec.go | 2 +- daemon/graphdriver/aufs/aufs_test.go | 2 +- daemon/logger/plugin.go | 2 +- daemon/names.go | 2 +- ...er_cli_external_volume_driver_unix_test.go | 2 +- integration/container/rename_test.go | 4 +- pkg/stringid/stringid.go | 44 ++----------------- pkg/stringid/stringid_test.go | 8 ---- pkg/truncindex/truncindex_test.go | 30 ++++++------- plugin/manager_linux_test.go | 2 +- volume/mounts/linux_parser.go | 2 +- volume/mounts/mounts.go | 2 +- volume/mounts/windows_parser.go | 2 +- volume/service/service.go | 2 +- 17 files changed, 34 insertions(+), 78 deletions(-) diff --git a/container/container_unix.go b/container/container_unix.go index 19f169f405..b5c9b66b58 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -136,7 +136,7 @@ func (container *Container) CopyImagePathContent(v volume.Volume, destination st return err } - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() path, err := v.Mount(id) if err != nil { return err diff --git a/daemon/create_unix.go b/daemon/create_unix.go index 13857bab06..e78415aaef 100644 --- a/daemon/create_unix.go +++ b/daemon/create_unix.go @@ -41,7 +41,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con } for spec := range config.Volumes { - name := stringid.GenerateNonCryptoID() + name := stringid.GenerateRandomID() destination := filepath.Clean(spec) // Skip volumes for which we already have something mounted on that diff --git a/daemon/create_windows.go b/daemon/create_windows.go index 37e425a014..3bce278773 100644 --- a/daemon/create_windows.go +++ b/daemon/create_windows.go @@ -38,7 +38,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con // If the mountpoint doesn't have a name, generate one. if len(mp.Name) == 0 { - mp.Name = stringid.GenerateNonCryptoID() + mp.Name = stringid.GenerateRandomID() } // Skip volumes for which we already have something mounted on that diff --git a/daemon/exec/exec.go b/daemon/exec/exec.go index c036c46a0c..cf2a9556c1 100644 --- a/daemon/exec/exec.go +++ b/daemon/exec/exec.go @@ -39,7 +39,7 @@ type Config struct { // NewConfig initializes the a new exec configuration func NewConfig() *Config { return &Config{ - ID: stringid.GenerateNonCryptoID(), + ID: stringid.GenerateRandomID(), StreamConfig: stream.NewConfig(), Started: make(chan struct{}), } diff --git a/daemon/graphdriver/aufs/aufs_test.go b/daemon/graphdriver/aufs/aufs_test.go index fdc502ba65..0752c842b4 100644 --- a/daemon/graphdriver/aufs/aufs_test.go +++ b/daemon/graphdriver/aufs/aufs_test.go @@ -731,7 +731,7 @@ func BenchmarkConcurrentAccess(b *testing.B) { // create a bunch of ids var ids []string for i := 0; i < numConcurrent; i++ { - ids = append(ids, stringid.GenerateNonCryptoID()) + ids = append(ids, stringid.GenerateRandomID()) } if err := d.Create(ids[0], "", nil); err != nil { diff --git a/daemon/logger/plugin.go b/daemon/logger/plugin.go index c66540ce52..8c155b0ddb 100644 --- a/daemon/logger/plugin.go +++ b/daemon/logger/plugin.go @@ -81,7 +81,7 @@ func makePluginCreator(name string, l logPlugin, scopePath func(s string) string return nil, err } - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() a := &pluginAdapter{ driverName: name, id: id, diff --git a/daemon/names.go b/daemon/names.go index 6c31949777..4fa39af2ee 100644 --- a/daemon/names.go +++ b/daemon/names.go @@ -38,7 +38,7 @@ func (daemon *Daemon) registerName(container *container.Container) error { func (daemon *Daemon) generateIDAndName(name string) (string, string, error) { var ( err error - id = stringid.GenerateNonCryptoID() + id = stringid.GenerateRandomID() ) if name == "" { diff --git a/integration-cli/docker_cli_external_volume_driver_unix_test.go b/integration-cli/docker_cli_external_volume_driver_unix_test.go index b876d9f6fe..e9e92d4372 100644 --- a/integration-cli/docker_cli_external_volume_driver_unix_test.go +++ b/integration-cli/docker_cli_external_volume_driver_unix_test.go @@ -558,7 +558,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverCapabilities(c *chec } func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverOutOfBandDelete(c *check.C) { - driverName := stringid.GenerateNonCryptoID() + driverName := stringid.GenerateRandomID() p := newVolumePlugin(c, driverName) defer p.Close() diff --git a/integration/container/rename_test.go b/integration/container/rename_test.go index e2334feb42..976e6007d3 100644 --- a/integration/container/rename_test.go +++ b/integration/container/rename_test.go @@ -61,7 +61,7 @@ func TestRenameStoppedContainer(t *testing.T) { assert.NilError(t, err) assert.Check(t, is.Equal("/"+oldName, inspect.Name)) - newName := "new_name" + stringid.GenerateNonCryptoID() + newName := "new_name" + stringid.GenerateRandomID() err = client.ContainerRename(ctx, oldName, newName) assert.NilError(t, err) @@ -79,7 +79,7 @@ func TestRenameRunningContainerAndReuse(t *testing.T) { cID := container.Run(t, ctx, client, container.WithName(oldName)) poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) - newName := "new_name" + stringid.GenerateNonCryptoID() + newName := "new_name" + stringid.GenerateRandomID() err := client.ContainerRename(ctx, oldName, newName) assert.NilError(t, err) diff --git a/pkg/stringid/stringid.go b/pkg/stringid/stringid.go index fa7d9166eb..5fe071d628 100644 --- a/pkg/stringid/stringid.go +++ b/pkg/stringid/stringid.go @@ -2,17 +2,12 @@ package stringid // import "github.com/docker/docker/pkg/stringid" import ( - cryptorand "crypto/rand" + "crypto/rand" "encoding/hex" "fmt" - "io" - "math" - "math/big" - "math/rand" "regexp" "strconv" "strings" - "time" ) const shortLen = 12 @@ -41,10 +36,11 @@ func TruncateID(id string) string { return id } -func generateID(r io.Reader) string { +// GenerateRandomID returns a unique id. +func GenerateRandomID() string { b := make([]byte, 32) for { - if _, err := io.ReadFull(r, b); err != nil { + if _, err := rand.Read(b); err != nil { panic(err) // This shouldn't happen } id := hex.EncodeToString(b) @@ -58,18 +54,6 @@ func generateID(r io.Reader) string { } } -// GenerateRandomID returns a unique id. -func GenerateRandomID() string { - return generateID(cryptorand.Reader) -} - -// GenerateNonCryptoID generates unique id without using cryptographically -// secure sources of random. -// It helps you to save entropy. -func GenerateNonCryptoID() string { - return generateID(readerFunc(rand.Read)) -} - // ValidateID checks whether an ID string is a valid image ID. func ValidateID(id string) error { if ok := validHex.MatchString(id); !ok { @@ -77,23 +61,3 @@ func ValidateID(id string) error { } return nil } - -func init() { - // safely set the seed globally so we generate random ids. Tries to use a - // crypto seed before falling back to time. - 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() - } else { - seed = cryptoseed.Int64() - } - - rand.Seed(seed) -} - -type readerFunc func(p []byte) (int, error) - -func (fn readerFunc) Read(p []byte) (int, error) { - return fn(p) -} diff --git a/pkg/stringid/stringid_test.go b/pkg/stringid/stringid_test.go index a7ccd5faae..2660d2e65f 100644 --- a/pkg/stringid/stringid_test.go +++ b/pkg/stringid/stringid_test.go @@ -13,14 +13,6 @@ func TestGenerateRandomID(t *testing.T) { } } -func TestGenerateNonCryptoID(t *testing.T) { - id := GenerateNonCryptoID() - - if len(id) != 64 { - t.Fatalf("Id returned is incorrect: %s", id) - } -} - func TestShortenId(t *testing.T) { id := "90435eec5c4e124e741ef731e118be2fc799a68aba0466ec17717f24ce2ae6a2" truncID := TruncateID(id) diff --git a/pkg/truncindex/truncindex_test.go b/pkg/truncindex/truncindex_test.go index e259017982..6d00a245aa 100644 --- a/pkg/truncindex/truncindex_test.go +++ b/pkg/truncindex/truncindex_test.go @@ -158,7 +158,7 @@ func assertIndexGet(t *testing.T, index *TruncIndex, input, expectedResult strin func BenchmarkTruncIndexAdd100(b *testing.B) { var testSet []string for i := 0; i < 100; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -174,7 +174,7 @@ func BenchmarkTruncIndexAdd100(b *testing.B) { func BenchmarkTruncIndexAdd250(b *testing.B) { var testSet []string for i := 0; i < 250; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -190,7 +190,7 @@ func BenchmarkTruncIndexAdd250(b *testing.B) { func BenchmarkTruncIndexAdd500(b *testing.B) { var testSet []string for i := 0; i < 500; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -207,7 +207,7 @@ func BenchmarkTruncIndexGet100(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 100; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } index := NewTruncIndex([]string{}) for _, id := range testSet { @@ -231,7 +231,7 @@ func BenchmarkTruncIndexGet250(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 250; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } index := NewTruncIndex([]string{}) for _, id := range testSet { @@ -255,7 +255,7 @@ func BenchmarkTruncIndexGet500(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 500; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } index := NewTruncIndex([]string{}) for _, id := range testSet { @@ -278,7 +278,7 @@ func BenchmarkTruncIndexGet500(b *testing.B) { func BenchmarkTruncIndexDelete100(b *testing.B) { var testSet []string for i := 0; i < 100; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -301,7 +301,7 @@ func BenchmarkTruncIndexDelete100(b *testing.B) { func BenchmarkTruncIndexDelete250(b *testing.B) { var testSet []string for i := 0; i < 250; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -324,7 +324,7 @@ func BenchmarkTruncIndexDelete250(b *testing.B) { func BenchmarkTruncIndexDelete500(b *testing.B) { var testSet []string for i := 0; i < 500; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -347,7 +347,7 @@ func BenchmarkTruncIndexDelete500(b *testing.B) { func BenchmarkTruncIndexNew100(b *testing.B) { var testSet []string for i := 0; i < 100; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -358,7 +358,7 @@ func BenchmarkTruncIndexNew100(b *testing.B) { func BenchmarkTruncIndexNew250(b *testing.B) { var testSet []string for i := 0; i < 250; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -369,7 +369,7 @@ func BenchmarkTruncIndexNew250(b *testing.B) { func BenchmarkTruncIndexNew500(b *testing.B) { var testSet []string for i := 0; i < 500; i++ { - testSet = append(testSet, stringid.GenerateNonCryptoID()) + testSet = append(testSet, stringid.GenerateRandomID()) } b.ResetTimer() for i := 0; i < b.N; i++ { @@ -381,7 +381,7 @@ func BenchmarkTruncIndexAddGet100(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 500; i++ { - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() testSet = append(testSet, id) l := rand.Intn(12) + 12 testKeys = append(testKeys, id[:l]) @@ -406,7 +406,7 @@ func BenchmarkTruncIndexAddGet250(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 500; i++ { - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() testSet = append(testSet, id) l := rand.Intn(12) + 12 testKeys = append(testKeys, id[:l]) @@ -431,7 +431,7 @@ func BenchmarkTruncIndexAddGet500(b *testing.B) { var testSet []string var testKeys []string for i := 0; i < 500; i++ { - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() testSet = append(testSet, id) l := rand.Intn(12) + 12 testKeys = append(testKeys, id[:l]) diff --git a/plugin/manager_linux_test.go b/plugin/manager_linux_test.go index fd8fa8523c..1b6a3bf773 100644 --- a/plugin/manager_linux_test.go +++ b/plugin/manager_linux_test.go @@ -70,7 +70,7 @@ func TestManagerWithPluginMounts(t *testing.T) { } func newTestPlugin(t *testing.T, name, cap, root string) *v2.Plugin { - id := stringid.GenerateNonCryptoID() + id := stringid.GenerateRandomID() rootfs := filepath.Join(root, id) if err := os.MkdirAll(rootfs, 0755); err != nil { t.Fatal(err) diff --git a/volume/mounts/linux_parser.go b/volume/mounts/linux_parser.go index 22ab2d62cf..b5ed4af6a3 100644 --- a/volume/mounts/linux_parser.go +++ b/volume/mounts/linux_parser.go @@ -298,7 +298,7 @@ func (p *linuxParser) parseMountSpec(cfg mount.Mount, validateBindSourceExists b switch cfg.Type { case mount.TypeVolume: if cfg.Source == "" { - mp.Name = stringid.GenerateNonCryptoID() + mp.Name = stringid.GenerateRandomID() } else { mp.Name = cfg.Source } diff --git a/volume/mounts/mounts.go b/volume/mounts/mounts.go index 63a1406814..5bf169f6e0 100644 --- a/volume/mounts/mounts.go +++ b/volume/mounts/mounts.go @@ -125,7 +125,7 @@ func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.Identity, checkFun if m.Volume != nil { id := m.ID if id == "" { - id = stringid.GenerateNonCryptoID() + id = stringid.GenerateRandomID() } path, err := m.Volume.Mount(id) if err != nil { diff --git a/volume/mounts/windows_parser.go b/volume/mounts/windows_parser.go index ac61044043..8f427d8c50 100644 --- a/volume/mounts/windows_parser.go +++ b/volume/mounts/windows_parser.go @@ -385,7 +385,7 @@ func (p *windowsParser) parseMountSpec(cfg mount.Mount, destRegex string, conver switch cfg.Type { case mount.TypeVolume: if cfg.Source == "" { - mp.Name = stringid.GenerateNonCryptoID() + mp.Name = stringid.GenerateRandomID() } else { mp.Name = cfg.Source } diff --git a/volume/service/service.go b/volume/service/service.go index 848606206a..8dbfa283c8 100644 --- a/volume/service/service.go +++ b/volume/service/service.go @@ -63,7 +63,7 @@ func (s *VolumesService) GetDriverList() []string { // When whatever is going to reference this volume is removed the caller should defeference the volume by calling `Release`. func (s *VolumesService) Create(ctx context.Context, name, driverName string, opts ...opts.CreateOption) (*types.Volume, error) { if name == "" { - name = stringid.GenerateNonCryptoID() + name = stringid.GenerateRandomID() } v, err := s.vs.Create(ctx, name, driverName, opts...) if err != nil {