From be632872ec37b9266069a96245dbac5bf6df3c3a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 2 Mar 2018 13:17:56 +0100 Subject: [PATCH] Fix AppArmor not being applied to Exec processes Exec processes do not automatically inherit AppArmor profiles from the container. This patch sets the AppArmor profile for the exec process. Before this change: apparmor_parser -q -r < profile deny-write flags=(attach_disconnected) { #include file, network, deny /tmp/** w, capability, } EOF docker run -dit --security-opt "apparmor=deny-write" --name aa busybox docker exec aa sh -c 'mkdir /tmp/test' (no error) With this change applied: docker exec aa sh -c 'mkdir /tmp/test' mkdir: can't create directory '/tmp/test': Permission denied Signed-off-by: Sebastiaan van Stijn Upstream-commit: 8f3308ae10ec9ad0dd4edfb46fde53a0e1e19b34 Component: engine --- components/engine/daemon/exec_linux.go | 3 ++ components/engine/daemon/exec_linux_test.go | 53 +++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 components/engine/daemon/exec_linux_test.go diff --git a/components/engine/daemon/exec_linux.go b/components/engine/daemon/exec_linux.go index 1ed26c2fcc..cd52f4886f 100644 --- a/components/engine/daemon/exec_linux.go +++ b/components/engine/daemon/exec_linux.go @@ -34,6 +34,8 @@ func (daemon *Daemon) execSetPlatformOpt(c *container.Container, ec *exec.Config if c.AppArmorProfile != "" { appArmorProfile = c.AppArmorProfile } else if c.HostConfig.Privileged { + // `docker exec --privileged` does not currently disable AppArmor + // profiles. Privileged configuration of the container is inherited appArmorProfile = "unconfined" } else { appArmorProfile = "docker-default" @@ -50,6 +52,7 @@ func (daemon *Daemon) execSetPlatformOpt(c *container.Container, ec *exec.Config return err } } + p.ApparmorProfile = appArmorProfile } daemon.setRlimits(&specs.Spec{Process: p}, c) return nil diff --git a/components/engine/daemon/exec_linux_test.go b/components/engine/daemon/exec_linux_test.go new file mode 100644 index 0000000000..9e5496ae4b --- /dev/null +++ b/components/engine/daemon/exec_linux_test.go @@ -0,0 +1,53 @@ +// +build linux + +package daemon + +import ( + "testing" + + containertypes "github.com/docker/docker/api/types/container" + "github.com/docker/docker/container" + "github.com/docker/docker/daemon/exec" + "github.com/gotestyourself/gotestyourself/assert" + "github.com/opencontainers/runc/libcontainer/apparmor" + "github.com/opencontainers/runtime-spec/specs-go" +) + +func TestExecSetPlatformOpt(t *testing.T) { + if !apparmor.IsEnabled() { + t.Skip("requires AppArmor to be enabled") + } + d := &Daemon{} + c := &container.Container{AppArmorProfile: "my-custom-profile"} + ec := &exec.Config{} + p := &specs.Process{} + + err := d.execSetPlatformOpt(c, ec, p) + assert.NilError(t, err) + assert.Equal(t, "my-custom-profile", p.ApparmorProfile) +} + +// TestExecSetPlatformOptPrivileged verifies that `docker exec --privileged` +// does not disable AppArmor profiles. Exec currently inherits the `Privileged` +// configuration of the container. See https://github.com/moby/moby/pull/31773#discussion_r105586900 +// +// This behavior may change in future, but test for the behavior to prevent it +// from being changed accidentally. +func TestExecSetPlatformOptPrivileged(t *testing.T) { + if !apparmor.IsEnabled() { + t.Skip("requires AppArmor to be enabled") + } + d := &Daemon{} + c := &container.Container{AppArmorProfile: "my-custom-profile"} + ec := &exec.Config{Privileged: true} + p := &specs.Process{} + + err := d.execSetPlatformOpt(c, ec, p) + assert.NilError(t, err) + assert.Equal(t, "my-custom-profile", p.ApparmorProfile) + + c.HostConfig = &containertypes.HostConfig{Privileged: true} + err = d.execSetPlatformOpt(c, ec, p) + assert.NilError(t, err) + assert.Equal(t, "unconfined", p.ApparmorProfile) +}