Skip to content

Commit b6072a4

Browse files
bcresseysamuelkarp
authored andcommitted
Do not propagate reserved labels from image configs
Image config labels are copied onto the container by both the CRI plugin (BuildLabels) and the client's WithImageConfigLabels option used by `ctr run`. 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. An image config is not a trusted source for labels in either namespace. Skip labels in both reserved namespaces when copying labels from an image config to a container, and warn about each label skipped: an image that tries to set them may be attempting to alter containerd behavior. Oversized image labels are already skipped this way by the CRI plugin. Labels set explicitly by clients, for example via `ctr run --label` or in the CRI request, are unaffected. Verified with the CRI plugin and with `ctr run` against an image whose config carries labels like these: the labels are no longer present on the created container and a warning is logged for each. Assisted-by: Claude Code Signed-off-by: Ben Cressey <ben@cressey.org> Signed-off-by: Samuel Karp <samuelkarp@google.com> (cherry picked from commit 0ec1af4) Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp> Signed-off-by: Samuel Karp <samuelkarp@google.com>
1 parent 6f9cbbf commit b6072a4

8 files changed

Lines changed: 158 additions & 6 deletions

File tree

client/container_opts.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,17 @@ import (
2121
"encoding/json"
2222
"errors"
2323
"fmt"
24+
"maps"
2425

2526
"github.com/containerd/containerd/v2/core/containers"
2627
"github.com/containerd/containerd/v2/core/content"
2728
"github.com/containerd/containerd/v2/core/images"
2829
"github.com/containerd/containerd/v2/core/snapshots"
30+
"github.com/containerd/containerd/v2/pkg/labels"
2931
"github.com/containerd/containerd/v2/pkg/namespaces"
3032
"github.com/containerd/containerd/v2/pkg/oci"
3133
"github.com/containerd/errdefs"
34+
"github.com/containerd/log"
3235
"github.com/containerd/typeurl/v2"
3336
"github.com/opencontainers/image-spec/identity"
3437
v1 "github.com/opencontainers/image-spec/specs-go/v1"
@@ -113,6 +116,10 @@ func WithContainerLabels(labels map[string]string) NewContainerOpts {
113116
// The existing labels are cleared as this is expected to be the first
114117
// operation in setting up a container's labels. Use WithAdditionalContainerLabels
115118
// to add/overwrite the existing image config labels.
119+
//
120+
// Image config labels in the namespaces reserved for containerd
121+
// (containerd.io/) and the CRI plugin (io.cri-containerd) are not copied
122+
// to the container.
116123
func WithImageConfigLabels(image Image) NewContainerOpts {
117124
return func(ctx context.Context, _ *Client, c *containers.Container) error {
118125
ic, err := image.Config(ctx)
@@ -138,6 +145,16 @@ func WithImageConfigLabels(image Image) NewContainerOpts {
138145
config = ociimage.Config
139146

140147
c.Labels = config.Labels
148+
// Labels in the containerd.io/* namespace are interpreted by containerd
149+
// itself, and labels in the io.cri-containerd.* namespace are interpreted
150+
// by the CRI plugin, so they are not copied from untrusted image configs.
151+
maps.DeleteFunc(c.Labels, func(k, _ string) bool {
152+
if labels.IsReserved(k) {
153+
log.G(ctx).Warnf("skipping image label %q: the label namespace is reserved for containerd; possible malicious image attempting to alter containerd behavior", k)
154+
return true
155+
}
156+
return false
157+
})
141158
return nil
142159
}
143160
}

client/container_opts_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package client
18+
19+
import (
20+
"context"
21+
"encoding/json"
22+
"testing"
23+
24+
"github.com/containerd/containerd/v2/core/containers"
25+
"github.com/containerd/containerd/v2/core/content"
26+
"github.com/opencontainers/go-digest"
27+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
28+
"github.com/stretchr/testify/assert"
29+
"github.com/stretchr/testify/require"
30+
)
31+
32+
// fakeImage implements the subset of Image used by WithImageConfigLabels:
33+
// Config returns a descriptor with the config blob inlined in Data, so the
34+
// content store is never consulted.
35+
type fakeImage struct {
36+
Image
37+
config ocispec.Descriptor
38+
}
39+
40+
func (i fakeImage) Config(context.Context) (ocispec.Descriptor, error) {
41+
return i.config, nil
42+
}
43+
44+
func (i fakeImage) ContentStore() content.Store {
45+
return nil
46+
}
47+
48+
func TestWithImageConfigLabels(t *testing.T) {
49+
blob, err := json.Marshal(ocispec.Image{
50+
Config: ocispec.ImageConfig{
51+
Labels: map[string]string{
52+
"foo": "bar",
53+
"containerd.io/restart.policy": "always",
54+
"io.cri-containerd.kind": "sandbox",
55+
},
56+
},
57+
})
58+
require.NoError(t, err)
59+
60+
img := fakeImage{
61+
config: ocispec.Descriptor{
62+
MediaType: ocispec.MediaTypeImageConfig,
63+
Digest: digest.FromBytes(blob),
64+
Size: int64(len(blob)),
65+
Data: blob,
66+
},
67+
}
68+
69+
var c containers.Container
70+
require.NoError(t, WithImageConfigLabels(img)(t.Context(), nil, &c))
71+
72+
// labels in the namespaces reserved for containerd and the CRI plugin
73+
// are not copied from the image config
74+
assert.Equal(t, map[string]string{"foo": "bar"}, c.Labels)
75+
}

internal/cri/labels/labels.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@
1616

1717
package labels
1818

19+
import (
20+
clabels "github.com/containerd/containerd/v2/pkg/labels"
21+
)
22+
1923
const (
2024
// criContainerdPrefix is common prefix for cri-containerd
21-
criContainerdPrefix = "io.cri-containerd"
25+
criContainerdPrefix = clabels.CRIContainerdPrefix
2226
// ImageLabelKey is the label key indicating the image is managed by cri plugin.
2327
ImageLabelKey = criContainerdPrefix + ".image"
2428
// ImageLabelValue is the label value indicating the image is managed by cri plugin.

internal/cri/util/util.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,21 @@ func GetPassthroughAnnotations(podAnnotations map[string]string,
8181
return passthroughAnnotations
8282
}
8383

84-
// BuildLabels builds the labels from config to be passed to containerd
84+
// BuildLabels builds the labels from config to be passed to containerd.
85+
// Image config labels in the namespaces reserved for containerd
86+
// (containerd.io/) and the CRI plugin (io.cri-containerd) are not copied
87+
// to the container.
8588
func BuildLabels(configLabels, imageConfigLabels map[string]string, containerType string) map[string]string {
8689
labels := make(map[string]string)
8790

8891
for k, v := range imageConfigLabels {
92+
// Labels in the containerd.io/* namespace are interpreted by containerd
93+
// itself, and labels in the io.cri-containerd.* namespace are interpreted
94+
// by the CRI plugin, so they are not copied from untrusted image configs.
95+
if clabels.IsReserved(k) {
96+
log.L.Warnf("skipping image label %q: the label namespace is reserved for containerd; possible malicious image attempting to alter containerd behavior", k)
97+
continue
98+
}
8999
if err := clabels.Validate(k, v); err == nil {
90100
labels[k] = v
91101
} else {

internal/cri/util/util_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,21 +145,29 @@ func TestPassThroughAnnotationsFilter(t *testing.T) {
145145

146146
func TestBuildLabels(t *testing.T) {
147147
imageConfigLabels := map[string]string{
148-
"a": "z",
149-
"d": "y",
150-
"long-label": strings.Repeat("example", 10000),
148+
"a": "z",
149+
"d": "y",
150+
"long-label": strings.Repeat("example", 10000),
151+
"containerd.io/restart.policy": "always",
152+
"io.cri-containerd.image": "managed",
151153
}
152154
configLabels := map[string]string{
153155
"a": "b",
154156
"c": "d",
157+
// reserved namespaces are only filtered for image config labels, not
158+
// for labels from the CRI request
159+
"containerd.io/restart.status": "stopped",
155160
}
156161
newLabels := BuildLabels(configLabels, imageConfigLabels, crilabels.ContainerKindSandbox)
157-
assert.Len(t, newLabels, 4)
162+
assert.Len(t, newLabels, 5)
158163
assert.Equal(t, "b", newLabels["a"])
159164
assert.Equal(t, "d", newLabels["c"])
160165
assert.Equal(t, "y", newLabels["d"])
166+
assert.Equal(t, "stopped", newLabels["containerd.io/restart.status"])
161167
assert.Equal(t, crilabels.ContainerKindSandbox, newLabels[crilabels.ContainerKindLabel])
162168
assert.NotContains(t, newLabels, "long-label")
169+
assert.NotContains(t, newLabels, "containerd.io/restart.policy")
170+
assert.NotContains(t, newLabels, "io.cri-containerd.image")
163171

164172
newLabels["a"] = "e"
165173
assert.Empty(t, configLabels[crilabels.ContainerKindLabel], "should not add new labels into original label")

pkg/labels/labels.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,18 @@
1616

1717
package labels
1818

19+
// ReservedPrefix is the prefix of the label namespace reserved for labels
20+
// defined and consumed by containerd itself. Labels in this namespace must
21+
// not be copied from untrusted sources such as image config labels. Use
22+
// IsReserved to check for such labels.
23+
const ReservedPrefix = "containerd.io/"
24+
25+
// CRIContainerdPrefix is the prefix of the label namespace reserved for
26+
// labels defined and consumed by containerd's CRI plugin. Labels in this
27+
// namespace must not be copied from untrusted sources such as image config
28+
// labels. Use IsReserved to check for such labels.
29+
const CRIContainerdPrefix = "io.cri-containerd"
30+
1931
// LabelUncompressed is added to compressed layer contents.
2032
// The value is digest of the uncompressed content.
2133
const LabelUncompressed = "containerd.io/uncompressed"

pkg/labels/validate.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package labels
1818

1919
import (
2020
"fmt"
21+
"strings"
2122

2223
"github.com/containerd/errdefs"
2324
)
@@ -39,3 +40,11 @@ func Validate(k, v string) error {
3940
}
4041
return nil
4142
}
43+
44+
// IsReserved returns true if the label key is in a namespace reserved for
45+
// containerd (ReservedPrefix) or its CRI plugin (CRIContainerdPrefix).
46+
// Reserved labels are interpreted by containerd and must not be copied from
47+
// untrusted sources such as image config labels.
48+
func IsReserved(k string) bool {
49+
return strings.HasPrefix(k, ReservedPrefix) || strings.HasPrefix(k, CRIContainerdPrefix)
50+
}

pkg/labels/validate_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,23 @@ func TestInvalidLabels(t *testing.T) {
5353
}
5454
}
5555

56+
func TestIsReserved(t *testing.T) {
57+
for key, reserved := range map[string]bool{
58+
"containerd.io/": true,
59+
"containerd.io/restart.status": true,
60+
"containerd.io/gc.ref.content": true,
61+
"io.cri-containerd": true,
62+
"io.cri-containerd.kind": true,
63+
"io.cri-containerd.image": true,
64+
"io.cri-containerdfoo": true,
65+
"containerd.io": false,
66+
"io.containerd.something": false,
67+
"com.example.app": false,
68+
} {
69+
assert.Equal(t, reserved, IsReserved(key), "IsReserved(%q)", key)
70+
}
71+
}
72+
5673
func TestLongKey(t *testing.T) {
5774
key := strings.Repeat("s", keyMaxLen+1)
5875
value := strings.Repeat("v", maxSize-len(key))

0 commit comments

Comments
 (0)