From b3456925ca8450dedba32752e3417eb6e1ebf336 Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Thu, 4 Nov 2021 14:41:21 -0700 Subject: [PATCH 1/3] vendor: update github.com/docker/distribution Signed-off-by: Samuel Karp --- vendor.conf | 2 +- .../manifest/manifestlist/manifestlist.go | 23 +++++++++++++++++++ .../manifest/ocischema/manifest.go | 22 ++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/vendor.conf b/vendor.conf index a88f05bd71..f16cab8452 100644 --- a/vendor.conf +++ b/vendor.conf @@ -76,7 +76,7 @@ github.com/ishidawataru/sctp f2269e66cdee387bd321445d5d30 go.etcd.io/bbolt 232d8fc87f50244f9c808f4745759e08a304c029 # v1.3.5 # get graph and distribution packages -github.com/docker/distribution 0d3efadf0154c2b8a4e7b6621fff9809655cc580 +github.com/docker/distribution 58f99e93b767ebacbf8e62a9074844712d31a177 github.com/samuelkarp/docker-distribution github.com/vbatts/tar-split 620714a4c508c880ac1bdda9c8370a2b19af1a55 # v0.11.1 github.com/opencontainers/go-digest ea51bea511f75cfa3ef6098cc253c5c3609b037a # v1.0.0 diff --git a/vendor/github.com/docker/distribution/manifest/manifestlist/manifestlist.go b/vendor/github.com/docker/distribution/manifest/manifestlist/manifestlist.go index 54c8f3c94c..09b3609737 100644 --- a/vendor/github.com/docker/distribution/manifest/manifestlist/manifestlist.go +++ b/vendor/github.com/docker/distribution/manifest/manifestlist/manifestlist.go @@ -54,6 +54,9 @@ func init() { } imageIndexFunc := func(b []byte) (distribution.Manifest, distribution.Descriptor, error) { + if err := validateIndex(b); err != nil { + return nil, distribution.Descriptor{}, err + } m := new(DeserializedManifestList) err := m.UnmarshalJSON(b) if err != nil { @@ -214,3 +217,23 @@ func (m DeserializedManifestList) Payload() (string, []byte, error) { return mediaType, m.canonical, nil } + +// unknownDocument represents a manifest, manifest list, or index that has not +// yet been validated +type unknownDocument struct { + Config interface{} `json:"config,omitempty"` + Layers interface{} `json:"layers,omitempty"` +} + +// validateIndex returns an error if the byte slice is invalid JSON or if it +// contains fields that belong to a manifest +func validateIndex(b []byte) error { + var doc unknownDocument + if err := json.Unmarshal(b, &doc); err != nil { + return err + } + if doc.Config != nil || doc.Layers != nil { + return errors.New("index: expected index but found manifest") + } + return nil +} diff --git a/vendor/github.com/docker/distribution/manifest/ocischema/manifest.go b/vendor/github.com/docker/distribution/manifest/ocischema/manifest.go index b8c4bab547..910a64afb4 100644 --- a/vendor/github.com/docker/distribution/manifest/ocischema/manifest.go +++ b/vendor/github.com/docker/distribution/manifest/ocischema/manifest.go @@ -22,6 +22,9 @@ var ( func init() { ocischemaFunc := func(b []byte) (distribution.Manifest, distribution.Descriptor, error) { + if err := validateManifest(b); err != nil { + return nil, distribution.Descriptor{}, err + } m := new(DeserializedManifest) err := m.UnmarshalJSON(b) if err != nil { @@ -122,3 +125,22 @@ func (m *DeserializedManifest) MarshalJSON() ([]byte, error) { func (m DeserializedManifest) Payload() (string, []byte, error) { return v1.MediaTypeImageManifest, m.canonical, nil } + +// unknownDocument represents a manifest, manifest list, or index that has not +// yet been validated +type unknownDocument struct { + Manifests interface{} `json:"manifests,omitempty"` +} + +// validateManifest returns an error if the byte slice is invalid JSON or if it +// contains fields that belong to a index +func validateManifest(b []byte) error { + var doc unknownDocument + if err := json.Unmarshal(b, &doc); err != nil { + return err + } + if doc.Manifests != nil { + return errors.New("ocimanifest: expected manifest but found index") + } + return nil +} From c96ed28f2f1aa2524564efe6ae02fe76203f1aa7 Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Thu, 4 Nov 2021 14:41:58 -0700 Subject: [PATCH 2/3] vendor: update github.com/containerd/containerd Signed-off-by: Samuel Karp --- vendor.conf | 2 +- .../containerd/containerd/images/image.go | 55 +++++++++++++++++++ .../remotes/docker/schema1/converter.go | 9 ++- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/vendor.conf b/vendor.conf index f16cab8452..72d5d5b126 100644 --- a/vendor.conf +++ b/vendor.conf @@ -130,7 +130,7 @@ github.com/googleapis/gax-go bd5b16380fd03dc758d11cef74ba google.golang.org/genproto 3f1135a288c9a07e340ae8ba4cc6c7065a3160e8 # containerd -github.com/containerd/containerd 0edc412565dcc6e3d6125ff9e4b009ad4b89c638 # master (v1.5.0-dev) +github.com/containerd/containerd e048c115a3a89caf63941d363858e207c28bccd6 github.com/moby/containerd # master (v1.5.0-dev) + patch for CVE-2021-41190 github.com/containerd/fifo 0724c46b320cf96bb172a0550c19a4b1fca4dacb github.com/containerd/continuity efbc4488d8fe1bdc16bde3b2d2990d9b3a899165 github.com/containerd/cgroups 0b889c03f102012f1d93a97ddd3ef71cd6f4f510 diff --git a/vendor/github.com/containerd/containerd/images/image.go b/vendor/github.com/containerd/containerd/images/image.go index 1868ee88dd..2e42ca09a6 100644 --- a/vendor/github.com/containerd/containerd/images/image.go +++ b/vendor/github.com/containerd/containerd/images/image.go @@ -19,6 +19,7 @@ package images import ( "context" "encoding/json" + "fmt" "sort" "time" @@ -154,6 +155,10 @@ func Manifest(ctx context.Context, provider content.Provider, image ocispec.Desc return nil, err } + if err := validateMediaType(p, desc.MediaType); err != nil { + return nil, errors.Wrapf(err, "manifest: invalid desc %s", desc.Digest) + } + var manifest ocispec.Manifest if err := json.Unmarshal(p, &manifest); err != nil { return nil, err @@ -194,6 +199,10 @@ func Manifest(ctx context.Context, provider content.Provider, image ocispec.Desc return nil, err } + if err := validateMediaType(p, desc.MediaType); err != nil { + return nil, errors.Wrapf(err, "manifest: invalid desc %s", desc.Digest) + } + var idx ocispec.Index if err := json.Unmarshal(p, &idx); err != nil { return nil, err @@ -336,6 +345,10 @@ func Children(ctx context.Context, provider content.Provider, desc ocispec.Descr return nil, err } + if err := validateMediaType(p, desc.MediaType); err != nil { + return nil, errors.Wrapf(err, "children: invalid desc %s", desc.Digest) + } + // TODO(stevvooe): We just assume oci manifest, for now. There may be // subtle differences from the docker version. var manifest ocispec.Manifest @@ -351,6 +364,10 @@ func Children(ctx context.Context, provider content.Provider, desc ocispec.Descr return nil, err } + if err := validateMediaType(p, desc.MediaType); err != nil { + return nil, errors.Wrapf(err, "children: invalid desc %s", desc.Digest) + } + var index ocispec.Index if err := json.Unmarshal(p, &index); err != nil { return nil, err @@ -368,6 +385,44 @@ func Children(ctx context.Context, provider content.Provider, desc ocispec.Descr return descs, nil } +// unknownDocument represents a manifest, manifest list, or index that has not +// yet been validated. +type unknownDocument struct { + MediaType string `json:"mediaType,omitempty"` + Config json.RawMessage `json:"config,omitempty"` + Layers json.RawMessage `json:"layers,omitempty"` + Manifests json.RawMessage `json:"manifests,omitempty"` + FSLayers json.RawMessage `json:"fsLayers,omitempty"` // schema 1 +} + +// validateMediaType returns an error if the byte slice is invalid JSON or if +// the media type identifies the blob as one format but it contains elements of +// another format. +func validateMediaType(b []byte, mt string) error { + var doc unknownDocument + if err := json.Unmarshal(b, &doc); err != nil { + return err + } + if len(doc.FSLayers) != 0 { + return fmt.Errorf("media-type: schema 1 not supported") + } + switch mt { + case MediaTypeDockerSchema2Manifest, ocispec.MediaTypeImageManifest: + if len(doc.Manifests) != 0 || + doc.MediaType == MediaTypeDockerSchema2ManifestList || + doc.MediaType == ocispec.MediaTypeImageIndex { + return fmt.Errorf("media-type: expected manifest but found index (%s)", mt) + } + case MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex: + if len(doc.Config) != 0 || len(doc.Layers) != 0 || + doc.MediaType == MediaTypeDockerSchema2Manifest || + doc.MediaType == ocispec.MediaTypeImageManifest { + return fmt.Errorf("media-type: expected index but found manifest (%s)", mt) + } + } + return nil +} + // RootFS returns the unpacked diffids that make up and images rootfs. // // These are used to verify that a set of layers unpacked to the expected diff --git a/vendor/github.com/containerd/containerd/remotes/docker/schema1/converter.go b/vendor/github.com/containerd/containerd/remotes/docker/schema1/converter.go index 8314c01d5a..f15a9acf3e 100644 --- a/vendor/github.com/containerd/containerd/remotes/docker/schema1/converter.go +++ b/vendor/github.com/containerd/containerd/remotes/docker/schema1/converter.go @@ -256,6 +256,9 @@ func (c *Converter) fetchManifest(ctx context.Context, desc ocispec.Descriptor) if err := json.Unmarshal(b, &m); err != nil { return err } + if len(m.Manifests) != 0 || len(m.Layers) != 0 { + return errors.New("converter: expected schema1 document but found extra keys") + } c.pulledManifest = &m return nil @@ -472,8 +475,10 @@ type history struct { } type manifest struct { - FSLayers []fsLayer `json:"fsLayers"` - History []history `json:"history"` + FSLayers []fsLayer `json:"fsLayers"` + History []history `json:"history"` + Layers json.RawMessage `json:"layers,omitempty"` // OCI manifest + Manifests json.RawMessage `json:"manifests,omitempty"` // OCI index } type v1History struct { From c1f352c4b13a1f562c59908f71a39fa40106ee7c Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Thu, 11 Nov 2021 17:45:40 -0800 Subject: [PATCH 3/3] 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, "") + }) + } + +}