From bdbc2ab3b3c389328009b85ccfb3b9cfa7b6c6d5 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Thu, 13 Feb 2020 22:50:17 -0800 Subject: [PATCH] Drop effective and permitted cap for non-root user. Signed-off-by: Lantao Liu --- integration/images/private-workdir/Dockerfile | 20 +++++ integration/images/private-workdir/Makefile | 27 ++++++ integration/main_test.go | 13 +++ integration/non_root_user_cap_test.go | 82 +++++++++++++++++++ pkg/containerd/opts/spec_unix.go | 15 +++- pkg/server/container_create_unix.go | 3 + 6 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 integration/images/private-workdir/Dockerfile create mode 100644 integration/images/private-workdir/Makefile create mode 100644 integration/non_root_user_cap_test.go diff --git a/integration/images/private-workdir/Dockerfile b/integration/images/private-workdir/Dockerfile new file mode 100644 index 000000000..60e3a9f81 --- /dev/null +++ b/integration/images/private-workdir/Dockerfile @@ -0,0 +1,20 @@ +# Copyright 2018 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +FROM busybox + +RUN mkdir /workdir +RUN chmod 700 /workdir +RUN chown nobody /workdir +WORKDIR /workdir diff --git a/integration/images/private-workdir/Makefile b/integration/images/private-workdir/Makefile new file mode 100644 index 000000000..58559acf4 --- /dev/null +++ b/integration/images/private-workdir/Makefile @@ -0,0 +1,27 @@ +# Copyright 2018 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +all: build + +PROJ=gcr.io/k8s-cri-containerd +VERSION=1.0 +IMAGE=$(PROJ)/private-workdir:$(VERSION) + +build: + docker build -t $(IMAGE) . + +push: + gcloud docker -- push $(IMAGE) + +.PHONY: build push diff --git a/integration/main_test.go b/integration/main_test.go index 7993bd4ac..79d8e67e3 100644 --- a/integration/main_test.go +++ b/integration/main_test.go @@ -251,6 +251,19 @@ func WithSupplementalGroups(gids []int64) ContainerOpts { } } +// WithRunAsUser adds RunAsUser uid. +func WithRunAsUser(uid int64) ContainerOpts { + return func(c *runtime.ContainerConfig) { + if c.Linux == nil { + c.Linux = &runtime.LinuxContainerConfig{} + } + if c.Linux.SecurityContext == nil { + c.Linux.SecurityContext = &runtime.LinuxContainerSecurityContext{} + } + c.Linux.SecurityContext.RunAsUser = &runtime.Int64Value{Value: uid} + } +} + // ContainerConfig creates a container config given a name and image name // and additional container config options func ContainerConfig(name, image string, opts ...ContainerOpts) *runtime.ContainerConfig { diff --git a/integration/non_root_user_cap_test.go b/integration/non_root_user_cap_test.go new file mode 100644 index 000000000..08ba3f6d4 --- /dev/null +++ b/integration/non_root_user_cap_test.go @@ -0,0 +1,82 @@ +/* +Copyright The containerd Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" +) + +func TestNonRootUserCap(t *testing.T) { + for name, test := range map[string]struct { + uid int64 + startOK bool + }{ + "shouldn't be able to run container with a private workdir only accessible to nobody as a non-root user": { + uid: 1234, + startOK: false, + }, + "shouldn be able to run container with a private workdir only accessible to nobody as root": { + uid: 0, + startOK: true, + }, + } { + t.Run(name, func(t *testing.T) { + t.Log("Create a sandbox") + sbConfig := PodSandboxConfig("sandbox", "non-root-user-cap") + sb, err := runtimeService.RunPodSandbox(sbConfig, *runtimeHandler) + require.NoError(t, err) + // Make sure the sandbox is cleaned up. + defer func() { + assert.NoError(t, runtimeService.StopPodSandbox(sb)) + assert.NoError(t, runtimeService.RemovePodSandbox(sb)) + }() + + const ( + testImage = "gcr.io/k8s-cri-containerd/private-workdir:1.0" + containerName = "test-container" + ) + t.Logf("Pull test image %q", testImage) + img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) + require.NoError(t, err) + defer func() { + assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) + }() + + t.Log("Create a container to print capabilities") + cnConfig := ContainerConfig( + containerName, + testImage, + WithCommand("ls", "/"), + WithLogPath(containerName), + WithRunAsUser(test.uid), + ) + cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig) + require.NoError(t, err) + + t.Log("Start the container") + if test.startOK { + require.NoError(t, runtimeService.StartContainer(cn)) + } else { + require.Error(t, runtimeService.StartContainer(cn)) + } + }) + } +} diff --git a/pkg/containerd/opts/spec_unix.go b/pkg/containerd/opts/spec_unix.go index d69e4d743..ecd6fe2bc 100644 --- a/pkg/containerd/opts/spec_unix.go +++ b/pkg/containerd/opts/spec_unix.go @@ -328,7 +328,7 @@ func WithDevices(osi osinterface.OS, config *runtime.ContainerConfig) oci.SpecOp } } -// WithCapabilities sets the provided capabilties from the security context +// WithCapabilities sets the provided capabilities from the security context func WithCapabilities(sc *runtime.LinuxContainerSecurityContext) oci.SpecOpts { capabilities := sc.GetCapabilities() if capabilities == nil { @@ -637,3 +637,16 @@ func GetUTSNamespace(pid uint32) string { func GetPIDNamespace(pid uint32) string { return fmt.Sprintf(pidNSFormat, pid) } + +// WithCapDropForNonRootUser drops effective and permitted capabilities +// if the user is non-root. +// See https://github.com/moby/moby/pull/36587. +// +// This option should be behind options for setting users and capabilities. +func WithCapDropForNonRootUser(_ context.Context, _ oci.Client, _ *containers.Container, s *runtimespec.Spec) error { + if s.Process != nil && s.Process.User.UID != 0 && s.Process.Capabilities != nil { + s.Process.Capabilities.Effective = []string{} + s.Process.Capabilities.Permitted = []string{} + } + return nil +} diff --git a/pkg/server/container_create_unix.go b/pkg/server/container_create_unix.go index 6b5de7528..d505367bb 100644 --- a/pkg/server/container_create_unix.go +++ b/pkg/server/container_create_unix.go @@ -288,6 +288,9 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon if seccompSpecOpts != nil { specOpts = append(specOpts, seccompSpecOpts) } + + // Add this at the end to make sure that capabilities and users have both been set. + specOpts = append(specOpts, customopts.WithCapDropForNonRootUser) return specOpts, nil }