[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1031630: bullseye-pu: package containerd/1.4.13~ds1-1~deb11u4



Package: release.debian.org
Severity: normal
Tags: bullseye
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: containerd@packages.debian.org, team@security.debian.org, zhsj@debian.org
Control: affects -1 + src:containerd

[ Reason ]

Backport patches for 2 CVE:

* CVE-2023-25153: OCI image importer memory exhaustion
* CVE-2023-25173: Supplementary groups are not set up properly

[ Impact ]

[ Tests ]

CVE-2023-25153 is simple fix without test.
CVE-2023-25173 is covered by some unit tests and I've tested
it on a Kubernetes cluster.

[ Risks ]

The patch for CVE-2023-25153 is simple and cherry-picked from upstream
1.5 branch directly. It should not be risky.

The patch for CVE-2023-25173 is difficult to backport (many conflicts
when apply patches from 1.5 branch).
So I have rewrite it based on the commits on 1.5 branch. Especially this
commit https://github.com/containerd/containerd/commit/a62c38bf.
The 1.4.x package doesn't include CRI integration tests so I have to test
it by hand on a real Kubernetes cluster. In upstream a62c38bf commit message,
they have shown the tests cases. So I use them to verify on Kubernetes.

With containerd/1.4.13~ds1-1~deb11u3, the log is

NAMESPACE NAME                            READY   STATUS RESTARTS AGE
default   test-group-add-1-group-add-1234 0/1     Error  0        14m
default   test-no-option                  0/1     Error  0        14m
default   test-user-1234                  0/1     Error  0        14m
default   test-user-1234-1234             0/1     Error  0        14m
default   test-user-1234-group-add-1234   0/1     Error  0        14m

The container output is like

# kubectl logs test-no-option
+ id
+ '[' 'uid=0(root) gid=0(root) groups=10(wheel)' '=' 'uid=0(root) gid=0(root) groups=0(root),10(wheel)' ]

With containerd/1.4.13~ds1-1~deb11u4, the log is

NAMESPACE NAME                            READY   STATUS      RESTARTS  AGE
default   test-group-add-1-group-add-1234 0/1     Completed   0         4s
default   test-no-option                  0/1     Completed   0         4s
default   test-user-1234                  0/1     Completed   0         4s
default   test-user-1234-1234             0/1     Completed   0         4s
default   test-user-1234-group-add-1234   0/1     Completed   0         4s

The container output is like

# kubectl logs test-no-option
+ id
+ '[' 'uid=0(root) gid=0(root) groups=0(root),10(wheel)' '=' 'uid=0(root) gid=0(root) groups=0(root),10(wheel)' ]

It has passed the upstream tests.

[ Checklist ]
  [x] *all* changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in (old)stable
  [x] the issue is verified as fixed in unstable

[ Changes ]

See attachment.

[ Other info ]

No
diff -Nru containerd-1.4.13~ds1/debian/changelog containerd-1.4.13~ds1/debian/changelog
--- containerd-1.4.13~ds1/debian/changelog	2022-12-08 10:24:34.000000000 +0800
+++ containerd-1.4.13~ds1/debian/changelog	2023-02-17 23:11:26.000000000 +0800
@@ -1,3 +1,10 @@
+containerd (1.4.13~ds1-1~deb11u4) bullseye; urgency=medium
+
+  * CVE-2023-25153: OCI image importer memory exhaustion
+  * CVE-2023-25173: Supplementary groups are not set up properly
+
+ -- Shengjing Zhu <zhsj@debian.org>  Fri, 17 Feb 2023 23:11:26 +0800
+
 containerd (1.4.13~ds1-1~deb11u3) bullseye; urgency=medium
 
   * CVE-2022-23471: CRI plugin: Fix goroutine leak during Exec
diff -Nru containerd-1.4.13~ds1/debian/patches/0012-CVE-2023-25153.patch containerd-1.4.13~ds1/debian/patches/0012-CVE-2023-25153.patch
--- containerd-1.4.13~ds1/debian/patches/0012-CVE-2023-25153.patch	1970-01-01 08:00:00.000000000 +0800
+++ containerd-1.4.13~ds1/debian/patches/0012-CVE-2023-25153.patch	2023-02-17 23:11:26.000000000 +0800
@@ -0,0 +1,41 @@
+From: Samuel Karp <samuelkarp@google.com>
+Date: Thu, 12 Jan 2023 18:06:41 -0800
+Subject: CVE-2023-25153
+
+Origin: backport, https://github.com/containerd/containerd/commit/959e1cf9
+---
+ images/archive/importer.go | 13 +++++++------
+ 1 file changed, 7 insertions(+), 6 deletions(-)
+
+diff --git a/images/archive/importer.go b/images/archive/importer.go
+index 2d04658..f24ab87 100644
+--- a/images/archive/importer.go
++++ b/images/archive/importer.go
+@@ -24,7 +24,6 @@ import (
+ 	"encoding/json"
+ 	"fmt"
+ 	"io"
+-	"io/ioutil"
+ 	"path"
+ 
+ 	"github.com/containerd/containerd/archive/compression"
+@@ -222,12 +221,14 @@ func ImportIndex(ctx context.Context, store content.Store, reader io.Reader, opt
+ 	return writeManifest(ctx, store, idx, ocispec.MediaTypeImageIndex)
+ }
+ 
++const (
++	kib       = 1024
++	mib       = 1024 * kib
++	jsonLimit = 20 * mib
++)
++
+ func onUntarJSON(r io.Reader, j interface{}) error {
+-	b, err := ioutil.ReadAll(r)
+-	if err != nil {
+-		return err
+-	}
+-	return json.Unmarshal(b, j)
++	return json.NewDecoder(io.LimitReader(r, jsonLimit)).Decode(j)
+ }
+ 
+ func onUntarBlob(ctx context.Context, r io.Reader, store content.Ingester, size int64, ref string) (digest.Digest, error) {
diff -Nru containerd-1.4.13~ds1/debian/patches/0013-CVE-2023-25173.patch containerd-1.4.13~ds1/debian/patches/0013-CVE-2023-25173.patch
--- containerd-1.4.13~ds1/debian/patches/0013-CVE-2023-25173.patch	1970-01-01 08:00:00.000000000 +0800
+++ containerd-1.4.13~ds1/debian/patches/0013-CVE-2023-25173.patch	2023-02-17 23:11:26.000000000 +0800
@@ -0,0 +1,316 @@
+From: Shengjing Zhu <zhsj@debian.org>
+Date: Fri, 17 Feb 2023 23:08:38 +0800
+Subject: CVE-2023-25173
+
+Origin: backport, https://github.com/containerd/containerd/commit/a62c38bf
+---
+ oci/spec_opts.go                                   |  21 ++
+ oci/spec_opts_linux_test.go                        | 212 +++++++++++++++++++++
+ .../cri/pkg/server/container_create_unix.go        |   3 +-
+ 3 files changed, 235 insertions(+), 1 deletion(-)
+ create mode 100644 oci/spec_opts_linux_test.go
+
+diff --git a/oci/spec_opts.go b/oci/spec_opts.go
+index 1372584..13ed7dd 100644
+--- a/oci/spec_opts.go
++++ b/oci/spec_opts.go
+@@ -114,6 +114,17 @@ func setCapabilities(s *Spec) {
+ 	}
+ }
+ 
++// ensureAdditionalGids ensures that the primary GID is also included in the additional GID list.
++func ensureAdditionalGids(s *Spec) {
++	setProcess(s)
++	for _, f := range s.Process.User.AdditionalGids {
++		if f == s.Process.User.GID {
++			return
++		}
++	}
++	s.Process.User.AdditionalGids = append([]uint32{s.Process.User.GID}, s.Process.User.AdditionalGids...)
++}
++
+ // WithDefaultSpec returns a SpecOpts that will populate the spec with default
+ // values.
+ //
+@@ -503,7 +514,9 @@ func WithNamespacedCgroup() SpecOpts {
+ //   user, uid, user:group, uid:gid, uid:group, user:gid
+ func WithUser(userstr string) SpecOpts {
+ 	return func(ctx context.Context, client Client, c *containers.Container, s *Spec) error {
++		defer ensureAdditionalGids(s)
+ 		setProcess(s)
++		s.Process.User.AdditionalGids = nil
+ 		parts := strings.Split(userstr, ":")
+ 		switch len(parts) {
+ 		case 1:
+@@ -582,7 +595,9 @@ func WithUser(userstr string) SpecOpts {
+ // WithUIDGID allows the UID and GID for the Process to be set
+ func WithUIDGID(uid, gid uint32) SpecOpts {
+ 	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
++		defer ensureAdditionalGids(s)
+ 		setProcess(s)
++		s.Process.User.AdditionalGids = nil
+ 		s.Process.User.UID = uid
+ 		s.Process.User.GID = gid
+ 		return nil
+@@ -595,7 +610,9 @@ func WithUIDGID(uid, gid uint32) SpecOpts {
+ // additionally sets the gid to 0, and does not return an error.
+ func WithUserID(uid uint32) SpecOpts {
+ 	return func(ctx context.Context, client Client, c *containers.Container, s *Spec) (err error) {
++		defer ensureAdditionalGids(s)
+ 		setProcess(s)
++		s.Process.User.AdditionalGids = nil
+ 		if c.Snapshotter == "" && c.SnapshotKey == "" {
+ 			if !isRootfsAbs(s.Root.Path) {
+ 				return errors.Errorf("rootfs absolute path is required")
+@@ -648,7 +665,9 @@ func WithUserID(uid uint32) SpecOpts {
+ // it returns error.
+ func WithUsername(username string) SpecOpts {
+ 	return func(ctx context.Context, client Client, c *containers.Container, s *Spec) (err error) {
++		defer ensureAdditionalGids(s)
+ 		setProcess(s)
++		s.Process.User.AdditionalGids = nil
+ 		if s.Linux != nil {
+ 			if c.Snapshotter == "" && c.SnapshotKey == "" {
+ 				if !isRootfsAbs(s.Root.Path) {
+@@ -703,7 +722,9 @@ func WithAdditionalGIDs(userstr string) SpecOpts {
+ 			return nil
+ 		}
+ 		setProcess(s)
++		s.Process.User.AdditionalGids = nil
+ 		setAdditionalGids := func(root string) error {
++			defer ensureAdditionalGids(s)
+ 			var username string
+ 			uid, err := strconv.Atoi(userstr)
+ 			if err == nil {
+diff --git a/oci/spec_opts_linux_test.go b/oci/spec_opts_linux_test.go
+new file mode 100644
+index 0000000..29cb3f8
+--- /dev/null
++++ b/oci/spec_opts_linux_test.go
+@@ -0,0 +1,212 @@
++/*
++   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 oci
++
++import (
++	"context"
++	"fmt"
++	"testing"
++
++	"github.com/containerd/containerd/containers"
++	"github.com/containerd/continuity/fs/fstest"
++	specs "github.com/opencontainers/runtime-spec/specs-go"
++	"github.com/stretchr/testify/assert"
++)
++
++// nolint:gosec
++func TestWithUserID(t *testing.T) {
++	t.Parallel()
++
++	expectedPasswd := `root:x:0:0:root:/root:/bin/ash
++guest:x:405:100:guest:/dev/null:/sbin/nologin
++`
++	td := t.TempDir()
++	apply := fstest.Apply(
++		fstest.CreateDir("/etc", 0777),
++		fstest.CreateFile("/etc/passwd", []byte(expectedPasswd), 0777),
++	)
++	if err := apply.Apply(td); err != nil {
++		t.Fatalf("failed to apply: %v", err)
++	}
++	c := containers.Container{ID: t.Name()}
++	testCases := []struct {
++		userID      uint32
++		expectedUID uint32
++		expectedGID uint32
++	}{
++		{
++			userID:      0,
++			expectedUID: 0,
++			expectedGID: 0,
++		},
++		{
++			userID:      405,
++			expectedUID: 405,
++			expectedGID: 100,
++		},
++		{
++			userID:      1000,
++			expectedUID: 1000,
++			expectedGID: 0,
++		},
++	}
++	for _, testCase := range testCases {
++		testCase := testCase
++		t.Run(fmt.Sprintf("user %d", testCase.userID), func(t *testing.T) {
++			t.Parallel()
++			s := Spec{
++				Version: specs.Version,
++				Root: &specs.Root{
++					Path: td,
++				},
++				Linux: &specs.Linux{},
++			}
++			err := WithUserID(testCase.userID)(context.Background(), nil, &c, &s)
++			assert.NoError(t, err)
++			assert.Equal(t, testCase.expectedUID, s.Process.User.UID)
++			assert.Equal(t, testCase.expectedGID, s.Process.User.GID)
++		})
++	}
++}
++
++// nolint:gosec
++func TestWithUsername(t *testing.T) {
++	t.Parallel()
++
++	expectedPasswd := `root:x:0:0:root:/root:/bin/ash
++guest:x:405:100:guest:/dev/null:/sbin/nologin
++`
++	td := t.TempDir()
++	apply := fstest.Apply(
++		fstest.CreateDir("/etc", 0777),
++		fstest.CreateFile("/etc/passwd", []byte(expectedPasswd), 0777),
++	)
++	if err := apply.Apply(td); err != nil {
++		t.Fatalf("failed to apply: %v", err)
++	}
++	c := containers.Container{ID: t.Name()}
++	testCases := []struct {
++		user        string
++		expectedUID uint32
++		expectedGID uint32
++		err         string
++	}{
++		{
++			user:        "root",
++			expectedUID: 0,
++			expectedGID: 0,
++		},
++		{
++			user:        "guest",
++			expectedUID: 405,
++			expectedGID: 100,
++		},
++		{
++			user: "1000",
++			err:  "no users found",
++		},
++		{
++			user: "unknown",
++			err:  "no users found",
++		},
++	}
++	for _, testCase := range testCases {
++		testCase := testCase
++		t.Run(testCase.user, func(t *testing.T) {
++			t.Parallel()
++			s := Spec{
++				Version: specs.Version,
++				Root: &specs.Root{
++					Path: td,
++				},
++				Linux: &specs.Linux{},
++			}
++			err := WithUsername(testCase.user)(context.Background(), nil, &c, &s)
++			if err != nil {
++				assert.EqualError(t, err, testCase.err)
++			}
++			assert.Equal(t, testCase.expectedUID, s.Process.User.UID)
++			assert.Equal(t, testCase.expectedGID, s.Process.User.GID)
++		})
++	}
++
++}
++
++// nolint:gosec
++func TestWithAdditionalGIDs(t *testing.T) {
++	t.Parallel()
++	expectedPasswd := `root:x:0:0:root:/root:/bin/ash
++bin:x:1:1:bin:/bin:/sbin/nologin
++daemon:x:2:2:daemon:/sbin:/sbin/nologin
++`
++	expectedGroup := `root:x:0:root
++bin:x:1:root,bin,daemon
++daemon:x:2:root,bin,daemon
++sys:x:3:root,bin,adm
++`
++	td := t.TempDir()
++	apply := fstest.Apply(
++		fstest.CreateDir("/etc", 0777),
++		fstest.CreateFile("/etc/passwd", []byte(expectedPasswd), 0777),
++		fstest.CreateFile("/etc/group", []byte(expectedGroup), 0777),
++	)
++	if err := apply.Apply(td); err != nil {
++		t.Fatalf("failed to apply: %v", err)
++	}
++	c := containers.Container{ID: t.Name()}
++
++	testCases := []struct {
++		user     string
++		expected []uint32
++	}{
++		{
++			user:     "root",
++			expected: []uint32{0, 1, 2, 3},
++		},
++		{
++			user:     "1000",
++			expected: []uint32{0},
++		},
++		{
++			user:     "bin",
++			expected: []uint32{0, 2, 3},
++		},
++		{
++			user:     "bin:root",
++			expected: []uint32{0},
++		},
++		{
++			user:     "daemon",
++			expected: []uint32{0, 1},
++		},
++	}
++	for _, testCase := range testCases {
++		testCase := testCase
++		t.Run(testCase.user, func(t *testing.T) {
++			t.Parallel()
++			s := Spec{
++				Version: specs.Version,
++				Root: &specs.Root{
++					Path: td,
++				},
++			}
++			err := WithAdditionalGIDs(testCase.user)(context.Background(), nil, &c, &s)
++			assert.NoError(t, err)
++			assert.Equal(t, testCase.expected, s.Process.User.AdditionalGids)
++		})
++	}
++}
+diff --git a/vendor/github.com/containerd/cri/pkg/server/container_create_unix.go b/vendor/github.com/containerd/cri/pkg/server/container_create_unix.go
+index bbe55e2..262ea44 100644
+--- a/vendor/github.com/containerd/cri/pkg/server/container_create_unix.go
++++ b/vendor/github.com/containerd/cri/pkg/server/container_create_unix.go
+@@ -299,7 +299,8 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
+ 		// Because it is still useful to get additional gids for uid 0.
+ 		userstr = strconv.FormatInt(securityContext.GetRunAsUser().GetValue(), 10)
+ 	}
+-	specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr))
++	specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr),
++			customopts.WithSupplementalGroups(securityContext.GetSupplementalGroups()))
+ 
+ 	apparmorSpecOpts, err := generateApparmorSpecOpts(
+ 		securityContext.GetApparmorProfile(),
diff -Nru containerd-1.4.13~ds1/debian/patches/series containerd-1.4.13~ds1/debian/patches/series
--- containerd-1.4.13~ds1/debian/patches/series	2022-12-08 10:24:34.000000000 +0800
+++ containerd-1.4.13~ds1/debian/patches/series	2023-02-17 23:11:26.000000000 +0800
@@ -9,3 +9,5 @@
 0009-CVE-2022-31030.patch
 0010-CVE-2022-24769.patch
 0011-CVE-2022-23471.patch
+0012-CVE-2023-25153.patch
+0013-CVE-2023-25173.patch

Reply to: