From 58ecc5ce14aa84576b714219609ce3bf36994afc Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 26 Oct 2021 22:56:42 -0700 Subject: [PATCH 1/3] executor: make sure supplementary groups are set for unset user Signed-off-by: Tonis Tiigi --- client/client_test.go | 23 ++++++++++++++++++++++- executor/oci/user.go | 9 +++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/client/client_test.go b/client/client_test.go index 9a1c6c26..7e68453f 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1692,13 +1692,21 @@ func testUser(t *testing.T, sb integration.Sandbox) { st := llb.Image("busybox:latest").Run(llb.Shlex(`sh -c "mkdir -m 0777 /wd"`)) run := func(user, cmd string) { - st = st.Run(llb.Shlex(cmd), llb.Dir("/wd"), llb.User(user)) + if user != "" { + st = st.Run(llb.Shlex(cmd), llb.Dir("/wd"), llb.User(user)) + } else { + st = st.Run(llb.Shlex(cmd), llb.Dir("/wd")) + } } run("daemon", `sh -c "id -nu > user"`) run("daemon:daemon", `sh -c "id -ng > group"`) run("daemon:nobody", `sh -c "id -ng > nobody"`) run("1:1", `sh -c "id -g > userone"`) + run("root", `sh -c "id -Gn > root_supplementary"`) + run("", `sh -c "id -Gn > default_supplementary"`) + run("", `rm /etc/passwd /etc/group`) // test that default user still works + run("", `sh -c "id -u > default_uid"`) st = st.Run(llb.Shlex("cp -a /wd/. /out/")) out := st.AddMount("/out", llb.Scratch()) @@ -1736,6 +1744,19 @@ func testUser(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) require.Contains(t, string(dt), "1") + dt, err = ioutil.ReadFile(filepath.Join(destDir, "root_supplementary")) + require.NoError(t, err) + require.True(t, strings.HasPrefix(string(dt), "root ")) + require.True(t, strings.Contains(string(dt), "wheel")) + + dt2, err := ioutil.ReadFile(filepath.Join(destDir, "default_supplementary")) + require.NoError(t, err) + require.Equal(t, string(dt), string(dt2)) + + dt, err = ioutil.ReadFile(filepath.Join(destDir, "default_uid")) + require.NoError(t, err) + require.Equal(t, "0", strings.TrimSpace(string(dt))) + checkAllReleasable(t, c, sb, true) } diff --git a/executor/oci/user.go b/executor/oci/user.go index e49eb8e4..eb459f39 100644 --- a/executor/oci/user.go +++ b/executor/oci/user.go @@ -15,6 +15,12 @@ import ( ) func GetUser(root, username string) (uint32, uint32, []uint32, error) { + var isDefault bool + if username == "" { + username = "0" + isDefault = true + } + // fast path from uid/gid if uid, gid, err := ParseUIDGID(username); err == nil { return uid, gid, nil, nil @@ -31,6 +37,9 @@ func GetUser(root, username string) (uint32, uint32, []uint32, error) { execUser, err := user.GetExecUser(username, nil, passwdFile, groupFile) if err != nil { + if isDefault { + return 0, 0, nil, nil + } return 0, 0, nil, err } var sgids []uint32 From e3ca502c09d40585a94be79322889f497a7262e1 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 27 Oct 2021 11:06:45 -0700 Subject: [PATCH 2/3] client: improve checks for user test Signed-off-by: Tonis Tiigi --- client/client_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 7e68453f..aac75d32 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1730,19 +1730,19 @@ func testUser(t *testing.T, sb integration.Sandbox) { dt, err := ioutil.ReadFile(filepath.Join(destDir, "user")) require.NoError(t, err) - require.Contains(t, string(dt), "daemon") + require.Equal(t, "daemon", strings.TrimSpace(string(dt))) dt, err = ioutil.ReadFile(filepath.Join(destDir, "group")) require.NoError(t, err) - require.Contains(t, string(dt), "daemon") + require.Equal(t, "daemon", strings.TrimSpace(string(dt))) dt, err = ioutil.ReadFile(filepath.Join(destDir, "nobody")) require.NoError(t, err) - require.Contains(t, string(dt), "nobody") + require.Equal(t, "nobody", strings.TrimSpace(string(dt))) dt, err = ioutil.ReadFile(filepath.Join(destDir, "userone")) require.NoError(t, err) - require.Contains(t, string(dt), "1") + require.Equal(t, "1", strings.TrimSpace(string(dt))) dt, err = ioutil.ReadFile(filepath.Join(destDir, "root_supplementary")) require.NoError(t, err) From e82ccdf958820574d3ed77556340200afef71276 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 27 Oct 2021 11:26:10 -0700 Subject: [PATCH 3/3] containerdexecutor: fix setting user Signed-off-by: Tonis Tiigi --- executor/containerdexecutor/executor.go | 32 +++++++++++-------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/executor/containerdexecutor/executor.go b/executor/containerdexecutor/executor.go index d85fa806..cd4bc1d2 100644 --- a/executor/containerdexecutor/executor.go +++ b/executor/containerdexecutor/executor.go @@ -119,27 +119,23 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root executor.M defer lm.Unmount() defer executor.MountStubsCleaner(rootfsPath, mounts)() - var sgids []uint32 - uid, gid, err := oci.ParseUIDGID(meta.User) + uid, gid, sgids, err := oci.GetUser(rootfsPath, meta.User) if err != nil { - uid, gid, sgids, err = oci.GetUser(rootfsPath, meta.User) - if err != nil { - return err - } + return err + } - identity := idtools.Identity{ - UID: int(uid), - GID: int(gid), - } + identity := idtools.Identity{ + UID: int(uid), + GID: int(gid), + } - newp, err := fs.RootPath(rootfsPath, meta.Cwd) - if err != nil { - return errors.Wrapf(err, "working dir %s points to invalid target", newp) - } - if _, err := os.Stat(newp); err != nil { - if err := idtools.MkdirAllAndChown(newp, 0755, identity); err != nil { - return errors.Wrapf(err, "failed to create working directory %s", newp) - } + newp, err := fs.RootPath(rootfsPath, meta.Cwd) + if err != nil { + return errors.Wrapf(err, "working dir %s points to invalid target", newp) + } + if _, err := os.Stat(newp); err != nil { + if err := idtools.MkdirAllAndChown(newp, 0755, identity); err != nil { + return errors.Wrapf(err, "failed to create working directory %s", newp) } }