From 1981706196164984ea6b75f0c8b7318c7fd6ca5a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 19 Oct 2022 14:34:45 +0200 Subject: [PATCH 1/5] daemon: remove migrateTrustKeyID() The migration code is in the 22.06 branch, and if we don't migrate the only side-effect is the daemon's ID being regenerated (as a UUID). Signed-off-by: Sebastiaan van Stijn --- daemon/daemon.go | 10 +--- daemon/id.go | 29 --------- integration-cli/docker_cli_daemon_test.go | 73 ----------------------- integration/daemon/daemon_test.go | 51 +++------------- 4 files changed, 9 insertions(+), 154 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 8c7a4b51b8..5aeb8ac2f2 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -931,14 +931,6 @@ 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") - } - // Check if Devices cgroup is mounted, it is hard requirement for container security, // on Linux. // @@ -951,7 +943,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S return nil, errors.New("Devices cgroup isn't mounted") } - d.id, err = loadOrCreateID(idPath) + d.id, err = loadOrCreateID(filepath.Join(config.Root, "engine-id")) if err != nil { return nil, err } diff --git a/daemon/id.go b/daemon/id.go index 9eb73d2292..cf520ca156 100644 --- a/daemon/id.go +++ b/daemon/id.go @@ -4,10 +4,8 @@ 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 @@ -32,30 +30,3 @@ func loadOrCreateID(idPath string) (string, error) { } 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-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index 804e77a37d..f3f8030dee 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -35,7 +35,6 @@ import ( "github.com/docker/docker/opts" testdaemon "github.com/docker/docker/testutil/daemon" units "github.com/docker/go-units" - "github.com/docker/libtrust" "github.com/moby/sys/mount" "golang.org/x/sys/unix" "gotest.tools/v3/assert" @@ -556,24 +555,6 @@ 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) - - k, err := libtrust.LoadKeyFile("/etc/docker/key.json") - if err != nil { - c.Fatalf("Error opening key file") - } - kid := k.KeyID() - // Test Key ID is a valid fingerprint (e.g. QQXN:JY5W:TBXI:MK3X:GX6P:PD5D:F56N:NHCS:LVRZ:JA46:R24J:XEFF) - if len(kid) != 59 { - c.Fatalf("Bad key ID: %s", kid) - } -} - // 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 @@ -1201,60 +1182,6 @@ func (s *DockerDaemonSuite) TestDaemonUnixSockCleanedUp(c *testing.T) { } } -func (s *DockerDaemonSuite) TestDaemonWithWrongkey(c *testing.T) { - type Config struct { - Crv string `json:"crv"` - D string `json:"d"` - Kid string `json:"kid"` - Kty string `json:"kty"` - X string `json:"x"` - Y string `json:"y"` - } - - os.Remove("/etc/docker/key.json") - c.Setenv("DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE", "1") - s.d.Start(c) - s.d.Stop(c) - - config := &Config{} - bytes, err := os.ReadFile("/etc/docker/key.json") - if err != nil { - c.Fatalf("Error reading key.json file: %s", err) - } - - // byte[] to Data-Struct - if err := json.Unmarshal(bytes, &config); err != nil { - c.Fatalf("Error Unmarshal: %s", err) - } - - // replace config.Kid with the fake value - config.Kid = "VSAJ:FUYR:X3H2:B2VZ:KZ6U:CJD5:K7BX:ZXHY:UZXT:P4FT:MJWG:HRJ4" - - // NEW Data-Struct to byte[] - newBytes, err := json.Marshal(&config) - if err != nil { - c.Fatalf("Error Marshal: %s", err) - } - - // write back - if err := os.WriteFile("/etc/docker/key.json", newBytes, 0400); err != nil { - c.Fatalf("Error os.WriteFile: %s", err) - } - - defer os.Remove("/etc/docker/key.json") - - if err := s.d.StartWithError(); err == nil { - c.Fatalf("It should not be successful to start daemon with wrong key: %v", err) - } - - content, err := s.d.ReadLogFile() - assert.Assert(c, err == nil) - - if !strings.Contains(string(content), "Public Key ID does not match") { - c.Fatalf("Missing KeyID message from daemon logs: %s", string(content)) - } -} - func (s *DockerDaemonSuite) TestDaemonRestartKillWait(c *testing.T) { s.d.StartWithBusybox(c) diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go index 42e4155269..2242d3d4f1 100644 --- a/integration/daemon/daemon_test.go +++ b/integration/daemon/daemon_test.go @@ -24,62 +24,27 @@ 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") - - 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) - - d.Start(t, "--config-file", cfg) - info := d.Info(t) - 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") + d.Start(t, "--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) + assert.Check(t, info.ID != "") 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) + idFile := filepath.Join(d.RootDir(), "engine-id") + assert.Check(t, os.Remove(idFile)) + // Using 0644 to allow rootless daemons to read the file (ideally + // we'd chown the file to have the remapped user as owner). + err := os.WriteFile(idFile, []byte(engineID), 0o644) assert.NilError(t, err) - d.Start(t, "--config-file", cfg, "--iptables=false") + d.Start(t, "--iptables=false") info = d.Info(t) assert.Equal(t, info.ID, engineID) d.Stop(t) From 5cdd6ab7cd4af97da2150cd649acfdb19ea8d700 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 19 Oct 2022 15:29:16 +0200 Subject: [PATCH 2/5] daemon/config: remove TrustKeyPath, and local utilities Turned out that the loadOrCreateTrustKey() utility was doing exactly the same as libtrust.LoadOrCreateTrustKey(), so making it a thin wrapped. I kept the tests to verify the behavior, but we could remove them as we only need this for our integration tests. The storage location for the generated key was changed (again as we only need this for some integration tests), so we can remove the TrustKeyPath from the config. Signed-off-by: Sebastiaan van Stijn --- cmd/dockerd/config.go | 3 -- cmd/dockerd/daemon.go | 8 ------ cmd/dockerd/daemon_unix.go | 4 --- cmd/dockerd/daemon_windows.go | 5 ---- cmd/dockerd/docker_windows.go | 6 +--- daemon/config/config.go | 6 ---- daemon/daemon.go | 8 +++--- daemon/trustkey.go | 54 ++--------------------------------- daemon/trustkey_test.go | 22 ++++---------- 9 files changed, 13 insertions(+), 103 deletions(-) diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go index 263efc28aa..c375f4b0d4 100644 --- a/cmd/dockerd/config.go +++ b/cmd/dockerd/config.go @@ -7,9 +7,6 @@ import ( "github.com/spf13/pflag" ) -// defaultTrustKeyFile is the default filename for the trust key -const defaultTrustKeyFile = "key.json" - // installCommonConfigFlags adds flags to the pflag.FlagSet to configure the daemon func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { var ( diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index b825c1a19c..95f89f5b2c 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -414,14 +414,6 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) { conf.CommonTLSOptions = config.CommonTLSOptions{} } - if conf.TrustKeyPath == "" { - daemonConfDir, err := getDaemonConfDir(conf.Root) - if err != nil { - return nil, err - } - conf.TrustKeyPath = filepath.Join(daemonConfDir, defaultTrustKeyFile) - } - if opts.configFile != "" { c, err := config.MergeDaemonConfigurations(conf, flags, opts.configFile) if err != nil { diff --git a/cmd/dockerd/daemon_unix.go b/cmd/dockerd/daemon_unix.go index 0265524be1..8b7fe066f2 100644 --- a/cmd/dockerd/daemon_unix.go +++ b/cmd/dockerd/daemon_unix.go @@ -56,10 +56,6 @@ func setDefaultUmask() error { return nil } -func getDaemonConfDir(_ string) (string, error) { - return getDefaultDaemonConfigDir() -} - func (cli *DaemonCli) getPlatformContainerdDaemonOpts() ([]supervisor.DaemonOpt, error) { opts := []supervisor.DaemonOpt{ // TODO(thaJeztah) change this to use /proc/self/oom_score_adj instead, diff --git a/cmd/dockerd/daemon_windows.go b/cmd/dockerd/daemon_windows.go index e7f41c0861..729b4da03d 100644 --- a/cmd/dockerd/daemon_windows.go +++ b/cmd/dockerd/daemon_windows.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "path/filepath" "time" "github.com/docker/docker/daemon/config" @@ -23,10 +22,6 @@ func setDefaultUmask() error { return nil } -func getDaemonConfDir(root string) (string, error) { - return filepath.Join(root, "config"), nil -} - // preNotifyReady sends a message to the host when the API is active, but before the daemon is func preNotifyReady() { // start the service now to prevent timeouts waiting for daemon to start diff --git a/cmd/dockerd/docker_windows.go b/cmd/dockerd/docker_windows.go index a132bdf285..815c036896 100644 --- a/cmd/dockerd/docker_windows.go +++ b/cmd/dockerd/docker_windows.go @@ -24,11 +24,7 @@ func runDaemon(opts *daemonOptions) error { // Windows specific settings as these are not defaulted. if opts.configFile == "" { - configDir, err := getDaemonConfDir(opts.daemonConfig.Root) - if err != nil { - return err - } - opts.configFile = filepath.Join(configDir, "daemon.json") + opts.configFile = filepath.Join(opts.daemonConfig.Root, "config", "daemon.json") } if runAsService { // If Windows SCM manages the service - no need for PID files diff --git a/daemon/config/config.go b/daemon/config/config.go index a80c1109c4..43fc3bbcc3 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -168,12 +168,6 @@ type CommonConfig struct { // Proxies holds the proxies that are configured for the daemon. Proxies `json:"proxies"` - // 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/daemon/daemon.go b/daemon/daemon.go index 5aeb8ac2f2..eb2377eea5 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1062,13 +1062,13 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S // 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) + // Previously, this was stored in the daemon's config-directory, but + // as pushing V1 is deprecated, and we only need this file during + // our integration tests, just store it within the "trust" directory. + imgSvcConfig.TrustKey, err = loadOrCreateTrustKey(filepath.Join(config.Root, "trust", "key.json")) if err != nil { return nil, err } - if err = os.Mkdir(filepath.Join(config.Root, "trust"), 0o700); err != nil && !errors.Is(err, os.ErrExist) { - return nil, err - } } // containerd is not currently supported with Windows. diff --git a/daemon/trustkey.go b/daemon/trustkey.go index a6b662d7c9..8d1da5aaa7 100644 --- a/daemon/trustkey.go +++ b/daemon/trustkey.go @@ -1,57 +1,9 @@ package daemon // import "github.com/docker/docker/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" -) +import "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 +// otherwise generates a new one. func loadOrCreateTrustKey(trustKeyPath string) (libtrust.PrivateKey, error) { - err := system.MkdirAll(filepath.Dir(trustKeyPath), 0755) - 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 + return libtrust.LoadOrCreateTrustKey(trustKeyPath) } diff --git a/daemon/trustkey_test.go b/daemon/trustkey_test.go index fcc57b12bf..1ea66b65b3 100644 --- a/daemon/trustkey_test.go +++ b/daemon/trustkey_test.go @@ -7,29 +7,20 @@ import ( "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" - "gotest.tools/v3/fs" ) // LoadOrCreateTrustKey func TestLoadOrCreateTrustKeyInvalidKeyFile(t *testing.T) { - tmpKeyFolderPath, err := os.MkdirTemp("", "api-trustkey-test") + tmpKeyFile, err := os.CreateTemp(t.TempDir(), "keyfile") assert.NilError(t, err) - defer os.RemoveAll(tmpKeyFolderPath) - - tmpKeyFile, err := os.CreateTemp(tmpKeyFolderPath, "keyfile") - assert.NilError(t, err) - defer tmpKeyFile.Close() + _ = tmpKeyFile.Close() _, err = loadOrCreateTrustKey(tmpKeyFile.Name()) - assert.Check(t, is.ErrorContains(err, "Error loading key file")) + assert.Check(t, is.ErrorContains(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") + tmpKeyFile := filepath.Join(t.TempDir(), "keyfile") key, err := loadOrCreateTrustKey(tmpKeyFile) assert.NilError(t, err) @@ -40,10 +31,7 @@ func TestLoadOrCreateTrustKeyCreateKeyWhenFileDoesNotExist(t *testing.T) { } func TestLoadOrCreateTrustKeyCreateKeyWhenDirectoryDoesNotExist(t *testing.T) { - tmpKeyFolderPath := fs.NewDir(t, "api-trustkey-test") - defer tmpKeyFolderPath.Remove() - tmpKeyFile := tmpKeyFolderPath.Join("folder/hierarchy/keyfile") - + tmpKeyFile := filepath.Join(t.TempDir(), "folder/hierarchy/keyfile") key, err := loadOrCreateTrustKey(tmpKeyFile) assert.NilError(t, err) assert.Check(t, key != nil) From e854b2a4592e07486be9111ca7715c546091522e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Nov 2022 15:16:16 +0100 Subject: [PATCH 3/5] distribution: use ad-hoc trustkey for tests Signed-off-by: Sebastiaan van Stijn --- distribution/push_v2.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 910123250c..5c540c0c5b 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -24,6 +24,7 @@ import ( "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/registry" + "github.com/docker/libtrust" "github.com/opencontainers/go-digest" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -187,7 +188,7 @@ func (p *pusher) pushTag(ctx context.Context, ref reference.NamedTagged, id dige putOptions := []distribution.ManifestServiceOption{distribution.WithTag(ref.Tag())} if _, err = manSvc.Put(ctx, manifest, putOptions...); err != nil { - if runtime.GOOS == "windows" || p.config.TrustKey == nil || p.config.RequireSchema2 { + if runtime.GOOS == "windows" || p.config.RequireSchema2 { logrus.Warnf("failed to upload schema2 manifest: %v", err) return err } @@ -211,7 +212,11 @@ func (p *pusher) pushTag(ctx context.Context, ref reference.NamedTagged, id dige if err != nil { return err } - builder = schema1.NewConfigManifestBuilder(p.repo.Blobs(ctx), p.config.TrustKey, manifestRef, imgConfig) + pk, err := libtrust.GenerateECP256PrivateKey() + if err != nil { + return errors.Wrap(err, "unexpected error generating private key") + } + builder = schema1.NewConfigManifestBuilder(p.repo.Blobs(ctx), pk, manifestRef, imgConfig) manifest, err = manifestFromBuilder(ctx, builder, descriptors) if err != nil { return err From 8feeaecb84cabb415a0d68db01f2bb425527f67c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Nov 2022 15:25:00 +0100 Subject: [PATCH 4/5] use ad-hoc libtrust key This is only used for tests, and the key is not verified anymore, so instead of creating a key and storing it, we can just use an ad-hoc one. Signed-off-by: Sebastiaan van Stijn --- cmd/dockerd/options.go | 2 -- daemon/daemon.go | 13 -------- daemon/images/image_push.go | 1 - daemon/images/service.go | 4 --- daemon/trustkey.go | 9 ------ daemon/trustkey_test.go | 59 ------------------------------------- distribution/config.go | 4 --- 7 files changed, 92 deletions(-) delete mode 100644 daemon/trustkey.go delete mode 100644 daemon/trustkey_test.go diff --git a/cmd/dockerd/options.go b/cmd/dockerd/options.go index c4649ded51..a9c336c357 100644 --- a/cmd/dockerd/options.go +++ b/cmd/dockerd/options.go @@ -65,8 +65,6 @@ func (o *daemonOptions) installFlags(flags *pflag.FlagSet) { flags.BoolVar(&o.TLS, FlagTLS, DefaultTLSValue, "Use TLS; implied by --tlsverify") flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify || DefaultTLSValue, "Use TLS and verify the remote") - // TODO use flag flags.String("identity"}, "i", "", "Path to libtrust key file") - o.TLSOptions = &tlsconfig.Options{} tlsOptions := o.TLSOptions flags.StringVar(&tlsOptions.CAFile, "tlscacert", filepath.Join(dockerCertPath, DefaultCaFile), "Trust certs signed only by this CA") diff --git a/daemon/daemon.go b/daemon/daemon.go index eb2377eea5..6c7d024595 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1058,19 +1058,6 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S 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") != "" { - // Previously, this was stored in the daemon's config-directory, but - // as pushing V1 is deprecated, and we only need this file during - // our integration tests, just store it within the "trust" directory. - imgSvcConfig.TrustKey, err = loadOrCreateTrustKey(filepath.Join(config.Root, "trust", "key.json")) - if 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/daemon/images/image_push.go b/daemon/images/image_push.go index 1bc1867291..c461133c30 100644 --- a/daemon/images/image_push.go +++ b/daemon/images/image_push.go @@ -54,7 +54,6 @@ func (i *ImageService) PushImage(ctx context.Context, image, tag string, metaHea }, ConfigMediaType: schema2.MediaTypeImageConfig, LayerStores: distribution.NewLayerProvidersFromStore(i.layerStore), - TrustKey: i.trustKey, UploadManager: i.uploadManager, } diff --git a/daemon/images/service.go b/daemon/images/service.go index 804492d8b8..2443321272 100644 --- a/daemon/images/service.go +++ b/daemon/images/service.go @@ -16,7 +16,6 @@ import ( "github.com/docker/docker/layer" dockerreference "github.com/docker/docker/reference" "github.com/docker/docker/registry" - "github.com/docker/libtrust" "github.com/opencontainers/go-digest" "github.com/pkg/errors" "golang.org/x/sync/singleflight" @@ -44,7 +43,6 @@ type ImageServiceConfig struct { MaxDownloadAttempts int ReferenceStore dockerreference.Store RegistryService registry.Service - TrustKey libtrust.PrivateKey ContentStore content.Store Leases leases.Manager ContentNamespace string @@ -61,7 +59,6 @@ func NewImageService(config ImageServiceConfig) *ImageService { layerStore: config.LayerStore, referenceStore: config.ReferenceStore, registryService: config.RegistryService, - trustKey: config.TrustKey, uploadManager: xfer.NewLayerUploadManager(config.MaxConcurrentUploads), leases: config.Leases, content: config.ContentStore, @@ -80,7 +77,6 @@ type ImageService struct { pruneRunning int32 referenceStore dockerreference.Store registryService registry.Service - trustKey libtrust.PrivateKey uploadManager *xfer.LayerUploadManager leases leases.Manager content content.Store diff --git a/daemon/trustkey.go b/daemon/trustkey.go deleted file mode 100644 index 8d1da5aaa7..0000000000 --- a/daemon/trustkey.go +++ /dev/null @@ -1,9 +0,0 @@ -package daemon // import "github.com/docker/docker/daemon" - -import "github.com/docker/libtrust" - -// LoadOrCreateTrustKey attempts to load the libtrust key at the given path, -// otherwise generates a new one. -func loadOrCreateTrustKey(trustKeyPath string) (libtrust.PrivateKey, error) { - return libtrust.LoadOrCreateTrustKey(trustKeyPath) -} diff --git a/daemon/trustkey_test.go b/daemon/trustkey_test.go deleted file mode 100644 index 1ea66b65b3..0000000000 --- a/daemon/trustkey_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package daemon // import "github.com/docker/docker/daemon" - -import ( - "os" - "path/filepath" - "testing" - - "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" -) - -// LoadOrCreateTrustKey -func TestLoadOrCreateTrustKeyInvalidKeyFile(t *testing.T) { - tmpKeyFile, err := os.CreateTemp(t.TempDir(), "keyfile") - assert.NilError(t, err) - _ = tmpKeyFile.Close() - - _, err = loadOrCreateTrustKey(tmpKeyFile.Name()) - assert.Check(t, is.ErrorContains(err, "error loading key file")) -} - -func TestLoadOrCreateTrustKeyCreateKeyWhenFileDoesNotExist(t *testing.T) { - tmpKeyFile := filepath.Join(t.TempDir(), "keyfile") - - key, err := loadOrCreateTrustKey(tmpKeyFile) - assert.NilError(t, err) - assert.Check(t, key != nil) - - _, err = os.Stat(tmpKeyFile) - assert.NilError(t, err, "key file doesn't exist") -} - -func TestLoadOrCreateTrustKeyCreateKeyWhenDirectoryDoesNotExist(t *testing.T) { - tmpKeyFile := filepath.Join(t.TempDir(), "folder/hierarchy/keyfile") - key, err := loadOrCreateTrustKey(tmpKeyFile) - assert.NilError(t, err) - assert.Check(t, key != nil) - - _, err = os.Stat(tmpKeyFile) - assert.NilError(t, err, "key file doesn't exist") -} - -func TestLoadOrCreateTrustKeyCreateKeyNoPath(t *testing.T) { - defer os.Remove("keyfile") - key, err := loadOrCreateTrustKey("keyfile") - assert.NilError(t, err) - assert.Check(t, key != nil) - - _, err = os.Stat("keyfile") - assert.NilError(t, err, "key file doesn't exist") -} - -func TestLoadOrCreateTrustKeyLoadValidKey(t *testing.T) { - tmpKeyFile := filepath.Join("testdata", "keyfile") - key, err := loadOrCreateTrustKey(tmpKeyFile) - assert.NilError(t, err) - expected := "AWX2:I27X:WQFX:IOMK:CNAK:O7PW:VYNB:ZLKC:CVAE:YJP2:SI4A:XXAY" - assert.Check(t, is.Contains(key.String(), expected)) -} diff --git a/distribution/config.go b/distribution/config.go index a00392199f..86c7ee1309 100644 --- a/distribution/config.go +++ b/distribution/config.go @@ -17,7 +17,6 @@ import ( "github.com/docker/docker/pkg/system" refstore "github.com/docker/docker/reference" registrypkg "github.com/docker/docker/registry" - "github.com/docker/libtrust" "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -74,9 +73,6 @@ type ImagePushConfig struct { ConfigMediaType string // LayerStores manages layers. LayerStores PushLayerProvider - // TrustKey is the private key for legacy signatures. This is typically - // an ephemeral key, since these signatures are no longer verified. - TrustKey libtrust.PrivateKey // UploadManager dispatches uploads. UploadManager *xfer.LayerUploadManager } From 85fddc0081339ea12e6c7bd71f702aa737f589b2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Nov 2022 15:32:02 +0100 Subject: [PATCH 5/5] distribution: remove unused RequireSchema2 It's never set, so we can remove it. Signed-off-by: Sebastiaan van Stijn --- distribution/config.go | 2 -- distribution/pull_v2.go | 4 ---- distribution/push_v2.go | 2 +- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/distribution/config.go b/distribution/config.go index 86c7ee1309..9dd6a5431d 100644 --- a/distribution/config.go +++ b/distribution/config.go @@ -46,8 +46,6 @@ type Config struct { // ReferenceStore manages tags. This value is optional, when excluded // content will not be tagged. ReferenceStore refstore.Store - // RequireSchema2 ensures that only schema2 manifests are used. - RequireSchema2 bool } // ImagePullConfig stores pull configuration. diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index 899ef09edb..0bef24d5f4 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -438,10 +438,6 @@ func (p *puller) pullTag(ctx context.Context, ref reference.Named, platform *spe switch v := manifest.(type) { case *schema1.SignedManifest: - if p.config.RequireSchema2 { - return false, fmt.Errorf("invalid manifest: not schema2") - } - // give registries time to upgrade to schema2 and only warn if we know a registry has been upgraded long time ago // TODO: condition to be removed if reference.Domain(ref) == "docker.io" { diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 5c540c0c5b..4332c44480 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -188,7 +188,7 @@ func (p *pusher) pushTag(ctx context.Context, ref reference.NamedTagged, id dige putOptions := []distribution.ManifestServiceOption{distribution.WithTag(ref.Tag())} if _, err = manSvc.Put(ctx, manifest, putOptions...); err != nil { - if runtime.GOOS == "windows" || p.config.RequireSchema2 { + if runtime.GOOS == "windows" { logrus.Warnf("failed to upload schema2 manifest: %v", err) return err }