From 74bb1ce9e9dbfa9dd866e84f891e865fca906d9a Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Sun, 21 Feb 2016 21:31:21 -0800 Subject: [PATCH] Add support for NoNewPrivileges in docker Signed-off-by: Mrunal Patel Add tests for no-new-privileges Signed-off-by: Mrunal Patel Update documentation for no-new-privileges Signed-off-by: Mrunal Patel --- container/container_unix.go | 1 + contrib/nnp-test/Dockerfile | 9 +++++++ contrib/nnp-test/nnp-test.c | 10 ++++++++ daemon/container_operations_unix.go | 1 + daemon/daemon_unix.go | 28 +++++++++++++-------- daemon/execdriver/driver_unix.go | 1 + daemon/execdriver/native/create.go | 2 ++ docs/reference/run.md | 9 +++++++ hack/make/.ensure-nnp-test | 22 ++++++++++++++++ hack/make/.integration-daemon-setup | 1 + integration-cli/docker_cli_run_unix_test.go | 12 +++++++++ man/docker-run.1.md | 2 ++ runconfig/opts/parse.go | 4 +-- 13 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 contrib/nnp-test/Dockerfile create mode 100644 contrib/nnp-test/nnp-test.c create mode 100644 hack/make/.ensure-nnp-test diff --git a/container/container_unix.go b/container/container_unix.go index 3fdafbffcc..61daa177b1 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -50,6 +50,7 @@ type Container struct { ShmPath string ResolvConfPath string SeccompProfile string + NoNewPrivileges bool } // CreateDaemonEnvironment returns the list of all environment variables given the list of diff --git a/contrib/nnp-test/Dockerfile b/contrib/nnp-test/Dockerfile new file mode 100644 index 0000000000..026d86954f --- /dev/null +++ b/contrib/nnp-test/Dockerfile @@ -0,0 +1,9 @@ +FROM buildpack-deps:jessie + +COPY . /usr/src/ + +WORKDIR /usr/src/ + +RUN gcc -g -Wall -static nnp-test.c -o /usr/bin/nnp-test + +RUN chmod +s /usr/bin/nnp-test diff --git a/contrib/nnp-test/nnp-test.c b/contrib/nnp-test/nnp-test.c new file mode 100644 index 0000000000..b767da7e1a --- /dev/null +++ b/contrib/nnp-test/nnp-test.c @@ -0,0 +1,10 @@ +#include +#include +#include + +int main(int argc, char *argv[]) +{ + printf("EUID=%d\n", geteuid()); + return 0; +} + diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index 54a086d495..79bd1a90dd 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -270,6 +270,7 @@ func (daemon *Daemon) populateCommand(c *container.Container, env []string) erro SeccompProfile: c.SeccompProfile, UIDMapping: uidMap, UTS: uts, + NoNewPrivileges: c.NoNewPrivileges, } if c.HostConfig.CgroupParent != "" { c.Command.CgroupParent = c.HostConfig.CgroupParent diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index d8d4e56e39..80b1ea076a 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -75,17 +75,23 @@ func parseSecurityOpt(container *container.Container, config *containertypes.Hos for _, opt := range config.SecurityOpt { con := strings.SplitN(opt, ":", 2) if len(con) == 1 { - return fmt.Errorf("Invalid --security-opt: %q", opt) - } - switch con[0] { - case "label": - labelOpts = append(labelOpts, con[1]) - case "apparmor": - container.AppArmorProfile = con[1] - case "seccomp": - container.SeccompProfile = con[1] - default: - return fmt.Errorf("Invalid --security-opt: %q", opt) + switch con[0] { + case "no-new-privileges": + container.NoNewPrivileges = true + default: + return fmt.Errorf("Invalid --security-opt 1: %q", opt) + } + } else { + switch con[0] { + case "label": + labelOpts = append(labelOpts, con[1]) + case "apparmor": + container.AppArmorProfile = con[1] + case "seccomp": + container.SeccompProfile = con[1] + default: + return fmt.Errorf("Invalid --security-opt 2: %q", opt) + } } } diff --git a/daemon/execdriver/driver_unix.go b/daemon/execdriver/driver_unix.go index 3ed3c8170f..851d6541c3 100644 --- a/daemon/execdriver/driver_unix.go +++ b/daemon/execdriver/driver_unix.go @@ -124,6 +124,7 @@ type Command struct { SeccompProfile string `json:"seccomp_profile"` UIDMapping []idtools.IDMap `json:"uidmapping"` UTS *UTS `json:"uts"` + NoNewPrivileges bool `json:"no_new_privileges"` } // SetRootPropagation sets the root mount propagation mode. diff --git a/daemon/execdriver/native/create.go b/daemon/execdriver/native/create.go index 103791d7c9..d85898f673 100644 --- a/daemon/execdriver/native/create.go +++ b/daemon/execdriver/native/create.go @@ -122,6 +122,8 @@ func (d *Driver) createContainer(c *execdriver.Command, hooks execdriver.Hooks) d.setupLabels(container, c) d.setupRlimits(container, c) + + container.NoNewPrivileges = c.NoNewPrivileges return container, nil } diff --git a/docs/reference/run.md b/docs/reference/run.md index ba2fc2d918..4be50a2d02 100644 --- a/docs/reference/run.md +++ b/docs/reference/run.md @@ -605,6 +605,8 @@ with the same logic -- if the original volume was specified with a name it will --security-opt="label:disable" : Turn off label confinement for the container --security-opt="apparmor:PROFILE" : Set the apparmor profile to be applied to the container + --security-opt="no-new-privileges" : Disable container processes from gaining + new privileges You can override the default labeling scheme for each container by specifying the `--security-opt` flag. For example, you can specify the MCS/MLS level, a @@ -631,6 +633,13 @@ command: > **Note**: You would have to write policy defining a `svirt_apache_t` type. +If you want to prevent your container processes from gaining additional +privileges, you can execute the following command: + + $ docker run --security-opt no-new-privileges -it centos bash + +For more details, see [kernel documentation](https://www.kernel.org/doc/Documentation/prctl/no_new_privs.txt). + ## Specifying custom cgroups Using the `--cgroup-parent` flag, you can pass a specific cgroup to run a diff --git a/hack/make/.ensure-nnp-test b/hack/make/.ensure-nnp-test new file mode 100644 index 0000000000..26b11b9a5c --- /dev/null +++ b/hack/make/.ensure-nnp-test @@ -0,0 +1,22 @@ +#!/bin/bash +set -e + +# Build a C binary for testing no-new-privileges +# and compile it for target daemon +if [ "$DOCKER_ENGINE_GOOS" = "linux" ]; then + if [ "$DOCKER_ENGINE_OSARCH" = "$DOCKER_CLIENT_OSARCH" ]; then + tmpdir=$(mktemp -d) + gcc -g -Wall -static contrib/nnp-test/nnp-test.c -o "${tmpdir}/nnp-test" + + dockerfile="${tmpdir}/Dockerfile" + cat <<-EOF > "$dockerfile" + FROM debian:jessie + COPY . /usr/bin/ + RUN chmod +s /usr/bin/nnp-test + EOF + docker build --force-rm ${DOCKER_BUILD_ARGS} -qt nnp-test "${tmpdir}" > /dev/null + rm -rf "${tmpdir}" + else + docker build ${DOCKER_BUILD_ARGS} -qt nnp-test contrib/nnp-test > /dev/null + fi +fi diff --git a/hack/make/.integration-daemon-setup b/hack/make/.integration-daemon-setup index 508a9d479e..b50f945416 100644 --- a/hack/make/.integration-daemon-setup +++ b/hack/make/.integration-daemon-setup @@ -7,6 +7,7 @@ if [ $DOCKER_ENGINE_GOOS != "windows" ]; then bundle .ensure-frozen-images bundle .ensure-httpserver bundle .ensure-syscall-test + bundle .ensure-nnp-test else # Note this is Windows to Windows CI, not Windows to Linux CI bundle .ensure-frozen-images-windows diff --git a/integration-cli/docker_cli_run_unix_test.go b/integration-cli/docker_cli_run_unix_test.go index 634765297b..c4aee18642 100644 --- a/integration-cli/docker_cli_run_unix_test.go +++ b/integration-cli/docker_cli_run_unix_test.go @@ -895,6 +895,18 @@ func (s *DockerSuite) TestRunSeccompDefaultProfile(c *check.C) { } } +// TestRunNoNewPrivSetuid checks that --security-opt=no-new-privileges prevents +// effective uid transtions on executing setuid binaries. +func (s *DockerSuite) TestRunNoNewPrivSetuid(c *check.C) { + testRequires(c, DaemonIsLinux, NotUserNamespace, SameHostDaemon) + + // test that running a setuid binary results in no effective uid transition + runCmd := exec.Command(dockerBinary, "run", "--security-opt", "no-new-privileges", "--user", "1000", "nnp-test", "/usr/bin/nnp-test") + if out, _, err := runCommandWithOutput(runCmd); err != nil || !strings.Contains(out, "EUID=1000") { + c.Fatalf("expected output to contain EUID=1000, got %s: %v", out, err) + } +} + func (s *DockerSuite) TestRunApparmorProcDirectory(c *check.C) { testRequires(c, SameHostDaemon, Apparmor) diff --git a/man/docker-run.1.md b/man/docker-run.1.md index bf75fb68ef..7f5c21046f 100644 --- a/man/docker-run.1.md +++ b/man/docker-run.1.md @@ -459,6 +459,8 @@ its root filesystem mounted as read only prohibiting any writes. "label:type:TYPE" : Set the label type for the container "label:level:LEVEL" : Set the label level for the container "label:disable" : Turn off label confinement for the container + "no-new-privileges" : Disable container processes from gaining additional privileges + **--stop-signal**=*SIGTERM* Signal to stop a container. Default is SIGTERM. diff --git a/runconfig/opts/parse.go b/runconfig/opts/parse.go index 18f9fc45bd..9772b82caf 100644 --- a/runconfig/opts/parse.go +++ b/runconfig/opts/parse.go @@ -500,8 +500,8 @@ func parseLoggingOpts(loggingDriver string, loggingOpts []string) (map[string]st func parseSecurityOpts(securityOpts []string) ([]string, error) { for key, opt := range securityOpts { con := strings.SplitN(opt, ":", 2) - if len(con) == 1 { - return securityOpts, fmt.Errorf("invalid --security-opt: %q", opt) + if len(con) == 1 && con[0] != "no-new-privileges" { + return securityOpts, fmt.Errorf("Invalid --security-opt: %q", opt) } if con[0] == "seccomp" && con[1] != "unconfined" { f, err := ioutil.ReadFile(con[1])