Expose trust key path in config

Allows storing key under any directory. In the case where the "/etc/docker" directory is not preserved, this file can be specified to a location where it will be preserved to ensure the ID does not change across restarts. Note this key is currently only used today to generate the ID used in Docker info and for manifest schema v1 pushes. The key signature and finger on these manifests are not checked or used any longer for security, deprecated by notary. Removes old key migration from a pre-release of Docker which put the key under the home directory and was used to preserve ID used for swarm v1 after the file moved. closes moby#32135 Signed-off-by: Derek McGowan <[email protected]>
ph · May 10, 2017 · e428c82 · e428c82
1 parent 7ca8679
commit e428c82
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 105 deletions.
diff --git a/cli/flags/common.go b/cli/flags/common.go
@@ -13,8 +13,6 @@ import (
 )
 
 const (
-	// DefaultTrustKeyFile is the default filename for the trust key
-	DefaultTrustKeyFile = "key.json"
 	// DefaultCaFile is the default filename for the CA pem file
 	DefaultCaFile = "ca.pem"
 	// DefaultKeyFile is the default filename for the key pem file
@@ -38,7 +36,6 @@ type CommonOptions struct {
 	TLS        bool
 	TLSVerify  bool
 	TLSOptions *tlsconfig.Options
-	TrustKey   string
 }
 
 // NewCommonOptions returns a new CommonOptions

diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go
@@ -9,6 +9,8 @@ import (
 const (
 	// defaultShutdownTimeout is the default shutdown timeout for the daemon
 	defaultShutdownTimeout = 15
+	// defaultTrustKeyFile is the default filename for the trust key
+	defaultTrustKeyFile = "key.json"
 )
 
 // installCommonConfigFlags adds flags to the pflag.FlagSet to configure the daemon
@@ -53,6 +55,13 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) {
 
 	flags.StringVar(&conf.MetricsAddress, "metrics-addr", "", "Set default address and port to serve the metrics api on")
 
+	// "--deprecated-key-path" is to allow configuration of the key used
+	// for the daemon ID and the deprecated image signing. It was never
+	// exposed as a command line option but is added here to allow
+	// overriding the default path in configuration.
+	flags.Var(opts.NewQuotedString(&conf.TrustKeyPath), "deprecated-key-path", "Path to key file for ID and image signing")
+	flags.MarkHidden("deprecated-key-path")
+
 	conf.MaxConcurrentDownloads = &maxConcurrentDownloads
 	conf.MaxConcurrentUploads = &maxConcurrentUploads
 }
diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go
@@ -3,10 +3,8 @@ package main
 import (
 	"crypto/tls"
 	"fmt"
-	"io"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"time"
 
@@ -26,7 +24,6 @@ import (
 	swarmrouter "github.com/docker/docker/api/server/router/swarm"
 	systemrouter "github.com/docker/docker/api/server/router/system"
 	"github.com/docker/docker/api/server/router/volume"
-	"github.com/docker/docker/cli"
 	"github.com/docker/docker/cli/debug"
 	cliflags "github.com/docker/docker/cli/flags"
 	"github.com/docker/docker/daemon"
@@ -42,7 +39,6 @@ import (
 	"github.com/docker/docker/pkg/pidfile"
 	"github.com/docker/docker/pkg/plugingetter"
 	"github.com/docker/docker/pkg/signal"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/plugin"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/runconfig"
@@ -66,52 +62,6 @@ func NewDaemonCli() *DaemonCli {
 	return &DaemonCli{}
 }
 
-func migrateKey(config *config.Config) (err error) {
-	// No migration necessary on Windows
-	if runtime.GOOS == "windows" {
-		return nil
-	}
-
-	// Migrate trust key if exists at ~/.docker/key.json and owned by current user
-	oldPath := filepath.Join(cli.ConfigurationDir(), cliflags.DefaultTrustKeyFile)
-	newPath := filepath.Join(getDaemonConfDir(config.Root), cliflags.DefaultTrustKeyFile)
-	if _, statErr := os.Stat(newPath); os.IsNotExist(statErr) && currentUserIsOwner(oldPath) {
-		defer func() {
-			// Ensure old path is removed if no error occurred
-			if err == nil {
-				err = os.Remove(oldPath)
-			} else {
-				logrus.Warnf("Key migration failed, key file not removed at %s", oldPath)
-				os.Remove(newPath)
-			}
-		}()
-
-		if err := system.MkdirAll(getDaemonConfDir(config.Root), os.FileMode(0644)); err != nil {
-			return fmt.Errorf("Unable to create daemon configuration directory: %s", err)
-		}
-
-		newFile, err := os.OpenFile(newPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
-		if err != nil {
-			return fmt.Errorf("error creating key file %q: %s", newPath, err)
-		}
-		defer newFile.Close()
-
-		oldFile, err := os.Open(oldPath)
-		if err != nil {
-			return fmt.Errorf("error opening key file %q: %s", oldPath, err)
-		}
-		defer oldFile.Close()
-
-		if _, err := io.Copy(newFile, oldFile); err != nil {
-			return fmt.Errorf("error copying key: %s", err)
-		}
-
-		logrus.Infof("Migrated key from %s to %s", oldPath, newPath)
-	}
-
-	return nil
-}
-
 func (cli *DaemonCli) start(opts daemonOptions) (err error) {
 	stopc := make(chan bool)
 	defer close(stopc)
@@ -127,12 +77,6 @@ func (cli *DaemonCli) start(opts daemonOptions) (err error) {
 	cli.configFile = &opts.configFile
 	cli.flags = opts.flags
 
-	if opts.common.TrustKey == "" {
-		opts.common.TrustKey = filepath.Join(
-			getDaemonConfDir(cli.Config.Root),
-			cliflags.DefaultTrustKeyFile)
-	}
-
 	if cli.Config.Debug {
 		debug.Enable()
 	}
@@ -241,13 +185,6 @@ func (cli *DaemonCli) start(opts daemonOptions) (err error) {
 		api.Accept(addr, ls...)
 	}
 
-	if err := migrateKey(cli.Config); err != nil {
-		return err
-	}
-
-	// FIXME: why is this down here instead of with the other TrustKey logic above?
-	cli.TrustKeyPath = opts.common.TrustKey
-
 	registryService := registry.NewService(cli.Config.ServiceOptions)
 	containerdRemote, err := libcontainerd.New(cli.getLibcontainerdRoot(), cli.getPlatformRemoteOptions()...)
 	if err != nil {
@@ -419,6 +356,12 @@ func loadDaemonCliConfig(opts daemonOptions) (*config.Config, error) {
 		conf.CommonTLSOptions.KeyFile = opts.common.TLSOptions.KeyFile
 	}
 
+	if conf.TrustKeyPath == "" {
+		conf.TrustKeyPath = filepath.Join(
+			getDaemonConfDir(conf.Root),
+			defaultTrustKeyFile)
+	}
+
 	if flags.Changed("graph") && flags.Changed("data-root") {
 		return nil, fmt.Errorf(`cannot specify both "--graph" and "--data-root" option`)
 	}

diff --git a/cmd/dockerd/daemon_unix.go b/cmd/dockerd/daemon_unix.go
@@ -14,23 +14,11 @@ import (
 	"github.com/docker/docker/cmd/dockerd/hack"
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/libcontainerd"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/libnetwork/portallocator"
 )
 
 const defaultDaemonConfigFile = "/etc/docker/daemon.json"
 
-// currentUserIsOwner checks whether the current user is the owner of the given
-// file.
-func currentUserIsOwner(f string) bool {
-	if fileInfo, err := system.Stat(f); err == nil && fileInfo != nil {
-		if int(fileInfo.UID()) == os.Getuid() {
-			return true
-		}
-	}
-	return false
-}
-
 // setDefaultUmask sets the umask to 0022 to avoid problems
 // caused by custom umask
 func setDefaultUmask() error {

diff --git a/daemon/config/config.go b/daemon/config/config.go
@@ -102,10 +102,15 @@ type CommonConfig struct {
 	RootDeprecated       string                    `json:"graph,omitempty"`
 	Root                 string                    `json:"data-root,omitempty"`
 	SocketGroup          string                    `json:"group,omitempty"`
-	TrustKeyPath         string                    `json:"-"`
 	CorsHeaders          string                    `json:"api-cors-header,omitempty"`
 	EnableCors           bool                      `json:"api-enable-cors,omitempty"`
 
+	// TrustKeyPath is used to generate the daemon ID and for signing schema 1 manifests
+	// when pushing to a registry which does not support schema 2. This field is marked as
+	// deprecated because schema 1 manifests are deprecated in favor of schema 2 and the
+	// daemon ID will use a dedicated identifier not shared with exported signatures.
+	TrustKeyPath string `json:"deprecated-key-path,omitempty"`
+
 	// LiveRestoreEnabled determines whether we should keep containers
 	// alive upon daemon shutdown/start
 	LiveRestoreEnabled bool `json:"live-restore,omitempty"`

diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go
@@ -535,32 +535,6 @@ func (s *DockerDaemonSuite) TestDaemonKeyGeneration(c *check.C) {
 	}
 }
 
-func (s *DockerDaemonSuite) TestDaemonKeyMigration(c *check.C) {
-	// TODO: skip or update for Windows daemon
-	os.Remove("/etc/docker/key.json")
-	k1, err := libtrust.GenerateECP256PrivateKey()
-	if err != nil {
-		c.Fatalf("Error generating private key: %s", err)
-	}
-	if err := os.MkdirAll(filepath.Join(os.Getenv("HOME"), ".docker"), 0755); err != nil {
-		c.Fatalf("Error creating .docker directory: %s", err)
-	}
-	if err := libtrust.SaveKey(filepath.Join(os.Getenv("HOME"), ".docker", "key.json"), k1); err != nil {
-		c.Fatalf("Error saving private key: %s", err)
-	}
-
-	s.d.Start(c)
-	s.d.Stop(c)
-
-	k2, err := libtrust.LoadKeyFile("/etc/docker/key.json")
-	if err != nil {
-		c.Fatalf("Error opening key file")
-	}
-	if k1.KeyID() != k2.KeyID() {
-		c.Fatalf("Key not migrated")
-	}
-}
-
 // GH#11320 - verify that the daemon exits on failure properly
 // Note that this explicitly tests the conflict of {-b,--bridge} and {--bip} options as the means
 // to get a daemon init failure; no other tests for -b/--bip conflict are therefore required