From e9864ce8b9d5676f23a46d67403bfba6c8a54cc8 Mon Sep 17 00:00:00 2001 From: Viktor Stanchev Date: Mon, 18 Dec 2017 15:06:04 -0800 Subject: [PATCH] disable schema1 by default, add a config flag to enable it port of #2473 Signed-off-by: Viktor Stanchev --- configuration/configuration.go | 2 ++ .../registry-config-notls.yml | 3 ++ .../tokenserver-oauth/registry-config.yml | 3 ++ .../tokenserver/registry-config.yml | 3 ++ docs/configuration.md | 3 ++ errors.go | 4 +++ notifications/listener_test.go | 2 +- registry/handlers/api_test.go | 4 +++ registry/handlers/app.go | 4 +++ registry/proxy/proxymanifeststore_test.go | 5 +-- registry/storage/catalog_test.go | 4 +-- registry/storage/garbagecollect_test.go | 2 +- registry/storage/manifeststore_test.go | 21 ++++++++++-- registry/storage/registry.go | 34 +++++++++++++++---- registry/storage/v1unsupportedhandler.go | 23 +++++++++++++ 15 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 registry/storage/v1unsupportedhandler.go diff --git a/configuration/configuration.go b/configuration/configuration.go index cdc996b9d..ac38eedf8 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -184,6 +184,8 @@ type Configuration struct { // TrustKey is the signing key to use for adding the signature to // schema1 manifests. TrustKey string `yaml:"signingkeyfile,omitempty"` + // Enabled determines if schema1 manifests should be pullable + Enabled bool `yaml:"enabled,omitempty"` } `yaml:"schema1,omitempty"` } `yaml:"compatibility,omitempty"` diff --git a/contrib/docker-integration/tokenserver-oauth/registry-config-notls.yml b/contrib/docker-integration/tokenserver-oauth/registry-config-notls.yml index ed6b3ea5d..a700d08c7 100644 --- a/contrib/docker-integration/tokenserver-oauth/registry-config-notls.yml +++ b/contrib/docker-integration/tokenserver-oauth/registry-config-notls.yml @@ -7,6 +7,9 @@ storage: rootdirectory: /tmp/registry-dev http: addr: 0.0.0.0:5000 +compatibility: + schema1: + enabled: true auth: token: realm: "https://auth.localregistry:5559/token/" diff --git a/contrib/docker-integration/tokenserver-oauth/registry-config.yml b/contrib/docker-integration/tokenserver-oauth/registry-config.yml index 630ef057c..226798b35 100644 --- a/contrib/docker-integration/tokenserver-oauth/registry-config.yml +++ b/contrib/docker-integration/tokenserver-oauth/registry-config.yml @@ -10,6 +10,9 @@ http: tls: certificate: "/etc/docker/registry/localregistry.cert" key: "/etc/docker/registry/localregistry.key" +compatibility: + schema1: + enabled: true auth: token: realm: "https://auth.localregistry:5559/token/" diff --git a/contrib/docker-integration/tokenserver/registry-config.yml b/contrib/docker-integration/tokenserver/registry-config.yml index bc269056a..b9efdd3a0 100644 --- a/contrib/docker-integration/tokenserver/registry-config.yml +++ b/contrib/docker-integration/tokenserver/registry-config.yml @@ -10,6 +10,9 @@ http: tls: certificate: "/etc/docker/registry/localregistry.cert" key: "/etc/docker/registry/localregistry.key" +compatibility: + schema1: + enabled: true auth: token: realm: "https://auth.localregistry:5556/token/" diff --git a/docs/configuration.md b/docs/configuration.md index 807353ccc..a876b33ca 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -271,6 +271,7 @@ proxy: compatibility: schema1: signingkeyfile: /etc/registry/key.json + enabled: true validation: manifests: urls: @@ -1028,6 +1029,7 @@ username (such as `batman`) and the password for that username. compatibility: schema1: signingkeyfile: /etc/registry/key.json + enabled: true ``` Use the `compatibility` structure to configure handling of older and deprecated @@ -1038,6 +1040,7 @@ features. Each subsection defines such a feature with configurable behavior. | Parameter | Required | Description | |-----------|----------|-------------------------------------------------------| | `signingkeyfile` | no | The signing private key used to add signatures to `schema1` manifests. If no signing key is provided, a new ECDSA key is generated when the registry starts. | +| `enabled` | no | If this is not set to true, `schema1` manifests cannot be pushed. | ## `validation` diff --git a/errors.go b/errors.go index 020d33258..8e0b788d6 100644 --- a/errors.go +++ b/errors.go @@ -20,6 +20,10 @@ var ErrManifestNotModified = errors.New("manifest not modified") // performed var ErrUnsupported = errors.New("operation unsupported") +// ErrSchemaV1Unsupported is returned when a client tries to upload a schema v1 +// manifest but the registry is configured to reject it +var ErrSchemaV1Unsupported = errors.New("manifest schema v1 unsupported") + // ErrTagUnknown is returned if the given tag is not known by the tag service type ErrTagUnknown struct { Tag string diff --git a/notifications/listener_test.go b/notifications/listener_test.go index a58498078..be9e81af8 100644 --- a/notifications/listener_test.go +++ b/notifications/listener_test.go @@ -25,7 +25,7 @@ func TestListener(t *testing.T) { t.Fatal(err) } - registry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableDelete, storage.EnableRedirect, storage.Schema1SigningKey(k)) + registry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableDelete, storage.EnableRedirect, storage.Schema1SigningKey(k), storage.EnableSchema1) if err != nil { t.Fatalf("error creating registry: %v", err) } diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 98dcaa71f..6b595f974 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -2027,6 +2027,7 @@ func newTestEnvMirror(t *testing.T, deleteEnabled bool) *testEnv { RemoteURL: "http://example.com", }, } + config.Compatibility.Schema1.Enabled = true return newTestEnvWithConfig(t, &config) @@ -2043,6 +2044,7 @@ func newTestEnv(t *testing.T, deleteEnabled bool) *testEnv { }, } + config.Compatibility.Schema1.Enabled = true config.HTTP.Headers = headerConfig return newTestEnvWithConfig(t, &config) @@ -2565,6 +2567,7 @@ func TestProxyManifestGetByTag(t *testing.T) { }}, }, } + truthConfig.Compatibility.Schema1.Enabled = true truthConfig.HTTP.Headers = headerConfig imageName, _ := reference.WithName("foo/bar") @@ -2583,6 +2586,7 @@ func TestProxyManifestGetByTag(t *testing.T) { RemoteURL: truthEnv.server.URL, }, } + proxyConfig.Compatibility.Schema1.Enabled = true proxyConfig.HTTP.Headers = headerConfig proxyEnv := newTestEnvWithConfig(t, &proxyConfig) diff --git a/registry/handlers/app.go b/registry/handlers/app.go index e38050cb7..9b774aa08 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -174,6 +174,10 @@ func NewApp(ctx context.Context, config *configuration.Configuration) *App { options = append(options, storage.Schema1SigningKey(app.trustKey)) + if config.Compatibility.Schema1.Enabled { + options = append(options, storage.EnableSchema1) + } + if config.HTTP.Host != "" { u, err := url.Parse(config.HTTP.Host) if err != nil { diff --git a/registry/proxy/proxymanifeststore_test.go b/registry/proxy/proxymanifeststore_test.go index 5f3bd34e6..b80b303eb 100644 --- a/registry/proxy/proxymanifeststore_test.go +++ b/registry/proxy/proxymanifeststore_test.go @@ -95,7 +95,8 @@ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestE ctx := context.Background() truthRegistry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), - storage.Schema1SigningKey(k)) + storage.Schema1SigningKey(k), + storage.EnableSchema1) if err != nil { t.Fatalf("error creating registry: %v", err) } @@ -117,7 +118,7 @@ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestE t.Fatalf(err.Error()) } - localRegistry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableRedirect, storage.DisableDigestResumption, storage.Schema1SigningKey(k)) + localRegistry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableRedirect, storage.DisableDigestResumption, storage.Schema1SigningKey(k), storage.EnableSchema1) if err != nil { t.Fatalf("error creating registry: %v", err) } diff --git a/registry/storage/catalog_test.go b/registry/storage/catalog_test.go index 696e024e0..45cd26439 100644 --- a/registry/storage/catalog_test.go +++ b/registry/storage/catalog_test.go @@ -26,7 +26,7 @@ type setupEnv struct { func setupFS(t *testing.T) *setupEnv { d := inmemory.New() ctx := context.Background() - registry, err := NewRegistry(ctx, d, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableRedirect) + registry, err := NewRegistry(ctx, d, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableRedirect, EnableSchema1) if err != nil { t.Fatalf("error creating registry: %v", err) } @@ -207,7 +207,7 @@ func testEq(a, b []string, size int) bool { func setupBadWalkEnv(t *testing.T) *setupEnv { d := newBadListDriver() ctx := context.Background() - registry, err := NewRegistry(ctx, d, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableRedirect) + registry, err := NewRegistry(ctx, d, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableRedirect, EnableSchema1) if err != nil { t.Fatalf("error creating registry: %v", err) } diff --git a/registry/storage/garbagecollect_test.go b/registry/storage/garbagecollect_test.go index 2e36fddb0..21debc5b0 100644 --- a/registry/storage/garbagecollect_test.go +++ b/registry/storage/garbagecollect_test.go @@ -27,7 +27,7 @@ func createRegistry(t *testing.T, driver driver.StorageDriver, options ...Regist if err != nil { t.Fatal(err) } - options = append([]RegistryOption{EnableDelete, Schema1SigningKey(k)}, options...) + options = append([]RegistryOption{EnableDelete, Schema1SigningKey(k), EnableSchema1}, options...) registry, err := NewRegistry(ctx, driver, options...) if err != nil { t.Fatalf("Failed to construct namespace") diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index 2fad7611d..a22332ba2 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -56,10 +56,18 @@ func TestManifestStorage(t *testing.T) { if err != nil { t.Fatal(err) } - testManifestStorage(t, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect, Schema1SigningKey(k)) + testManifestStorage(t, true, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect, Schema1SigningKey(k), EnableSchema1) } -func testManifestStorage(t *testing.T, options ...RegistryOption) { +func TestManifestStorageV1Unsupported(t *testing.T) { + k, err := libtrust.GenerateECP256PrivateKey() + if err != nil { + t.Fatal(err) + } + testManifestStorage(t, false, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect, Schema1SigningKey(k)) +} + +func testManifestStorage(t *testing.T, schema1Enabled bool, options ...RegistryOption) { repoName, _ := reference.WithName("foo/bar") env := newManifestStoreTestEnv(t, repoName, "thetag", options...) ctx := context.Background() @@ -111,6 +119,15 @@ func testManifestStorage(t *testing.T, options ...RegistryOption) { t.Fatalf("expected errors putting manifest with full verification") } + // If schema1 is not enabled, do a short version of this test, just checking + // if we get the right error when we Put + if !schema1Enabled { + if err != distribution.ErrSchemaV1Unsupported { + t.Fatalf("got the wrong error when schema1 is disabled: %s", err) + } + return + } + switch err := err.(type) { case distribution.ErrManifestVerification: if len(err) != 2 { diff --git a/registry/storage/registry.go b/registry/storage/registry.go index 46b968539..66149e955 100644 --- a/registry/storage/registry.go +++ b/registry/storage/registry.go @@ -19,6 +19,7 @@ type registry struct { statter *blobStatter // global statter service. blobDescriptorCacheProvider cache.BlobDescriptorCacheProvider deleteEnabled bool + schema1Enabled bool resumableDigestEnabled bool schema1SigningKey libtrust.PrivateKey blobDescriptorServiceFactory distribution.BlobDescriptorServiceFactory @@ -48,6 +49,13 @@ func EnableDelete(registry *registry) error { return nil } +// EnableSchema1 is a functional option for NewRegistry. It enables pushing of +// schema1 manifests. +func EnableSchema1(registry *registry) error { + registry.schema1Enabled = true + return nil +} + // DisableDigestResumption is a functional option for NewRegistry. It should be // used if the registry is acting as a caching proxy. func DisableDigestResumption(registry *registry) error { @@ -237,16 +245,30 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M linkDirectoryPathSpec: manifestDirectoryPathSpec, } - ms := &manifestStore{ - ctx: ctx, - repository: repo, - blobStore: blobStore, - schema1Handler: &signedManifestHandler{ + var v1Handler ManifestHandler + if repo.schema1Enabled { + v1Handler = &signedManifestHandler{ ctx: ctx, schema1SigningKey: repo.schema1SigningKey, repository: repo, blobStore: blobStore, - }, + } + } else { + v1Handler = &v1UnsupportedHandler{ + innerHandler: &signedManifestHandler{ + ctx: ctx, + schema1SigningKey: repo.schema1SigningKey, + repository: repo, + blobStore: blobStore, + }, + } + } + + ms := &manifestStore{ + ctx: ctx, + repository: repo, + blobStore: blobStore, + schema1Handler: v1Handler, schema2Handler: &schema2ManifestHandler{ ctx: ctx, repository: repo, diff --git a/registry/storage/v1unsupportedhandler.go b/registry/storage/v1unsupportedhandler.go new file mode 100644 index 000000000..fb6ca14e3 --- /dev/null +++ b/registry/storage/v1unsupportedhandler.go @@ -0,0 +1,23 @@ +package storage + +import ( + "context" + + "github.com/docker/distribution" + digest "github.com/opencontainers/go-digest" +) + +// signedManifestHandler is a ManifestHandler that unmarshals v1 manifests but +// refuses to Put v1 manifests +type v1UnsupportedHandler struct { + innerHandler ManifestHandler +} + +var _ ManifestHandler = &v1UnsupportedHandler{} + +func (v *v1UnsupportedHandler) Unmarshal(ctx context.Context, dgst digest.Digest, content []byte) (distribution.Manifest, error) { + return v.innerHandler.Unmarshal(ctx, dgst, content) +} +func (v *v1UnsupportedHandler) Put(ctx context.Context, manifest distribution.Manifest, skipDependencyVerification bool) (digest.Digest, error) { + return digest.Digest(""), distribution.ErrSchemaV1Unsupported +}