Merge commit from fork

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
 	}
 }

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)
+}

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.

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 {

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")

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"

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)
+}

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))