beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #44329 from thaJeztah/remove_trustkey_id_migration

Browse source 
Remove trustkey id migration and config.TrustKeyPath
This commit is contained in:
Tianon Gravi 2022-12-01 12:49:54 -08:00 committed by GitHub
commit cd8a090e67
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 17 additions and 345 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
cmd/dockerd/config.go
View file

@ -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 (

8
cmd/dockerd/daemon.go
View file

@ -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 {

4
cmd/dockerd/daemon_unix.go
View file

@ -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,

5
cmd/dockerd/daemon_windows.go
View file

@ -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

6
cmd/dockerd/docker_windows.go
View file

@ -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

2
cmd/dockerd/options.go
View file

@ -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")

6
daemon/config/config.go
View file

@ -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"`

23
daemon/daemon.go
View file

@ -935,14 +935,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.
//
@ -955,7 +947,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
}
@ -1070,19 +1062,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") != "" {
imgSvcConfig.TrustKey, err = loadOrCreateTrustKey(config.TrustKeyPath)
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.
// So sometimes d.containerdCli will be nil
// In that case we'll create a local content store... but otherwise we'll use containerd

29
daemon/id.go
View file

@ -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
}

1
daemon/images/image_push.go
View file

@ -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,
}

4
daemon/images/service.go
View file

@ -14,7 +14,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"
)
@ -41,7 +40,6 @@ type ImageServiceConfig struct {
MaxDownloadAttempts int
ReferenceStore dockerreference.Store
RegistryService registry.Service
TrustKey libtrust.PrivateKey
ContentStore content.Store
Leases leases.Manager
ContentNamespace string
@ -58,7 +56,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,
@ -77,7 +74,6 @@ type ImageService struct {
pruneRunning int32
referenceStore dockerreference.Store
registryService registry.Service
trustKey libtrust.PrivateKey
uploadManager *xfer.LayerUploadManager
leases leases.Manager
content content.Store

57
daemon/trustkey.go
View file

@ -1,57 +0,0 @@
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
}

71
daemon/trustkey_test.go
View file

@ -1,71 +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"
"gotest.tools/v3/fs"
)
// LoadOrCreateTrustKey
func TestLoadOrCreateTrustKeyInvalidKeyFile(t *testing.T) {
tmpKeyFolderPath, err := os.MkdirTemp("", "api-trustkey-test")
assert.NilError(t, err)
defer os.RemoveAll(tmpKeyFolderPath)
tmpKeyFile, err := os.CreateTemp(tmpKeyFolderPath, "keyfile")
assert.NilError(t, err)
defer tmpKeyFile.Close()
_, 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))
}

6
distribution/config.go
View file

@ -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"
@ -47,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.
@ -74,9 +71,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
}

4
distribution/pull_v2.go
View file

@ -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" {

9
distribution/push_v2.go
View file

@ -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" {
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

73
integration-cli/docker_cli_daemon_test.go
View file

@ -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)

51
integration/daemon/daemon_test.go
View file

@ -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)