From c1f352c4b13a1f562c59908f71a39fa40106ee7c Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Thu, 11 Nov 2021 17:45:40 -0800 Subject: [PATCH] distribution: validate blob type Signed-off-by: Samuel Karp --- distribution/manifest.go | 45 +++++++++++++++++----- distribution/manifest_test.go | 72 +++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 9 deletions(-) diff --git a/distribution/manifest.go b/distribution/manifest.go index a97373bd61..3b5a18bad2 100644 --- a/distribution/manifest.go +++ b/distribution/manifest.go @@ -3,6 +3,7 @@ package distribution import ( "context" "encoding/json" + "fmt" "io" "io/ioutil" @@ -11,7 +12,9 @@ import ( "github.com/containerd/containerd/log" "github.com/containerd/containerd/remotes" "github.com/docker/distribution" + "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/schema1" + "github.com/docker/distribution/manifest/schema2" digest "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -166,8 +169,10 @@ func detectManifestMediaType(ra content.ReaderAt) (string, error) { func detectManifestBlobMediaType(dt []byte) (string, error) { var mfst struct { MediaType string `json:"mediaType"` - Config json.RawMessage `json:"config"` // schema2 Manifest - FSLayers json.RawMessage `json:"fsLayers"` // schema1 Manifest + Manifests json.RawMessage `json:"manifests"` // oci index, manifest list + Config json.RawMessage `json:"config"` // schema2 Manifest + Layers json.RawMessage `json:"layers"` // schema2 Manifest + FSLayers json.RawMessage `json:"fsLayers"` // schema1 Manifest } if err := json.Unmarshal(dt, &mfst); err != nil { @@ -178,18 +183,40 @@ func detectManifestBlobMediaType(dt []byte) (string, error) { // Docker types should generally have a media type set. // OCI (golang) types do not have a `mediaType` defined, and it is optional in the spec. // - // `distrubtion.UnmarshalManifest`, which is used to unmarshal this for real, checks these media type values. + // `distribution.UnmarshalManifest`, which is used to unmarshal this for real, checks these media type values. // If the specified media type does not match it will error, and in some cases (docker media types) it is required. // So pretty much if we don't have a media type we can fall back to OCI. // This does have a special fallback for schema1 manifests just because it is easy to detect. - switch { - case mfst.MediaType != "": + switch mfst.MediaType { + case schema2.MediaTypeManifest, specs.MediaTypeImageManifest: + if mfst.Manifests != nil || mfst.FSLayers != nil { + return "", fmt.Errorf(`media-type: %q should not have "manifests" or "fsLayers"`, mfst.MediaType) + } + return mfst.MediaType, nil + case manifestlist.MediaTypeManifestList, specs.MediaTypeImageIndex: + if mfst.Config != nil || mfst.Layers != nil || mfst.FSLayers != nil { + return "", fmt.Errorf(`media-type: %q should not have "config", "layers", or "fsLayers"`, mfst.MediaType) + } + return mfst.MediaType, nil + case schema1.MediaTypeManifest: + if mfst.Manifests != nil || mfst.Layers != nil { + return "", fmt.Errorf(`media-type: %q should not have "manifests" or "layers"`, mfst.MediaType) + } return mfst.MediaType, nil - case mfst.FSLayers != nil: - return schema1.MediaTypeManifest, nil - case mfst.Config != nil: - return specs.MediaTypeImageManifest, nil default: + if mfst.MediaType != "" { + return mfst.MediaType, nil + } + } + switch { + case mfst.FSLayers != nil && mfst.Manifests == nil && mfst.Layers == nil && mfst.Config == nil: + return schema1.MediaTypeManifest, nil + case mfst.Config != nil && mfst.Manifests == nil && mfst.FSLayers == nil, + mfst.Layers != nil && mfst.Manifests == nil && mfst.FSLayers == nil: + return specs.MediaTypeImageManifest, nil + case mfst.Config == nil && mfst.Layers == nil && mfst.FSLayers == nil: + // fallback to index return specs.MediaTypeImageIndex, nil } + return "", errors.New("media-type: cannot determine") } diff --git a/distribution/manifest_test.go b/distribution/manifest_test.go index 0976a712ec..578f8ccce8 100644 --- a/distribution/manifest_test.go +++ b/distribution/manifest_test.go @@ -14,8 +14,10 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/remotes" "github.com/docker/distribution" + "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/ocischema" "github.com/docker/distribution/manifest/schema1" + "github.com/docker/distribution/manifest/schema2" "github.com/google/go-cmp/cmp/cmpopts" digest "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go/v1" @@ -349,3 +351,73 @@ func TestDetectManifestBlobMediaType(t *testing.T) { } } + +func TestDetectManifestBlobMediaTypeInvalid(t *testing.T) { + type testCase struct { + json []byte + expected string + } + cases := map[string]testCase{ + "schema 1 mediaType with manifests": { + []byte(`{"mediaType": "` + schema1.MediaTypeManifest + `","manifests":[]}`), + `media-type: "application/vnd.docker.distribution.manifest.v1+json" should not have "manifests" or "layers"`, + }, + "schema 1 mediaType with layers": { + []byte(`{"mediaType": "` + schema1.MediaTypeManifest + `","layers":[]}`), + `media-type: "application/vnd.docker.distribution.manifest.v1+json" should not have "manifests" or "layers"`, + }, + "schema 2 mediaType with manifests": { + []byte(`{"mediaType": "` + schema2.MediaTypeManifest + `","manifests":[]}`), + `media-type: "application/vnd.docker.distribution.manifest.v2+json" should not have "manifests" or "fsLayers"`, + }, + "schema 2 mediaType with fsLayers": { + []byte(`{"mediaType": "` + schema2.MediaTypeManifest + `","fsLayers":[]}`), + `media-type: "application/vnd.docker.distribution.manifest.v2+json" should not have "manifests" or "fsLayers"`, + }, + "oci manifest mediaType with manifests": { + []byte(`{"mediaType": "` + specs.MediaTypeImageManifest + `","manifests":[]}`), + `media-type: "application/vnd.oci.image.manifest.v1+json" should not have "manifests" or "fsLayers"`, + }, + "manifest list mediaType with fsLayers": { + []byte(`{"mediaType": "` + manifestlist.MediaTypeManifestList + `","fsLayers":[]}`), + `media-type: "application/vnd.docker.distribution.manifest.list.v2+json" should not have "config", "layers", or "fsLayers"`, + }, + "index mediaType with layers": { + []byte(`{"mediaType": "` + specs.MediaTypeImageIndex + `","layers":[]}`), + `media-type: "application/vnd.oci.image.index.v1+json" should not have "config", "layers", or "fsLayers"`, + }, + "index mediaType with config": { + []byte(`{"mediaType": "` + specs.MediaTypeImageIndex + `","config":{}}`), + `media-type: "application/vnd.oci.image.index.v1+json" should not have "config", "layers", or "fsLayers"`, + }, + "config and manifests": { + []byte(`{"config":{}, "manifests":[]}`), + `media-type: cannot determine`, + }, + "layers and manifests": { + []byte(`{"layers":[], "manifests":[]}`), + `media-type: cannot determine`, + }, + "layers and fsLayers": { + []byte(`{"layers":[], "fsLayers":[]}`), + `media-type: cannot determine`, + }, + "fsLayers and manifests": { + []byte(`{"fsLayers":[], "manifests":[]}`), + `media-type: cannot determine`, + }, + "config and fsLayers": { + []byte(`{"config":{}, "fsLayers":[]}`), + `media-type: cannot determine`, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + mt, err := detectManifestBlobMediaType(tc.json) + assert.Error(t, err, tc.expected) + assert.Equal(t, mt, "") + }) + } + +}