Merge pull request from GHSA-hmfx-3pcx-653p

[release/1.5] oci: fix additional GIDs
containerd · Feb 15, 2023 · 28e4618 · 28e4618
2 parents 959e1cf + a62c38b
commit 28e4618
Show file tree

Hide file tree

Showing 5 changed files with 513 additions and 69 deletions.
diff --git a/integration/addition_gids_test.go b/integration/addition_gids_test.go
@@ -20,6 +20,7 @@
 package integration
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -47,8 +48,7 @@ func TestAdditionalGids(t *testing.T) {
 	}()
 
 	var (
-		testImage     = GetImage(BusyBox)
-		containerName = "test-container"
+		testImage = GetImage(BusyBox)
 	)
 	t.Logf("Pull test image %q", testImage)
 	img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig)
@@ -57,34 +57,102 @@ func TestAdditionalGids(t *testing.T) {
 		assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img}))
 	}()
 
-	t.Log("Create a container to print id")
-	cnConfig := ContainerConfig(
-		containerName,
-		testImage,
-		WithCommand("id"),
-		WithLogPath(containerName),
-		WithSupplementalGroups([]int64{1 /*daemon*/, 1234 /*new group*/}),
-	)
-	cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig)
-	require.NoError(t, err)
+	type testCase struct {
+		description string
+		opts        []ContainerOpts
+		expected    string
+	}
+
+	testCases := []testCase{
+		{
+			description: "Equivalent of `docker run` (no option)",
+			opts:        nil,
+			expected:    "groups=0(root),10(wheel)",
+		},
+		{
+			description: "Equivalent of `docker run --group-add 1 --group-add 1234`",
+			opts:        []ContainerOpts{WithSupplementalGroups([]int64{1 /*daemon*/, 1234 /*new group*/})},
+			expected:    "groups=0(root),1(daemon),10(wheel),1234",
+		},
+		{
+			description: "Equivalent of `docker run --user 1234`",
+			opts:        []ContainerOpts{WithRunAsUser(1234)},
+			expected:    "groups=0(root)",
+		},
+		{
+			description: "Equivalent of `docker run --user 1234:1234`",
+			opts:        []ContainerOpts{WithRunAsUser(1234), WithRunAsGroup(1234)},
+			expected:    "groups=1234",
+		},
+		{
+			description: "Equivalent of `docker run --user 1234 --group-add 1234`",
+			opts:        []ContainerOpts{WithRunAsUser(1234), WithSupplementalGroups([]int64{1234})},
+			expected:    "groups=0(root),1234",
+		},
+		{
+			description: "Equivalent of `docker run --user daemon` (Supported by CRI, although unsupported by kube-apiserver)",
+			opts:        []ContainerOpts{WithRunAsUsername("daemon")},
+			expected:    "groups=1(daemon)",
+		},
+		{
+			description: "Equivalent of `docker run --user daemon --group-add 1234` (Supported by CRI, although unsupported by kube-apiserver)",
+			opts:        []ContainerOpts{WithRunAsUsername("daemon"), WithSupplementalGroups([]int64{1234})},
+			expected:    "groups=1(daemon),1234",
+		},
+	}
+
+	for i, tc := range testCases {
+		i, tc := i, tc
+		tBasename := fmt.Sprintf("case-%d", i)
+		t.Run(tBasename, func(t *testing.T) {
+			t.Log(tc.description)
+			t.Logf("Expected=%q", tc.expected)
+
+			testPodLogDir := t.TempDir()
+
+			t.Log("Create a sandbox with log directory")
+			sbConfig := PodSandboxConfig("sandbox", tBasename,
+				WithPodLogDirectory(testPodLogDir))
+			sb, err := runtimeService.RunPodSandbox(sbConfig, *runtimeHandler)
+			require.NoError(t, err)
+			defer func() {
+				assert.NoError(t, runtimeService.StopPodSandbox(sb))
+				assert.NoError(t, runtimeService.RemovePodSandbox(sb))
+			}()
+
+			t.Log("Create a container to print id")
+			containerName := tBasename
+			cnConfig := ContainerConfig(
+				containerName,
+				testImage,
+				append(
+					[]ContainerOpts{
+						WithCommand("id"),
+						WithLogPath(containerName),
+					}, tc.opts...)...,
+			)
+			cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig)
+			require.NoError(t, err)
+
+			t.Log("Start the container")
+			require.NoError(t, runtimeService.StartContainer(cn))
+
+			t.Log("Wait for container to finish running")
+			require.NoError(t, Eventually(func() (bool, error) {
+				s, err := runtimeService.ContainerStatus(cn)
+				if err != nil {
+					return false, err
+				}
+				if s.GetState() == runtime.ContainerState_CONTAINER_EXITED {
+					return true, nil
+				}
+				return false, nil
+			}, time.Second, 30*time.Second))
 
-	t.Log("Start the container")
-	require.NoError(t, runtimeService.StartContainer(cn))
-
-	t.Log("Wait for container to finish running")
-	require.NoError(t, Eventually(func() (bool, error) {
-		s, err := runtimeService.ContainerStatus(cn)
-		if err != nil {
-			return false, err
-		}
-		if s.GetState() == runtime.ContainerState_CONTAINER_EXITED {
-			return true, nil
-		}
-		return false, nil
-	}, time.Second, 30*time.Second))
-
-	t.Log("Search additional groups in container log")
-	content, err := ioutil.ReadFile(filepath.Join(testPodLogDir, containerName))
-	assert.NoError(t, err)
-	assert.Contains(t, string(content), "groups=1(daemon),10(wheel),1234")
+			t.Log("Search additional groups in container log")
+			content, err := os.ReadFile(filepath.Join(testPodLogDir, containerName))
+			assert.NoError(t, err)
+			assert.Contains(t, string(content), tc.expected+"\n")
+		})
+	}
 }
diff --git a/integration/main_test.go b/integration/main_test.go
@@ -256,6 +256,45 @@ func WithLogPath(path string) ContainerOpts {
 	}
 }
 
+// WithRunAsUser sets the uid.
+func WithRunAsUser(uid int64) ContainerOpts {
+	return func(c *runtime.ContainerConfig) {
+		if c.Linux == nil {
+			c.Linux = &runtime.LinuxContainerConfig{}
+		}
+		if c.Linux.SecurityContext == nil {
+			c.Linux.SecurityContext = &runtime.LinuxContainerSecurityContext{}
+		}
+		c.Linux.SecurityContext.RunAsUser = &runtime.Int64Value{Value: uid}
+	}
+}
+
+// WithRunAsUsername sets the username.
+func WithRunAsUsername(username string) ContainerOpts {
+	return func(c *runtime.ContainerConfig) {
+		if c.Linux == nil {
+			c.Linux = &runtime.LinuxContainerConfig{}
+		}
+		if c.Linux.SecurityContext == nil {
+			c.Linux.SecurityContext = &runtime.LinuxContainerSecurityContext{}
+		}
+		c.Linux.SecurityContext.RunAsUsername = username
+	}
+}
+
+// WithRunAsGroup sets the gid.
+func WithRunAsGroup(gid int64) ContainerOpts {
+	return func(c *runtime.ContainerConfig) {
+		if c.Linux == nil {
+			c.Linux = &runtime.LinuxContainerConfig{}
+		}
+		if c.Linux.SecurityContext == nil {
+			c.Linux.SecurityContext = &runtime.LinuxContainerSecurityContext{}
+		}
+		c.Linux.SecurityContext.RunAsGroup = &runtime.Int64Value{Value: gid}
+	}
+}
+
 // WithSupplementalGroups adds supplemental groups.
 func WithSupplementalGroups(gids []int64) ContainerOpts {
 	return func(c *runtime.ContainerConfig) {