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

Bug#1031630: marked as done (bullseye-pu: package containerd/1.4.13~ds1-1~deb11u4)



Your message dated Sat, 29 Apr 2023 10:54:14 +0100
with message-id <502b8fb37ece620c9723446611a9287974ba5a0c.camel@adam-barratt.org.uk>
and subject line Closing p-u requests for fixes included in 11.7
has caused the Debian Bug report #1031630,
regarding bullseye-pu: package containerd/1.4.13~ds1-1~deb11u4
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1031630: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1031630
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
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

--- End Message ---
--- Begin Message ---
Package: release.debian.org
Version: 11.7

Hi,

Each of the updates referred to in these requests was included in this
morning's 11.7 point release.

Regards,

Adam

--- End Message ---

Reply to: