From bb1208639b4cf92522d56b8cba2a4c30033a2144 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 3 May 2022 13:52:17 +0200 Subject: [PATCH 1/2] daemon: separate daemon ID from trust-key This change is in preparation of deprecating support for old manifests. Currently the daemon's ID is based on the trust-key ID, which will be removed once we fully deprecate support for old manifests (the trust key is currently only used in tests). This patch: - looks if a trust-key is present; if so, it migrates the trust-key ID to the new "engine-id" file within the daemon's root. - if no trust-key is present (so in case it's a "fresh" install), we generate a UUID instead and use that as ID. The migration is to prevent engines from getting a new ID on upgrades; while we don't provide any guarantees on the engine's ID, users may expect the ID to be "stable" (not change) between upgrades. A test has been added, which can be ran with; make DOCKER_GRAPHDRIVER=vfs TEST_FILTER='TestConfigDaemonID' test-integration Signed-off-by: Sebastiaan van Stijn --- daemon/daemon.go | 13 ++++++- daemon/id.go | 61 +++++++++++++++++++++++++++++++ integration/daemon/daemon_test.go | 52 +++++++++++++++++++++++--- 3 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 daemon/id.go diff --git a/daemon/daemon.go b/daemon/daemon.go index 5d9e57cca5..5dc4b0cd8a 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -977,6 +977,14 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S return nil, err } + // Try to preserve the daemon ID (which is the trust-key's ID) when upgrading + // an existing installation; this is a "best-effort". + idPath := filepath.Join(config.Root, "engine-id") + err = migrateTrustKeyID(config.TrustKeyPath, idPath) + if err != nil { + logrus.WithError(err).Warnf("unable to migrate engine ID; a new engine ID will be generated") + } + trustKey, err := loadOrCreateTrustKey(config.TrustKeyPath) if err != nil { return nil, err @@ -1019,7 +1027,10 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S return nil, errors.New("Devices cgroup isn't mounted") } - d.id = trustKey.PublicKey().KeyID() + d.id, err = loadOrCreateID(idPath) + if err != nil { + return nil, err + } d.repository = daemonRepo d.containers = container.NewMemoryStore() if d.containersReplica, err = container.NewViewDB(); err != nil { diff --git a/daemon/id.go b/daemon/id.go new file mode 100644 index 0000000000..9eb73d2292 --- /dev/null +++ b/daemon/id.go @@ -0,0 +1,61 @@ +package daemon // import "github.com/docker/docker/daemon" + +import ( + "os" + + "github.com/docker/docker/pkg/ioutils" + "github.com/docker/libtrust" + "github.com/google/uuid" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +// loadOrCreateID loads the engine's ID from idPath, or generates a new ID +// if it doesn't exist. It returns the ID, and any error that occurred when +// saving the file. +// +// Note that this function expects the daemon's root directory to already have +// been created with the right permissions and ownership (usually this would +// be done by daemon.CreateDaemonRoot(). +func loadOrCreateID(idPath string) (string, error) { + var id string + idb, err := os.ReadFile(idPath) + if os.IsNotExist(err) { + id = uuid.New().String() + if err := ioutils.AtomicWriteFile(idPath, []byte(id), os.FileMode(0600)); err != nil { + return "", errors.Wrap(err, "error saving ID file") + } + } else if err != nil { + return "", errors.Wrapf(err, "error loading ID file %s", idPath) + } else { + id = string(idb) + } + return id, nil +} + +// migrateTrustKeyID migrates the daemon ID of existing installations. It returns +// an error when a trust-key was found, but we failed to read it, or failed to +// complete the migration. +// +// We migrate the ID so that engines don't get a new ID generated on upgrades, +// which may be unexpected (and users may be using the ID for various purposes). +func migrateTrustKeyID(deprecatedTrustKeyPath, idPath string) error { + if _, err := os.Stat(idPath); err == nil { + // engine ID file already exists; no migration needed + return nil + } + trustKey, err := libtrust.LoadKeyFile(deprecatedTrustKeyPath) + if err != nil { + if err == libtrust.ErrKeyFileDoesNotExist { + // no existing trust-key found; no migration needed + return nil + } + return err + } + id := trustKey.PublicKey().KeyID() + if err := ioutils.AtomicWriteFile(idPath, []byte(id), os.FileMode(0600)); err != nil { + return errors.Wrap(err, "error saving ID file") + } + logrus.Info("successfully migrated engine ID") + return nil +} diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go index 346857d9ae..e4649a7ab3 100644 --- a/integration/daemon/daemon_test.go +++ b/integration/daemon/daemon_test.go @@ -22,6 +22,11 @@ import ( "gotest.tools/v3/skip" ) +const ( + libtrustKey = `{"crv":"P-256","d":"dm28PH4Z4EbyUN8L0bPonAciAQa1QJmmyYd876mnypY","kid":"WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB","kty":"EC","x":"Mh5-JINSjaa_EZdXDttri255Z5fbCEOTQIZjAcScFTk","y":"eUyuAjfxevb07hCCpvi4Zi334Dy4GDWQvEToGEX4exQ"}` + libtrustKeyID = "WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB" +) + func TestConfigDaemonLibtrustID(t *testing.T) { skip.If(t, runtime.GOOS == "windows") @@ -29,16 +34,53 @@ func TestConfigDaemonLibtrustID(t *testing.T) { defer d.Stop(t) trustKey := filepath.Join(d.RootDir(), "key.json") - err := os.WriteFile(trustKey, []byte(`{"crv":"P-256","d":"dm28PH4Z4EbyUN8L0bPonAciAQa1QJmmyYd876mnypY","kid":"WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB","kty":"EC","x":"Mh5-JINSjaa_EZdXDttri255Z5fbCEOTQIZjAcScFTk","y":"eUyuAjfxevb07hCCpvi4Zi334Dy4GDWQvEToGEX4exQ"}`), 0644) + err := os.WriteFile(trustKey, []byte(libtrustKey), 0644) assert.NilError(t, err) - config := filepath.Join(d.RootDir(), "daemon.json") - err = os.WriteFile(config, []byte(`{"deprecated-key-path": "`+trustKey+`"}`), 0644) + cfg := filepath.Join(d.RootDir(), "daemon.json") + err = os.WriteFile(cfg, []byte(`{"deprecated-key-path": "`+trustKey+`"}`), 0644) assert.NilError(t, err) - d.Start(t, "--config-file", config) + d.Start(t, "--config-file", cfg) info := d.Info(t) - assert.Equal(t, info.ID, "WTJ3:YSIP:CE2E:G6KJ:PSBD:YX2Y:WEYD:M64G:NU2V:XPZV:H2CR:VLUB") + assert.Equal(t, info.ID, libtrustKeyID) +} + +func TestConfigDaemonID(t *testing.T) { + skip.If(t, runtime.GOOS == "windows") + + d := daemon.New(t) + defer d.Stop(t) + + trustKey := filepath.Join(d.RootDir(), "key.json") + err := os.WriteFile(trustKey, []byte(libtrustKey), 0644) + assert.NilError(t, err) + + cfg := filepath.Join(d.RootDir(), "daemon.json") + err = os.WriteFile(cfg, []byte(`{"deprecated-key-path": "`+trustKey+`"}`), 0644) + assert.NilError(t, err) + + // Verify that on an installation with a trust-key present, the ID matches + // the trust-key ID, and that the ID has been migrated to the engine-id file. + d.Start(t, "--config-file", cfg, "--iptables=false") + info := d.Info(t) + assert.Equal(t, info.ID, libtrustKeyID) + + idFile := filepath.Join(d.RootDir(), "engine-id") + id, err := os.ReadFile(idFile) + assert.NilError(t, err) + assert.Equal(t, string(id), libtrustKeyID) + d.Stop(t) + + // Verify that (if present) the engine-id file takes precedence + const engineID = "this-is-the-engine-id" + err = os.WriteFile(idFile, []byte(engineID), 0600) + assert.NilError(t, err) + + d.Start(t, "--config-file", cfg, "--iptables=false") + info = d.Info(t) + assert.Equal(t, info.ID, engineID) + d.Stop(t) } func TestDaemonConfigValidation(t *testing.T) { From 070da63310cabb855bc0b34ce0fde229949ff303 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 3 May 2022 23:10:14 +0200 Subject: [PATCH 2/2] daemon: only create trust-key if DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE is set The libtrust trust-key is only used for pushing legacy image manifests; pushing these images has been deprecated, and we only need to be able to push them in our CI. This patch disables generating the trust-key (and related paths) unless the DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE env-var is set (which we do in our CI). Signed-off-by: Sebastiaan van Stijn --- daemon/daemon.go | 25 ++++++++++++----------- integration-cli/docker_cli_daemon_test.go | 2 ++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 5dc4b0cd8a..dd97426035 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -985,17 +985,6 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S logrus.WithError(err).Warnf("unable to migrate engine ID; a new engine ID will be generated") } - trustKey, err := loadOrCreateTrustKey(config.TrustKeyPath) - if err != nil { - return nil, err - } - - trustDir := filepath.Join(config.Root, "trust") - - if err := system.MkdirAll(trustDir, 0700); err != nil { - return nil, err - } - // We have a single tag/reference store for the daemon globally. However, it's // stored under the graphdriver. On host platforms which only support a single // container OS, but multiple selectable graphdrivers, this means depending on which @@ -1057,10 +1046,22 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S MaxDownloadAttempts: *config.MaxDownloadAttempts, ReferenceStore: rs, RegistryService: registryService, - TrustKey: trustKey, ContentNamespace: config.ContainerdNamespace, } + // This is a temporary environment variables used in CI to allow pushing + // manifest v2 schema 1 images to test-registries used for testing *pulling* + // these images. + if os.Getenv("DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE") != "" { + imgSvcConfig.TrustKey, err = loadOrCreateTrustKey(config.TrustKeyPath) + if err != nil { + return nil, err + } + if err = system.MkdirAll(filepath.Join(config.Root, "trust"), 0700); err != nil { + return nil, err + } + } + // containerd is not currently supported with Windows. // So sometimes d.containerdCli will be nil // In that case we'll create a local content store... but otherwise we'll use containerd diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index 94f07f56b4..96c0d0a906 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -559,6 +559,7 @@ func (s *DockerDaemonSuite) TestDaemonAllocatesListeningPort(c *testing.T) { func (s *DockerDaemonSuite) TestDaemonKeyGeneration(c *testing.T) { // TODO: skip or update for Windows daemon os.Remove("/etc/docker/key.json") + c.Setenv("DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE", "1") s.d.Start(c) s.d.Stop(c) @@ -1212,6 +1213,7 @@ func (s *DockerDaemonSuite) TestDaemonWithWrongkey(c *testing.T) { } os.Remove("/etc/docker/key.json") + c.Setenv("DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE", "1") s.d.Start(c) s.d.Stop(c)