Merge pull request #574 from crosbymichael/symlink-dev-secfix

Security fixes for docker 1.6.1

SPEC.md

@@ -217,17 +217,6 @@ profile <profile_name> flags=(attach_disconnected,mediate_deleted) {
   file,
   umount,
 
-  mount fstype=tmpfs,
-  mount fstype=mqueue,
-  mount fstype=fuse.*,
-  mount fstype=binfmt_misc -> /proc/sys/fs/binfmt_misc/,
-  mount fstype=efivarfs -> /sys/firmware/efi/efivars/,
-  mount fstype=fusectl -> /sys/fs/fuse/connections/,
-  mount fstype=securityfs -> /sys/kernel/security/,
-  mount fstype=debugfs -> /sys/kernel/debug/,
-  mount fstype=proc -> /proc/,
-  mount fstype=sysfs -> /sys/,
-
   deny @{PROC}/sys/fs/** wklx,
   deny @{PROC}/sysrq-trigger rwklx,
   deny @{PROC}/mem rwklx,
@@ -235,9 +224,7 @@ profile <profile_name> flags=(attach_disconnected,mediate_deleted) {
   deny @{PROC}/sys/kernel/[^s][^h][^m]* wklx,
   deny @{PROC}/sys/kernel/*/** wklx,
 
-  deny mount options=(ro, remount) -> /,
+  deny mount,
-  deny mount fstype=debugfs -> /var/lib/ureadahead/debugfs/,
-  deny mount fstype=devpts,
 
   deny /sys/[^f]*/** wklx,
   deny /sys/f[^s]*/** wklx,

apparmor/gen.go

@@ -27,17 +27,6 @@ profile {{.Name}} flags=(attach_disconnected,mediate_deleted) {
   file,
   umount,
 
-  mount fstype=tmpfs,
-  mount fstype=mqueue,
-  mount fstype=fuse.*,
-  mount fstype=binfmt_misc -> /proc/sys/fs/binfmt_misc/,
-  mount fstype=efivarfs -> /sys/firmware/efi/efivars/,
-  mount fstype=fusectl -> /sys/fs/fuse/connections/,
-  mount fstype=securityfs -> /sys/kernel/security/,
-  mount fstype=debugfs -> /sys/kernel/debug/,
-  mount fstype=proc -> /proc/,
-  mount fstype=sysfs -> /sys/,
-
   deny @{PROC}/sys/fs/** wklx,
   deny @{PROC}/sysrq-trigger rwklx,
   deny @{PROC}/mem rwklx,
@@ -45,9 +34,7 @@ profile {{.Name}} flags=(attach_disconnected,mediate_deleted) {
   deny @{PROC}/sys/kernel/[^s][^h][^m]* wklx,
   deny @{PROC}/sys/kernel/*/** wklx,
 
-  deny mount options=(ro, remount) -> /,
+  deny mount,
-  deny mount fstype=debugfs -> /var/lib/ureadahead/debugfs/,
-  deny mount fstype=devpts,
 
   deny /sys/[^f]*/** wklx,
   deny /sys/f[^s]*/** wklx,

integration/exec_test.go

@@ -651,7 +651,7 @@ func TestMountCmds(t *testing.T) {
 	config := newTemplateConfig(rootfs)
 	config.Mounts = append(config.Mounts, &configs.Mount{
 		Source:      tmpDir,
-		Destination: filepath.Join(rootfs, "tmp"),
+		Destination: "/tmp",
 		Device:      "bind",
 		Flags:       syscall.MS_BIND | syscall.MS_REC,
 		PremountCmds: []configs.Command{

rootfs_linux.go

@@ -13,6 +13,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/libcontainer/cgroups"
 	"github.com/docker/libcontainer/configs"
 	"github.com/docker/libcontainer/label"
@@ -48,11 +49,6 @@ func setupRootfs(config *configs.Config, console *linuxConsole) (err error) {
 	if err := setupPtmx(config, console); err != nil {
 		return newSystemError(err)
 	}
-	// stdin, stdout and stderr could be pointing to /dev/null from parent namespace.
-	// re-open them inside this namespace.
-	if err := reOpenDevNull(config.Rootfs); err != nil {
-		return newSystemError(err)
-	}
 	if err := setupDevSymlinks(config.Rootfs); err != nil {
 		return newSystemError(err)
 	}
@@ -67,6 +63,9 @@ func setupRootfs(config *configs.Config, console *linuxConsole) (err error) {
 	if err != nil {
 		return newSystemError(err)
 	}
+	if err := reOpenDevNull(config.Rootfs); err != nil {
+		return newSystemError(err)
+	}
 	if config.Readonlyfs {
 		if err := setReadonly(); err != nil {
 			return newSystemError(err)
@@ -139,6 +138,16 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error {
 			// unable to bind anything to it.
 			return err
 		}
+		// ensure that the destination of the bind mount is resolved of symlinks at mount time because
+		// any previous mounts can invalidate the next mount's destination.
+		// this can happen when a user specifies mounts within other mounts to cause breakouts or other
+		// evil stuff to try to escape the container's rootfs.
+		if dest, err = symlink.FollowSymlinkInScope(filepath.Join(rootfs, m.Destination), rootfs); err != nil {
+			return err
+		}
+		if err := checkMountDestination(rootfs, dest); err != nil {
+			return err
+		}
 		if err := createIfNotExists(dest, stat.IsDir()); err != nil {
 			return err
 		}
@@ -197,6 +206,29 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error {
 	return nil
 }
 
+// checkMountDestination checks to ensure that the mount destination is not over the
+// top of /proc or /sys.
+// dest is required to be an abs path and have any symlinks resolved before calling this function.
+func checkMountDestination(rootfs, dest string) error {
+	if filepath.Clean(rootfs) == filepath.Clean(dest) {
+		return fmt.Errorf("mounting into / is prohibited")
+	}
+	invalidDestinations := []string{
+		"/proc",
+		"/sys",
+	}
+	for _, invalid := range invalidDestinations {
+		path, err := filepath.Rel(filepath.Join(rootfs, invalid), dest)
+		if err != nil {
+			return err
+		}
+		if path == "." || !strings.HasPrefix(path, "..") {
+			return fmt.Errorf("%q cannot be mounted because it is located inside %q", dest, invalid)
+		}
+	}
+	return nil
+}
+
 func setupDevSymlinks(rootfs string) error {
 	var links = [][2]string{
 		{"/proc/self/fd", "/dev/fd"},
@@ -221,11 +253,13 @@ func setupDevSymlinks(rootfs string) error {
 	return nil
 }
 
-// If stdin, stdout or stderr are pointing to '/dev/null' in the global mount namespace,
+// If stdin, stdout, and/or stderr are pointing to `/dev/null` in the parent's rootfs
-// this method will make them point to '/dev/null' in this namespace.
+// this method will make them point to `/dev/null` in this container's rootfs.  This
+// needs to be called after we chroot/pivot into the container's rootfs so that any
+// symlinks are resolved locally.
 func reOpenDevNull(rootfs string) error {
 	var stat, devNullStat syscall.Stat_t
-	file, err := os.Open(filepath.Join(rootfs, "/dev/null"))
+	file, err := os.Open("/dev/null")
 	if err != nil {
 		return fmt.Errorf("Failed to open /dev/null - %s", err)
 	}

rootfs_linux_test.go

@@ -0,0 +1,37 @@
+// +build linux
+
+package libcontainer
+
+import "testing"
+
+func TestCheckMountDestOnProc(t *testing.T) {
+	dest := "/rootfs/proc/"
+	err := checkMountDestination("/rootfs", dest)
+	if err == nil {
+		t.Fatal("destination inside proc should return an error")
+	}
+}
+
+func TestCheckMountDestInSys(t *testing.T) {
+	dest := "/rootfs//sys/fs/cgroup"
+	err := checkMountDestination("/rootfs", dest)
+	if err == nil {
+		t.Fatal("destination inside proc should return an error")
+	}
+}
+
+func TestCheckMountDestFalsePositive(t *testing.T) {
+	dest := "/rootfs/sysfiles/fs/cgroup"
+	err := checkMountDestination("/rootfs", dest)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestCheckMountRoot(t *testing.T) {
+	dest := "/rootfs"
+	err := checkMountDestination("/rootfs", dest)
+	if err == nil {
+		t.Fatal(err)
+	}
+}