From c0a981629cf44688687548490c5e665d851afe06 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Fri, 18 Jan 2019 16:07:28 -0800 Subject: [PATCH] Start a sandbox process in a new userns only if CAP_SETUID is set In addition, it fixes a race condition in TestMultiContainerGoferStop. There are two scripts copy the same set of files into the same directory and sometime one of this command fails with EXIST. PiperOrigin-RevId: 230011247 Change-Id: I9289f72e65dc407cdcd0e6cd632a509e01f43e9c --- runsc/container/multi_container_test.go | 11 ++++++----- runsc/sandbox/sandbox.go | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index 8490999ea..8922e6dbe 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -739,11 +739,6 @@ func TestMultiContainerGoferStop(t *testing.T) { t.Fatal("error finding test_app:", err) } - dir, err := ioutil.TempDir(testutil.TmpDir(), "gofer-stop-test") - if err != nil { - t.Fatal("ioutil.TempDir failed:", err) - } - // Setup containers. Root container just reaps children, while the others // perform some IOs. Children are executed in 3 batches of 10. Within the // batch there is overlap between containers starting and being destroyed. In @@ -751,6 +746,12 @@ func TestMultiContainerGoferStop(t *testing.T) { cmds := [][]string{{app, "reaper"}} const batchSize = 10 for i := 0; i < 3*batchSize; i++ { + dir, err := ioutil.TempDir(testutil.TmpDir(), "gofer-stop-test") + if err != nil { + t.Fatal("ioutil.TempDir failed:", err) + } + defer os.RemoveAll(dir) + cmd := "find /bin -type f | head | xargs -I SRC cp SRC " + dir cmds = append(cmds, []string{"sh", "-c", cmd}) } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index d28d93b0a..df4c3c787 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -500,15 +500,15 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund return fmt.Errorf("can't run sandbox process in minimal chroot since we don't have CAP_SYS_ADMIN") } } else { - log.Infof("Sandbox will be started in new user namespace") - nss = append(nss, specs.LinuxNamespace{Type: specs.UserNamespace}) - // If we have CAP_SETUID and CAP_SETGID, then we can also run // as user nobody. if conf.TestOnlyAllowRunAsCurrentUserWithoutChroot { log.Warningf("Running sandbox in test mode as current user (uid=%d gid=%d). This is only safe in tests!", os.Getuid(), os.Getgid()) log.Warningf("Running sandbox in test mode without chroot. This is only safe in tests!") } else if specutils.HasCapabilities(capability.CAP_SETUID, capability.CAP_SETGID) { + log.Infof("Sandbox will be started in new user namespace") + nss = append(nss, specs.LinuxNamespace{Type: specs.UserNamespace}) + // Map nobody in the new namespace to nobody in the parent namespace. // // A sandbox process will construct an empty