From f695e98cb7cbb39eb7e5bd0a268ae1cd2e594fc1 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 18 Jun 2019 00:23:44 +0000 Subject: [PATCH] Revert "Remove the rest of v1 manifest support" This reverts commit 98fc09128b3f8221680bd22c2de5b893bd3d7ef8 in order to keep registry v2 schema1 handling and libtrust-key-based engine ID. Because registry v2 schema1 was not officially deprecated and registries are still relying on it, this patch puts its logic back. However, registry v1 relics are not added back since v1 logic has been removed a while ago. This also fixes an engine upgrade issue in a swarm cluster. It was relying on the Engine ID to be the same upon upgrade, but the mentioned commit modified the logic to use UUID and from a different file. Since the libtrust key is always needed to support v2 schema1 pushes, that the old engine ID is based on the libtrust key, and that the engine ID needs to be conserved across upgrades, adding a UUID-based engine ID logic seems to add more complexity than it solves the problems. Hence reverting the engine ID changes as well. Signed-off-by: Tibor Vass --- cmd/dockerd/config.go | 15 +++++ cmd/dockerd/daemon.go | 19 ++++++ cmd/dockerd/daemon_unix.go | 4 ++ cmd/dockerd/daemon_windows.go | 5 ++ daemon/config/config.go | 10 ++++ daemon/daemon.go | 5 +- daemon/images/image_push.go | 1 + daemon/images/service.go | 4 ++ daemon/trustkey.go | 57 ++++++++++++++++++ daemon/trustkey_test.go | 71 +++++++++++++++++++++++ daemon/uuid.go | 35 ----------- distribution/config.go | 4 ++ distribution/push_v2.go | 23 +++++++- integration-cli/docker_cli_daemon_test.go | 23 ++++---- internal/test/fixtures/plugin/plugin.go | 2 +- registry/config.go | 6 ++ registry/service.go | 17 +++++- 17 files changed, 247 insertions(+), 54 deletions(-) create mode 100644 daemon/trustkey.go create mode 100644 daemon/trustkey_test.go delete mode 100644 daemon/uuid.go diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go index 317315414f..bf64108ef4 100644 --- a/cmd/dockerd/config.go +++ b/cmd/dockerd/config.go @@ -12,6 +12,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 @@ -81,6 +83,13 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { flags.IntVar(&conf.NetworkControlPlaneMTU, "network-control-plane-mtu", config.DefaultNetworkMtu, "Network Control plane MTU") + // "--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 return nil @@ -94,4 +103,10 @@ func installRegistryServiceFlags(options *registry.ServiceOptions, flags *pflag. flags.Var(ana, "allow-nondistributable-artifacts", "Allow push of nondistributable artifacts to registry") flags.Var(mirrors, "registry-mirror", "Preferred Docker registry mirror") flags.Var(insecureRegistries, "insecure-registry", "Enable insecure registry communication") + + if runtime.GOOS != "windows" { + // TODO: Remove this flag after 3 release cycles (18.03) + flags.BoolVar(&options.V2Only, "disable-legacy-registry", true, "Disable contacting legacy registries") + flags.MarkHidden("disable-legacy-registry") + } } diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 4a2962409a..9347946d28 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -429,6 +429,14 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) { conf.CommonTLSOptions.KeyFile = opts.TLSOptions.KeyFile } + if conf.TrustKeyPath == "" { + daemonConfDir, err := getDaemonConfDir(conf.Root) + if err != nil { + return nil, err + } + conf.TrustKeyPath = filepath.Join(daemonConfDir, defaultTrustKeyFile) + } + if flags.Changed("graph") && flags.Changed("data-root") { return nil, errors.New(`cannot specify both "--graph" and "--data-root" option`) } @@ -451,6 +459,17 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) { return nil, err } + if runtime.GOOS != "windows" { + if flags.Changed("disable-legacy-registry") { + // TODO: Remove this error after 3 release cycles (18.03) + return nil, errors.New("ERROR: The '--disable-legacy-registry' flag has been removed. Interacting with legacy (v1) registries is no longer supported") + } + if !conf.V2Only { + // TODO: Remove this error after 3 release cycles (18.03) + return nil, errors.New("ERROR: The 'disable-legacy-registry' configuration option has been removed. Interacting with legacy (v1) registries is no longer supported") + } + } + if flags.Changed("graph") { logrus.Warnf(`The "-g / --graph" flag is deprecated. Please use "--data-root" instead`) } diff --git a/cmd/dockerd/daemon_unix.go b/cmd/dockerd/daemon_unix.go index a0f1b893b4..a6685bb668 100644 --- a/cmd/dockerd/daemon_unix.go +++ b/cmd/dockerd/daemon_unix.go @@ -58,6 +58,10 @@ func setDefaultUmask() error { return nil } +func getDaemonConfDir(_ string) (string, error) { + return getDefaultDaemonConfigDir() +} + func (cli *DaemonCli) getPlatformContainerdDaemonOpts() ([]supervisor.DaemonOpt, error) { opts := []supervisor.DaemonOpt{ supervisor.WithOOMScore(cli.Config.OOMScoreAdjust), diff --git a/cmd/dockerd/daemon_windows.go b/cmd/dockerd/daemon_windows.go index bf7b9efa3f..46932760cd 100644 --- a/cmd/dockerd/daemon_windows.go +++ b/cmd/dockerd/daemon_windows.go @@ -5,6 +5,7 @@ import ( "fmt" "net" "os" + "path/filepath" "time" "github.com/docker/docker/daemon/config" @@ -23,6 +24,10 @@ func setDefaultUmask() error { return nil } +func getDaemonConfDir(root string) (string, error) { + return filepath.Join(root, `\config`), nil +} + // preNotifySystem sends a message to the host when the API is active, but before the daemon is func preNotifySystem() { // start the service now to prevent timeouts waiting for daemon to start diff --git a/daemon/config/config.go b/daemon/config/config.go index 75430ed4b8..1b1dc9ca16 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -8,6 +8,7 @@ import ( "io/ioutil" "os" "reflect" + "runtime" "strings" "sync" @@ -134,6 +135,12 @@ type CommonConfig struct { SocketGroup string `json:"group,omitempty"` CorsHeaders string `json:"api-cors-header,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"` @@ -240,6 +247,9 @@ func New() *Config { config.LogConfig.Config = make(map[string]string) config.ClusterOpts = make(map[string]string) + if runtime.GOOS != "linux" { + config.V2Only = true + } return &config } diff --git a/daemon/daemon.go b/daemon/daemon.go index 420cfb12b6..6bde9c94e6 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -973,7 +973,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S return nil, err } - uuid, err := loadOrCreateUUID(filepath.Join(config.Root, "engine_uuid")) + trustKey, err := loadOrCreateTrustKey(config.TrustKeyPath) if err != nil { return nil, err } @@ -1018,7 +1018,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S return nil, errors.New("Devices cgroup isn't mounted") } - d.ID = uuid + d.ID = trustKey.PublicKey().KeyID() d.repository = daemonRepo d.containers = container.NewMemoryStore() if d.containersReplica, err = container.NewViewDB(); err != nil { @@ -1049,6 +1049,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S MaxConcurrentUploads: *config.MaxConcurrentUploads, ReferenceStore: rs, RegistryService: registryService, + TrustKey: trustKey, }) go d.execCommandGC() diff --git a/daemon/images/image_push.go b/daemon/images/image_push.go index c397b1cd52..4c7be8d2e9 100644 --- a/daemon/images/image_push.go +++ b/daemon/images/image_push.go @@ -54,6 +54,7 @@ func (i *ImageService) PushImage(ctx context.Context, image, tag string, metaHea }, ConfigMediaType: schema2.MediaTypeImageConfig, LayerStores: distribution.NewLayerProvidersFromStores(i.layerStores), + TrustKey: i.trustKey, UploadManager: i.uploadManager, } diff --git a/daemon/images/service.go b/daemon/images/service.go index 9034e5f37c..e8df5cb649 100644 --- a/daemon/images/service.go +++ b/daemon/images/service.go @@ -14,6 +14,7 @@ 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" "github.com/sirupsen/logrus" @@ -39,6 +40,7 @@ type ImageServiceConfig struct { MaxConcurrentUploads int ReferenceStore dockerreference.Store RegistryService registry.Service + TrustKey libtrust.PrivateKey } // NewImageService returns a new ImageService from a configuration @@ -54,6 +56,7 @@ func NewImageService(config ImageServiceConfig) *ImageService { layerStores: config.LayerStores, referenceStore: config.ReferenceStore, registryService: config.RegistryService, + trustKey: config.TrustKey, uploadManager: xfer.NewLayerUploadManager(config.MaxConcurrentUploads), } } @@ -69,6 +72,7 @@ type ImageService struct { pruneRunning int32 referenceStore dockerreference.Store registryService registry.Service + trustKey libtrust.PrivateKey uploadManager *xfer.LayerUploadManager } diff --git a/daemon/trustkey.go b/daemon/trustkey.go new file mode 100644 index 0000000000..4d72c932f1 --- /dev/null +++ b/daemon/trustkey.go @@ -0,0 +1,57 @@ +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" +) + +// 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), 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 +} diff --git a/daemon/trustkey_test.go b/daemon/trustkey_test.go new file mode 100644 index 0000000000..e49e76aa3e --- /dev/null +++ b/daemon/trustkey_test.go @@ -0,0 +1,71 @@ +package daemon // import "github.com/docker/docker/daemon" + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "gotest.tools/assert" + is "gotest.tools/assert/cmp" + "gotest.tools/fs" +) + +// LoadOrCreateTrustKey +func TestLoadOrCreateTrustKeyInvalidKeyFile(t *testing.T) { + tmpKeyFolderPath, err := ioutil.TempDir("", "api-trustkey-test") + assert.NilError(t, err) + defer os.RemoveAll(tmpKeyFolderPath) + + tmpKeyFile, err := ioutil.TempFile(tmpKeyFolderPath, "keyfile") + assert.NilError(t, err) + + _, err = loadOrCreateTrustKey(tmpKeyFile.Name()) + 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") + + 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) { + tmpKeyFolderPath := fs.NewDir(t, "api-trustkey-test") + defer tmpKeyFolderPath.Remove() + tmpKeyFile := tmpKeyFolderPath.Join("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/daemon/uuid.go b/daemon/uuid.go deleted file mode 100644 index 9640866f2a..0000000000 --- a/daemon/uuid.go +++ /dev/null @@ -1,35 +0,0 @@ -package daemon // import "github.com/docker/docker/daemon" - -import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" - - "github.com/docker/docker/pkg/ioutils" - "github.com/google/uuid" -) - -func loadOrCreateUUID(path string) (string, error) { - err := os.MkdirAll(filepath.Dir(path), 0755) - if err != nil { - return "", err - } - var id string - idb, err := ioutil.ReadFile(path) - if os.IsNotExist(err) { - id = uuid.New().String() - if err := ioutils.AtomicWriteFile(path, []byte(id), os.FileMode(0600)); err != nil { - return "", fmt.Errorf("Error saving uuid file: %s", err) - } - } else if err != nil { - return "", fmt.Errorf("Error loading uuid file %s: %s", path, err) - } else { - idp, err := uuid.Parse(string(idb)) - if err != nil { - return "", fmt.Errorf("Error parsing uuid in file %s: %s", path, err) - } - id = idp.String() - } - return id, nil -} diff --git a/distribution/config.go b/distribution/config.go index e9631d1b8c..438051c296 100644 --- a/distribution/config.go +++ b/distribution/config.go @@ -18,6 +18,7 @@ import ( "github.com/docker/docker/pkg/system" refstore "github.com/docker/docker/reference" "github.com/docker/docker/registry" + "github.com/docker/libtrust" "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -72,6 +73,9 @@ type ImagePushConfig struct { ConfigMediaType string // LayerStores (indexed by operating system) manages layers. LayerStores map[string]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 } diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 8bc0edd4c1..bbd14da1cb 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "runtime" "sort" "strings" "sync" @@ -180,8 +181,26 @@ func (p *v2Pusher) pushV2Tag(ctx context.Context, ref reference.NamedTagged, id putOptions := []distribution.ManifestServiceOption{distribution.WithTag(ref.Tag())} if _, err = manSvc.Put(ctx, manifest, putOptions...); err != nil { - logrus.Warnf("failed to upload schema2 manifest: %v", err) - return err + if runtime.GOOS == "windows" || p.config.TrustKey == nil || p.config.RequireSchema2 { + logrus.Warnf("failed to upload schema2 manifest: %v", err) + return err + } + + logrus.Warnf("failed to upload schema2 manifest: %v - falling back to schema1", err) + + manifestRef, err := reference.WithTag(p.repo.Named(), ref.Tag()) + if err != nil { + return err + } + builder = schema1.NewConfigManifestBuilder(p.repo.Blobs(ctx), p.config.TrustKey, manifestRef, imgConfig) + manifest, err = manifestFromBuilder(ctx, builder, descriptors) + if err != nil { + return err + } + + if _, err = manSvc.Put(ctx, manifest, putOptions...); err != nil { + return err + } } var canonicalManifest []byte diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index 9d4938993d..fd3384e39d 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -18,7 +18,6 @@ import ( "path" "path/filepath" "regexp" - "runtime" "strconv" "strings" "sync" @@ -36,6 +35,7 @@ import ( "github.com/docker/docker/pkg/mount" "github.com/docker/go-units" "github.com/docker/libnetwork/iptables" + "github.com/docker/libtrust" "github.com/go-check/check" "github.com/kr/pty" "golang.org/x/sys/unix" @@ -551,23 +551,20 @@ func (s *DockerDaemonSuite) TestDaemonAllocatesListeningPort(c *check.C) { } } -func (s *DockerDaemonSuite) TestDaemonUUIDGeneration(c *check.C) { - dir := "/var/lib/docker" - if runtime.GOOS == "windows" { - dir = filepath.Join(os.Getenv("programdata"), "docker") - } - file := filepath.Join(dir, "engine_uuid") - os.Remove(file) +func (s *DockerDaemonSuite) TestDaemonKeyGeneration(c *check.C) { + // TODO: skip or update for Windows daemon + os.Remove("/etc/docker/key.json") s.d.Start(c) s.d.Stop(c) - fi, err := os.Stat(file) + k, err := libtrust.LoadKeyFile("/etc/docker/key.json") if err != nil { - c.Fatalf("Error opening uuid file") + c.Fatalf("Error opening key file") } - // Test for uuid length - if fi.Size() != 36 { - c.Fatalf("Bad UUID size %d", fi.Size()) + 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) } } diff --git a/internal/test/fixtures/plugin/plugin.go b/internal/test/fixtures/plugin/plugin.go index 93f5f99fed..1af6584269 100644 --- a/internal/test/fixtures/plugin/plugin.go +++ b/internal/test/fixtures/plugin/plugin.go @@ -92,7 +92,7 @@ func CreateInRegistry(ctx context.Context, repo string, auth *types.AuthConfig, return nil, nil } - regService, err := registry.NewService(registry.ServiceOptions{}) + regService, err := registry.NewService(registry.ServiceOptions{V2Only: true}) if err != nil { return err } diff --git a/registry/config.go b/registry/config.go index 6bb9258c9b..de5a526b69 100644 --- a/registry/config.go +++ b/registry/config.go @@ -19,11 +19,16 @@ type ServiceOptions struct { AllowNondistributableArtifacts []string `json:"allow-nondistributable-artifacts,omitempty"` Mirrors []string `json:"registry-mirrors,omitempty"` InsecureRegistries []string `json:"insecure-registries,omitempty"` + + // V2Only controls access to legacy registries. If it is set to true via the + // command line flag the daemon will not attempt to contact v1 legacy registries + V2Only bool `json:"disable-legacy-registry,omitempty"` } // serviceConfig holds daemon configuration for the registry service. type serviceConfig struct { registrytypes.ServiceConfig + V2Only bool } var ( @@ -71,6 +76,7 @@ func newServiceConfig(options ServiceOptions) (*serviceConfig, error) { // Hack: Bypass setting the mirrors to IndexConfigs since they are going away // and Mirrors are only for the official registry anyways. }, + V2Only: options.V2Only, } if err := config.LoadAllowNondistributableArtifacts(options.AllowNondistributableArtifacts); err != nil { return nil, err diff --git a/registry/service.go b/registry/service.go index 08f5c7a4e1..b441970ff1 100644 --- a/registry/service.go +++ b/registry/service.go @@ -309,5 +309,20 @@ func (s *DefaultService) LookupPushEndpoints(hostname string) (endpoints []APIEn } func (s *DefaultService) lookupEndpoints(hostname string) (endpoints []APIEndpoint, err error) { - return s.lookupV2Endpoints(hostname) + endpoints, err = s.lookupV2Endpoints(hostname) + if err != nil { + return nil, err + } + + if s.config.V2Only { + return endpoints, nil + } + + legacyEndpoints, err := s.lookupV1Endpoints(hostname) + if err != nil { + return nil, err + } + endpoints = append(endpoints, legacyEndpoints...) + + return endpoints, nil }