Skip to content

Commit e428c82

Browse files
dmcgowandmcgowan
committed
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 #32135 Signed-off-by: Derek McGowan <[email protected]>
1 parent 7ca8679 commit e428c82

6 files changed

Lines changed: 21 additions & 105 deletions

File tree

‎cli/flags/common.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ import (
1313
)
1414

1515
const (
16-
// DefaultTrustKeyFile is the default filename for the trust key
17-
DefaultTrustKeyFile = "key.json"
1816
// DefaultCaFile is the default filename for the CA pem file
1917
DefaultCaFile = "ca.pem"
2018
// DefaultKeyFile is the default filename for the key pem file
@@ -38,7 +36,6 @@ type CommonOptions struct {
3836
TLS bool
3937
TLSVerify bool
4038
TLSOptions *tlsconfig.Options
41-
TrustKey string
4239
}
4340

4441
// NewCommonOptions returns a new CommonOptions

‎cmd/dockerd/config.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
const (
1010
// defaultShutdownTimeout is the default shutdown timeout for the daemon
1111
defaultShutdownTimeout = 15
12+
// defaultTrustKeyFile is the default filename for the trust key
13+
defaultTrustKeyFile = "key.json"
1214
)
1315

1416
// installCommonConfigFlags adds flags to the pflag.FlagSet to configure the daemon
@@ -53,6 +55,13 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) {
5355

5456
flags.StringVar(&conf.MetricsAddress, "metrics-addr", "", "Set default address and port to serve the metrics api on")
5557

58+
// "--deprecated-key-path" is to allow configuration of the key used
59+
// for the daemon ID and the deprecated image signing. It was never
60+
// exposed as a command line option but is added here to allow
61+
// overriding the default path in configuration.
62+
flags.Var(opts.NewQuotedString(&conf.TrustKeyPath), "deprecated-key-path", "Path to key file for ID and image signing")
63+
flags.MarkHidden("deprecated-key-path")
64+
5665
conf.MaxConcurrentDownloads = &maxConcurrentDownloads
5766
conf.MaxConcurrentUploads = &maxConcurrentUploads
5867
}

‎cmd/dockerd/daemon.go‎

Lines changed: 6 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@ package main
33
import (
44
"crypto/tls"
55
"fmt"
6-
"io"
76
"os"
87
"path/filepath"
9-
"runtime"
108
"strings"
119
"time"
1210

@@ -26,7 +24,6 @@ import (
2624
swarmrouter "github.com/docker/docker/api/server/router/swarm"
2725
systemrouter "github.com/docker/docker/api/server/router/system"
2826
"github.com/docker/docker/api/server/router/volume"
29-
"github.com/docker/docker/cli"
3027
"github.com/docker/docker/cli/debug"
3128
cliflags "github.com/docker/docker/cli/flags"
3229
"github.com/docker/docker/daemon"
@@ -42,7 +39,6 @@ import (
4239
"github.com/docker/docker/pkg/pidfile"
4340
"github.com/docker/docker/pkg/plugingetter"
4441
"github.com/docker/docker/pkg/signal"
45-
"github.com/docker/docker/pkg/system"
4642
"github.com/docker/docker/plugin"
4743
"github.com/docker/docker/registry"
4844
"github.com/docker/docker/runconfig"
@@ -66,52 +62,6 @@ func NewDaemonCli() *DaemonCli {
6662
return &DaemonCli{}
6763
}
6864

69-
func migrateKey(config *config.Config) (err error) {
70-
// No migration necessary on Windows
71-
if runtime.GOOS == "windows" {
72-
return nil
73-
}
74-
75-
// Migrate trust key if exists at ~/.docker/key.json and owned by current user
76-
oldPath := filepath.Join(cli.ConfigurationDir(), cliflags.DefaultTrustKeyFile)
77-
newPath := filepath.Join(getDaemonConfDir(config.Root), cliflags.DefaultTrustKeyFile)
78-
if _, statErr := os.Stat(newPath); os.IsNotExist(statErr) && currentUserIsOwner(oldPath) {
79-
defer func() {
80-
// Ensure old path is removed if no error occurred
81-
if err == nil {
82-
err = os.Remove(oldPath)
83-
} else {
84-
logrus.Warnf("Key migration failed, key file not removed at %s", oldPath)
85-
os.Remove(newPath)
86-
}
87-
}()
88-
89-
if err := system.MkdirAll(getDaemonConfDir(config.Root), os.FileMode(0644)); err != nil {
90-
return fmt.Errorf("Unable to create daemon configuration directory: %s", err)
91-
}
92-
93-
newFile, err := os.OpenFile(newPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
94-
if err != nil {
95-
return fmt.Errorf("error creating key file %q: %s", newPath, err)
96-
}
97-
defer newFile.Close()
98-
99-
oldFile, err := os.Open(oldPath)
100-
if err != nil {
101-
return fmt.Errorf("error opening key file %q: %s", oldPath, err)
102-
}
103-
defer oldFile.Close()
104-
105-
if _, err := io.Copy(newFile, oldFile); err != nil {
106-
return fmt.Errorf("error copying key: %s", err)
107-
}
108-
109-
logrus.Infof("Migrated key from %s to %s", oldPath, newPath)
110-
}
111-
112-
return nil
113-
}
114-
11565
func (cli *DaemonCli) start(opts daemonOptions) (err error) {
11666
stopc := make(chan bool)
11767
defer close(stopc)
@@ -127,12 +77,6 @@ func (cli *DaemonCli) start(opts daemonOptions) (err error) {
12777
cli.configFile = &opts.configFile
12878
cli.flags = opts.flags
12979

130-
if opts.common.TrustKey == "" {
131-
opts.common.TrustKey = filepath.Join(
132-
getDaemonConfDir(cli.Config.Root),
133-
cliflags.DefaultTrustKeyFile)
134-
}
135-
13680
if cli.Config.Debug {
13781
debug.Enable()
13882
}
@@ -241,13 +185,6 @@ func (cli *DaemonCli) start(opts daemonOptions) (err error) {
241185
api.Accept(addr, ls...)
242186
}
243187

244-
if err := migrateKey(cli.Config); err != nil {
245-
return err
246-
}
247-
248-
// FIXME: why is this down here instead of with the other TrustKey logic above?
249-
cli.TrustKeyPath = opts.common.TrustKey
250-
251188
registryService := registry.NewService(cli.Config.ServiceOptions)
252189
containerdRemote, err := libcontainerd.New(cli.getLibcontainerdRoot(), cli.getPlatformRemoteOptions()...)
253190
if err != nil {
@@ -419,6 +356,12 @@ func loadDaemonCliConfig(opts daemonOptions) (*config.Config, error) {
419356
conf.CommonTLSOptions.KeyFile = opts.common.TLSOptions.KeyFile
420357
}
421358

359+
if conf.TrustKeyPath == "" {
360+
conf.TrustKeyPath = filepath.Join(
361+
getDaemonConfDir(conf.Root),
362+
defaultTrustKeyFile)
363+
}
364+
422365
if flags.Changed("graph") && flags.Changed("data-root") {
423366
return nil, fmt.Errorf(`cannot specify both "--graph" and "--data-root" option`)
424367
}

‎cmd/dockerd/daemon_unix.go‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,11 @@ import (
1414
"github.com/docker/docker/cmd/dockerd/hack"
1515
"github.com/docker/docker/daemon"
1616
"github.com/docker/docker/libcontainerd"
17-
"github.com/docker/docker/pkg/system"
1817
"github.com/docker/libnetwork/portallocator"
1918
)
2019

2120
const defaultDaemonConfigFile = "/etc/docker/daemon.json"
2221

23-
// currentUserIsOwner checks whether the current user is the owner of the given
24-
// file.
25-
func currentUserIsOwner(f string) bool {
26-
if fileInfo, err := system.Stat(f); err == nil && fileInfo != nil {
27-
if int(fileInfo.UID()) == os.Getuid() {
28-
return true
29-
}
30-
}
31-
return false
32-
}
33-
3422
// setDefaultUmask sets the umask to 0022 to avoid problems
3523
// caused by custom umask
3624
func setDefaultUmask() error {

‎daemon/config/config.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,15 @@ type CommonConfig struct {
102102
RootDeprecated string `json:"graph,omitempty"`
103103
Root string `json:"data-root,omitempty"`
104104
SocketGroup string `json:"group,omitempty"`
105-
TrustKeyPath string `json:"-"`
106105
CorsHeaders string `json:"api-cors-header,omitempty"`
107106
EnableCors bool `json:"api-enable-cors,omitempty"`
108107

108+
// TrustKeyPath is used to generate the daemon ID and for signing schema 1 manifests
109+
// when pushing to a registry which does not support schema 2. This field is marked as
110+
// deprecated because schema 1 manifests are deprecated in favor of schema 2 and the
111+
// daemon ID will use a dedicated identifier not shared with exported signatures.
112+
TrustKeyPath string `json:"deprecated-key-path,omitempty"`
113+
109114
// LiveRestoreEnabled determines whether we should keep containers
110115
// alive upon daemon shutdown/start
111116
LiveRestoreEnabled bool `json:"live-restore,omitempty"`

‎integration-cli/docker_cli_daemon_test.go‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -535,32 +535,6 @@ func (s *DockerDaemonSuite) TestDaemonKeyGeneration(c *check.C) {
535535
}
536536
}
537537

538-
func (s *DockerDaemonSuite) TestDaemonKeyMigration(c *check.C) {
539-
// TODO: skip or update for Windows daemon
540-
os.Remove("/etc/docker/key.json")
541-
k1, err := libtrust.GenerateECP256PrivateKey()
542-
if err != nil {
543-
c.Fatalf("Error generating private key: %s", err)
544-
}
545-
if err := os.MkdirAll(filepath.Join(os.Getenv("HOME"), ".docker"), 0755); err != nil {
546-
c.Fatalf("Error creating .docker directory: %s", err)
547-
}
548-
if err := libtrust.SaveKey(filepath.Join(os.Getenv("HOME"), ".docker", "key.json"), k1); err != nil {
549-
c.Fatalf("Error saving private key: %s", err)
550-
}
551-
552-
s.d.Start(c)
553-
s.d.Stop(c)
554-
555-
k2, err := libtrust.LoadKeyFile("/etc/docker/key.json")
556-
if err != nil {
557-
c.Fatalf("Error opening key file")
558-
}
559-
if k1.KeyID() != k2.KeyID() {
560-
c.Fatalf("Key not migrated")
561-
}
562-
}
563-
564538
// GH#11320 - verify that the daemon exits on failure properly
565539
// Note that this explicitly tests the conflict of {-b,--bridge} and {--bip} options as the means
566540
// to get a daemon init failure; no other tests for -b/--bip conflict are therefore required

0 commit comments

Comments
 (0)