[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>
Gbp-Pq: Name cve-2021-21284-3.patch

diff --git a/engine/daemon/container_operations_unix.go b/engine/daemon/container_operations_unix.go
index 9953c7f3fddc58acaf367780202804b705c27feb..6c58cb7bcfa36bf7a8017ec370e4d699894b3e24 100644 (file)
--- a/engine/daemon/container_operations_unix.go
+++ b/engine/daemon/container_operations_unix.go
@@ -407,5 +407,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 1afb1bebea328468711fe2127da3bb49a4713590..eba24532bbcede86897f5f753282fd5fc16d5c39 100644 (file)
--- a/engine/daemon/create.go
+++ b/engine/daemon/create.go
@@ -161,12 +161,10 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) (
        }
        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 be4e8d5d10cd70f7bd500a5294e4cfb03d32a6ca..11edf29b8f47cf6ed4a44e0f48fbb9b6ad01ccfa 100644 (file)
--- a/engine/daemon/daemon.go
+++ b/engine/daemon/daemon.go
@@ -740,7 +740,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 dab30a27f5b9a0d6337567fbe8459d560561c6b5..dce8767a2973bbd6f20bbd8872b7f7653ef46631 100644 (file)
--- a/engine/daemon/daemon_unix.go
+++ b/engine/daemon/daemon_unix.go
@@ -1133,7 +1133,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
@@ -1158,10 +1158,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
@@ -1174,7 +1180,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 114aa9a615e0943923d024adfcd6d2f25ab399fd..b6d8466bd5f8790e09166eaa06231263740fa112 100644 (file)
--- 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 7ce7edef36e918fa6d498c3002679accdabe9338..91d20eadf603a20019917fe4b400c52515f33d81 100644 (file)
--- 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 08c05e192f61c402bce3c3fafc73118683f05a02..a108be3545dd2d58848b3ba3a8ba813c04536d28 100644 (file)
--- a/engine/daemon/graphdriver/overlay/overlay.go
+++ b/engine/daemon/graphdriver/overlay/overlay.go
@@ -163,12 +163,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
        }
 
@@ -293,10 +289,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
        }
 
@@ -309,6 +306,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)
        }
 
@@ -329,7 +327,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
@@ -339,7 +337,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 6da1f25a34e62911c0e32353b5559013659e1312..ff54ff5ba672db6387a4a4a86a4443d014570c39 100644 (file)
--- a/engine/daemon/graphdriver/overlay2/overlay.go
+++ b/engine/daemon/graphdriver/overlay2/overlay.go
@@ -196,12 +196,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
        }
 
@@ -391,11 +386,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 33e6bf6cc9c2fda1170a0bcf5ac75a8aaab5717d..ae540a4b7b6dc1d9e0f140585e09288ee5815c57 100644 (file)
--- a/engine/daemon/graphdriver/vfs/driver.go
+++ b/engine/daemon/graphdriver/vfs/driver.go
@@ -30,8 +30,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
        }
 
@@ -106,7 +105,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 8a798778d291452a5efffc355ce8f939dad3c098..3d2431d6a075ea55c36bf281fdec2c1cdbdb0a23 100644 (file)
--- a/engine/daemon/graphdriver/zfs/zfs.go
+++ b/engine/daemon/graphdriver/zfs/zfs.go
@@ -102,11 +102,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 7190de9ed60e063140fcb90afc746ea049fd076c..a4e1e614e2ddd6164af2aa7cb5ef93028428e07f 100644 (file)
--- 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