pkg/idtools: remove CanAccess(), and move to daemon

The implementation of CanAccess() is very rudimentary, and should
not be used for anything other than a basic check (and maybe not
even for that). It's only used in a single location in the daemon,
so move it there, and un-export it to not encourage others to use
it out of context.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2022-10-08 22:23:41 +02:00
parent ee34a8ac29
commit 69f72417f4
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
3 changed files with 38 additions and 26 deletions

View file

@ -15,6 +15,7 @@ import (
"strconv"
"strings"
"sync"
"syscall"
"time"
"github.com/containerd/cgroups"
@ -1247,7 +1248,7 @@ func setupDaemonRoot(config *config.Config, rootDir string, remappedRoot idtools
if dirPath == "/" {
break
}
if !idtools.CanAccess(dirPath, remappedRoot) {
if !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)
}
}
@ -1259,6 +1260,34 @@ func setupDaemonRoot(config *config.Config, rootDir string, remappedRoot idtools
return nil
}
// canAccess takes a valid (existing) directory and a uid, gid pair and determines
// if that uid, gid pair has access (execute bit) to the directory.
//
// Note: this is a very rudimentary check, and may not produce accurate results,
// so should not be used for anything other than the current use, see:
// https://github.com/moby/moby/issues/43724
func canAccess(path string, pair idtools.Identity) bool {
statInfo, err := os.Stat(path)
if err != nil {
return false
}
perms := statInfo.Mode().Perm()
if perms&0o001 == 0o001 {
// world access
return true
}
ssi := statInfo.Sys().(*syscall.Stat_t)
if ssi.Uid == uint32(pair.UID) && (perms&0o100 == 0o100) {
// owner access.
return true
}
if ssi.Gid == uint32(pair.GID) && (perms&0o010 == 0o010) {
// group access.
return true
}
return false
}
func setupDaemonRootPropagation(cfg *config.Config) error {
rootParentMount, mountOptions, err := getSourceMount(cfg.Root)
if err != nil {

View file

@ -79,30 +79,6 @@ func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting
return nil
}
// CanAccess takes a valid (existing) directory and a uid, gid pair and determines
// if that uid, gid pair has access (execute bit) to the directory
func CanAccess(path string, pair Identity) bool {
statInfo, err := os.Stat(path)
if err != nil {
return false
}
perms := statInfo.Mode().Perm()
if perms&0o001 == 0o001 {
// world access
return true
}
ssi := statInfo.Sys().(*syscall.Stat_t)
if ssi.Uid == uint32(pair.UID) && (perms&0o100 == 0o100) {
// owner access.
return true
}
if ssi.Gid == uint32(pair.GID) && (perms&0o010 == 0o010) {
// group access.
return true
}
return false
}
// LookupUser uses traditional local system files lookup (from libcontainer/user) on a username,
// followed by a call to `getent` for supporting host configured non-files passwd and group dbs
func LookupUser(name string) (user.User, error) {

View file

@ -6,8 +6,10 @@ package idtools // import "github.com/docker/docker/pkg/idtools"
import (
"fmt"
"os"
"os/exec"
"os/user"
"path/filepath"
"syscall"
"testing"
"golang.org/x/sys/unix"
@ -425,7 +427,12 @@ func TestNewIDMappings(t *testing.T) {
err = MkdirAllAndChown(dirName, 0o700, Identity{UID: rootUID, GID: rootGID})
assert.Check(t, err, "Couldn't change ownership of file path. Got error")
assert.Check(t, CanAccess(dirName, idMapping.RootPair()), "Unable to access %s directory with user UID:%d and GID:%d", dirName, rootUID, rootGID)
cmd := exec.Command("ls", "-la", dirName)
cmd.SysProcAttr = &syscall.SysProcAttr{
Credential: &syscall.Credential{Uid: uint32(rootUID), Gid: uint32(rootGID)},
}
out, err := cmd.CombinedOutput()
assert.Check(t, err, "Unable to access %s directory with user UID:%d and GID:%d:\n%s", dirName, rootUID, rootGID, string(out))
}
func TestLookupUserAndGroup(t *testing.T) {