From bf2b8a05a0b7bd07eb35ab73d4e6af50651a8625 Mon Sep 17 00:00:00 2001 From: Luboslav Pivarc Date: Wed, 10 May 2023 10:09:21 +0200 Subject: [PATCH 1/2] Do not drop effective&permitted set Currently moby drops ep sets before the entrypoint is executed. This does mean that with combination of no-new-privileges the file capabilities stops working with non-root containers. This is undesired as the usability of such containers is harmed comparing to running root containers. This commit therefore sets the effective/permitted set in order to allow use of file capabilities or libcap(3)/prctl(2) respectively with combination of no-new-privileges and without respectively. For no-new-privileges the container will be able to obtain capabilities that are requested. Signed-off-by: Luboslav Pivarc Signed-off-by: Bjorn Neergaard (cherry picked from commit 3aef732e61ec8ae0ea0bd8ad31116194e0fc21a6) Signed-off-by: Sebastiaan van Stijn --- oci/oci.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/oci/oci.go b/oci/oci.go index 864ccf5b60..45ed7979ee 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -23,19 +23,10 @@ func SetCapabilities(s *specs.Spec, caplist []string) error { if s.Process == nil { s.Process = &specs.Process{} } - // setUser has already been executed here - if s.Process.User.UID == 0 { - s.Process.Capabilities = &specs.LinuxCapabilities{ - Effective: caplist, - Bounding: caplist, - Permitted: caplist, - } - } else { - // Do not set Effective and Permitted capabilities for non-root users, - // to match what execve does. - s.Process.Capabilities = &specs.LinuxCapabilities{ - Bounding: caplist, - } + s.Process.Capabilities = &specs.LinuxCapabilities{ + Effective: caplist, + Bounding: caplist, + Permitted: caplist, } return nil } From cc39fb9f6b6863998f061f303f0a10377336cc6b Mon Sep 17 00:00:00 2001 From: Luboslav Pivarc Date: Mon, 17 Jul 2023 11:48:02 +0200 Subject: [PATCH 2/2] Integration test for capabilities Verify non-root containers are able to use file capabilities. Signed-off-by: Luboslav Pivarc Co-authored-by: Cory Snider Signed-off-by: Cory Snider (cherry picked from commit 42fa7a1951f192a0038b904f529e86beeb056093) Signed-off-by: Sebastiaan van Stijn --- .../capabilities/capabilities_linux_test.go | 108 ++++++++++++++++++ integration/capabilities/main_linux_test.go | 33 ++++++ integration/internal/container/ops.go | 18 +++ 3 files changed, 159 insertions(+) create mode 100644 integration/capabilities/capabilities_linux_test.go create mode 100644 integration/capabilities/main_linux_test.go diff --git a/integration/capabilities/capabilities_linux_test.go b/integration/capabilities/capabilities_linux_test.go new file mode 100644 index 0000000000..272f3dcdb7 --- /dev/null +++ b/integration/capabilities/capabilities_linux_test.go @@ -0,0 +1,108 @@ +package capabilities + +import ( + "bytes" + "context" + "io" + "strings" + "testing" + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/pkg/stdcopy" + "github.com/docker/docker/testutil/fakecontext" + + "gotest.tools/v3/assert" + "gotest.tools/v3/poll" +) + +func TestNoNewPrivileges(t *testing.T) { + defer setupTest(t)() + + withFileCapability := ` + FROM debian:bullseye-slim + RUN apt-get update && apt-get install -y libcap2-bin --no-install-recommends + RUN setcap CAP_DAC_OVERRIDE=+eip /bin/cat + RUN echo "hello" > /txt && chown 0:0 /txt && chmod 700 /txt + RUN useradd -u 1500 test + ` + imageTag := "captest" + + source := fakecontext.New(t, "", fakecontext.WithDockerfile(withFileCapability)) + defer source.Close() + + client := testEnv.APIClient() + + // Build image + ctx := context.TODO() + resp, err := client.ImageBuild(ctx, + source.AsTarReader(t), + types.ImageBuildOptions{ + Tags: []string{imageTag}, + }) + assert.NilError(t, err) + _, err = io.Copy(io.Discard, resp.Body) + assert.NilError(t, err) + resp.Body.Close() + + testCases := []struct { + doc string + opts []func(*container.TestContainerConfig) + stdOut, stdErr string + }{ + { + doc: "CapabilityRequested=true", + opts: []func(*container.TestContainerConfig){ + container.WithUser("test"), + container.WithCapability("CAP_DAC_OVERRIDE"), + }, + stdOut: "hello", + }, + { + doc: "CapabilityRequested=false", + opts: []func(*container.TestContainerConfig){ + container.WithUser("test"), + container.WithDropCapability("CAP_DAC_OVERRIDE"), + }, + stdErr: "exec /bin/cat: operation not permitted", + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + // Run the container with the image + opts := append(tc.opts, + container.WithImage(imageTag), + container.WithCmd("/bin/cat", "/txt"), + container.WithSecurityOpt("no-new-privileges=true"), + ) + cid := container.Run(ctx, t, client, opts...) + poll.WaitOn(t, container.IsInState(ctx, client, cid, "exited"), poll.WithDelay(100*time.Millisecond)) + + // Assert on outputs + logReader, err := client.ContainerLogs(ctx, cid, types.ContainerLogsOptions{ + ShowStdout: true, + ShowStderr: true, + }) + assert.NilError(t, err) + defer logReader.Close() + + var actualStdout, actualStderr bytes.Buffer + _, err = stdcopy.StdCopy(&actualStdout, &actualStderr, logReader) + assert.NilError(t, err) + + stdOut := strings.TrimSpace(actualStdout.String()) + stdErr := strings.TrimSpace(actualStderr.String()) + if stdOut != tc.stdOut { + t.Fatalf("test produced invalid output: %q, expected %q. Stderr:%q", stdOut, tc.stdOut, stdErr) + } + if stdErr != tc.stdErr { + t.Fatalf("test produced invalid error: %q, expected %q. Stdout:%q", stdErr, tc.stdErr, stdOut) + + } + }) + } + +} diff --git a/integration/capabilities/main_linux_test.go b/integration/capabilities/main_linux_test.go new file mode 100644 index 0000000000..0f074dd156 --- /dev/null +++ b/integration/capabilities/main_linux_test.go @@ -0,0 +1,33 @@ +package capabilities + +import ( + "fmt" + "os" + "testing" + + "github.com/docker/docker/testutil/environment" +) + +var testEnv *environment.Execution + +func TestMain(m *testing.M) { + var err error + testEnv, err = environment.New() + if err != nil { + fmt.Println(err) + os.Exit(1) + } + err = environment.EnsureFrozenImagesLinux(testEnv) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + testEnv.Print() + os.Exit(m.Run()) +} + +func setupTest(t *testing.T) func() { + environment.ProtectAll(t, testEnv) + return func() { testEnv.Clean(t) } +} diff --git a/integration/internal/container/ops.go b/integration/internal/container/ops.go index 33d9777006..838f515e9f 100644 --- a/integration/internal/container/ops.go +++ b/integration/internal/container/ops.go @@ -237,3 +237,21 @@ func WithRuntime(name string) func(*TestContainerConfig) { c.HostConfig.Runtime = name } } + +func WithCapability(capabilities ...string) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + c.HostConfig.CapAdd = append(c.HostConfig.CapAdd, capabilities...) + } +} + +func WithDropCapability(capabilities ...string) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + c.HostConfig.CapDrop = append(c.HostConfig.CapDrop, capabilities...) + } +} + +func WithSecurityOpt(opt string) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + c.HostConfig.SecurityOpt = append(c.HostConfig.SecurityOpt, opt) + } +}