Merge pull request moby#3651 from AkihiroSuda/fix-additional-gids

Ensures that the primary GID is also included in the additional GIDs

Author: tonistiigi

executor/oci/user.go

@@ -91,6 +91,7 @@ func parseUID(str string) (uint32, error) {
 // once the PR in containerd is merged we should remove this function.
 func WithUIDGID(uid, gid uint32, sgids []uint32) containerdoci.SpecOpts {
 	return func(_ context.Context, _ containerdoci.Client, _ *containers.Container, s *containerdoci.Spec) error {
+		defer ensureAdditionalGids(s)
 		setProcess(s)
 		s.Process.User.UID = uid
 		s.Process.User.GID = gid
@@ -106,3 +107,15 @@ func setProcess(s *containerdoci.Spec) {
 		s.Process = &specs.Process{}
 	}
 }
+
+// ensureAdditionalGids ensures that the primary GID is also included in the additional GID list.
+// From https://github.com/containerd/containerd/blob/v1.7.0-beta.4/oci/spec_opts.go#L124-L133
+func ensureAdditionalGids(s *containerdoci.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...)
+}

frontend/dockerfile/dockerfile_test.go

@@ -71,6 +71,7 @@ var allTests = integration.TestFuncs(
 	testExportedHistory,
 	testExposeExpansion,
 	testUser,
+	testUserAdditionalGids,
 	testCacheReleased,
 	testDockerignore,
 	testDockerignoreInvalid,
@@ -3005,6 +3006,43 @@ USER nobody
 	require.Equal(t, "nobody", ociimg.Config.User)
 }
 
+// testUserAdditionalGids ensures that that the primary GID is also included in the additional GID list.
+// CVE-2023-25173: https://github.com/advisories/GHSA-hmfx-3pcx-653p
+func testUserAdditionalGids(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+# Mimics the tests in https://github.com/containerd/containerd/commit/3eda46af12b1deedab3d0802adb2e81cb3521950
+FROM busybox
+SHELL ["/bin/sh", "-euxc"]
+RUN [ "$(id)" = "uid=0(root) gid=0(root) groups=0(root),10(wheel)" ]
+USER 1234
+RUN [ "$(id)" = "uid=1234 gid=0(root) groups=0(root)" ]
+USER 1234:1234
+RUN [ "$(id)" = "uid=1234 gid=1234 groups=1234" ]
+USER daemon
+RUN [ "$(id)" = "uid=1(daemon) gid=1(daemon) groups=1(daemon)" ]
+`)
+
+	dir, err := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+}
+
 func testCopyChown(t *testing.T, sb integration.Sandbox) {
 	f := getFrontend(t, sb)