Skip to content

Commit

Permalink
Merge pull request #41964 from thaJeztah/CVE-2021-21284_master
Browse files Browse the repository at this point in the history
[master] Fix Access to remapped root allows privilege escalation to real root (CVE-2021-21284)
  • Loading branch information
tiborvass committed Feb 2, 2021
2 parents 3e0025e + 7f5e39b commit 64bd448
Show file tree
Hide file tree
Showing 14 changed files with 69 additions and 71 deletions.
2 changes: 1 addition & 1 deletion daemon/container_operations_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
6 changes: 2 additions & 4 deletions daemon/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,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
}

Expand Down
10 changes: 4 additions & 6 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
}

// set up the tmpDir to use a canonical path
tmp, err := prepareTempDir(config.Root, rootIDs)
tmp, err := prepareTempDir(config.Root)
if err != nil {
return nil, fmt.Errorf("Unable to get the TempDir under %s: %s", config.Root, err)
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -1374,7 +1374,7 @@ func (daemon *Daemon) Subnets() ([]net.IPNet, []net.IPNet) {
// prepareTempDir prepares and returns the default directory to use
// for temporary files.
// If it doesn't exist, it is created. If it exists, its content is removed.
func prepareTempDir(rootDir string, rootIdentity idtools.Identity) (string, error) {
func prepareTempDir(rootDir string) (string, error) {
var tmpDir string
if tmpDir = os.Getenv("DOCKER_TMPDIR"); tmpDir == "" {
tmpDir = filepath.Join(rootDir, "tmp")
Expand All @@ -1392,9 +1392,7 @@ func prepareTempDir(rootDir string, rootIdentity idtools.Identity) (string, erro
}
}
}
// We don't remove the content of tmpdir if it's not the default,
// it may hold things that do not belong to us.
return tmpDir, idtools.MkdirAllAndChown(tmpDir, 0700, rootIdentity)
return tmpDir, idtools.MkdirAllAndChown(tmpDir, 0700, idtools.CurrentIdentity())
}

func (daemon *Daemon) setGenericResources(conf *config.Config) error {
Expand Down
14 changes: 10 additions & 4 deletions daemon/daemon_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
}
}
Expand Down
9 changes: 3 additions & 6 deletions daemon/graphdriver/aufs/aufs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
10 changes: 3 additions & 7 deletions daemon/graphdriver/btrfs/btrfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 5 additions & 9 deletions daemon/graphdriver/fuse-overlayfs/fuseoverlayfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
16 changes: 7 additions & 9 deletions daemon/graphdriver/overlay/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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)
}

Expand All @@ -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
Expand All @@ -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
}

Expand Down
12 changes: 4 additions & 8 deletions daemon/graphdriver/overlay2/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 2 additions & 3 deletions daemon/graphdriver/vfs/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 1 addition & 5 deletions daemon/graphdriver/zfs/zfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
11 changes: 8 additions & 3 deletions pkg/idtools/idtools.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ const (

// MkdirAllAndChown creates a directory (include any along the path) and then modifies
// ownership to the requested uid/gid. If the directory already exists, this
// function will still change ownership to the requested uid/gid pair.
// function will still change ownership and permissions.
func MkdirAllAndChown(path string, mode os.FileMode, owner Identity) error {
return mkdirAs(path, mode, owner, true, true)
}

// MkdirAndChown creates a directory and then modifies ownership to the requested uid/gid.
// If the directory already exists, this function still changes ownership.
// If the directory already exists, this function still changes ownership and permissions.
// Note that unlike os.Mkdir(), this function does not return IsExist error
// in case path already exists.
func MkdirAndChown(path string, mode os.FileMode, owner Identity) error {
Expand All @@ -50,7 +50,7 @@ func MkdirAndChown(path string, mode os.FileMode, owner Identity) error {

// MkdirAllAndChownNew creates a directory (include any along the path) and then modifies
// ownership ONLY of newly created directories to the requested uid/gid. If the
// directories along the path exist, no change of ownership will be performed
// directories along the path exist, no change of ownership or permissions will be performed
func MkdirAllAndChownNew(path string, mode os.FileMode, owner Identity) error {
return mkdirAs(path, mode, owner, true, false)
}
Expand Down Expand Up @@ -234,3 +234,8 @@ func parseSubidFile(path, username string) (ranges, error) {

return rangeList, s.Err()
}

// CurrentIdentity returns the identity of the current process
func CurrentIdentity() Identity {
return Identity{UID: os.Getuid(), GID: os.Getegid()}
}
14 changes: 10 additions & 4 deletions pkg/idtools/idtools_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting
}

// short-circuit--we were called with an existing directory and chown was requested
return lazyChown(path, owner.UID, owner.GID, stat)
return setPermissions(path, mode, owner.UID, owner.GID, stat)
}

if os.IsNotExist(err) {
Expand Down Expand Up @@ -71,7 +71,7 @@ func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting
// even if it existed, we will chown the requested path + any subpaths that
// didn't exist when we called MkdirAll
for _, pathComponent := range paths {
if err := lazyChown(pathComponent, owner.UID, owner.GID, nil); err != nil {
if err := setPermissions(pathComponent, mode, owner.UID, owner.GID, nil); err != nil {
return err
}
}
Expand Down Expand Up @@ -213,17 +213,23 @@ func callGetent(database, key string) (io.Reader, error) {
return bytes.NewReader(out), nil
}

// lazyChown performs a chown only if the uid/gid don't match what's requested
// setPermissions performs a chown/chmod only if the uid/gid don't match what's requested
// Normally a Chown is a no-op if uid/gid match, but in some cases this can still cause an error, e.g. if the
// dir is on an NFS share, so don't call chown unless we absolutely must.
func lazyChown(p string, uid, gid int, stat *system.StatT) error {
// Likewise for setting permissions.
func setPermissions(p string, mode os.FileMode, uid, gid int, stat *system.StatT) error {
if stat == nil {
var err error
stat, err = system.Stat(p)
if err != nil {
return err
}
}
if os.FileMode(stat.Mode()).Perm() != mode.Perm() {
if err := os.Chmod(p, mode.Perm()); err != nil {
return err
}
}
if stat.UID() == uint32(uid) && stat.GID() == uint32(gid) {
return nil
}
Expand Down

0 comments on commit 64bd448

Please sign in to comment.