From e908cc39018c015084ffbffbc5703ccba5c2fbb7 Mon Sep 17 00:00:00 2001 From: Brian Goff 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 --- 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 +++------- .../graphdriver/fuse-overlayfs/fuseoverlayfs.go | 14 +++++--------- 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 +++++++++-- 12 files changed, 48 insertions(+), 59 deletions(-) diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index f4f1bd2c0b6a33e30cd178c611c98fdad10a0657..5521adbd2749474165ec21d4071dd8a45e903bb3 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -466,5 +466,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/daemon/create.go b/daemon/create.go index d4aeb77c7b7677f19a1ce8e7a57921d5f89ef809..e0d6eeea8b90ef43184f2733ffee5584f9b94bdd 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -211,12 +211,10 @@ func (daemon *Daemon) create(opts createOpts) (retC *container.Container, retErr } ctr.RWLayer = rwLayer - rootIDs := daemon.idMapping.RootPair() - - if err := idtools.MkdirAndChown(ctr.Root, 0700, rootIDs); err != nil { + if err := idtools.MkdirAndChown(ctr.Root, 0701, idtools.CurrentIdentity()); err != nil { return nil, err } - if err := idtools.MkdirAndChown(ctr.CheckpointDir(), 0700, rootIDs); err != nil { + if err := idtools.MkdirAndChown(ctr.CheckpointDir(), 0700, idtools.CurrentIdentity()); err != nil { return nil, err } diff --git a/daemon/daemon.go b/daemon/daemon.go index ab0db4fa0668067684dad87f97871ebfddf9d61b..794ff9712d08dcdaa38ec6e3d0686aa13e1c2f4b 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -861,7 +861,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/daemon/daemon_unix.go b/daemon/daemon_unix.go index 5fa688dff4c7609bd85931d8fa4b15b29a88b740..b477275228a24a70030e22b27671f6ea74b34ec7 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -1196,7 +1196,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 @@ -1221,10 +1221,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 @@ -1237,7 +1243,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/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index 4ecc647bdd8d51c99d6cb88b397b25dce7eb285e..b007274e13f19290eb5e6d0fe9e8b7c5e0fffe4c 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/daemon/graphdriver/aufs/aufs.go @@ -129,18 +129,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/daemon/graphdriver/btrfs/btrfs.go b/daemon/graphdriver/btrfs/btrfs.go index 1866b07b884a5308eac679bc1fa4ec060a147968..f9127472f8b0f9cc5ba2254399823a8859ebc860 100644 --- a/daemon/graphdriver/btrfs/btrfs.go +++ b/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/daemon/graphdriver/fuse-overlayfs/fuseoverlayfs.go b/daemon/graphdriver/fuse-overlayfs/fuseoverlayfs.go index e906cda74db24d38698588ad0a8fd4bed0d42f29..782e8be9844692e775628b6f7f5ca93301596eb8 100644 --- a/daemon/graphdriver/fuse-overlayfs/fuseoverlayfs.go +++ b/daemon/graphdriver/fuse-overlayfs/fuseoverlayfs.go @@ -88,12 +88,7 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, graphdriver.ErrNotSupported } - 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 } @@ -178,10 +173,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 } @@ -215,7 +211,7 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts) (retErr return nil } - if err := idtools.MkdirAndChown(path.Join(dir, workDirName), 0700, root); err != nil { + if err := idtools.MkdirAndChown(path.Join(dir, workDirName), 0701, currentID); err != nil { return err } diff --git a/daemon/graphdriver/overlay/overlay.go b/daemon/graphdriver/overlay/overlay.go index 1cffc1b896c814733cf9abca41dddef5e2997132..90be0e3d645b263822ef876ffd1eb49bce8cf321 100644 --- a/daemon/graphdriver/overlay/overlay.go +++ b/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/daemon/graphdriver/overlay2/overlay.go b/daemon/graphdriver/overlay2/overlay.go index 11d53029ca9e1ebdc521b1f496f4432251b5980f..e26fa07c827ddb237503309de8422e90fbd6e783 100644 --- a/daemon/graphdriver/overlay2/overlay.go +++ b/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/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index b341de5b893c616d00d952f3291be8275bb007f5..af9b107609e47d943208a1107aa85f83ec13452c 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/daemon/graphdriver/vfs/driver.go @@ -38,8 +38,7 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, err } - 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/daemon/graphdriver/zfs/zfs.go b/daemon/graphdriver/zfs/zfs.go index 21c271e2ec07c545fd0da7f6c46550de852ed927..f9099a2094124ff05a1cc176138f69d9db0c9758 100644 --- a/daemon/graphdriver/zfs/zfs.go +++ b/daemon/graphdriver/zfs/zfs.go @@ -104,11 +104,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/volume/local/local.go b/volume/local/local.go index 89252cf627420d72fd6a20ef35bbdf805d12e18b..088502f43bd834828765a0d69c2d01a69e7ff5e1 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -50,7 +50,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 } @@ -153,8 +153,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