From 22b246417f52aa6bd0e358e41e2bfb9c0a59c867 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 6 Sep 2017 11:35:06 -0400 Subject: [PATCH 1/3] Move names to a more appropriate package. Signed-off-by: Daniel Nephin --- daemon/checkpoint.go | 6 +++--- daemon/names.go | 6 +++--- {api => daemon/names}/names.go | 2 +- volume/local/local.go | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) rename {api => daemon/names}/names.go (96%) diff --git a/daemon/checkpoint.go b/daemon/checkpoint.go index d3028f1e28..7bdcae5154 100644 --- a/daemon/checkpoint.go +++ b/daemon/checkpoint.go @@ -7,13 +7,13 @@ import ( "os" "path/filepath" - "github.com/docker/docker/api" "github.com/docker/docker/api/types" + "github.com/docker/docker/daemon/names" ) var ( - validCheckpointNameChars = api.RestrictedNameChars - validCheckpointNamePattern = api.RestrictedNamePattern + validCheckpointNameChars = names.RestrictedNameChars + validCheckpointNamePattern = names.RestrictedNamePattern ) // getCheckpointDir verifies checkpoint directory for create,remove, list options and checks if checkpoint already exists diff --git a/daemon/names.go b/daemon/names.go index e61e94008f..712df9fd0f 100644 --- a/daemon/names.go +++ b/daemon/names.go @@ -4,8 +4,8 @@ import ( "fmt" "strings" - "github.com/docker/docker/api" "github.com/docker/docker/container" + "github.com/docker/docker/daemon/names" "github.com/docker/docker/pkg/namesgenerator" "github.com/docker/docker/pkg/stringid" "github.com/pkg/errors" @@ -13,8 +13,8 @@ import ( ) var ( - validContainerNameChars = api.RestrictedNameChars - validContainerNamePattern = api.RestrictedNamePattern + validContainerNameChars = names.RestrictedNameChars + validContainerNamePattern = names.RestrictedNamePattern ) func (daemon *Daemon) registerName(container *container.Container) error { diff --git a/api/names.go b/daemon/names/names.go similarity index 96% rename from api/names.go rename to daemon/names/names.go index f147d1f4ce..26f6748a8f 100644 --- a/api/names.go +++ b/daemon/names/names.go @@ -1,4 +1,4 @@ -package api +package names import "regexp" diff --git a/volume/local/local.go b/volume/local/local.go index eb78d875a5..c85122d63a 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -13,7 +13,7 @@ import ( "strings" "sync" - "github.com/docker/docker/api" + "github.com/docker/docker/daemon/names" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/mount" "github.com/docker/docker/volume" @@ -35,7 +35,7 @@ var ( // volumeNameRegex ensures the name assigned for the volume is valid. // This name is used to create the bind directory, so we need to avoid characters that // would make the path to escape the root directory. - volumeNameRegex = api.RestrictedNamePattern + volumeNameRegex = names.RestrictedNamePattern ) type activeMount struct { @@ -298,7 +298,7 @@ func (r *Root) validateName(name string) error { return validationError("volume name is too short, names should be at least two alphanumeric characters") } if !volumeNameRegex.MatchString(name) { - return validationError(fmt.Sprintf("%q includes invalid characters for a local volume name, only %q are allowed. If you intended to pass a host directory, use absolute path", name, api.RestrictedNameChars)) + return validationError(fmt.Sprintf("%q includes invalid characters for a local volume name, only %q are allowed. If you intended to pass a host directory, use absolute path", name, names.RestrictedNameChars)) } return nil } From 2f007e46d0100d865a061c1a8e544bddc0b7a368 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 6 Sep 2017 11:44:11 -0400 Subject: [PATCH 2/3] Remove libtrust dep from api Signed-off-by: Daniel Nephin --- api/common.go | 54 ---------------- api/common_test.go | 77 ----------------------- daemon/daemon.go | 3 +- {api/fixtures => daemon/testdata}/keyfile | 0 daemon/trustkey.go | 57 +++++++++++++++++ daemon/trustkey_test.go | 72 +++++++++++++++++++++ 6 files changed, 130 insertions(+), 133 deletions(-) delete mode 100644 api/common_test.go rename {api/fixtures => daemon/testdata}/keyfile (100%) create mode 100644 daemon/trustkey.go create mode 100644 daemon/trustkey_test.go diff --git a/api/common.go b/api/common.go index 6e462aeda7..ff87a94b58 100644 --- a/api/common.go +++ b/api/common.go @@ -1,17 +1,5 @@ package api -import ( - "encoding/json" - "encoding/pem" - "fmt" - "os" - "path/filepath" - - "github.com/docker/docker/pkg/ioutils" - "github.com/docker/docker/pkg/system" - "github.com/docker/libtrust" -) - // Common constants for daemon and client. const ( // DefaultVersion of Current REST API @@ -21,45 +9,3 @@ const ( // command to specify that no base image is to be used. NoBaseImageSpecifier string = "scratch" ) - -// LoadOrCreateTrustKey attempts to load the libtrust key at the given path, -// otherwise generates a new one -func LoadOrCreateTrustKey(trustKeyPath string) (libtrust.PrivateKey, error) { - err := system.MkdirAll(filepath.Dir(trustKeyPath), 0700, "") - if err != nil { - return nil, err - } - trustKey, err := libtrust.LoadKeyFile(trustKeyPath) - if err == libtrust.ErrKeyFileDoesNotExist { - trustKey, err = libtrust.GenerateECP256PrivateKey() - if err != nil { - return nil, fmt.Errorf("Error generating key: %s", err) - } - encodedKey, err := serializePrivateKey(trustKey, filepath.Ext(trustKeyPath)) - if err != nil { - return nil, fmt.Errorf("Error serializing key: %s", err) - } - if err := ioutils.AtomicWriteFile(trustKeyPath, encodedKey, os.FileMode(0600)); err != nil { - return nil, fmt.Errorf("Error saving key file: %s", err) - } - } else if err != nil { - return nil, fmt.Errorf("Error loading key file %s: %s", trustKeyPath, err) - } - return trustKey, nil -} - -func serializePrivateKey(key libtrust.PrivateKey, ext string) (encoded []byte, err error) { - if ext == ".json" || ext == ".jwk" { - encoded, err = json.Marshal(key) - if err != nil { - return nil, fmt.Errorf("unable to encode private key JWK: %s", err) - } - } else { - pemBlock, err := key.PEMBlock() - if err != nil { - return nil, fmt.Errorf("unable to encode private key PEM: %s", err) - } - encoded = pem.EncodeToMemory(pemBlock) - } - return -} diff --git a/api/common_test.go b/api/common_test.go deleted file mode 100644 index f466616b0f..0000000000 --- a/api/common_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package api - -import ( - "io/ioutil" - "path/filepath" - "testing" - - "os" -) - -// LoadOrCreateTrustKey -func TestLoadOrCreateTrustKeyInvalidKeyFile(t *testing.T) { - tmpKeyFolderPath, err := ioutil.TempDir("", "api-trustkey-test") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpKeyFolderPath) - - tmpKeyFile, err := ioutil.TempFile(tmpKeyFolderPath, "keyfile") - if err != nil { - t.Fatal(err) - } - - if _, err := LoadOrCreateTrustKey(tmpKeyFile.Name()); err == nil { - t.Fatal("expected an error, got nothing.") - } - -} - -func TestLoadOrCreateTrustKeyCreateKey(t *testing.T) { - tmpKeyFolderPath, err := ioutil.TempDir("", "api-trustkey-test") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpKeyFolderPath) - - // Without the need to create the folder hierarchy - tmpKeyFile := filepath.Join(tmpKeyFolderPath, "keyfile") - - if key, err := LoadOrCreateTrustKey(tmpKeyFile); err != nil || key == nil { - t.Fatalf("expected a new key file, got : %v and %v", err, key) - } - - if _, err := os.Stat(tmpKeyFile); err != nil { - t.Fatalf("Expected to find a file %s, got %v", tmpKeyFile, err) - } - - // With the need to create the folder hierarchy as tmpKeyFie is in a path - // where some folders do not exist. - tmpKeyFile = filepath.Join(tmpKeyFolderPath, "folder/hierarchy/keyfile") - - if key, err := LoadOrCreateTrustKey(tmpKeyFile); err != nil || key == nil { - t.Fatalf("expected a new key file, got : %v and %v", err, key) - } - - if _, err := os.Stat(tmpKeyFile); err != nil { - t.Fatalf("Expected to find a file %s, got %v", tmpKeyFile, err) - } - - // With no path at all - defer os.Remove("keyfile") - if key, err := LoadOrCreateTrustKey("keyfile"); err != nil || key == nil { - t.Fatalf("expected a new key file, got : %v and %v", err, key) - } - - if _, err := os.Stat("keyfile"); err != nil { - t.Fatalf("Expected to find a file keyfile, got %v", err) - } -} - -func TestLoadOrCreateTrustKeyLoadValidKey(t *testing.T) { - tmpKeyFile := filepath.Join("fixtures", "keyfile") - - if key, err := LoadOrCreateTrustKey(tmpKeyFile); err != nil || key == nil { - t.Fatalf("expected a key file, got : %v and %v", err, key) - } -} diff --git a/daemon/daemon.go b/daemon/daemon.go index a11a1f8691..7d23dd87a6 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -19,7 +19,6 @@ import ( "time" containerd "github.com/containerd/containerd/api/grpc/types" - "github.com/docker/docker/api" "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/swarm" @@ -713,7 +712,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe return nil, err } - trustKey, err := api.LoadOrCreateTrustKey(config.TrustKeyPath) + trustKey, err := loadOrCreateTrustKey(config.TrustKeyPath) if err != nil { return nil, err } diff --git a/api/fixtures/keyfile b/daemon/testdata/keyfile similarity index 100% rename from api/fixtures/keyfile rename to daemon/testdata/keyfile diff --git a/daemon/trustkey.go b/daemon/trustkey.go new file mode 100644 index 0000000000..cb33146f9b --- /dev/null +++ b/daemon/trustkey.go @@ -0,0 +1,57 @@ +package daemon + +import ( + "encoding/json" + "encoding/pem" + "fmt" + "os" + "path/filepath" + + "github.com/docker/docker/pkg/ioutils" + "github.com/docker/docker/pkg/system" + "github.com/docker/libtrust" +) + +// LoadOrCreateTrustKey attempts to load the libtrust key at the given path, +// otherwise generates a new one +// TODO: this should use more of libtrust.LoadOrCreateTrustKey which may need +// a refactor or this function to be moved into libtrust +func loadOrCreateTrustKey(trustKeyPath string) (libtrust.PrivateKey, error) { + err := system.MkdirAll(filepath.Dir(trustKeyPath), 0700, "") + if err != nil { + return nil, err + } + trustKey, err := libtrust.LoadKeyFile(trustKeyPath) + if err == libtrust.ErrKeyFileDoesNotExist { + trustKey, err = libtrust.GenerateECP256PrivateKey() + if err != nil { + return nil, fmt.Errorf("Error generating key: %s", err) + } + encodedKey, err := serializePrivateKey(trustKey, filepath.Ext(trustKeyPath)) + if err != nil { + return nil, fmt.Errorf("Error serializing key: %s", err) + } + if err := ioutils.AtomicWriteFile(trustKeyPath, encodedKey, os.FileMode(0600)); err != nil { + return nil, fmt.Errorf("Error saving key file: %s", err) + } + } else if err != nil { + return nil, fmt.Errorf("Error loading key file %s: %s", trustKeyPath, err) + } + return trustKey, nil +} + +func serializePrivateKey(key libtrust.PrivateKey, ext string) (encoded []byte, err error) { + if ext == ".json" || ext == ".jwk" { + encoded, err = json.Marshal(key) + if err != nil { + return nil, fmt.Errorf("unable to encode private key JWK: %s", err) + } + } else { + pemBlock, err := key.PEMBlock() + if err != nil { + return nil, fmt.Errorf("unable to encode private key PEM: %s", err) + } + encoded = pem.EncodeToMemory(pemBlock) + } + return +} diff --git a/daemon/trustkey_test.go b/daemon/trustkey_test.go new file mode 100644 index 0000000000..2ade2aa80d --- /dev/null +++ b/daemon/trustkey_test.go @@ -0,0 +1,72 @@ +package daemon + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/docker/docker/internal/testutil" + "github.com/gotestyourself/gotestyourself/fs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// LoadOrCreateTrustKey +func TestLoadOrCreateTrustKeyInvalidKeyFile(t *testing.T) { + tmpKeyFolderPath, err := ioutil.TempDir("", "api-trustkey-test") + require.NoError(t, err) + defer os.RemoveAll(tmpKeyFolderPath) + + tmpKeyFile, err := ioutil.TempFile(tmpKeyFolderPath, "keyfile") + require.NoError(t, err) + + _, err = loadOrCreateTrustKey(tmpKeyFile.Name()) + testutil.ErrorContains(t, err, "Error loading key file") +} + +func TestLoadOrCreateTrustKeyCreateKeyWhenFileDoesNotExist(t *testing.T) { + tmpKeyFolderPath := fs.NewDir(t, "api-trustkey-test") + defer tmpKeyFolderPath.Remove() + + // Without the need to create the folder hierarchy + tmpKeyFile := tmpKeyFolderPath.Join("keyfile") + + key, err := loadOrCreateTrustKey(tmpKeyFile) + require.NoError(t, err) + assert.NotNil(t, key) + + _, err = os.Stat(tmpKeyFile) + require.NoError(t, err, "key file doesn't exist") +} + +func TestLoadOrCreateTrustKeyCreateKeyWhenDirectoryDoesNotExist(t *testing.T) { + tmpKeyFolderPath := fs.NewDir(t, "api-trustkey-test") + defer tmpKeyFolderPath.Remove() + tmpKeyFile := tmpKeyFolderPath.Join("folder/hierarchy/keyfile") + + key, err := loadOrCreateTrustKey(tmpKeyFile) + require.NoError(t, err) + assert.NotNil(t, key) + + _, err = os.Stat(tmpKeyFile) + require.NoError(t, err, "key file doesn't exist") +} + +func TestLoadOrCreateTrustKeyCreateKeyNoPath(t *testing.T) { + defer os.Remove("keyfile") + key, err := loadOrCreateTrustKey("keyfile") + require.NoError(t, err) + assert.NotNil(t, key) + + _, err = os.Stat("keyfile") + require.NoError(t, err, "key file doesn't exist") +} + +func TestLoadOrCreateTrustKeyLoadValidKey(t *testing.T) { + tmpKeyFile := filepath.Join("testdata", "keyfile") + key, err := loadOrCreateTrustKey(tmpKeyFile) + require.NoError(t, err) + expected := "AWX2:I27X:WQFX:IOMK:CNAK:O7PW:VYNB:ZLKC:CVAE:YJP2:SI4A:XXAY" + assert.Contains(t, key.String(), expected) +} From 6916c215b00ab4d7edf4be14848ab2e695697381 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 6 Sep 2017 12:13:01 -0400 Subject: [PATCH 3/3] Move tlsconfig to client package. Signed-off-by: Daniel Nephin --- client/hijack.go | 3 +-- client/tlsconfig_clone.go | 11 +++++++++++ {pkg/tlsconfig => client}/tlsconfig_clone_go17.go | 6 +++--- pkg/tlsconfig/tlsconfig_clone.go | 11 ----------- 4 files changed, 15 insertions(+), 16 deletions(-) create mode 100644 client/tlsconfig_clone.go rename {pkg/tlsconfig => client}/tlsconfig_clone_go17.go (89%) delete mode 100644 pkg/tlsconfig/tlsconfig_clone.go diff --git a/client/hijack.go b/client/hijack.go index 8cf0119f3d..dc754fca7d 100644 --- a/client/hijack.go +++ b/client/hijack.go @@ -12,7 +12,6 @@ import ( "time" "github.com/docker/docker/api/types" - "github.com/docker/docker/pkg/tlsconfig" "github.com/docker/go-connections/sockets" "github.com/pkg/errors" "golang.org/x/net/context" @@ -115,7 +114,7 @@ func tlsDialWithDialer(dialer *net.Dialer, network, addr string, config *tls.Con // from the hostname we're connecting to. if config.ServerName == "" { // Make a copy to avoid polluting argument or default. - config = tlsconfig.Clone(config) + config = tlsConfigClone(config) config.ServerName = hostname } diff --git a/client/tlsconfig_clone.go b/client/tlsconfig_clone.go new file mode 100644 index 0000000000..99b6be1cea --- /dev/null +++ b/client/tlsconfig_clone.go @@ -0,0 +1,11 @@ +// +build go1.8 + +package client + +import "crypto/tls" + +// tlsConfigClone returns a clone of tls.Config. This function is provided for +// compatibility for go1.7 that doesn't include this method in stdlib. +func tlsConfigClone(c *tls.Config) *tls.Config { + return c.Clone() +} diff --git a/pkg/tlsconfig/tlsconfig_clone_go17.go b/client/tlsconfig_clone_go17.go similarity index 89% rename from pkg/tlsconfig/tlsconfig_clone_go17.go rename to client/tlsconfig_clone_go17.go index 0d5b448fec..b837b2ade0 100644 --- a/pkg/tlsconfig/tlsconfig_clone_go17.go +++ b/client/tlsconfig_clone_go17.go @@ -1,12 +1,12 @@ // +build go1.7,!go1.8 -package tlsconfig +package client import "crypto/tls" -// Clone returns a clone of tls.Config. This function is provided for +// tlsConfigClone returns a clone of tls.Config. This function is provided for // compatibility for go1.7 that doesn't include this method in stdlib. -func Clone(c *tls.Config) *tls.Config { +func tlsConfigClone(c *tls.Config) *tls.Config { return &tls.Config{ Rand: c.Rand, Time: c.Time, diff --git a/pkg/tlsconfig/tlsconfig_clone.go b/pkg/tlsconfig/tlsconfig_clone.go deleted file mode 100644 index e4dec3a5d1..0000000000 --- a/pkg/tlsconfig/tlsconfig_clone.go +++ /dev/null @@ -1,11 +0,0 @@ -// +build go1.8 - -package tlsconfig - -import "crypto/tls" - -// Clone returns a clone of tls.Config. This function is provided for -// compatibility for go1.7 that doesn't include this method in stdlib. -func Clone(c *tls.Config) *tls.Config { - return c.Clone() -}