From f9bc9220eb49e04ff9ba8a4a9139a380ce127979 Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 28 Aug 2023 12:44:46 +0200 Subject: [PATCH 1/2] feat(storage)!: remove schema1 except manifeststore_test Signed-off-by: David van der Spek --- registry/root.go | 9 +- registry/storage/catalog_test.go | 15 +- registry/storage/garbagecollect_test.go | 76 +++---- registry/storage/manifeststore.go | 6 - registry/storage/manifeststore_test.go | 220 +++++++-------------- registry/storage/registry.go | 45 +---- registry/storage/schema2manifesthandler.go | 3 +- registry/storage/signedmanifesthandler.go | 142 ------------- registry/storage/v1unsupportedhandler.go | 24 --- testutil/manifests.go | 35 ---- 10 files changed, 116 insertions(+), 459 deletions(-) delete mode 100644 registry/storage/signedmanifesthandler.go delete mode 100644 registry/storage/v1unsupportedhandler.go diff --git a/registry/root.go b/registry/root.go index 49d10946..51c8c7ff 100644 --- a/registry/root.go +++ b/registry/root.go @@ -8,7 +8,6 @@ import ( "github.com/distribution/distribution/v3/registry/storage" "github.com/distribution/distribution/v3/registry/storage/driver/factory" "github.com/distribution/distribution/v3/version" - "github.com/docker/libtrust" "github.com/spf13/cobra" ) @@ -67,13 +66,7 @@ var GCCmd = &cobra.Command{ os.Exit(1) } - k, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - fmt.Fprint(os.Stderr, err) - os.Exit(1) - } - - registry, err := storage.NewRegistry(ctx, driver, storage.Schema1SigningKey(k)) + registry, err := storage.NewRegistry(ctx, driver) if err != nil { fmt.Fprintf(os.Stderr, "failed to construct registry: %v", err) os.Exit(1) diff --git a/registry/storage/catalog_test.go b/registry/storage/catalog_test.go index 14ba4e83..0e3e0361 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(memory.UnlimitedSize)), EnableRedirect, EnableSchema1) + registry, err := NewRegistry(ctx, d, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize)), EnableRedirect) if err != nil { t.Fatalf("error creating registry: %v", err) } @@ -81,19 +81,18 @@ func makeRepo(ctx context.Context, t *testing.T, name string, reg distribution.N t.Fatal(err) } + // upload the layers err = testutil.UploadBlobs(repo, layers) if err != nil { t.Fatalf("failed to upload layers: %v", err) } - getKeys := func(digests map[digest.Digest]io.ReadSeeker) (ds []digest.Digest) { - for d := range digests { - ds = append(ds, d) - } - return + digests := []digest.Digest{} + for digest := range layers { + digests = append(digests, digest) } - manifest, err := testutil.MakeSchema1Manifest(getKeys(layers)) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + manifest, err := testutil.MakeSchema2Manifest(repo, digests) if err != nil { t.Fatal(err) } @@ -206,7 +205,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(memory.UnlimitedSize)), EnableRedirect, EnableSchema1) + registry, err := NewRegistry(ctx, d, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize)), EnableRedirect) 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 bdbf9ab2..35e6cd71 100644 --- a/registry/storage/garbagecollect_test.go +++ b/registry/storage/garbagecollect_test.go @@ -11,7 +11,6 @@ import ( "github.com/distribution/distribution/v3/registry/storage/driver" "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" "github.com/distribution/distribution/v3/testutil" - "github.com/docker/libtrust" "github.com/opencontainers/go-digest" ) @@ -23,11 +22,7 @@ type image struct { func createRegistry(t *testing.T, driver driver.StorageDriver, options ...RegistryOption) distribution.Namespace { ctx := context.Background() - k, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - t.Fatal(err) - } - options = append([]RegistryOption{EnableDelete, Schema1SigningKey(k), EnableSchema1}, options...) + options = append(options, EnableDelete) registry, err := NewRegistry(ctx, driver, options...) if err != nil { t.Fatalf("Failed to construct namespace") @@ -110,30 +105,6 @@ func uploadImage(t *testing.T, repository distribution.Repository, im image) dig return manifestDigest } -func uploadRandomSchema1Image(t *testing.T, repository distribution.Repository) image { - randomLayers, err := testutil.CreateRandomLayers(2) - if err != nil { - t.Fatalf("%v", err) - } - - digests := []digest.Digest{} - for digest := range randomLayers { - digests = append(digests, digest) - } - - manifest, err := testutil.MakeSchema1Manifest(digests) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - if err != nil { - t.Fatalf("%v", err) - } - - manifestDigest := uploadImage(t, repository, image{manifest: manifest, layers: randomLayers}) - return image{ - manifest: manifest, - manifestDigest: manifestDigest, - layers: randomLayers, - } -} - func uploadRandomSchema2Image(t *testing.T, repository distribution.Repository) image { randomLayers, err := testutil.CreateRandomLayers(2) if err != nil { @@ -166,8 +137,8 @@ func TestNoDeletionNoEffect(t *testing.T) { repo := makeRepository(t, registry, "palailogos") manifestService, _ := repo.Manifests(ctx) - image1 := uploadRandomSchema1Image(t, repo) - image2 := uploadRandomSchema1Image(t, repo) + image1 := uploadRandomSchema2Image(t, repo) + image2 := uploadRandomSchema2Image(t, repo) uploadRandomSchema2Image(t, repo) // construct manifestlist for fun. @@ -215,11 +186,21 @@ func TestDeleteManifestIfTagNotFound(t *testing.T) { t.Fatalf("failed to make layers: %v", err) } + digests1 := []digest.Digest{} + for digest := range randomLayers1 { + digests1 = append(digests1, digest) + } + randomLayers2, err := testutil.CreateRandomLayers(3) if err != nil { t.Fatalf("failed to make layers: %v", err) } + digests2 := []digest.Digest{} + for digest := range randomLayers2 { + digests2 = append(digests2, digest) + } + // Upload all layers err = testutil.UploadBlobs(repo, randomLayers1) if err != nil { @@ -232,12 +213,12 @@ func TestDeleteManifestIfTagNotFound(t *testing.T) { } // Construct manifests - manifest1, err := testutil.MakeSchema1Manifest(getKeys(randomLayers1)) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + manifest1, err := testutil.MakeSchema2Manifest(repo, digests1) if err != nil { t.Fatalf("failed to make manifest: %v", err) } - manifest2, err := testutil.MakeSchema1Manifest(getKeys(randomLayers2)) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + manifest2, err := testutil.MakeSchema2Manifest(repo, digests2) if err != nil { t.Fatalf("failed to make manifest: %v", err) } @@ -303,7 +284,7 @@ func TestGCWithMissingManifests(t *testing.T) { registry := createRegistry(t, d) repo := makeRepository(t, registry, "testrepo") - uploadRandomSchema1Image(t, repo) + uploadRandomSchema2Image(t, repo) // Simulate a missing _manifests directory revPath, err := pathFor(manifestRevisionsPathSpec{"testrepo"}) @@ -339,8 +320,8 @@ func TestDeletionHasEffect(t *testing.T) { repo := makeRepository(t, registry, "komnenos") manifests, _ := repo.Manifests(ctx) - image1 := uploadRandomSchema1Image(t, repo) - image2 := uploadRandomSchema1Image(t, repo) + image1 := uploadRandomSchema2Image(t, repo) + image2 := uploadRandomSchema2Image(t, repo) image3 := uploadRandomSchema2Image(t, repo) manifests.Delete(ctx, image2.manifestDigest) @@ -389,13 +370,6 @@ func getAnyKey(digests map[digest.Digest]io.ReadSeeker) (d digest.Digest) { return } -func getKeys(digests map[digest.Digest]io.ReadSeeker) (ds []digest.Digest) { - for d := range digests { - ds = append(ds, d) - } - return -} - func TestDeletionWithSharedLayer(t *testing.T) { ctx := context.Background() inmemoryDriver := inmemory.New() @@ -409,11 +383,21 @@ func TestDeletionWithSharedLayer(t *testing.T) { t.Fatalf("failed to make layers: %v", err) } + digests1 := []digest.Digest{} + for digest := range randomLayers1 { + digests1 = append(digests1, digest) + } + randomLayers2, err := testutil.CreateRandomLayers(3) if err != nil { t.Fatalf("failed to make layers: %v", err) } + digests2 := []digest.Digest{} + for digest := range randomLayers2 { + digests2 = append(digests2, digest) + } + // Upload all layers err = testutil.UploadBlobs(repo, randomLayers1) if err != nil { @@ -426,13 +410,13 @@ func TestDeletionWithSharedLayer(t *testing.T) { } // Construct manifests - manifest1, err := testutil.MakeSchema1Manifest(getKeys(randomLayers1)) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + manifest1, err := testutil.MakeSchema2Manifest(repo, digests1) if err != nil { t.Fatalf("failed to make manifest: %v", err) } sharedKey := getAnyKey(randomLayers1) - manifest2, err := testutil.MakeSchema2Manifest(repo, append(getKeys(randomLayers2), sharedKey)) + manifest2, err := testutil.MakeSchema2Manifest(repo, append(digests2, sharedKey)) if err != nil { t.Fatalf("failed to make manifest: %v", err) } diff --git a/registry/storage/manifeststore.go b/registry/storage/manifeststore.go index 1d55a080..37c86495 100644 --- a/registry/storage/manifeststore.go +++ b/registry/storage/manifeststore.go @@ -10,7 +10,6 @@ import ( "github.com/distribution/distribution/v3/manifest" "github.com/distribution/distribution/v3/manifest/manifestlist" "github.com/distribution/distribution/v3/manifest/ocischema" - "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. "github.com/distribution/distribution/v3/manifest/schema2" "github.com/opencontainers/go-digest" v1 "github.com/opencontainers/image-spec/specs-go/v1" @@ -48,7 +47,6 @@ type manifestStore struct { skipDependencyVerification bool - schema1Handler ManifestHandler schema2Handler ManifestHandler manifestListHandler ManifestHandler ocischemaHandler ManifestHandler @@ -96,8 +94,6 @@ func (ms *manifestStore) Get(ctx context.Context, dgst digest.Digest, options .. } switch versioned.SchemaVersion { - case 1: - return ms.schema1Handler.Unmarshal(ctx, dgst, content) case 2: // This can be an image manifest or a manifest list switch versioned.MediaType { @@ -133,8 +129,6 @@ func (ms *manifestStore) Put(ctx context.Context, manifest distribution.Manifest dcontext.GetLogger(ms.ctx).Debug("(*manifestStore).Put") switch manifest.(type) { - case *schema1.SignedManifest: //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - return ms.schema1Handler.Put(ctx, manifest, ms.skipDependencyVerification) case *schema2.DeserializedManifest: return ms.schema2Handler.Put(ctx, manifest, ms.skipDependencyVerification) case *ocischema.DeserializedManifest: diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index d40315a9..54e19913 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -11,13 +11,12 @@ import ( "github.com/distribution/distribution/v3" "github.com/distribution/distribution/v3/manifest" "github.com/distribution/distribution/v3/manifest/ocischema" - "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + "github.com/distribution/distribution/v3/manifest/schema2" "github.com/distribution/distribution/v3/reference" "github.com/distribution/distribution/v3/registry/storage/cache/memory" "github.com/distribution/distribution/v3/registry/storage/driver" "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" "github.com/distribution/distribution/v3/testutil" - "github.com/docker/libtrust" "github.com/opencontainers/go-digest" v1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -55,22 +54,10 @@ func newManifestStoreTestEnv(t *testing.T, name reference.Named, tag string, opt } func TestManifestStorage(t *testing.T) { - k, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - t.Fatal(err) - } - testManifestStorage(t, true, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize)), EnableDelete, EnableRedirect, Schema1SigningKey(k), EnableSchema1) + testManifestStorage(t, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize)), EnableDelete, EnableRedirect) } -func TestManifestStorageV1Unsupported(t *testing.T) { - k, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - t.Fatal(err) - } - testManifestStorage(t, false, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize)), EnableDelete, EnableRedirect, Schema1SigningKey(k)) -} - -func testManifestStorage(t *testing.T, schema1Enabled bool, options ...RegistryOption) { +func testManifestStorage(t *testing.T, options ...RegistryOption) { repoName, _ := reference.WithName("foo/bar") env := newManifestStoreTestEnv(t, repoName, "thetag", options...) ctx := context.Background() @@ -79,13 +66,59 @@ func testManifestStorage(t *testing.T, schema1Enabled bool, options ...RegistryO t.Fatal(err) } - m := schema1.Manifest{ //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - Versioned: manifest.Versioned{ - SchemaVersion: 1, - }, - Name: env.name.Name(), - Tag: env.tag, + // Push a config, and reference it in the manifest + sampleConfig := []byte(`{ + "architecture": "amd64", + "history": [ + { + "created": "2015-10-31T22:22:54.690851953Z", + "created_by": "/bin/sh -c #(nop) ADD file:a3bc1e842b69636f9df5256c49c5374fb4eef1e281fe3f282c65fb853ee171c5 in /" + }, + ], + "rootfs": { + "diff_ids": [ + "sha256:c6f988f4874bb0add23a778f753c65efe992244e148a1d2ec2a8b664fb66bbd1", + ], + "type": "layers" + } + }`) + + // Build a manifest and store it and its layers in the registry + + blobStore := env.repository.Blobs(ctx) + d, err := blobStore.Put(ctx, schema2.MediaTypeImageConfig, sampleConfig) + if err != nil { + t.Fatal(err) } + builder := schema2.NewManifestBuilder(d, sampleConfig) + + m := &schema2.Manifest{ + Versioned: manifest.Versioned{ + SchemaVersion: 2, + MediaType: schema2.MediaTypeManifest, + }, + Config: distribution.Descriptor{ + Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", + Size: 3253, + MediaType: schema2.MediaTypeImageConfig, + }, + Layers: []distribution.Descriptor{ + { + Digest: "sha256:463434349086340864309863409683460843608348608934092322395278926a", + Size: 6323, + MediaType: schema2.MediaTypeLayer, + }, + { + Digest: "sha256:630923423623623423352523525237238023652897356239852383652aaaaaaa", + Size: 6863, + MediaType: schema2.MediaTypeLayer, + }, + }, + } + sampleConfigDigest := digest.FromBytes(sampleConfig) + sampleConfigSize := int64(len(sampleConfig)) + m.Config.Digest = sampleConfigDigest + m.Config.Size = sampleConfigSize // Build up some test layers and add them to the manifest, saving the // readseekers for upload later. @@ -97,52 +130,7 @@ func testManifestStorage(t *testing.T, schema1Enabled bool, options ...RegistryO } testLayers[dgst] = rs - m.FSLayers = append(m.FSLayers, schema1.FSLayer{ //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - BlobSum: dgst, - }) - m.History = append(m.History, schema1.History{ //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - V1Compatibility: "", - }) - - } - - pk, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - t.Fatalf("unexpected error generating private key: %v", err) - } - - sm, merr := schema1.Sign(&m, pk) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - if merr != nil { - t.Fatalf("error signing manifest: %v", err) - } - - _, err = ms.Put(ctx, sm) - if err == nil { - 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 { - t.Fatalf("expected 2 verification errors: %#v", err) - } - - for _, err := range err { - if _, ok := err.(distribution.ErrManifestBlobUnknown); !ok { - t.Fatalf("unexpected error type: %v", err) - } - } - default: - t.Fatalf("unexpected error verifying manifest: %v", err) + m.Layers[i].Digest = dgst } // Now, upload the layers that were missing! @@ -159,6 +147,12 @@ func testManifestStorage(t *testing.T, schema1Enabled bool, options ...RegistryO if _, err := wr.Commit(env.ctx, distribution.Descriptor{Digest: dgst}); err != nil { t.Fatalf("unexpected error finishing upload: %v", err) } + builder.AppendReference(distribution.Descriptor{Digest: dgst}) + } + + sm, err := builder.Build(ctx) + if err != nil { + t.Fatalf("%s: unexpected error generating manifest: %v", repoName, err) } var manifestDigest digest.Digest @@ -180,34 +174,19 @@ func testManifestStorage(t *testing.T, schema1Enabled bool, options ...RegistryO t.Fatalf("unexpected error fetching manifest: %v", err) } - fetchedManifest, ok := fromStore.(*schema1.SignedManifest) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + fetchedManifest, ok := fromStore.(*schema2.DeserializedManifest) if !ok { t.Fatalf("unexpected manifest type from signedstore") } - - if !bytes.Equal(fetchedManifest.Canonical, sm.Canonical) { - t.Fatalf("fetched payload does not match original payload: %q != %q", fetchedManifest.Canonical, sm.Canonical) - } - - _, pl, err := fetchedManifest.Payload() //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + _, pl, err := fetchedManifest.Payload() if err != nil { - t.Fatalf("error getting payload %#v", err) - } - - fetchedJWS, err := libtrust.ParsePrettySignature(pl, "signatures") - if err != nil { - t.Fatalf("unexpected error parsing jws: %v", err) - } - - payload, err := fetchedJWS.Payload() - if err != nil { - t.Fatalf("unexpected error extracting payload: %v", err) + t.Fatalf("could not get manifest payload: %v", err) } // Now that we have a payload, take a moment to check that the manifest is // return by the payload digest. - dgst := digest.FromBytes(payload) + dgst := digest.FromBytes(pl) exists, err = ms.Exists(ctx, dgst) if err != nil { t.Fatalf("error checking manifest existence by digest: %v", err) @@ -222,55 +201,18 @@ func testManifestStorage(t *testing.T, schema1Enabled bool, options ...RegistryO t.Fatalf("unexpected error fetching manifest by digest: %v", err) } - byDigestManifest, ok := fetchedByDigest.(*schema1.SignedManifest) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + byDigestManifest, ok := fetchedByDigest.(*schema2.DeserializedManifest) if !ok { t.Fatalf("unexpected manifest type from signedstore") } - if !bytes.Equal(byDigestManifest.Canonical, fetchedManifest.Canonical) { - t.Fatalf("fetched manifest not equal: %q != %q", byDigestManifest.Canonical, fetchedManifest.Canonical) - } - - sigs, err := fetchedJWS.Signatures() + _, byDigestCanonical, err := byDigestManifest.Payload() if err != nil { - t.Fatalf("unable to extract signatures: %v", err) + t.Fatalf("could not get manifest payload: %v", err) } - if len(sigs) != 1 { - t.Fatalf("unexpected number of signatures: %d != %d", len(sigs), 1) - } - - // Now, push the same manifest with a different key - pk2, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - t.Fatalf("unexpected error generating private key: %v", err) - } - - sm2, err := schema1.Sign(&m, pk2) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - if err != nil { - t.Fatalf("unexpected error signing manifest: %v", err) - } - _, pl, err = sm2.Payload() //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - if err != nil { - t.Fatalf("error getting payload %#v", err) - } - - jws2, err := libtrust.ParsePrettySignature(pl, "signatures") - if err != nil { - t.Fatalf("error parsing signature: %v", err) - } - - sigs2, err := jws2.Signatures() - if err != nil { - t.Fatalf("unable to extract signatures: %v", err) - } - - if len(sigs2) != 1 { - t.Fatalf("unexpected number of signatures: %d != %d", len(sigs2), 1) - } - - if manifestDigest, err = ms.Put(ctx, sm2); err != nil { - t.Fatalf("unexpected error putting manifest: %v", err) + if !bytes.Equal(byDigestCanonical, pl) { + t.Fatalf("fetched manifest not equal: %q != %q", byDigestCanonical, pl) } fromStore, err = ms.Get(ctx, manifestDigest) @@ -278,31 +220,17 @@ func testManifestStorage(t *testing.T, schema1Enabled bool, options ...RegistryO t.Fatalf("unexpected error fetching manifest: %v", err) } - fetched, ok := fromStore.(*schema1.SignedManifest) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + fetched, ok := fromStore.(*schema2.DeserializedManifest) if !ok { t.Fatalf("unexpected type from signed manifeststore : %T", fetched) } - if _, err := schema1.Verify(fetched); err != nil { //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - t.Fatalf("unexpected error verifying manifest: %v", err) - } - - _, pl, err = fetched.Payload() //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + _, receivedPL, err := fetched.Payload() if err != nil { t.Fatalf("error getting payload %#v", err) } - receivedJWS, err := libtrust.ParsePrettySignature(pl, "signatures") - if err != nil { - t.Fatalf("unexpected error parsing jws: %v", err) - } - - receivedPayload, err := receivedJWS.Payload() - if err != nil { - t.Fatalf("unexpected error extracting received payload: %v", err) - } - - if !bytes.Equal(receivedPayload, payload) { + if !bytes.Equal(receivedPL, pl) { t.Fatalf("payloads are not equal") } diff --git a/registry/storage/registry.go b/registry/storage/registry.go index 7bf24487..6c263e7c 100644 --- a/registry/storage/registry.go +++ b/registry/storage/registry.go @@ -8,7 +8,6 @@ import ( "github.com/distribution/distribution/v3/reference" "github.com/distribution/distribution/v3/registry/storage/cache" storagedriver "github.com/distribution/distribution/v3/registry/storage/driver" - "github.com/docker/libtrust" ) // registry is the top-level implementation of Registry for use in the storage @@ -19,9 +18,7 @@ type registry struct { statter *blobStatter // global statter service. blobDescriptorCacheProvider cache.BlobDescriptorCacheProvider deleteEnabled bool - schema1Enabled bool resumableDigestEnabled bool - schema1SigningKey libtrust.PrivateKey blobDescriptorServiceFactory distribution.BlobDescriptorServiceFactory manifestURLs manifestURLs driver storagedriver.StorageDriver @@ -50,13 +47,6 @@ 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 { @@ -80,15 +70,6 @@ func ManifestURLsDenyRegexp(r *regexp.Regexp) RegistryOption { } } -// Schema1SigningKey returns a functional option for NewRegistry. It sets the -// key for signing all schema1 manifests. -func Schema1SigningKey(key libtrust.PrivateKey) RegistryOption { - return func(registry *registry) error { - registry.schema1SigningKey = key - return nil - } -} - // BlobDescriptorServiceFactory returns a functional option for NewRegistry. It sets the // factory to create BlobDescriptorServiceFactory middleware. func BlobDescriptorServiceFactory(factory distribution.BlobDescriptorServiceFactory) RegistryOption { @@ -240,25 +221,6 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M linkDirectoryPathSpec: manifestDirectoryPathSpec, } - 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, - }, - } - } - manifestListHandler := &manifestListHandler{ ctx: ctx, repository: repo, @@ -266,10 +228,9 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M } ms := &manifestStore{ - ctx: ctx, - repository: repo, - blobStore: blobStore, - schema1Handler: v1Handler, + ctx: ctx, + repository: repo, + blobStore: blobStore, schema2Handler: &schema2ManifestHandler{ ctx: ctx, repository: repo, diff --git a/registry/storage/schema2manifesthandler.go b/registry/storage/schema2manifesthandler.go index 7716289a..aa8980ec 100644 --- a/registry/storage/schema2manifesthandler.go +++ b/registry/storage/schema2manifesthandler.go @@ -8,7 +8,6 @@ import ( "github.com/distribution/distribution/v3" dcontext "github.com/distribution/distribution/v3/context" - "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. "github.com/distribution/distribution/v3/manifest/schema2" "github.com/opencontainers/go-digest" ) @@ -110,7 +109,7 @@ func (ms *schema2ManifestHandler) verifyManifest(ctx context.Context, mnfst sche break } } - case schema2.MediaTypeManifest, schema1.MediaTypeManifest: //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. + case schema2.MediaTypeManifest: var exists bool exists, err = manifestService.Exists(ctx, descriptor.Digest) if err != nil || !exists { diff --git a/registry/storage/signedmanifesthandler.go b/registry/storage/signedmanifesthandler.go deleted file mode 100644 index edb32e07..00000000 --- a/registry/storage/signedmanifesthandler.go +++ /dev/null @@ -1,142 +0,0 @@ -package storage - -import ( - "context" - "encoding/json" - "fmt" - - "github.com/distribution/distribution/v3" - dcontext "github.com/distribution/distribution/v3/context" - "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - "github.com/distribution/distribution/v3/reference" - "github.com/docker/libtrust" - "github.com/opencontainers/go-digest" -) - -// signedManifestHandler is a ManifestHandler that covers schema1 manifests. It -// can unmarshal and put schema1 manifests that have been signed by libtrust. -type signedManifestHandler struct { - repository distribution.Repository - schema1SigningKey libtrust.PrivateKey - blobStore distribution.BlobStore - ctx context.Context -} - -var _ ManifestHandler = &signedManifestHandler{} - -func (ms *signedManifestHandler) Unmarshal(ctx context.Context, dgst digest.Digest, content []byte) (distribution.Manifest, error) { - dcontext.GetLogger(ms.ctx).Debug("(*signedManifestHandler).Unmarshal") - - var ( - signatures [][]byte - err error - ) - - jsig, err := libtrust.NewJSONSignature(content, signatures...) - if err != nil { - return nil, err - } - - if ms.schema1SigningKey != nil { - if err := jsig.Sign(ms.schema1SigningKey); err != nil { - return nil, err - } - } - - // Extract the pretty JWS - raw, err := jsig.PrettySignature("signatures") - if err != nil { - return nil, err - } - - var sm schema1.SignedManifest //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - if err := json.Unmarshal(raw, &sm); err != nil { - return nil, err - } - return &sm, nil -} - -func (ms *signedManifestHandler) Put(ctx context.Context, manifest distribution.Manifest, skipDependencyVerification bool) (digest.Digest, error) { - dcontext.GetLogger(ms.ctx).Debug("(*signedManifestHandler).Put") - - sm, ok := manifest.(*schema1.SignedManifest) //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - if !ok { - return "", fmt.Errorf("non-schema1 manifest put to signedManifestHandler: %T", manifest) - } - - if err := ms.verifyManifest(ms.ctx, *sm, skipDependencyVerification); err != nil { - return "", err - } - - mt := schema1.MediaTypeManifest //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - payload := sm.Canonical - - revision, err := ms.blobStore.Put(ctx, mt, payload) - if err != nil { - dcontext.GetLogger(ctx).Errorf("error putting payload into blobstore: %v", err) - return "", err - } - - return revision.Digest, nil -} - -// verifyManifest ensures that the manifest content is valid from the -// perspective of the registry. It ensures that the signature is valid for the -// enclosed payload. As a policy, the registry only tries to store valid -// content, leaving trust policies of that content up to consumers. -func (ms *signedManifestHandler) verifyManifest(ctx context.Context, mnfst schema1.SignedManifest, skipDependencyVerification bool) error { //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - var errs distribution.ErrManifestVerification - - if len(mnfst.Name) > reference.NameTotalLengthMax { - errs = append(errs, - distribution.ErrManifestNameInvalid{ - Name: mnfst.Name, - Reason: fmt.Errorf("manifest name must not be more than %v characters", reference.NameTotalLengthMax), - }) - } - - if !reference.NameRegexp.MatchString(mnfst.Name) { - errs = append(errs, - distribution.ErrManifestNameInvalid{ - Name: mnfst.Name, - Reason: fmt.Errorf("invalid manifest name format"), - }) - } - - if len(mnfst.History) != len(mnfst.FSLayers) { - errs = append(errs, fmt.Errorf("mismatched history and fslayer cardinality %d != %d", - len(mnfst.History), len(mnfst.FSLayers))) - } - - if _, err := schema1.Verify(&mnfst); err != nil { //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - switch err { - case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey: - errs = append(errs, distribution.ErrManifestUnverified{}) - default: - if err.Error() == "invalid signature" { // TODO(stevvooe): This should be exported by libtrust - errs = append(errs, distribution.ErrManifestUnverified{}) - } else { - errs = append(errs, err) - } - } - } - - if !skipDependencyVerification { - for _, fsLayer := range mnfst.References() { //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. - _, err := ms.repository.Blobs(ctx).Stat(ctx, fsLayer.Digest) - if err != nil { - if err != distribution.ErrBlobUnknown { - errs = append(errs, err) - } - - // On error here, we always append unknown blob errors. - errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest}) - } - } - } - if len(errs) != 0 { - return errs - } - - return nil -} diff --git a/registry/storage/v1unsupportedhandler.go b/registry/storage/v1unsupportedhandler.go deleted file mode 100644 index d6ed95fe..00000000 --- a/registry/storage/v1unsupportedhandler.go +++ /dev/null @@ -1,24 +0,0 @@ -package storage - -import ( - "context" - - "github.com/distribution/distribution/v3" - 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 -} diff --git a/testutil/manifests.go b/testutil/manifests.go index 3a6a333b..9501aace 100644 --- a/testutil/manifests.go +++ b/testutil/manifests.go @@ -5,11 +5,8 @@ import ( "github.com/distribution/distribution/v3" "github.com/distribution/distribution/v3/context" - "github.com/distribution/distribution/v3/manifest" "github.com/distribution/distribution/v3/manifest/manifestlist" - "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck // Ignore SA1019: "github.com/distribution/distribution/v3/manifest/schema1" is deprecated, as it's used for backward compatibility. "github.com/distribution/distribution/v3/manifest/schema2" - "github.com/docker/libtrust" "github.com/opencontainers/go-digest" ) @@ -39,38 +36,6 @@ func MakeManifestList(blobstatter distribution.BlobStatter, manifestDigests []di return manifestlist.FromDescriptors(manifestDescriptors) } -// MakeSchema1Manifest constructs a schema 1 manifest from a given list of digests and returns -// the digest of the manifest. -// -// Deprecated: Docker Image Manifest v2, Schema 1 is deprecated since 2015. -// Use Docker Image Manifest v2, Schema 2, or the OCI Image Specification. -func MakeSchema1Manifest(digests []digest.Digest) (*schema1.SignedManifest, error) { - mfst := schema1.Manifest{ - Versioned: manifest.Versioned{ - SchemaVersion: 1, - }, - Name: "who", - Tag: "cares", - } - - for _, d := range digests { - mfst.FSLayers = append(mfst.FSLayers, schema1.FSLayer{BlobSum: d}) - mfst.History = append(mfst.History, schema1.History{V1Compatibility: ""}) - } - - pk, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - return nil, fmt.Errorf("unexpected error generating private key: %v", err) - } - - signedManifest, err := schema1.Sign(&mfst, pk) - if err != nil { - return nil, fmt.Errorf("error signing manifest: %v", err) - } - - return signedManifest, nil -} - // MakeSchema2Manifest constructs a schema 2 manifest from a given list of digests and returns // the digest of the manifest func MakeSchema2Manifest(repository distribution.Repository, digests []digest.Digest) (distribution.Manifest, error) { From c7bdabadcf34334368d80aaf1ea9495a3118ee94 Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 28 Aug 2023 12:44:49 +0200 Subject: [PATCH 2/2] add back getKeys + cleanup manifeststore test Signed-off-by: David van der Spek --- registry/storage/garbagecollect_test.go | 35 ++++++++----------------- registry/storage/manifeststore_test.go | 28 +++++++------------- 2 files changed, 20 insertions(+), 43 deletions(-) diff --git a/registry/storage/garbagecollect_test.go b/registry/storage/garbagecollect_test.go index 35e6cd71..695dbe9d 100644 --- a/registry/storage/garbagecollect_test.go +++ b/registry/storage/garbagecollect_test.go @@ -186,21 +186,11 @@ func TestDeleteManifestIfTagNotFound(t *testing.T) { t.Fatalf("failed to make layers: %v", err) } - digests1 := []digest.Digest{} - for digest := range randomLayers1 { - digests1 = append(digests1, digest) - } - randomLayers2, err := testutil.CreateRandomLayers(3) if err != nil { t.Fatalf("failed to make layers: %v", err) } - digests2 := []digest.Digest{} - for digest := range randomLayers2 { - digests2 = append(digests2, digest) - } - // Upload all layers err = testutil.UploadBlobs(repo, randomLayers1) if err != nil { @@ -213,12 +203,12 @@ func TestDeleteManifestIfTagNotFound(t *testing.T) { } // Construct manifests - manifest1, err := testutil.MakeSchema2Manifest(repo, digests1) + manifest1, err := testutil.MakeSchema2Manifest(repo, getKeys(randomLayers1)) if err != nil { t.Fatalf("failed to make manifest: %v", err) } - manifest2, err := testutil.MakeSchema2Manifest(repo, digests2) + manifest2, err := testutil.MakeSchema2Manifest(repo, getKeys(randomLayers2)) if err != nil { t.Fatalf("failed to make manifest: %v", err) } @@ -370,6 +360,13 @@ func getAnyKey(digests map[digest.Digest]io.ReadSeeker) (d digest.Digest) { return } +func getKeys(digests map[digest.Digest]io.ReadSeeker) (ds []digest.Digest) { + for d := range digests { + ds = append(ds, d) + } + return +} + func TestDeletionWithSharedLayer(t *testing.T) { ctx := context.Background() inmemoryDriver := inmemory.New() @@ -383,21 +380,11 @@ func TestDeletionWithSharedLayer(t *testing.T) { t.Fatalf("failed to make layers: %v", err) } - digests1 := []digest.Digest{} - for digest := range randomLayers1 { - digests1 = append(digests1, digest) - } - randomLayers2, err := testutil.CreateRandomLayers(3) if err != nil { t.Fatalf("failed to make layers: %v", err) } - digests2 := []digest.Digest{} - for digest := range randomLayers2 { - digests2 = append(digests2, digest) - } - // Upload all layers err = testutil.UploadBlobs(repo, randomLayers1) if err != nil { @@ -410,13 +397,13 @@ func TestDeletionWithSharedLayer(t *testing.T) { } // Construct manifests - manifest1, err := testutil.MakeSchema2Manifest(repo, digests1) + manifest1, err := testutil.MakeSchema2Manifest(repo, getKeys(randomLayers1)) if err != nil { t.Fatalf("failed to make manifest: %v", err) } sharedKey := getAnyKey(randomLayers1) - manifest2, err := testutil.MakeSchema2Manifest(repo, append(digests2, sharedKey)) + manifest2, err := testutil.MakeSchema2Manifest(repo, append(getKeys(randomLayers2), sharedKey)) if err != nil { t.Fatalf("failed to make manifest: %v", err) } diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index 54e19913..3285d541 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -98,27 +98,12 @@ func testManifestStorage(t *testing.T, options ...RegistryOption) { MediaType: schema2.MediaTypeManifest, }, Config: distribution.Descriptor{ - Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", - Size: 3253, + Digest: digest.FromBytes(sampleConfig), + Size: int64(len(sampleConfig)), MediaType: schema2.MediaTypeImageConfig, }, - Layers: []distribution.Descriptor{ - { - Digest: "sha256:463434349086340864309863409683460843608348608934092322395278926a", - Size: 6323, - MediaType: schema2.MediaTypeLayer, - }, - { - Digest: "sha256:630923423623623423352523525237238023652897356239852383652aaaaaaa", - Size: 6863, - MediaType: schema2.MediaTypeLayer, - }, - }, + Layers: []distribution.Descriptor{}, } - sampleConfigDigest := digest.FromBytes(sampleConfig) - sampleConfigSize := int64(len(sampleConfig)) - m.Config.Digest = sampleConfigDigest - m.Config.Size = sampleConfigSize // Build up some test layers and add them to the manifest, saving the // readseekers for upload later. @@ -130,7 +115,12 @@ func testManifestStorage(t *testing.T, options ...RegistryOption) { } testLayers[dgst] = rs - m.Layers[i].Digest = dgst + layer := distribution.Descriptor{ + Digest: dgst, + Size: 6323, + MediaType: schema2.MediaTypeLayer, + } + m.Layers = append(m.Layers, layer) } // Now, upload the layers that were missing!