diff --git a/.github/workflows/oci-conformance-action.yml b/.github/workflows/oci-conformance-action.yml index be816301..021dc6ef 100644 --- a/.github/workflows/oci-conformance-action.yml +++ b/.github/workflows/oci-conformance-action.yml @@ -38,8 +38,8 @@ jobs: with: # TODO: change to upstream once the foloowing PR is merged: # https://github.com/opencontainers/distribution-spec/pull/436 - repository: sudo-bmitch/distribution-spec - ref: pr-conformance-index-subject + repository: opencontainers/distribution-spec + ref: main path: distribution-spec - name: build conformance binary from main run: | diff --git a/go.sum b/go.sum index 4da711d0..ee527097 100644 --- a/go.sum +++ b/go.sum @@ -1460,6 +1460,7 @@ github.com/rs/zerolog v1.29.1 h1:cO+d60CHkknCbvzEWxP0S9K6KqyTjrCNUy1LdQLCGPc= github.com/rs/zerolog v1.29.1/go.mod h1:Le6ESbR7hc+DP6Lt1THiV8CQSdkkNrd3R0XbEgp3ZBU= github.com/rubenv/sql-migrate v1.2.0 h1:fOXMPLMd41sK7Tg75SXDec15k3zg5WNV6SjuDRiNfcU= github.com/rubenv/sql-migrate v1.2.0/go.mod h1:Z5uVnq7vrIrPmHbVFfR4YLHRZquxeHpckCnRq0P/K9Y= +github.com/russross/blackfriday v1.6.0 h1:KqfZb0pUVN2lYqZUYRddxF4OR8ZMURnJIG5Y3VRLtww= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 2ff28f6d..c1aa65c9 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -5443,6 +5443,239 @@ func TestImageSignatures(t *testing.T) { }) } +func TestManifestValidation(t *testing.T) { + Convey("Validate manifest", t, func() { + // start a new server + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + + conf := config.New() + conf.HTTP.Port = port + + dir := t.TempDir() + ctlr := makeController(conf, dir, "") + cm := test.NewControllerManager(ctlr) + // this blocks + cm.StartServer() + time.Sleep(1000 * time.Millisecond) + defer cm.StopServer() + + repoName := "validation" + blobContent := []byte("this is a blob") + blobDigest := godigest.FromBytes(blobContent) + So(blobDigest, ShouldNotBeNil) + + cfg, layers, manifest, err := test.GetImageComponents(2) + So(err, ShouldBeNil) + + err = test.UploadImage( + test.Image{ + Config: cfg, + Layers: layers, + Manifest: manifest, + Reference: "1.0", + }, baseURL, repoName) + So(err, ShouldBeNil) + + content, err := json.Marshal(manifest) + So(err, ShouldBeNil) + digest := godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + + configBlob, err := json.Marshal(cfg) + So(err, ShouldBeNil) + + configDigest := godigest.FromBytes(configBlob) + + Convey("empty layers should pass validation", func() { + // create a manifest + manifest := ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: ispec.MediaTypeImageConfig, + Size: int64(len(configBlob)), + Digest: configDigest, + }, + Layers: []ispec.Descriptor{}, + Annotations: map[string]string{ + "key": "val", + }, + } + manifest.SchemaVersion = 2 + + mcontent, err := json.Marshal(manifest) + So(err, ShouldBeNil) + + resp, err := resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + }) + + Convey("empty layers and schemaVersion missing should fail validation", func() { + // create a manifest + manifest := ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: ispec.MediaTypeImageConfig, + Size: int64(len(configBlob)), + Digest: configDigest, + }, + Layers: []ispec.Descriptor{}, + Annotations: map[string]string{ + "key": "val", + }, + } + + mcontent, err := json.Marshal(manifest) + So(err, ShouldBeNil) + + resp, err := resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + }) + + Convey("missing layer should fail validation", func() { + missingLayer := []byte("missing layer") + missingLayerDigest := godigest.FromBytes(missingLayer) + // create a manifest + manifest := ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: ispec.MediaTypeImageConfig, + Size: int64(len(configBlob)), + Digest: configDigest, + }, + Layers: []ispec.Descriptor{ + { + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: digest, + Size: int64(len(content)), + }, + { + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: missingLayerDigest, + Size: int64(len(missingLayer)), + }, + }, + Annotations: map[string]string{ + "key": "val", + }, + } + manifest.SchemaVersion = 2 + + mcontent, err := json.Marshal(manifest) + So(err, ShouldBeNil) + + resp, err := resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + }) + + Convey("wrong mediatype should fail validation", func() { + // create a manifest + manifest := ispec.Manifest{ + MediaType: "bad.mediatype", + Config: ispec.Descriptor{ + MediaType: ispec.MediaTypeImageConfig, + Size: int64(len(configBlob)), + Digest: configDigest, + }, + Layers: []ispec.Descriptor{ + { + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: digest, + Size: int64(len(content)), + }, + }, + Annotations: map[string]string{ + "key": "val", + }, + } + manifest.SchemaVersion = 2 + + mcontent, err := json.Marshal(manifest) + So(err, ShouldBeNil) + + resp, err := resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + }) + + Convey("multiarch image should pass validation", func() { + index := ispec.Index{ + MediaType: ispec.MediaTypeImageIndex, + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: digest, + Size: int64(len((content))), + }, + }, + } + + index.SchemaVersion = 2 + + indexContent, err := json.Marshal(index) + So(err, ShouldBeNil) + + resp, err := resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex). + SetBody(indexContent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/index", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + }) + + Convey("multiarch image without schemaVersion should fail validation", func() { + index := ispec.Index{ + MediaType: ispec.MediaTypeImageIndex, + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: digest, + Size: int64(len((content))), + }, + }, + } + + indexContent, err := json.Marshal(index) + So(err, ShouldBeNil) + + resp, err := resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex). + SetBody(indexContent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/index", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + }) + + Convey("multiarch image with missing manifest should fail validation", func() { + index := ispec.Index{ + MediaType: ispec.MediaTypeImageIndex, + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: digest, + Size: int64(len((content))), + }, + { + MediaType: ispec.MediaTypeImageManifest, + Digest: godigest.FromString("missing layer"), + Size: 10, + }, + }, + } + + index.SchemaVersion = 2 + + indexContent, err := json.Marshal(index) + So(err, ShouldBeNil) + + resp, err := resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex). + SetBody(indexContent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/index", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + }) + }) +} + func TestArtifactReferences(t *testing.T) { Convey("Validate Artifact References", t, func() { // start a new server @@ -5557,7 +5790,113 @@ func TestArtifactReferences(t *testing.T) { Get(baseURL + fmt.Sprintf("/v2/%s/referrers/%s", repoName, digest.String())) So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // create a bad manifest (constructed manually) + content := `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","subject":{"mediaType":"application/vnd.oci.image.manifest.v1+json","digest":"sha256:71dbae9d7e6445fb5e0b11328e941b8e8937fdd52465079f536ce44bb78796ed","size":406}}` //nolint: lll + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(content).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + + // missing layers + mcontent := []byte("this is a missing blob") + digest = godigest.FromBytes(mcontent) + So(digest, ShouldNotBeNil) + + manifest.Layers = append(manifest.Layers, ispec.Descriptor{ + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: digest, + Size: int64(len(mcontent)), + }) + + mcontent, err = json.Marshal(manifest) + So(err, ShouldBeNil) + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + + // invalid schema version + manifest.SchemaVersion = 1 + + mcontent, err = json.Marshal(manifest) + So(err, ShouldBeNil) + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + + // upload image config blob + resp, err = resty.R().Post(baseURL + fmt.Sprintf("/v2/%s/blobs/uploads/", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) + loc := test.Location(baseURL, resp) + cblob = []byte("{}") + cdigest = godigest.FromBytes(cblob) + So(cdigest, ShouldNotBeNil) + + resp, err = resty.R(). + SetContentLength(true). + SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))). + SetHeader("Content-Type", "application/octet-stream"). + SetQueryParam("digest", cdigest.String()). + SetBody(cblob). + Put(loc) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + + manifest := ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: "application/vnd.oci.image.config.v1+json", + Digest: cdigest, + Size: int64(len(cblob)), + }, + } + + manifest.SchemaVersion = 2 + mcontent, err = json.Marshal(manifest) + So(err, ShouldBeNil) + digest = godigest.FromBytes(mcontent) + So(digest, ShouldNotBeNil) + + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + + // missing layers + mcontent = []byte("this is a missing blob") + digest = godigest.FromBytes(mcontent) + So(digest, ShouldNotBeNil) + + manifest.Layers = append(manifest.Layers, ispec.Descriptor{ + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: digest, + Size: int64(len(mcontent)), + }) + + mcontent, err = json.Marshal(manifest) + So(err, ShouldBeNil) + + // should fail because config is of type image and blob is not uploaded + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + + // no layers at all + manifest.Layers = []ispec.Descriptor{} + + mcontent, err = json.Marshal(manifest) + So(err, ShouldBeNil) + + // should not fail + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageManifest). + SetBody(mcontent).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/1.0", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) }) + Convey("Using valid content", func() { content, err = json.Marshal(manifest) So(err, ShouldBeNil) diff --git a/pkg/cli/image_cmd_test.go b/pkg/cli/image_cmd_test.go index b7f6e1ac..7c338986 100644 --- a/pkg/cli/image_cmd_test.go +++ b/pkg/cli/image_cmd_test.go @@ -1503,11 +1503,11 @@ func runDisplayIndexTests(baseURL string) { actual := strings.TrimSpace(str) // Actual cli output should be something similar to (order of images may differ): // REPOSITORY TAG OS/ARCH DIGEST SIGNED SIZE - // repo multi-arch * 59b25ae4 false 1.5kB + // repo multi-arch * 0f844b3e false 1.5kB // linux/amd64 97b0d65c false 634B // windows/arm64/v6 dcfa3a9c false 444B So(actual, ShouldContainSubstring, "REPOSITORY TAG OS/ARCH DIGEST SIGNED SIZE") - So(actual, ShouldContainSubstring, "repo multi-arch * 59b25ae4 false 1.5kB ") + So(actual, ShouldContainSubstring, "repo multi-arch * 0f844b3e false 1.5kB ") So(actual, ShouldContainSubstring, "linux/amd64 2ab1a275 false 634B ") So(actual, ShouldContainSubstring, "windows/arm64/v6 55fdd23a false 501B") }) @@ -1531,14 +1531,14 @@ func runDisplayIndexTests(baseURL string) { actual := strings.TrimSpace(str) // Actual cli output should be something similar to (order of images may differ): // REPOSITORY TAG OS/ARCH DIGEST CONFIG SIGNED LAYERS SIZE - // repo multi-arch * 59b25ae4 false 1.5kB + // repo multi-arch * 0f844b3e false 1.5kB // linux/amd64 2ab1a275 58cc9abe false 634B // cbb5b121 4B // a00291e8 4B // windows/arm64/v6 55fdd23a 5132a1cd false 501B // 7d08ce29 4B So(actual, ShouldContainSubstring, "REPOSITORY TAG OS/ARCH DIGEST CONFIG SIGNED LAYERS SIZE") - So(actual, ShouldContainSubstring, "repo multi-arch * 59b25ae4 false 1.5kB") + So(actual, ShouldContainSubstring, "repo multi-arch * 0f844b3e false 1.5kB") So(actual, ShouldContainSubstring, "linux/amd64 2ab1a275 58cc9abe false 634B") So(actual, ShouldContainSubstring, "cbb5b121 4B") So(actual, ShouldContainSubstring, "a00291e8 4B") diff --git a/pkg/cli/search_cmd_referrers_test.go b/pkg/cli/search_cmd_referrers_test.go index a8fe2367..09bed5da 100644 --- a/pkg/cli/search_cmd_referrers_test.go +++ b/pkg/cli/search_cmd_referrers_test.go @@ -28,8 +28,8 @@ func ref[T any](input T) *T { } const ( - customArtTypeV1 = "custom.art.type.v1" - customArtTypeV2 = "custom.art.type.v2" + customArtTypeV1 = "application/custom.art.type.v1" + customArtTypeV2 = "application/custom.art.type.v2" repoName = "repo" ) @@ -263,8 +263,8 @@ func TestReferrerCLI(t *testing.T) { str := strings.TrimSpace(space.ReplaceAllString(buff.String(), " ")) So(str, ShouldContainSubstring, "ARTIFACT TYPE SIZE DIGEST") So(str, ShouldContainSubstring, "application/vnd.oci.image.config.v1+json 557 B "+ref1Digest.String()) - So(str, ShouldContainSubstring, "custom.art.type.v1 535 B "+ref2Digest.String()) - So(str, ShouldContainSubstring, "custom.art.type.v2 598 B "+ref3Digest.String()) + So(str, ShouldContainSubstring, "application/custom.art.type.v1 547 B "+ref2Digest.String()) + So(str, ShouldContainSubstring, "application/custom.art.type.v2 610 B "+ref3Digest.String()) fmt.Println(buff.String()) @@ -287,8 +287,8 @@ func TestReferrerCLI(t *testing.T) { str = strings.TrimSpace(space.ReplaceAllString(buff.String(), " ")) So(str, ShouldContainSubstring, "ARTIFACT TYPE SIZE DIGEST") So(str, ShouldContainSubstring, "application/vnd.oci.image.config.v1+json 557 B "+ref1Digest.String()) - So(str, ShouldContainSubstring, "custom.art.type.v1 535 B "+ref2Digest.String()) - So(str, ShouldContainSubstring, "custom.art.type.v2 598 B "+ref3Digest.String()) + So(str, ShouldContainSubstring, "application/custom.art.type.v1 547 B "+ref2Digest.String()) + So(str, ShouldContainSubstring, "application/custom.art.type.v2 610 B "+ref3Digest.String()) fmt.Println(buff.String()) }) @@ -370,8 +370,8 @@ func TestReferrerCLI(t *testing.T) { str := strings.TrimSpace(space.ReplaceAllString(buff.String(), " ")) So(str, ShouldContainSubstring, "ARTIFACT TYPE SIZE DIGEST") So(str, ShouldContainSubstring, "application/vnd.oci.image.config.v1+json 557 B "+ref1Digest.String()) - So(str, ShouldContainSubstring, "custom.art.type.v1 535 B "+ref2Digest.String()) - So(str, ShouldContainSubstring, "custom.art.type.v2 598 B "+ref3Digest.String()) + So(str, ShouldContainSubstring, "application/custom.art.type.v1 547 B "+ref2Digest.String()) + So(str, ShouldContainSubstring, "application/custom.art.type.v2 610 B "+ref3Digest.String()) fmt.Println(buff.String()) os.Remove(configPath) @@ -391,8 +391,8 @@ func TestReferrerCLI(t *testing.T) { str = strings.TrimSpace(space.ReplaceAllString(buff.String(), " ")) So(str, ShouldContainSubstring, "ARTIFACT TYPE SIZE DIGEST") So(str, ShouldContainSubstring, "application/vnd.oci.image.config.v1+json 557 B "+ref1Digest.String()) - So(str, ShouldContainSubstring, "custom.art.type.v1 535 B "+ref2Digest.String()) - So(str, ShouldContainSubstring, "custom.art.type.v2 598 B "+ref3Digest.String()) + So(str, ShouldContainSubstring, "application/custom.art.type.v1 547 B "+ref2Digest.String()) + So(str, ShouldContainSubstring, "application/custom.art.type.v2 610 B "+ref3Digest.String()) fmt.Println(buff.String()) }) } diff --git a/pkg/extensions/search/search_test.go b/pkg/extensions/search/search_test.go index d61247e4..7d678b12 100644 --- a/pkg/extensions/search/search_test.go +++ b/pkg/extensions/search/search_test.go @@ -876,7 +876,7 @@ func TestGetReferrersGQL(t *testing.T) { artifactContentBlobSize := int64(len(artifactContentBlob)) artifactContentType := "application/octet-stream" artifactContentBlobDigest := godigest.FromBytes(artifactContentBlob) - artifactType := "com.artifact.test" + artifactType := "com.artifact.test/type1" artifactImg := Image{ Manifest: ispec.Manifest{ @@ -903,6 +903,8 @@ func TestGetReferrersGQL(t *testing.T) { Layers: [][]byte{artifactContentBlob}, } + artifactImg.Manifest.SchemaVersion = 2 + artifactManifestBlob, err := json.Marshal(artifactImg.Manifest) So(err, ShouldBeNil) artifactManifestDigest := godigest.FromBytes(artifactManifestBlob) @@ -1001,7 +1003,7 @@ func TestGetReferrersGQL(t *testing.T) { artifactContentBlobSize := int64(len(artifactContentBlob)) artifactContentType := "application/octet-stream" artifactContentBlobDigest := godigest.FromBytes(artifactContentBlob) - artifactType := "com.artifact.test" + artifactType := "com.artifact.test/type2" configBlob, err := json.Marshal(ispec.Image{}) So(err, ShouldBeNil) @@ -1025,6 +1027,8 @@ func TestGetReferrersGQL(t *testing.T) { }, } + artifactManifest.SchemaVersion = 2 + artifactManifestBlob, err := json.Marshal(artifactManifest) So(err, ShouldBeNil) artifactManifestDigest := godigest.FromBytes(artifactManifestBlob) @@ -1117,7 +1121,7 @@ func TestGetReferrersGQL(t *testing.T) { indexReferrer, err := GetRandomMultiarchImage("ref") So(err, ShouldBeNil) - artifactType := "art.type" + artifactType := "com.artifact.art/type" indexReferrer.Index.ArtifactType = artifactType indexReferrer.Index.Subject = &ispec.Descriptor{ MediaType: ispec.MediaTypeImageManifest, @@ -6202,7 +6206,7 @@ func TestImageSummary(t *testing.T) { Digest: manifestDigest, MediaType: ispec.MediaTypeImageManifest, } - referrerImage.Manifest.Config.MediaType = "test.artifact.type" + referrerImage.Manifest.Config.MediaType = "application/test.artifact.type" referrerImage.Manifest.Annotations = map[string]string{"testAnnotationKey": "testAnnotationValue"} referrerManifestDigest, err := referrerImage.Digest() So(err, ShouldBeNil) @@ -6277,7 +6281,7 @@ func TestImageSummary(t *testing.T) { So(len(imgSummary.Referrers), ShouldEqual, 1) So(imgSummary.Referrers[0], ShouldResemble, zcommon.Referrer{ MediaType: ispec.MediaTypeImageManifest, - ArtifactType: "test.artifact.type", + ArtifactType: "application/test.artifact.type", Digest: referrerManifestDigest.String(), Annotations: []zcommon.Annotation{{Key: "testAnnotationKey", Value: "testAnnotationValue"}}, }) diff --git a/pkg/extensions/sync/sync_test.go b/pkg/extensions/sync/sync_test.go index 1d15bd17..754a5203 100644 --- a/pkg/extensions/sync/sync_test.go +++ b/pkg/extensions/sync/sync_test.go @@ -23,6 +23,7 @@ import ( dockerManifest "github.com/containers/image/v5/manifest" notreg "github.com/notaryproject/notation-go/registry" godigest "github.com/opencontainers/go-digest" + "github.com/opencontainers/image-spec/specs-go" ispec "github.com/opencontainers/image-spec/specs-go/v1" artifactspec "github.com/oras-project/artifacts-spec/specs-go/v1" "github.com/sigstore/cosign/v2/cmd/cosign/cli/attach" @@ -409,6 +410,9 @@ func TestORAS(t *testing.T) { _ = pushBlob(srcBaseURL, repoName, ispec.DescriptorEmptyJSON.Data) artifactManifest := ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, MediaType: artifactspec.MediaTypeArtifactManifest, ArtifactType: "application/vnd.oras.artifact", Layers: []ispec.Descriptor{ @@ -743,6 +747,9 @@ func TestOnDemand(t *testing.T) { _ = pushBlob(srcBaseURL, "remote-repo", ispec.DescriptorEmptyJSON.Data) OCIRefManifest := ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, Subject: &ispec.Descriptor{ MediaType: ispec.MediaTypeImageManifest, Digest: manifestDigest, @@ -954,6 +961,9 @@ func TestSyncReferenceInLoop(t *testing.T) { _ = pushBlob(srcBaseURL, testImage, ispec.DescriptorEmptyJSON.Data) OCIRefManifest := ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, Subject: &ispec.Descriptor{ MediaType: ispec.MediaTypeImageManifest, Digest: sbomDigest, @@ -3896,6 +3906,9 @@ func TestSignatures(t *testing.T) { _ = pushBlob(srcBaseURL, repoName, ispec.DescriptorEmptyJSON.Data) OCIRefManifest := ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, Subject: &ispec.Descriptor{ MediaType: ispec.MediaTypeImageManifest, Digest: sbomDigest, @@ -6357,6 +6370,10 @@ func pushRepo(url, repoName string) godigest.Digest { // create a manifest manifest := ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, + MediaType: ispec.MediaTypeImageManifest, Config: ispec.Descriptor{ MediaType: "application/vnd.oci.image.config.v1+json", Digest: cdigest, @@ -6396,6 +6413,9 @@ func pushRepo(url, repoName string) godigest.Digest { // push a referrer artifact manifest = ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, MediaType: ispec.MediaTypeImageManifest, Config: ispec.Descriptor{ MediaType: "application/vnd.cncf.icecream", @@ -6417,6 +6437,9 @@ func pushRepo(url, repoName string) godigest.Digest { } artifactManifest := ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, MediaType: ispec.MediaTypeImageManifest, ArtifactType: "application/vnd.cncf.icecream", Config: ispec.Descriptor{ diff --git a/pkg/storage/common/common.go b/pkg/storage/common/common.go index 0bf4b0af..ea73c5dc 100644 --- a/pkg/storage/common/common.go +++ b/pkg/storage/common/common.go @@ -1,6 +1,7 @@ package storage import ( + "bytes" "encoding/json" "errors" "path" @@ -8,6 +9,7 @@ import ( notreg "github.com/notaryproject/notation-go/registry" godigest "github.com/opencontainers/go-digest" + "github.com/opencontainers/image-spec/schema" imeta "github.com/opencontainers/image-spec/specs-go" ispec "github.com/opencontainers/image-spec/specs-go/v1" oras "github.com/oras-project/artifacts-spec/specs-go/v1" @@ -21,6 +23,8 @@ import ( storageTypes "zotregistry.io/zot/pkg/storage/types" ) +const manifestWithEmptyLayersErrMsg = "layers: Array must have at least 1 items" + func GetTagsByIndex(index ispec.Index) []string { tags := make([]string, 0) @@ -71,6 +75,14 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy switch mediaType { case ispec.MediaTypeImageManifest: var manifest ispec.Manifest + + // validate manifest + if err := ValidateManifestSchema(body); err != nil { + log.Error().Err(err).Msg("OCIv1 image manifest schema validation failed") + + return "", zerr.ErrBadManifest + } + if err := json.Unmarshal(body, &manifest); err != nil { log.Error().Err(err).Msg("unable to unmarshal JSON") @@ -78,20 +90,21 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy } if manifest.Config.MediaType == ispec.MediaTypeImageConfig { - digest, err := validateOCIManifest(imgStore, repo, reference, &manifest, log) - if err != nil { - log.Error().Err(err).Msg("invalid oci image manifest") + // validate layers - a lightweight check if the blob is present + for _, layer := range manifest.Layers { + if IsNonDistributable(layer.MediaType) { + log.Debug().Str("digest", layer.Digest.String()).Str("mediaType", layer.MediaType). + Msg("skip checking non-distributable layer exists") - return digest, err - } - } + continue + } - if manifest.Subject != nil { - var m ispec.Descriptor - if err := json.Unmarshal(body, &m); err != nil { - log.Error().Err(err).Msg("unable to unmarshal JSON") + ok, _, err := imgStore.StatBlob(repo, layer.Digest) + if !ok || err != nil { + log.Error().Err(err).Str("digest", layer.Digest.String()).Msg("missing layer blob") - return "", zerr.ErrBadManifest + return "", zerr.ErrBadManifest + } } } case oras.MediaTypeArtifactManifest: @@ -101,46 +114,27 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy return "", zerr.ErrBadManifest } - } + case ispec.MediaTypeImageIndex: + // validate manifest + if err := ValidateImageIndexSchema(body); err != nil { + log.Error().Err(err).Msg("OCIv1 image index manifest schema validation failed") - return "", nil -} - -func validateOCIManifest(imgStore storageTypes.ImageStore, repo, reference string, //nolint:unparam - manifest *ispec.Manifest, log zerolog.Logger, -) (godigest.Digest, error) { - if manifest.SchemaVersion != storageConstants.SchemaVersion { - log.Error().Int("SchemaVersion", manifest.SchemaVersion).Msg("invalid manifest") - - return "", zerr.ErrBadManifest - } - - // validate image config - config := manifest.Config - - blobBuf, err := imgStore.GetBlobContent(repo, config.Digest) - if err != nil { - return config.Digest, zerr.ErrBlobNotFound - } - - var cspec ispec.Image - - err = json.Unmarshal(blobBuf, &cspec) - if err != nil { - return "", zerr.ErrBadManifest - } - - // validate the layers - for _, layer := range manifest.Layers { - if IsNonDistributable(layer.MediaType) { - log.Warn().Str("digest", layer.Digest.String()).Str("mediaType", layer.MediaType).Msg("not validating layer exists") - - continue + return "", err } - ok, _, err := imgStore.StatBlob(repo, layer.Digest) - if err != nil || !ok { - return layer.Digest, zerr.ErrBlobNotFound + var indexManifest ispec.Index + if err := json.Unmarshal(body, &indexManifest); err != nil { + log.Error().Err(err).Msg("unable to unmarshal JSON") + + return "", zerr.ErrBadManifest + } + + for _, manifest := range indexManifest.Manifests { + if ok, _, err := imgStore.StatBlob(repo, manifest.Digest); !ok || err != nil { + log.Error().Err(err).Str("digest", manifest.Digest.String()).Msg("missing manifest blob") + + return "", zerr.ErrBadManifest + } } } @@ -777,6 +771,37 @@ func IsNonDistributable(mediaType string) bool { mediaType == ispec.MediaTypeImageLayerNonDistributableZstd //nolint:staticcheck } +func ValidateManifestSchema(buf []byte) error { + if err := schema.ValidatorMediaTypeManifest.Validate(bytes.NewBuffer(buf)); err != nil { + if !IsEmptyLayersError(err) { + return zerr.ErrBadManifest + } + } + + return nil +} + +func ValidateImageIndexSchema(buf []byte) error { + if err := schema.ValidatorMediaTypeImageIndex.Validate(bytes.NewBuffer(buf)); err != nil { + return zerr.ErrBadManifest + } + + return nil +} + +func IsEmptyLayersError(err error) bool { + var validationErr schema.ValidationError + if errors.As(err, &validationErr) { + if len(validationErr.Errs) == 1 && strings.Contains(err.Error(), manifestWithEmptyLayersErrMsg) { + return true + } else { + return false + } + } + + return false +} + /* DedupeTaskGenerator takes all blobs paths found in the storage.imagestore and groups them by digest diff --git a/pkg/storage/local/local.go b/pkg/storage/local/local.go index e0b31776..74707952 100644 --- a/pkg/storage/local/local.go +++ b/pkg/storage/local/local.go @@ -475,11 +475,6 @@ func (is *ImageStoreLocal) PutImageManifest(repo, reference, mediaType string, / } }() - digest, err := common.ValidateManifest(is, repo, reference, mediaType, body, is.log) - if err != nil { - return digest, "", err - } - refIsDigest := true mDigest, err := common.GetAndValidateRequestDigest(body, reference, is.log) @@ -491,6 +486,11 @@ func (is *ImageStoreLocal) PutImageManifest(repo, reference, mediaType string, / refIsDigest = false } + digest, err := common.ValidateManifest(is, repo, reference, mediaType, body, is.log) + if err != nil { + return digest, "", err + } + index, err := common.GetIndex(is, repo, is.log) if err != nil { return "", "", err diff --git a/pkg/storage/s3/s3.go b/pkg/storage/s3/s3.go index 26a8ec46..b02e5d2d 100644 --- a/pkg/storage/s3/s3.go +++ b/pkg/storage/s3/s3.go @@ -386,11 +386,6 @@ func (is *ObjectStorage) PutImageManifest(repo, reference, mediaType string, //n } }() - dig, err := common.ValidateManifest(is, repo, reference, mediaType, body, is.log) - if err != nil { - return dig, "", err - } - refIsDigest := true mDigest, err := common.GetAndValidateRequestDigest(body, reference, is.log) @@ -402,6 +397,11 @@ func (is *ObjectStorage) PutImageManifest(repo, reference, mediaType string, //n refIsDigest = false } + dig, err := common.ValidateManifest(is, repo, reference, mediaType, body, is.log) + if err != nil { + return dig, "", err + } + index, err := common.GetIndex(is, repo, is.log) if err != nil { return "", "", err diff --git a/pkg/storage/s3/s3_test.go b/pkg/storage/s3/s3_test.go index 05547152..bd01938e 100644 --- a/pkg/storage/s3/s3_test.go +++ b/pkg/storage/s3/s3_test.go @@ -569,6 +569,8 @@ func TestGetOrasAndOCIReferrers(t *testing.T) { }, } + artifactManifest.SchemaVersion = 2 + manBuf, err := json.Marshal(artifactManifest) So(err, ShouldBeNil) diff --git a/pkg/test/common.go b/pkg/test/common.go index ca94b2a5..4519379a 100644 --- a/pkg/test/common.go +++ b/pkg/test/common.go @@ -51,6 +51,7 @@ import ( zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/meta/repodb" "zotregistry.io/zot/pkg/storage" + storageCommon "zotregistry.io/zot/pkg/storage/common" "zotregistry.io/zot/pkg/test/inject" ) @@ -1020,6 +1021,11 @@ func UploadImage(img Image, baseURL, repo string) error { return err } + // validate manifest + if err := storageCommon.ValidateManifestSchema(manifestBlob); err != nil { + return err + } + if img.Reference == "" { img.Reference = godigest.FromBytes(manifestBlob).String() } @@ -1886,6 +1892,8 @@ func GetRandomMultiarchImage(reference string) (MultiarchImage, error) { return MultiarchImage{}, err } + index.SchemaVersion = 2 + return MultiarchImage{ Index: index, Images: images, Reference: reference, }, err @@ -1905,6 +1913,8 @@ func GetMultiarchImageForImages(reference string, images []Image) MultiarchImage images[i].Reference = getManifestDigest(image.Manifest).String() } + index.SchemaVersion = 2 + return MultiarchImage{Index: index, Images: images, Reference: reference} } @@ -1940,6 +1950,11 @@ func UploadMultiarchImage(multiImage MultiarchImage, baseURL string, repo string return err } + // validate manifest + if err := storageCommon.ValidateImageIndexSchema(indexBlob); err != nil { + return err + } + resp, err := resty.R(). SetHeader("Content-type", ispec.MediaTypeImageIndex). SetBody(indexBlob). diff --git a/pkg/test/common_test.go b/pkg/test/common_test.go index c519ce0e..ca90c6ca 100644 --- a/pkg/test/common_test.go +++ b/pkg/test/common_test.go @@ -16,6 +16,7 @@ import ( notconfig "github.com/notaryproject/notation-go/config" godigest "github.com/opencontainers/go-digest" + "github.com/opencontainers/image-spec/specs-go" ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" "golang.org/x/crypto/bcrypt" @@ -377,7 +378,125 @@ func TestUploadBlob(t *testing.T) { }) } +func TestUploadMultiarchImage(t *testing.T) { + Convey("make controller", t, func() { + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + + conf := config.New() + conf.HTTP.Port = port + conf.Storage.RootDirectory = t.TempDir() + + ctlr := api.NewController(conf) + + ctlrManager := test.NewControllerManager(ctlr) + ctlrManager.StartAndWait(port) + defer ctlrManager.StopServer() + + layerBlob := []byte("test") + + img := test.Image{ + Layers: [][]byte{ + layerBlob, + }, + Manifest: ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, + Layers: []ispec.Descriptor{ + { + Digest: godigest.FromBytes(layerBlob), + Size: int64(len(layerBlob)), + MediaType: ispec.MediaTypeImageLayerGzip, + }, + }, + Config: ispec.DescriptorEmptyJSON, + }, + Config: ispec.Image{}, + } + + manifestBuf, err := json.Marshal(img.Manifest) + So(err, ShouldBeNil) + + Convey("Multiarch image uploaded successfully", func() { + err = test.UploadMultiarchImage(test.MultiarchImage{ + Index: ispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, + MediaType: ispec.MediaTypeImageIndex, + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: godigest.FromBytes(manifestBuf), + Size: int64(len(manifestBuf)), + }, + }, + }, + Images: []test.Image{img}, + Reference: "index", + }, baseURL, "test") + So(err, ShouldBeNil) + }) + + Convey("Multiarch image without schemaVersion should fail validation", func() { + err = test.UploadMultiarchImage(test.MultiarchImage{ + Index: ispec.Index{ + MediaType: ispec.MediaTypeImageIndex, + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: godigest.FromBytes(manifestBuf), + Size: int64(len(manifestBuf)), + }, + }, + }, + Images: []test.Image{img}, + Reference: "index", + }, baseURL, "test") + So(err, ShouldNotBeNil) + }) + }) +} + func TestUploadImage(t *testing.T) { + Convey("Manifest without schemaVersion should fail validation", t, func() { + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + + conf := config.New() + conf.HTTP.Port = port + conf.Storage.RootDirectory = t.TempDir() + + ctlr := api.NewController(conf) + + ctlrManager := test.NewControllerManager(ctlr) + ctlrManager.StartAndWait(port) + defer ctlrManager.StopServer() + + layerBlob := []byte("test") + + img := test.Image{ + Layers: [][]byte{ + layerBlob, + }, + Manifest: ispec.Manifest{ + Layers: []ispec.Descriptor{ + { + Digest: godigest.FromBytes(layerBlob), + Size: int64(len(layerBlob)), + MediaType: ispec.MediaTypeImageLayerGzip, + }, + }, + Config: ispec.DescriptorEmptyJSON, + }, + Config: ispec.Image{}, + } + + err := test.UploadImage(img, baseURL, "test") + So(err, ShouldNotBeNil) + }) + Convey("Post request results in an error", t, func() { port := test.GetFreePort() baseURL := test.GetBaseURL(port) @@ -465,6 +584,19 @@ func TestUploadImage(t *testing.T) { Layers: [][]byte{ layerBlob, }, + Manifest: ispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, + Layers: []ispec.Descriptor{ + { + Digest: godigest.FromBytes(layerBlob), + Size: int64(len(layerBlob)), + MediaType: ispec.MediaTypeImageLayerGzip, + }, + }, + Config: ispec.DescriptorEmptyJSON, + }, Config: ispec.Image{}, } diff --git a/test/blackbox/referrers.bats b/test/blackbox/referrers.bats index 3ee25151..05ed7033 100644 --- a/test/blackbox/referrers.bats +++ b/test/blackbox/referrers.bats @@ -60,13 +60,13 @@ EOF [ "$status" -eq 0 ] [ $(echo "${lines[-1]}" | jq '.repositories[]') = '"golang"' ] - run oras attach --plain-http --image-spec v1.1-image --artifact-type image.type 127.0.0.1:8080/golang:1.20 ${IMAGE_MANIFEST_REFERRER} + run oras attach --plain-http --image-spec v1.1-image --artifact-type image.artifact/type 127.0.0.1:8080/golang:1.20 ${IMAGE_MANIFEST_REFERRER} [ "$status" -eq 0 ] MANIFEST_DIGEST=$(skopeo inspect --tls-verify=false docker://localhost:8080/golang:1.20 | jq -r '.Digest') echo ${MANIFEST_DIGEST} - curl -X GET http://127.0.0.1:8080/v2/golang/referrers/${MANIFEST_DIGEST}?artifactType=image.type + curl -X GET http://127.0.0.1:8080/v2/golang/referrers/${MANIFEST_DIGEST}?artifactType=image.artifact/type } function teardown() { @@ -78,13 +78,13 @@ function teardown() { @test "add referrers, one artifact and one image" { # Check referrers API using the normal REST endpoint - run curl -X GET http://127.0.0.1:8080/v2/golang/referrers/${MANIFEST_DIGEST}?artifactType=image.type + run curl -X GET http://127.0.0.1:8080/v2/golang/referrers/${MANIFEST_DIGEST}?artifactType=image.artifact/type [ "$status" -eq 0 ] - [ $(echo "${lines[-1]}" | jq '.manifests[].artifactType') = '"image.type"' ] + [ $(echo "${lines[-1]}" | jq '.manifests[].artifactType') = '"image.artifact/type"' ] # Check referrers API using the GQL endpoint - REFERRER_QUERY_DATA="{ \"query\": \"{ Referrers(repo:\\\"golang\\\", digest:\\\"${MANIFEST_DIGEST}\\\", type:[\\\"image.type\\\"]) { MediaType ArtifactType Digest Size} }\"}" + REFERRER_QUERY_DATA="{ \"query\": \"{ Referrers(repo:\\\"golang\\\", digest:\\\"${MANIFEST_DIGEST}\\\", type:[\\\"image.artifact/type\\\"]) { MediaType ArtifactType Digest Size} }\"}" run curl -X POST -H "Content-Type: application/json" --data "${REFERRER_QUERY_DATA}" http://localhost:8080/v2/_zot/ext/search [ "$status" -eq 0 ] - [ $(echo "${lines[-1]}" | jq '.data.Referrers[].ArtifactType') = '"image.type"' ] + [ $(echo "${lines[-1]}" | jq '.data.Referrers[].ArtifactType') = '"image.artifact/type"' ] }