Move AppArmor policy to contrib & deb packaging

The automatic installation of AppArmor policies prevents the
management of custom, site-specific apparmor policies for the
default container profile. Furthermore, this change will allow
a future policy for the engine itself to be written without demanding
the engine be able to arbitrarily create and manage AppArmor policies.

- Add deb package suggests for apparmor.
- Ubuntu postinst use aa-status & fix policy path
- Add the policies to the debian packages.
- Add apparmor tests for writing proc files
Additional restrictions against modifying files in proc
are enforced by AppArmor. Ensure that AppArmor is preventing
access to these files, not simply Docker's configuration of proc.
- Remove /proc/k?mem from AA policy
The path to mem and kmem are in /dev, not /proc
and cannot be restricted successfully through AppArmor.
The device cgroup will need to be sufficient here.
- Load contrib/apparmor during integration tests
Note that this is somewhat dirty because we
cannot restore the host to its original configuration.
However, it should be noted that prior to this patch
series, the Docker daemon itself was loading apparmor
policy from within the tests, so this is no dirtier or
uglier than the status-quo.

Signed-off-by: Eric Windisch <eric@windisch.us>

contrib/apparmor/docker

@@ -0,0 +1,25 @@
+#include <tunables/global>
+
+profile docker-default flags=(attach_disconnected,mediate_deleted) {
+  #include <abstractions/base>
+
+  network,
+  capability,
+  file,
+  umount,
+
+  deny @{PROC}/sys/fs/** wklx,
+  deny @{PROC}/sysrq-trigger rwklx,
+  deny @{PROC}/sys/kernel/[^s][^h][^m]* wklx,
+  deny @{PROC}/sys/kernel/*/** wklx,
+
+  deny mount,
+
+  deny /sys/[^f]*/** wklx,
+  deny /sys/f[^s]*/** wklx,
+  deny /sys/fs/[^c]*/** wklx,
+  deny /sys/fs/c[^g]*/** wklx,
+  deny /sys/fs/cg[^r]*/** wklx,
+  deny /sys/firmware/efi/efivars/** rwklx,
+  deny /sys/kernel/security/** rwklx,
+}

contrib/builder/deb/generate.sh

@@ -50,6 +50,7 @@ for version in "${versions[@]}"; do
 		build-essential # "essential for building Debian packages"
 		curl ca-certificates # for downloading Go
 		debhelper # for easy ".deb" building
+		dh-apparmor # for apparmor debhelper
 		dh-systemd # for systemd debhelper integration
 		git # for "git commit" info in "docker -v"
 		libapparmor-dev # for "sys/apparmor.h"

daemon/execdriver/native/apparmor.go

@@ -1,124 +0,0 @@
-// +build linux
-
-package native
-
-import (
-	"fmt"
-	"io"
-	"os"
-	"os/exec"
-	"path"
-	"text/template"
-
-	"github.com/opencontainers/runc/libcontainer/apparmor"
-)
-
-const (
-	apparmorProfilePath = "/etc/apparmor.d/docker"
-)
-
-type data struct {
-	Name         string
-	Imports      []string
-	InnerImports []string
-}
-
-const baseTemplate = `
-{{range $value := .Imports}}
-{{$value}}
-{{end}}
-
-profile {{.Name}} flags=(attach_disconnected,mediate_deleted) {
-{{range $value := .InnerImports}}
-  {{$value}}
-{{end}}
-
-  network,
-  capability,
-  file,
-  umount,
-
-  deny @{PROC}/sys/fs/** wklx,
-  deny @{PROC}/sysrq-trigger rwklx,
-  deny @{PROC}/mem rwklx,
-  deny @{PROC}/kmem rwklx,
-  deny @{PROC}/sys/kernel/[^s][^h][^m]* wklx,
-  deny @{PROC}/sys/kernel/*/** wklx,
-
-  deny mount,
-
-  deny /sys/[^f]*/** wklx,
-  deny /sys/f[^s]*/** wklx,
-  deny /sys/fs/[^c]*/** wklx,
-  deny /sys/fs/c[^g]*/** wklx,
-  deny /sys/fs/cg[^r]*/** wklx,
-  deny /sys/firmware/efi/efivars/** rwklx,
-  deny /sys/kernel/security/** rwklx,
-}
-`
-
-func generateProfile(out io.Writer) error {
-	compiled, err := template.New("apparmor_profile").Parse(baseTemplate)
-	if err != nil {
-		return err
-	}
-	data := &data{
-		Name: "docker-default",
-	}
-	if tunablesExists() {
-		data.Imports = append(data.Imports, "#include <tunables/global>")
-	} else {
-		data.Imports = append(data.Imports, "@{PROC}=/proc/")
-	}
-	if abstractionsExists() {
-		data.InnerImports = append(data.InnerImports, "#include <abstractions/base>")
-	}
-	if err := compiled.Execute(out, data); err != nil {
-		return err
-	}
-	return nil
-}
-
-// check if the tunables/global exist
-func tunablesExists() bool {
-	_, err := os.Stat("/etc/apparmor.d/tunables/global")
-	return err == nil
-}
-
-// check if abstractions/base exist
-func abstractionsExists() bool {
-	_, err := os.Stat("/etc/apparmor.d/abstractions/base")
-	return err == nil
-}
-
-func installApparmorProfile() error {
-	if !apparmor.IsEnabled() {
-		return nil
-	}
-
-	// Make sure /etc/apparmor.d exists
-	if err := os.MkdirAll(path.Dir(apparmorProfilePath), 0755); err != nil {
-		return err
-	}
-
-	f, err := os.OpenFile(apparmorProfilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
-	if err != nil {
-		return err
-	}
-	if err := generateProfile(f); err != nil {
-		f.Close()
-		return err
-	}
-	f.Close()
-
-	cmd := exec.Command("/sbin/apparmor_parser", "-r", "-W", "docker")
-	// to use the parser directly we have to make sure we are in the correct
-	// dir with the profile
-	cmd.Dir = "/etc/apparmor.d"
-
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("Error loading docker apparmor profile: %s (%s)", err, output)
-	}
-	return nil
-}

daemon/execdriver/native/driver.go

@@ -50,10 +50,6 @@ func NewDriver(root, initPath string, options []string) (*driver, error) {
 	if err := sysinfo.MkdirAll(root, 0700); err != nil {
 		return nil, err
 	}
-	// native driver root is at docker_root/execdriver/native. Put apparmor at docker_root
-	if err := installApparmorProfile(); err != nil {
-		return nil, err
-	}
 
 	// choose cgroup manager
 	// this makes sure there are no breaking changes to people

hack/make/.build-deb/docker-engine.install

@@ -9,3 +9,4 @@ contrib/init/systemd/docker.socket lib/systemd/system/
 contrib/mk* usr/share/docker-engine/contrib/
 contrib/nuke-graph-directory.sh usr/share/docker-engine/contrib/
 contrib/syntax/nano/Dockerfile.nanorc usr/share/nano/
+contrib/apparmor/* etc/apparmor.d/

hack/make/.build-deb/rules

@@ -32,5 +32,8 @@ override_dh_installudev:
 	# match our existing priority
 	dh_installudev --priority=z80
 
+override_dh_install:
+	dh_apparmor --profile-name=docker -pdocker-engine
+
 %:
 	dh $@ --with=bash-completion $(shell command -v dh_systemd_enable > /dev/null 2>&1 && echo --with=systemd)

hack/make/.integration-daemon-start

@@ -35,6 +35,8 @@ if [ -z "$DOCKER_TEST_HOST" ]; then
 		(
 			set -x
 			/etc/init.d/apparmor start
+
+			/sbin/apparmor_parser -r -W -T contrib/apparmor/
 		)
 	fi
 

hack/make/ubuntu

@@ -72,6 +72,10 @@ bundle_ubuntu() {
 		done
 	done
 
+	# Include contributed apparmor policy
+	mkdir -p "$DIR/etc/apparmor.d/"
+	cp contrib/apparmor/docker "$DIR/etc/apparmor.d/"
+
 	# Copy the binary
 	# This will fail if the binary bundle hasn't been built
 	mkdir -p "$DIR/usr/bin"
@@ -89,6 +93,10 @@ if [ "$1" = 'configure' ] && [ -z "$2" ]; then
 	fi
 fi
 
+if ( aa-status --enabled ); then
+	/sbin/apparmor_parser -r -W -T /etc/apparmor.d/docker
+fi
+
 if ! { [ -x /sbin/initctl ] && /sbin/initctl version 2>/dev/null | grep -q upstart; }; then
 	# we only need to do this if upstart isn't in charge
 	update-rc.d docker defaults > /dev/null || true
@@ -149,6 +157,7 @@ EOF
 			--deb-recommends git \
 			--deb-recommends xz-utils \
 			--deb-recommends 'cgroupfs-mount | cgroup-lite' \
+			--deb-suggests apparmor \
 			--description "$PACKAGE_DESCRIPTION" \
 			--maintainer "$PACKAGE_MAINTAINER" \
 			--conflicts docker \

integration-cli/docker_cli_run_test.go

@@ -2518,3 +2518,25 @@ func (s *DockerSuite) TestVolumeFromMixedRWOptions(c *check.C) {
 		c.Fatalf("Expected RW volume was RO")
 	}
 }
+
+func (s *DockerSuite) TestRunWriteFilteredProc(c *check.C) {
+	testRequires(c, Apparmor)
+
+	testWritePaths := []string{
+		/* modprobe and core_pattern should both be denied by generic
+		 * policy of denials for /proc/sys/kernel. These files have been
+		 * picked to be checked as they are particularly sensitive to writes */
+		"/proc/sys/kernel/modprobe",
+		"/proc/sys/kernel/core_pattern",
+		"/proc/sysrq-trigger",
+	}
+	for i, filePath := range testWritePaths {
+		name := fmt.Sprintf("writeprocsieve-%d", i)
+
+		shellCmd := fmt.Sprintf("exec 3>%s", filePath)
+		runCmd := exec.Command(dockerBinary, "run", "--privileged", "--security-opt", "apparmor:docker-default", "--name", name, "busybox", "sh", "-c", shellCmd)
+		if out, exitCode, err := runCommandWithOutput(runCmd); err == nil || exitCode == 0 {
+			c.Fatalf("Open FD for write should have failed with permission denied, got: %s, %v", out, err)
+		}
+	}
+}