Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.10 backport] Ensures that the primary GID is also included in the additional GIDs #3669

Merged
merged 1 commit into from
Mar 15, 2023
+50 −0
Merged

[0.10 backport] Ensures that the primary GID is also included in the additional GIDs #3669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions executor/oci/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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...)
}
37 changes: 37 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ var allTests = integration.TestFuncs(
testExportedHistory,
testExposeExpansion,
testUser,
testUserAdditionalGids,
testCacheReleased,
testDockerignore,
testDockerignoreInvalid,
Expand Down Expand Up @@ -3108,6 +3109,42 @@ 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 := tmpdir(
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{
builder.DefaultLocalNameDockerfile: dir,
builder.DefaultLocalNameContext: dir,
},
}, nil)
require.NoError(t, err)
}

func testCopyChown(t *testing.T, sb integration.Sandbox) {
f := getFrontend(t, sb)
isFileOp := getFileOp(t, sb)
Expand Down