From 67de83e70bca92ae6a08e28a03b3fc8fcca9f3f1 Mon Sep 17 00:00:00 2001
From: Brian Goff <cpuguy83@gmail.com>
Date: Tue, 6 Oct 2020 19:43:24 +0000
Subject: [PATCH] Use real root with 0701 perms

Various dirs in /var/lib/docker contain data that needs to be mounted
into a container. For this reason, these dirs are set to be owned by the
remapped root user, otherwise there can be permissions issues.
However, this uneccessarily exposes these dirs to an unprivileged user
on the host.

Instead, set the ownership of these dirs to the real root (or rather the
UID/GID of dockerd) with 0701 permissions, which allows the remapped
root to enter the directories but not read/write to them.
The remapped root needs to enter these dirs so the container's rootfs
can be configured... e.g. to mount /etc/resolve.conf.

This prevents an unprivileged user from having read/write access to
these dirs on the host.
The flip side of this is now any user can enter these directories.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit e908cc39018c015084ffbffbc5703ccba5c2fbb7)

Cherry-pick conflict with eb14d936bfc296f0a85bf4dc9e9bb1f4b4a01282:
Kept old `container` variable name.
Signed-off-by: Tibor Vass <tibor@docker.com>
---
 daemon/container_operations_unix.go    |  2 +-
 daemon/create.go                       |  6 ++----
 daemon/daemon.go                       |  2 +-
 daemon/daemon_unix.go                  | 14 ++++++++++----
 daemon/graphdriver/aufs/aufs.go        |  9 +++------
 daemon/graphdriver/btrfs/btrfs.go      | 10 +++-------
 daemon/graphdriver/overlay/overlay.go  | 16 +++++++---------
 daemon/graphdriver/overlay2/overlay.go | 12 ++++--------
 daemon/graphdriver/vfs/driver.go       |  5 ++---
 daemon/graphdriver/zfs/zfs.go          |  6 +-----
 volume/local/local.go                  | 11 +++++++++--
 11 files changed, 43 insertions(+), 50 deletions(-)

diff --git a/engine/daemon/container_operations_unix.go b/engine/daemon/container_operations_unix.go
index 3fcdc1913bed..ad8cb4c83aa7 100644
--- a/engine/daemon/container_operations_unix.go
+++ b/engine/daemon/container_operations_unix.go
@@ -411,5 +411,5 @@ func (daemon *Daemon) setupContainerMountsRoot(c *container.Container) error {
 	if err != nil {
 		return err
 	}
-	return idtools.MkdirAllAndChown(p, 0700, daemon.idMapping.RootPair())
+	return idtools.MkdirAllAndChown(p, 0701, idtools.CurrentIdentity())
 }
diff --git a/engine/daemon/create.go b/engine/daemon/create.go
index f9db0ca83454..9f8590b860d1 100644
--- a/engine/daemon/create.go
+++ b/engine/daemon/create.go
@@ -194,12 +194,10 @@ func (daemon *Daemon) create(opts createOpts) (retC *container.Container, retErr
 	}
 	container.RWLayer = rwLayer
 
-	rootIDs := daemon.idMapping.RootPair()
-
-	if err := idtools.MkdirAndChown(container.Root, 0700, rootIDs); err != nil {
+	if err := idtools.MkdirAndChown(container.Root, 0701, idtools.CurrentIdentity()); err != nil {
 		return nil, err
 	}
-	if err := idtools.MkdirAndChown(container.CheckpointDir(), 0700, rootIDs); err != nil {
+	if err := idtools.MkdirAndChown(container.CheckpointDir(), 0700, idtools.CurrentIdentity()); err != nil {
 		return nil, err
 	}
 
diff --git a/engine/daemon/daemon.go b/engine/daemon/daemon.go
index 24205cd1dc3d..e75cee6c8c18 100644
--- a/engine/daemon/daemon.go
+++ b/engine/daemon/daemon.go
@@ -813,7 +813,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 	}
 
 	daemonRepo := filepath.Join(config.Root, "containers")
-	if err := idtools.MkdirAllAndChown(daemonRepo, 0700, rootIDs); err != nil {
+	if err := idtools.MkdirAllAndChown(daemonRepo, 0701, idtools.CurrentIdentity()); err != nil {
 		return nil, err
 	}
 
diff --git a/engine/daemon/daemon_unix.go b/engine/daemon/daemon_unix.go
index a14f029c92cc..44dd67a61dab 100644
--- a/engine/daemon/daemon_unix.go
+++ b/engine/daemon/daemon_unix.go
@@ -1206,7 +1206,7 @@ func setupRemappedRoot(config *config.Config) (*idtools.IdentityMapping, error)
 	return &idtools.IdentityMapping{}, nil
 }
 
-func setupDaemonRoot(config *config.Config, rootDir string, rootIdentity idtools.Identity) error {
+func setupDaemonRoot(config *config.Config, rootDir string, remappedRoot idtools.Identity) error {
 	config.Root = rootDir
 	// the docker root metadata directory needs to have execute permissions for all users (g+x,o+x)
 	// so that syscalls executing as non-root, operating on subdirectories of the graph root
@@ -1231,10 +1231,16 @@ func setupDaemonRoot(config *config.Config, rootDir string, rootIdentity idtools
 	// a new subdirectory with ownership set to the remapped uid/gid (so as to allow
 	// `chdir()` to work for containers namespaced to that uid/gid)
 	if config.RemappedRoot != "" {
-		config.Root = filepath.Join(rootDir, fmt.Sprintf("%d.%d", rootIdentity.UID, rootIdentity.GID))
+		id := idtools.CurrentIdentity()
+		// First make sure the current root dir has the correct perms.
+		if err := idtools.MkdirAllAndChown(config.Root, 0701, id); err != nil {
+			return errors.Wrapf(err, "could not create or set daemon root permissions: %s", config.Root)
+		}
+
+		config.Root = filepath.Join(rootDir, fmt.Sprintf("%d.%d", remappedRoot.UID, remappedRoot.GID))
 		logrus.Debugf("Creating user namespaced daemon root: %s", config.Root)
 		// Create the root directory if it doesn't exist
-		if err := idtools.MkdirAllAndChown(config.Root, 0700, rootIdentity); err != nil {
+		if err := idtools.MkdirAllAndChown(config.Root, 0701, id); err != nil {
 			return fmt.Errorf("Cannot create daemon root: %s: %v", config.Root, err)
 		}
 		// we also need to verify that any pre-existing directories in the path to
@@ -1247,7 +1253,7 @@ func setupDaemonRoot(config *config.Config, rootDir string, rootIdentity idtools
 			if dirPath == "/" {
 				break
 			}
-			if !idtools.CanAccess(dirPath, rootIdentity) {
+			if !idtools.CanAccess(dirPath, remappedRoot) {
 				return fmt.Errorf("a subdirectory in your graphroot path (%s) restricts access to the remapped root uid/gid; please fix by allowing 'o+x' permissions on existing directories", config.Root)
 			}
 		}
diff --git a/engine/daemon/graphdriver/aufs/aufs.go b/engine/daemon/graphdriver/aufs/aufs.go
index bbd19a82b000..086e75b97866 100644
--- a/engine/daemon/graphdriver/aufs/aufs.go
+++ b/engine/daemon/graphdriver/aufs/aufs.go
@@ -130,18 +130,15 @@ func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
 		locker:    locker.New(),
 	}
 
-	rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
-	if err != nil {
-		return nil, err
-	}
+	currentID := idtools.CurrentIdentity()
 	// Create the root aufs driver dir
-	if err := idtools.MkdirAllAndChown(root, 0700, idtools.Identity{UID: rootUID, GID: rootGID}); err != nil {
+	if err := idtools.MkdirAllAndChown(root, 0701, currentID); err != nil {
 		return nil, err
 	}
 
 	// Populate the dir structure
 	for _, p := range paths {
-		if err := idtools.MkdirAllAndChown(path.Join(root, p), 0700, idtools.Identity{UID: rootUID, GID: rootGID}); err != nil {
+		if err := idtools.MkdirAllAndChown(path.Join(root, p), 0701, currentID); err != nil {
 			return nil, err
 		}
 	}
diff --git a/engine/daemon/graphdriver/btrfs/btrfs.go b/engine/daemon/graphdriver/btrfs/btrfs.go
index fcaedc6eab18..b1b287904c5e 100644
--- a/engine/daemon/graphdriver/btrfs/btrfs.go
+++ b/engine/daemon/graphdriver/btrfs/btrfs.go
@@ -70,11 +70,7 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
 		return nil, graphdriver.ErrPrerequisites
 	}
 
-	rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
-	if err != nil {
-		return nil, err
-	}
-	if err := idtools.MkdirAllAndChown(home, 0700, idtools.Identity{UID: rootUID, GID: rootGID}); err != nil {
+	if err := idtools.MkdirAllAndChown(home, 0701, idtools.CurrentIdentity()); err != nil {
 		return nil, err
 	}
 
@@ -525,7 +521,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error {
 	if err != nil {
 		return err
 	}
-	if err := idtools.MkdirAllAndChown(subvolumes, 0700, idtools.Identity{UID: rootUID, GID: rootGID}); err != nil {
+	if err := idtools.MkdirAllAndChown(subvolumes, 0701, idtools.CurrentIdentity()); err != nil {
 		return err
 	}
 	if parent == "" {
@@ -560,7 +556,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error {
 		if err := d.setStorageSize(path.Join(subvolumes, id), driver); err != nil {
 			return err
 		}
-		if err := idtools.MkdirAllAndChown(quotas, 0700, idtools.Identity{UID: rootUID, GID: rootGID}); err != nil {
+		if err := idtools.MkdirAllAndChown(quotas, 0700, idtools.CurrentIdentity()); err != nil {
 			return err
 		}
 		if err := ioutil.WriteFile(path.Join(quotas, id), []byte(fmt.Sprint(driver.options.size)), 0644); err != nil {
diff --git a/engine/daemon/graphdriver/overlay/overlay.go b/engine/daemon/graphdriver/overlay/overlay.go
index e837a595e17c..3edffd252bd4 100644
--- a/engine/daemon/graphdriver/overlay/overlay.go
+++ b/engine/daemon/graphdriver/overlay/overlay.go
@@ -156,12 +156,8 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
 		logrus.WithField("storage-driver", "overlay").Warn(overlayutils.ErrDTypeNotSupported("overlay", backingFs))
 	}
 
-	rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
-	if err != nil {
-		return nil, err
-	}
 	// Create the driver home dir
-	if err := idtools.MkdirAllAndChown(home, 0700, idtools.Identity{UID: rootUID, GID: rootGID}); err != nil {
+	if err := idtools.MkdirAllAndChown(home, 0701, idtools.CurrentIdentity()); err != nil {
 		return nil, err
 	}
 
@@ -265,10 +261,11 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr
 	}
 	root := idtools.Identity{UID: rootUID, GID: rootGID}
 
-	if err := idtools.MkdirAllAndChown(path.Dir(dir), 0700, root); err != nil {
+	currentID := idtools.CurrentIdentity()
+	if err := idtools.MkdirAllAndChown(path.Dir(dir), 0701, currentID); err != nil {
 		return err
 	}
-	if err := idtools.MkdirAndChown(dir, 0700, root); err != nil {
+	if err := idtools.MkdirAndChown(dir, 0701, currentID); err != nil {
 		return err
 	}
 
@@ -281,6 +278,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr
 
 	// Toplevel images are just a "root" dir
 	if parent == "" {
+		// This must be 0755 otherwise unprivileged users will in the container will not be able to read / in the container
 		return idtools.MkdirAndChown(path.Join(dir, "root"), 0755, root)
 	}
 
@@ -301,7 +299,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr
 		if err := idtools.MkdirAndChown(path.Join(dir, "work"), 0700, root); err != nil {
 			return err
 		}
-		return ioutil.WriteFile(path.Join(dir, "lower-id"), []byte(parent), 0666)
+		return ioutil.WriteFile(path.Join(dir, "lower-id"), []byte(parent), 0600)
 	}
 
 	// Otherwise, copy the upper and the lower-id from the parent
@@ -311,7 +309,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr
 		return err
 	}
 
-	if err := ioutil.WriteFile(path.Join(dir, "lower-id"), lowerID, 0666); err != nil {
+	if err := ioutil.WriteFile(path.Join(dir, "lower-id"), lowerID, 0600); err != nil {
 		return err
 	}
 
diff --git a/engine/daemon/graphdriver/overlay2/overlay.go b/engine/daemon/graphdriver/overlay2/overlay.go
index 7b2475ea7e8d..1db8ac4cd8ab 100644
--- a/engine/daemon/graphdriver/overlay2/overlay.go
+++ b/engine/daemon/graphdriver/overlay2/overlay.go
@@ -165,12 +165,7 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
 		logger.Warn(overlayutils.ErrDTypeNotSupported("overlay2", backingFs))
 	}
 
-	rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
-	if err != nil {
-		return nil, err
-	}
-	// Create the driver home dir
-	if err := idtools.MkdirAllAndChown(path.Join(home, linkDir), 0700, idtools.Identity{UID: rootUID, GID: rootGID}); err != nil {
+	if err := idtools.MkdirAllAndChown(path.Join(home, linkDir), 0701, idtools.CurrentIdentity()); err != nil {
 		return nil, err
 	}
 
@@ -339,11 +334,12 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts) (retErr
 		return err
 	}
 	root := idtools.Identity{UID: rootUID, GID: rootGID}
+	current := idtools.CurrentIdentity()
 
-	if err := idtools.MkdirAllAndChown(path.Dir(dir), 0700, root); err != nil {
+	if err := idtools.MkdirAllAndChown(path.Dir(dir), 0701, current); err != nil {
 		return err
 	}
-	if err := idtools.MkdirAndChown(dir, 0700, root); err != nil {
+	if err := idtools.MkdirAndChown(dir, 0701, current); err != nil {
 		return err
 	}
 
diff --git a/engine/daemon/graphdriver/vfs/driver.go b/engine/daemon/graphdriver/vfs/driver.go
index d7f14ecb622d..3134c6631fdd 100644
--- a/engine/daemon/graphdriver/vfs/driver.go
+++ b/engine/daemon/graphdriver/vfs/driver.go
@@ -38,8 +38,7 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
 		home:      home,
 		idMapping: idtools.NewIDMappingsFromMaps(uidMaps, gidMaps),
 	}
-	rootIDs := d.idMapping.RootPair()
-	if err := idtools.MkdirAllAndChown(home, 0700, rootIDs); err != nil {
+	if err := idtools.MkdirAllAndChown(home, 0701, idtools.CurrentIdentity()); err != nil {
 		return nil, err
 	}
 
@@ -141,7 +140,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error {
 func (d *Driver) create(id, parent string, size uint64) error {
 	dir := d.dir(id)
 	rootIDs := d.idMapping.RootPair()
-	if err := idtools.MkdirAllAndChown(filepath.Dir(dir), 0700, rootIDs); err != nil {
+	if err := idtools.MkdirAllAndChown(filepath.Dir(dir), 0701, idtools.CurrentIdentity()); err != nil {
 		return err
 	}
 	if err := idtools.MkdirAndChown(dir, 0755, rootIDs); err != nil {
diff --git a/engine/daemon/graphdriver/zfs/zfs.go b/engine/daemon/graphdriver/zfs/zfs.go
index c83446cf8f77..b887b0950203 100644
--- a/engine/daemon/graphdriver/zfs/zfs.go
+++ b/engine/daemon/graphdriver/zfs/zfs.go
@@ -103,11 +103,7 @@ func Init(base string, opt []string, uidMaps, gidMaps []idtools.IDMap) (graphdri
 		return nil, fmt.Errorf("BUG: zfs get all -t filesystem -rHp '%s' should contain '%s'", options.fsName, options.fsName)
 	}
 
-	rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
-	if err != nil {
-		return nil, fmt.Errorf("Failed to get root uid/guid: %v", err)
-	}
-	if err := idtools.MkdirAllAndChown(base, 0700, idtools.Identity{UID: rootUID, GID: rootGID}); err != nil {
+	if err := idtools.MkdirAllAndChown(base, 0701, idtools.CurrentIdentity()); err != nil {
 		return nil, fmt.Errorf("Failed to create '%s': %v", base, err)
 	}
 
diff --git a/engine/volume/local/local.go b/engine/volume/local/local.go
index 6dc894873dd8..aeb98aba61ff 100644
--- a/engine/volume/local/local.go
+++ b/engine/volume/local/local.go
@@ -49,7 +49,7 @@ type activeMount struct {
 func New(scope string, rootIdentity idtools.Identity) (*Root, error) {
 	rootDirectory := filepath.Join(scope, volumesPathName)
 
-	if err := idtools.MkdirAllAndChown(rootDirectory, 0700, rootIdentity); err != nil {
+	if err := idtools.MkdirAllAndChown(rootDirectory, 0701, idtools.CurrentIdentity()); err != nil {
 		return nil, err
 	}
 
@@ -146,8 +146,15 @@ func (r *Root) Create(name string, opts map[string]string) (volume.Volume, error
 	}
 
 	path := r.DataPath(name)
+	volRoot := filepath.Dir(path)
+	// Root dir does not need to be accessed by the remapped root
+	if err := idtools.MkdirAllAndChown(volRoot, 0701, idtools.CurrentIdentity()); err != nil {
+		return nil, errors.Wrapf(errdefs.System(err), "error while creating volume root path '%s'", volRoot)
+	}
+
+	// Remapped root does need access to the data path
 	if err := idtools.MkdirAllAndChown(path, 0755, r.rootIdentity); err != nil {
-		return nil, errors.Wrapf(errdefs.System(err), "error while creating volume path '%s'", path)
+		return nil, errors.Wrapf(errdefs.System(err), "error while creating volume data path '%s'", path)
 	}
 
 	var err error