0ct0pu5/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #44254 from thaJeztah/idtools_cleanup2

Browse source 
pkg/idtools: various cleanups
This commit is contained in:
Sebastiaan van Stijn 2022-10-15 22:42:09 +02:00 committed by GitHub
commit 1311687d0d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 51 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
pkg/idtools/idtools_unix.go
View file

@ -16,7 +16,6 @@ import (
"github.com/docker/docker/pkg/system"
"github.com/opencontainers/runc/libcontainer/user"
"github.com/pkg/errors"
)
var (
@ -25,12 +24,6 @@ var (
)
func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting bool) error {
// make an array containing the original path asked for, plus (for mkAll == true)
// all path components leading up to the complete path that don't exist before we MkdirAll
// so that we can chown all of them properly at the end. If chownExisting is false, we won't
// chown the full directory path if it exists
var paths []string
path, err := filepath.Abs(path)
if err != nil {
return err
@ -49,6 +42,11 @@ func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting
return setPermissions(path, mode, owner.UID, owner.GID, stat)
}
// make an array containing the original path asked for, plus (for mkAll == true)
// all path components leading up to the complete path that don't exist before we MkdirAll
// so that we can chown all of them properly at the end. If chownExisting is false, we won't
// chown the full directory path if it exists
var paths []string
if os.IsNotExist(err) {
paths = []string{path}
}
@ -66,7 +64,7 @@ func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting
paths = append(paths, dirPath)
}
}
if err := system.MkdirAll(path, mode); err != nil {
if err := os.MkdirAll(path, mode); err != nil {
return err
}
} else {
@ -91,20 +89,17 @@ func CanAccess(path string, pair Identity) bool {
if err != nil {
return false
}
fileMode := os.FileMode(statInfo.Mode())
permBits := fileMode.Perm()
return accessible(statInfo.UID() == uint32(pair.UID),
statInfo.GID() == uint32(pair.GID), permBits)
}
func accessible(isOwner, isGroup bool, perms os.FileMode) bool {
if isOwner && (perms&0100 == 0100) {
perms := os.FileMode(statInfo.Mode()).Perm()
if perms&0o001 == 0o001 {
// world access
return true
}
if isGroup && (perms&0010 == 0010) {
if statInfo.UID() == uint32(pair.UID) && (perms&0o100 == 0o100) {
// owner access.
return true
}
if perms&0001 == 0001 {
if statInfo.GID() == uint32(pair.GID) && (perms&0o010 == 0o010) {
// group access.
return true
}
return false
@ -259,7 +254,7 @@ func setPermissions(p string, mode os.FileMode, uid, gid int, stat *system.StatT
func LoadIdentityMapping(name string) (IdentityMapping, error) {
usr, err := LookupUser(name)
if err != nil {
return IdentityMapping{}, fmt.Errorf("Could not get user for username %s: %v", name, err)
return IdentityMapping{}, fmt.Errorf("could not get user for username %s: %v", name, err)
}
subuidRanges, err := lookupSubUIDRanges(usr)
@ -289,7 +284,7 @@ func lookupSubUIDRanges(usr user.User) ([]IDMap, error) {
}
}
if len(rangeList) == 0 {
return nil, errors.Errorf("no subuid ranges found for user %q", usr.Name)
return nil, fmt.Errorf("no subuid ranges found for user %q", usr.Name)
}
return createIDMap(rangeList), nil
}
@ -306,7 +301,7 @@ func lookupSubGIDRanges(usr user.User) ([]IDMap, error) {
}
}
if len(rangeList) == 0 {
return nil, errors.Errorf("no subgid ranges found for user %q", usr.Name)
return nil, fmt.Errorf("no subgid ranges found for user %q", usr.Name)
}
return createIDMap(rangeList), nil
}

40
pkg/idtools/idtools_unix_test.go
View file

@ -46,7 +46,7 @@ func TestMkdirAllAndChown(t *testing.T) {
}
// test adding a directory to a pre-existing dir; only the new dir is owned by the uid/gid
if err := MkdirAllAndChown(filepath.Join(dirName, "usr", "share"), 0755, Identity{UID: 99, GID: 99}); err != nil {
if err := MkdirAllAndChown(filepath.Join(dirName, "usr", "share"), 0o755, Identity{UID: 99, GID: 99}); err != nil {
t.Fatal(err)
}
testTree["usr/share"] = node{99, 99}
@ -59,7 +59,7 @@ func TestMkdirAllAndChown(t *testing.T) {
}
// test 2-deep new directories--both should be owned by the uid/gid pair
if err := MkdirAllAndChown(filepath.Join(dirName, "lib", "some", "other"), 0755, Identity{UID: 101, GID: 101}); err != nil {
if err := MkdirAllAndChown(filepath.Join(dirName, "lib", "some", "other"), 0o755, Identity{UID: 101, GID: 101}); err != nil {
t.Fatal(err)
}
testTree["lib/some"] = node{101, 101}
@ -73,7 +73,7 @@ func TestMkdirAllAndChown(t *testing.T) {
}
// test a directory that already exists; should be chowned, but nothing else
if err := MkdirAllAndChown(filepath.Join(dirName, "usr"), 0755, Identity{UID: 102, GID: 102}); err != nil {
if err := MkdirAllAndChown(filepath.Join(dirName, "usr"), 0o755, Identity{UID: 102, GID: 102}); err != nil {
t.Fatal(err)
}
testTree["usr"] = node{102, 102}
@ -102,7 +102,7 @@ func TestMkdirAllAndChownNew(t *testing.T) {
assert.NilError(t, buildTree(dirName, testTree))
// test adding a directory to a pre-existing dir; only the new dir is owned by the uid/gid
err = MkdirAllAndChownNew(filepath.Join(dirName, "usr", "share"), 0755, Identity{UID: 99, GID: 99})
err = MkdirAllAndChownNew(filepath.Join(dirName, "usr", "share"), 0o755, Identity{UID: 99, GID: 99})
assert.NilError(t, err)
testTree["usr/share"] = node{99, 99}
@ -111,7 +111,7 @@ func TestMkdirAllAndChownNew(t *testing.T) {
assert.NilError(t, compareTrees(testTree, verifyTree))
// test 2-deep new directories--both should be owned by the uid/gid pair
err = MkdirAllAndChownNew(filepath.Join(dirName, "lib", "some", "other"), 0755, Identity{UID: 101, GID: 101})
err = MkdirAllAndChownNew(filepath.Join(dirName, "lib", "some", "other"), 0o755, Identity{UID: 101, GID: 101})
assert.NilError(t, err)
testTree["lib/some"] = node{101, 101}
testTree["lib/some/other"] = node{101, 101}
@ -120,7 +120,7 @@ func TestMkdirAllAndChownNew(t *testing.T) {
assert.NilError(t, compareTrees(testTree, verifyTree))
// test a directory that already exists; should NOT be chowned
err = MkdirAllAndChownNew(filepath.Join(dirName, "usr"), 0755, Identity{UID: 102, GID: 102})
err = MkdirAllAndChownNew(filepath.Join(dirName, "usr"), 0o755, Identity{UID: 102, GID: 102})
assert.NilError(t, err)
verifyTree, err = readTree(dirName, "")
assert.NilError(t, err)
@ -191,7 +191,7 @@ func TestMkdirAllAndChownNewRelative(t *testing.T) {
assert.ErrorIs(t, err, os.ErrNotExist)
}
err := MkdirAllAndChownNew(tc.in, 0755, Identity{UID: expectedUIDGID, GID: expectedUIDGID})
err := MkdirAllAndChownNew(tc.in, 0o755, Identity{UID: expectedUIDGID, GID: expectedUIDGID})
assert.Check(t, err)
for _, p := range tc.out {
@ -235,7 +235,7 @@ func TestMkdirAndChown(t *testing.T) {
}
// test a directory that already exists; should just chown to the requested uid/gid
if err := MkdirAndChown(filepath.Join(dirName, "usr"), 0755, Identity{UID: 99, GID: 99}); err != nil {
if err := MkdirAndChown(filepath.Join(dirName, "usr"), 0o755, Identity{UID: 99, GID: 99}); err != nil {
t.Fatal(err)
}
testTree["usr"] = node{99, 99}
@ -248,12 +248,12 @@ func TestMkdirAndChown(t *testing.T) {
}
// create a subdir under a dir which doesn't exist--should fail
if err := MkdirAndChown(filepath.Join(dirName, "usr", "bin", "subdir"), 0755, Identity{UID: 102, GID: 102}); err == nil {
if err := MkdirAndChown(filepath.Join(dirName, "usr", "bin", "subdir"), 0o755, Identity{UID: 102, GID: 102}); err == nil {
t.Fatalf("Trying to create a directory with Mkdir where the parent doesn't exist should have failed")
}
// create a subdir under an existing dir; should only change the ownership of the new subdir
if err := MkdirAndChown(filepath.Join(dirName, "usr", "bin"), 0755, Identity{UID: 102, GID: 102}); err != nil {
if err := MkdirAndChown(filepath.Join(dirName, "usr", "bin"), 0o755, Identity{UID: 102, GID: 102}); err != nil {
t.Fatal(err)
}
testTree["usr/bin"] = node{102, 102}
@ -269,11 +269,11 @@ func TestMkdirAndChown(t *testing.T) {
func buildTree(base string, tree map[string]node) error {
for path, node := range tree {
fullPath := filepath.Join(base, path)
if err := os.MkdirAll(fullPath, 0755); err != nil {
return fmt.Errorf("Couldn't create path: %s; error: %v", fullPath, err)
if err := os.MkdirAll(fullPath, 0o755); err != nil {
return fmt.Errorf("couldn't create path: %s; error: %v", fullPath, err)
}
if err := os.Chown(fullPath, node.uid, node.gid); err != nil {
return fmt.Errorf("Couldn't chown path: %s; error: %v", fullPath, err)
return fmt.Errorf("couldn't chown path: %s; error: %v", fullPath, err)
}
}
return nil
@ -284,13 +284,13 @@ func readTree(base, root string) (map[string]node, error) {
dirInfos, err := os.ReadDir(base)
if err != nil {
return nil, fmt.Errorf("Couldn't read directory entries for %q: %v", base, err)
return nil, fmt.Errorf("couldn't read directory entries for %q: %v", base, err)
}
for _, info := range dirInfos {
s := &unix.Stat_t{}
if err := unix.Stat(filepath.Join(base, info.Name()), s); err != nil {
return nil, fmt.Errorf("Can't stat file %q: %v", filepath.Join(base, info.Name()), err)
return nil, fmt.Errorf("can't stat file %q: %v", filepath.Join(base, info.Name()), err)
}
tree[filepath.Join(root, info.Name())] = node{int(s.Uid), int(s.Gid)}
if info.IsDir() {
@ -309,7 +309,7 @@ func readTree(base, root string) (map[string]node, error) {
func compareTrees(left, right map[string]node) error {
if len(left) != len(right) {
return fmt.Errorf("Trees aren't the same size")
return fmt.Errorf("trees aren't the same size")
}
for path, nodeLeft := range left {
if nodeRight, ok := right[path]; ok {
@ -340,7 +340,7 @@ func TestParseSubidFileWithNewlinesAndComments(t *testing.T) {
# empty default subuid/subgid file
dockremap:231072:65536`
if err := os.WriteFile(fnamePath, []byte(fcontent), 0644); err != nil {
if err := os.WriteFile(fnamePath, []byte(fcontent), 0o644); err != nil {
t.Fatal(err)
}
ranges, err := parseSubidFile(fnamePath, "dockremap")
@ -423,9 +423,9 @@ func TestNewIDMappings(t *testing.T) {
assert.Check(t, err, "Couldn't create temp directory")
defer os.RemoveAll(dirName)
err = MkdirAllAndChown(dirName, 0700, Identity{UID: rootUID, GID: rootGID})
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()), fmt.Sprintf("Unable to access %s directory with user UID:%d and GID:%d", dirName, rootUID, rootGID))
assert.Check(t, CanAccess(dirName, idMapping.RootPair()), "Unable to access %s directory with user UID:%d and GID:%d", dirName, rootUID, rootGID)
}
func TestLookupUserAndGroup(t *testing.T) {
@ -475,7 +475,7 @@ func TestMkdirIsNotDir(t *testing.T) {
}
defer os.Remove(file.Name())
err = mkdirAs(file.Name(), 0755, Identity{UID: 0, GID: 0}, false, false)
err = mkdirAs(file.Name(), 0o755, Identity{UID: 0, GID: 0}, false, false)
assert.Check(t, is.Error(err, "mkdir "+file.Name()+": not a directory"))
}

14
pkg/idtools/idtools_windows.go
View file

@ -19,16 +19,6 @@ const (
// permissions aren't set through this path, the identity isn't utilized.
// Ownership is handled elsewhere, but in the future could be support here
// too.
func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting bool) error {
if err := system.MkdirAll(path, mode); err != nil {
return err
}
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
// Windows does not require/support this function, so always return true
func CanAccess(path string, identity Identity) bool {
return true
func mkdirAs(path string, _ os.FileMode, _ Identity, _, _ bool) error {
return system.MkdirAll(path, 0)
}

26
pkg/idtools/usergroupadd_linux.go
View file

@ -32,21 +32,21 @@ const (
// mapping ranges in containers.
func AddNamespaceRangesUser(name string) (int, int, error) {
if err := addUser(name); err != nil {
return -1, -1, fmt.Errorf("Error adding user %q: %v", name, err)
return -1, -1, fmt.Errorf("error adding user %q: %v", name, err)
}
// Query the system for the created uid and gid pair
out, err := execCmd("id", name)
if err != nil {
return -1, -1, fmt.Errorf("Error trying to find uid/gid for new user %q: %v", name, err)
return -1, -1, fmt.Errorf("error trying to find uid/gid for new user %q: %v", name, err)
}
matches := idOutRegexp.FindStringSubmatch(strings.TrimSpace(string(out)))
if len(matches) != 3 {
return -1, -1, fmt.Errorf("Can't find uid, gid from `id` output: %q", string(out))
return -1, -1, fmt.Errorf("can't find uid, gid from `id` output: %q", string(out))
}
uid, err := strconv.Atoi(matches[1])
if err != nil {
return -1, -1, fmt.Errorf("Can't convert found uid (%s) to int: %v", matches[1], err)
return -1, -1, fmt.Errorf("can't convert found uid (%s) to int: %v", matches[1], err)
}
gid, err := strconv.Atoi(matches[2])
if err != nil {
@ -57,7 +57,7 @@ func AddNamespaceRangesUser(name string) (int, int, error) {
// do not get auto-created ranges in subuid/subgid)
if err := createSubordinateRanges(name); err != nil {
return -1, -1, fmt.Errorf("Couldn't create subordinate ID ranges: %v", err)
return -1, -1, fmt.Errorf("couldn't create subordinate ID ranges: %v", err)
}
return uid, gid, nil
}
@ -92,33 +92,33 @@ func createSubordinateRanges(name string) error {
// by the distro tooling
ranges, err := parseSubuid(name)
if err != nil {
return fmt.Errorf("Error while looking for subuid ranges for user %q: %v", name, err)
return fmt.Errorf("error while looking for subuid ranges for user %q: %v", name, err)
}
if len(ranges) == 0 {
// no UID ranges; let's create one
startID, err := findNextUIDRange()
if err != nil {
return fmt.Errorf("Can't find available subuid range: %v", err)
return fmt.Errorf("can't find available subuid range: %v", err)
}
out, err := execCmd("usermod", "-v", fmt.Sprintf("%d-%d", startID, startID+defaultRangeLen-1), name)
if err != nil {
return fmt.Errorf("Unable to add subuid range to user: %q; output: %s, err: %v", name, out, err)
return fmt.Errorf("unable to add subuid range to user: %q; output: %s, err: %v", name, out, err)
}
}
ranges, err = parseSubgid(name)
if err != nil {
return fmt.Errorf("Error while looking for subgid ranges for user %q: %v", name, err)
return fmt.Errorf("error while looking for subgid ranges for user %q: %v", name, err)
}
if len(ranges) == 0 {
// no GID ranges; let's create one
startID, err := findNextGIDRange()
if err != nil {
return fmt.Errorf("Can't find available subgid range: %v", err)
return fmt.Errorf("can't find available subgid range: %v", err)
}
out, err := execCmd("usermod", "-w", fmt.Sprintf("%d-%d", startID, startID+defaultRangeLen-1), name)
if err != nil {
return fmt.Errorf("Unable to add subgid range to user: %q; output: %s, err: %v", name, out, err)
return fmt.Errorf("unable to add subgid range to user: %q; output: %s, err: %v", name, out, err)
}
}
return nil
@ -127,7 +127,7 @@ func createSubordinateRanges(name string) error {
func findNextUIDRange() (int, error) {
ranges, err := parseSubuid("ALL")
if err != nil {
return -1, fmt.Errorf("Couldn't parse all ranges in /etc/subuid file: %v", err)
return -1, fmt.Errorf("couldn't parse all ranges in /etc/subuid file: %v", err)
}
sort.Sort(ranges)
return findNextRangeStart(ranges)
@ -136,7 +136,7 @@ func findNextUIDRange() (int, error) {
func findNextGIDRange() (int, error) {
ranges, err := parseSubgid("ALL")
if err != nil {
return -1, fmt.Errorf("Couldn't parse all ranges in /etc/subgid file: %v", err)
return -1, fmt.Errorf("couldn't parse all ranges in /etc/subgid file: %v", err)
}
sort.Sort(ranges)
return findNextRangeStart(ranges)