diff --git a/client/container_opts.go b/client/container_opts.go index 574a87875..79b03da2d 100644 --- a/client/container_opts.go +++ b/client/container_opts.go @@ -27,9 +27,11 @@ import ( "github.com/containerd/containerd/v2/core/content" "github.com/containerd/containerd/v2/core/images" "github.com/containerd/containerd/v2/core/snapshots" + "github.com/containerd/containerd/v2/pkg/labels" "github.com/containerd/containerd/v2/pkg/namespaces" "github.com/containerd/containerd/v2/pkg/oci" "github.com/containerd/errdefs" + "github.com/containerd/log" "github.com/containerd/typeurl/v2" "github.com/opencontainers/image-spec/identity" v1 "github.com/opencontainers/image-spec/specs-go/v1" @@ -114,6 +116,10 @@ func WithContainerLabels(labels map[string]string) NewContainerOpts { // The existing labels are cleared as this is expected to be the first // operation in setting up a container's labels. Use WithAdditionalContainerLabels // to add/overwrite the existing image config labels. +// +// Image config labels in the namespaces reserved for containerd +// (containerd.io/) and the CRI plugin (io.cri-containerd) are not copied +// to the container. func WithImageConfigLabels(image Image) NewContainerOpts { return func(ctx context.Context, _ *Client, c *containers.Container) error { ic, err := image.Config(ctx) @@ -139,6 +145,16 @@ func WithImageConfigLabels(image Image) NewContainerOpts { config = ociimage.Config c.Labels = config.Labels + // Labels in the containerd.io/* namespace are interpreted by containerd + // itself, and labels in the io.cri-containerd.* namespace are interpreted + // by the CRI plugin, so they are not copied from untrusted image configs. + maps.DeleteFunc(c.Labels, func(k, _ string) bool { + if labels.IsReserved(k) { + log.G(ctx).Warnf("skipping image label %q: the label namespace is reserved for containerd; possible malicious image attempting to alter containerd behavior", k) + return true + } + return false + }) return nil } } diff --git a/client/container_opts_test.go b/client/container_opts_test.go new file mode 100644 index 000000000..fb01e6a33 --- /dev/null +++ b/client/container_opts_test.go @@ -0,0 +1,75 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package client + +import ( + "context" + "encoding/json" + "testing" + + "github.com/containerd/containerd/v2/core/containers" + "github.com/containerd/containerd/v2/core/content" + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeImage implements the subset of Image used by WithImageConfigLabels: +// Config returns a descriptor with the config blob inlined in Data, so the +// content store is never consulted. +type fakeImage struct { + Image + config ocispec.Descriptor +} + +func (i fakeImage) Config(context.Context) (ocispec.Descriptor, error) { + return i.config, nil +} + +func (i fakeImage) ContentStore() content.Store { + return nil +} + +func TestWithImageConfigLabels(t *testing.T) { + blob, err := json.Marshal(ocispec.Image{ + Config: ocispec.ImageConfig{ + Labels: map[string]string{ + "foo": "bar", + "containerd.io/restart.policy": "always", + "io.cri-containerd.kind": "sandbox", + }, + }, + }) + require.NoError(t, err) + + img := fakeImage{ + config: ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageConfig, + Digest: digest.FromBytes(blob), + Size: int64(len(blob)), + Data: blob, + }, + } + + var c containers.Container + require.NoError(t, WithImageConfigLabels(img)(t.Context(), nil, &c)) + + // labels in the namespaces reserved for containerd and the CRI plugin + // are not copied from the image config + assert.Equal(t, map[string]string{"foo": "bar"}, c.Labels) +} diff --git a/internal/cri/labels/labels.go b/internal/cri/labels/labels.go index 95aa7cf34..7385e101c 100644 --- a/internal/cri/labels/labels.go +++ b/internal/cri/labels/labels.go @@ -32,9 +32,13 @@ limitations under the License. package labels +import ( + clabels "github.com/containerd/containerd/v2/pkg/labels" +) + const ( // criContainerdPrefix is common prefix for cri-containerd - criContainerdPrefix = "io.cri-containerd" + criContainerdPrefix = clabels.CRIContainerdPrefix // ImageLabelKey is the label key indicating the image is managed by cri plugin. ImageLabelKey = criContainerdPrefix + ".image" // ImageLabelValue is the label value indicating the image is managed by cri plugin. diff --git a/internal/cri/util/util.go b/internal/cri/util/util.go index db45c89c8..cc68f99e0 100644 --- a/internal/cri/util/util.go +++ b/internal/cri/util/util.go @@ -82,11 +82,21 @@ func GetPassthroughAnnotations(podAnnotations map[string]string, return passthroughAnnotations } -// BuildLabels builds the labels from config to be passed to containerd +// BuildLabels builds the labels from config to be passed to containerd. +// Image config labels in the namespaces reserved for containerd +// (containerd.io/) and the CRI plugin (io.cri-containerd) are not copied +// to the container. func BuildLabels(configLabels, imageConfigLabels map[string]string, containerType string) map[string]string { labels := make(map[string]string) for k, v := range imageConfigLabels { + // Labels in the containerd.io/* namespace are interpreted by containerd + // itself, and labels in the io.cri-containerd.* namespace are interpreted + // by the CRI plugin, so they are not copied from untrusted image configs. + if clabels.IsReserved(k) { + log.L.Warnf("skipping image label %q: the label namespace is reserved for containerd; possible malicious image attempting to alter containerd behavior", k) + continue + } if err := clabels.Validate(k, v); err == nil { labels[k] = v } else { diff --git a/internal/cri/util/util_test.go b/internal/cri/util/util_test.go index 3974920f2..1664feb63 100644 --- a/internal/cri/util/util_test.go +++ b/internal/cri/util/util_test.go @@ -145,21 +145,29 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { func TestBuildLabels(t *testing.T) { imageConfigLabels := map[string]string{ - "a": "z", - "d": "y", - "long-label": strings.Repeat("example", 10000), + "a": "z", + "d": "y", + "long-label": strings.Repeat("example", 10000), + "containerd.io/restart.policy": "always", + "io.cri-containerd.image": "managed", } configLabels := map[string]string{ "a": "b", "c": "d", + // reserved namespaces are only filtered for image config labels, not + // for labels from the CRI request + "containerd.io/restart.status": "stopped", } newLabels := BuildLabels(configLabels, imageConfigLabels, crilabels.ContainerKindSandbox) - assert.Len(t, newLabels, 4) + assert.Len(t, newLabels, 5) assert.Equal(t, "b", newLabels["a"]) assert.Equal(t, "d", newLabels["c"]) assert.Equal(t, "y", newLabels["d"]) + assert.Equal(t, "stopped", newLabels["containerd.io/restart.status"]) assert.Equal(t, crilabels.ContainerKindSandbox, newLabels[crilabels.ContainerKindLabel]) assert.NotContains(t, newLabels, "long-label") + assert.NotContains(t, newLabels, "containerd.io/restart.policy") + assert.NotContains(t, newLabels, "io.cri-containerd.image") newLabels["a"] = "e" assert.Empty(t, configLabels[crilabels.ContainerKindLabel], "should not add new labels into original label") diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 0f9bab5c5..ba4c245e4 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -16,6 +16,18 @@ package labels +// ReservedPrefix is the prefix of the label namespace reserved for labels +// defined and consumed by containerd itself. Labels in this namespace must +// not be copied from untrusted sources such as image config labels. Use +// IsReserved to check for such labels. +const ReservedPrefix = "containerd.io/" + +// CRIContainerdPrefix is the prefix of the label namespace reserved for +// labels defined and consumed by containerd's CRI plugin. Labels in this +// namespace must not be copied from untrusted sources such as image config +// labels. Use IsReserved to check for such labels. +const CRIContainerdPrefix = "io.cri-containerd" + // LabelUncompressed is added to compressed layer contents. // The value is digest of the uncompressed content. const LabelUncompressed = "containerd.io/uncompressed" diff --git a/pkg/labels/validate.go b/pkg/labels/validate.go index 6f23cdd7c..495427bb4 100644 --- a/pkg/labels/validate.go +++ b/pkg/labels/validate.go @@ -18,6 +18,7 @@ package labels import ( "fmt" + "strings" "github.com/containerd/errdefs" ) @@ -39,3 +40,11 @@ func Validate(k, v string) error { } return nil } + +// IsReserved returns true if the label key is in a namespace reserved for +// containerd (ReservedPrefix) or its CRI plugin (CRIContainerdPrefix). +// Reserved labels are interpreted by containerd and must not be copied from +// untrusted sources such as image config labels. +func IsReserved(k string) bool { + return strings.HasPrefix(k, ReservedPrefix) || strings.HasPrefix(k, CRIContainerdPrefix) +} diff --git a/pkg/labels/validate_test.go b/pkg/labels/validate_test.go index 16be11df3..fb97e5b69 100644 --- a/pkg/labels/validate_test.go +++ b/pkg/labels/validate_test.go @@ -53,6 +53,23 @@ func TestInvalidLabels(t *testing.T) { } } +func TestIsReserved(t *testing.T) { + for key, reserved := range map[string]bool{ + "containerd.io/": true, + "containerd.io/restart.status": true, + "containerd.io/gc.ref.content": true, + "io.cri-containerd": true, + "io.cri-containerd.kind": true, + "io.cri-containerd.image": true, + "io.cri-containerdfoo": true, + "containerd.io": false, + "io.containerd.something": false, + "com.example.app": false, + } { + assert.Equal(t, reserved, IsReserved(key), "IsReserved(%q)", key) + } +} + func TestLongKey(t *testing.T) { key := strings.Repeat("s", keyMaxLen+1) value := strings.Repeat("v", maxSize-len(key))