Merge pull request #13628 from samuelkarp/prepare-2.2.5

[release/2.2] Prepare release notes for v2.2.5

Author: dmcgowan

client/container_opts.go

@@ -21,14 +21,17 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"maps"
 
 	"github.com/containerd/containerd/v2/core/containers"
 	"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"
@@ -113,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)
@@ -138,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)
+}

contrib/checkpoint/checkpoint-restore-cri-test.sh

@@ -56,6 +56,15 @@ TESTDATA=testdata
 # shellcheck disable=SC2034
 export CONTAINERD_ADDRESS="$TESTDIR/c.sock"
 export CONTAINER_RUNTIME_ENDPOINT="unix:///${CONTAINERD_ADDRESS}"
+
+# Generate crictl config file with 30s timeout
+export CRI_CONFIG_FILE="${TESTDIR}/crictl.yaml"
+cat <<EOF > "${CRI_CONFIG_FILE}"
+runtime-endpoint: unix://${CONTAINERD_ADDRESS}
+image-endpoint: unix://${CONTAINERD_ADDRESS}
+timeout: 30
+EOF
+
 TEST_IMAGE=ghcr.io/containerd/alpine
 
 function test_from_archive() {
@@ -69,9 +78,12 @@ function test_from_archive() {
 	echo -n "--> Start pod: "
 	pod_id=$(crictl runp "$POD_JSON")
 	echo "$pod_id"
+	CTR_JSON=$(mktemp)
+	jq '.annotations = {"cdi.k8s.io/device":"gpu","safe.annotation":"true"}' "$TESTDATA"/container_sleep.json >"$CTR_JSON"
 	echo -n "--> Create container: "
-	ctr_id=$(crictl create "$pod_id" "$TESTDATA"/container_sleep.json "$POD_JSON")
+	ctr_id=$(crictl create "$pod_id" "$CTR_JSON" "$POD_JSON")
 	echo "$ctr_id"
+	rm -f "$CTR_JSON"
 	echo -n "--> Start container: "
 	crictl start "$ctr_id"
 	lines_before=$(crictl logs "$ctr_id" | wc -l)
@@ -108,9 +120,15 @@ function test_from_archive() {
 			"should be larger than before checkpointing ($lines_before)"
 		false
 	fi
+	echo "--> Verifying CDI annotation filtering on restore: "
+	actual_annots=$(crictl inspect "$ctr_id" | jq -c '.status.annotations')
+	if jq -e 'has("cdi.k8s.io/device") or (has("safe.annotation") | not)' <<<"$actual_annots" >/dev/null; then
+		echo "error: CDI annotation was not filtered or safe annotation missing: $actual_annots"
+		exit 1
+	fi
 	# Cleanup
 	echo "--> Cleanup images: "
-	crictl rmi "${TEST_IMAGE}" | sed 's/^/----> \t/'
+	(crictl rmi "${TEST_IMAGE}" || true) | sed 's/^/----> \t/'
 	echo -n "--> Verifying container rootfs: "
 	crictl exec "$ctr_id" ls -la /root/testfile
 	if crictl exec "$ctr_id" ls -la /etc/motd >/dev/null 2>&1; then
@@ -184,14 +202,16 @@ function test_from_oci() {
 	echo "--> Cleanup images: "
 	../../bin/ctr -n k8s.io images rm localhost/checkpoint-image:latest | sed 's/^/----> \t/'
 	echo "--> Cleanup images: "
-	crictl rmi "${TEST_IMAGE}" | sed 's/^/----> \t/'
+	(crictl rmi "${TEST_IMAGE}" || true) | sed 's/^/----> \t/'
 	echo "--> Deleting all pods: "
 	crictl -t 5s rmp -fa | sed 's/^/----> \t/'
 	SUCCESS=1
 }
 
 cat >"${TESTDIR}/config.toml" <<EOF
 version = 3
+[plugins."io.containerd.cri.v1.runtime"]
+  enable_cdi = false
 [plugins."io.containerd.cri.v1.runtime".containerd]
   default_runtime_name = "test-runtime"
 [plugins.'io.containerd.cri.v1.runtime'.containerd.runtimes.test-runtime]

internal/cri/labels/labels.go

@@ -16,9 +16,13 @@
 
 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.