From b5c99c0e95c331647e9c4352ac6adae862c05b2e Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Sat, 1 Mar 2025 00:11:49 -0800 Subject: [PATCH] Update moby/sys/user to version which includes mapping Update idtools to use Mkdir funcs from moby sys/user package Add deprecation exception to golanci until move off idtools is complete Signed-off-by: Derek McGowan --- .golangci.yml | 4 + pkg/idtools/idtools.go | 28 +++- pkg/idtools/idtools_unix_test.go | 18 +-- pkg/idtools/idtools_windows.go | 12 -- vendor.mod | 2 +- vendor.sum | 4 +- vendor/github.com/moby/sys/user/idtools.go | 141 ++++++++++++++++++ .../github.com/moby/sys/user}/idtools_unix.go | 63 +++----- .../moby/sys/user/idtools_windows.go | 13 ++ vendor/modules.txt | 2 +- 10 files changed, 208 insertions(+), 79 deletions(-) create mode 100644 vendor/github.com/moby/sys/user/idtools.go rename {pkg/idtools => vendor/github.com/moby/sys/user}/idtools_unix.go (62%) create mode 100644 vendor/github.com/moby/sys/user/idtools_windows.go diff --git a/.golangci.yml b/.golangci.yml index ae92a4fdf3..bda9aad4fb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -212,6 +212,10 @@ issues: linters: - staticcheck + - text: "SA1019: idtools\\.(CurrentIdentity|FromUserIdentityMapping|IDMap|MkdirAndChown|MkdirAllAndChown|MkdirAllAndChownNew) is deprecated" + linters: + - staticcheck + # Ignore "nested context in function literal (fatcontext)" as we intentionally set up tracing on a base-context for tests. # FIXME(thaJeztah): see if there's a more iodiomatic way to do this. - text: 'nested context in function literal' diff --git a/pkg/idtools/idtools.go b/pkg/idtools/idtools.go index d2fbd943a6..37b184cc62 100644 --- a/pkg/idtools/idtools.go +++ b/pkg/idtools/idtools.go @@ -3,11 +3,15 @@ package idtools import ( "fmt" "os" + + "github.com/moby/sys/user" ) // IDMap contains a single entry for user namespace range remapping. An array // of IDMap entries represents the structure that will be provided to the Linux // kernel for creating a user namespace. +// +// Deprecated: use [user.IDMap] instead. type IDMap struct { ContainerID int `json:"container_id"` HostID int `json:"host_id"` @@ -17,28 +21,42 @@ type IDMap struct { // 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 and permissions. +// +// Deprecated: use [user.MkdirAllAndChown] instead. func MkdirAllAndChown(path string, mode os.FileMode, owner Identity) error { - return mkdirAs(path, mode, owner, true, true) + return user.MkdirAllAndChown(path, mode, owner.UID, owner.GID) } // MkdirAndChown creates a directory and then modifies ownership to the requested uid/gid. // 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. +// +// Deprecated: use [user.MkdirAndChown] instead. func MkdirAndChown(path string, mode os.FileMode, owner Identity) error { - return mkdirAs(path, mode, owner, false, true) + return user.MkdirAndChown(path, mode, owner.UID, owner.GID) } // 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 or permissions will be performed +// +// Deprecated: use [user.MkdirAllAndChown] instead. func MkdirAllAndChownNew(path string, mode os.FileMode, owner Identity) error { - return mkdirAs(path, mode, owner, true, false) + return user.MkdirAllAndChown(path, mode, owner.UID, owner.GID, user.WithOnlyNew) } // GetRootUIDGID retrieves the remapped root uid/gid pair from the set of maps. // If the maps are empty, then the root uid/gid will default to "real" 0/0 +// +// Deprecated: use [(user.IdentityMapping).RootPair] instead. func GetRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) { + return getRootUIDGID(uidMap, gidMap) +} + +// getRootUIDGID retrieves the remapped root uid/gid pair from the set of maps. +// If the maps are empty, then the root uid/gid will default to "real" 0/0 +func getRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) { uid, err := toHost(0, uidMap) if err != nil { return -1, -1, err @@ -105,7 +123,7 @@ type IdentityMapping struct { // because a root user always exists, and the defaults are correct when the uid // and gid maps are empty. func (i IdentityMapping) RootPair() Identity { - uid, gid, _ := GetRootUIDGID(i.UIDMaps, i.GIDMaps) + uid, gid, _ := getRootUIDGID(i.UIDMaps, i.GIDMaps) return Identity{UID: uid, GID: gid} } @@ -144,6 +162,8 @@ func (i IdentityMapping) Empty() bool { } // CurrentIdentity returns the identity of the current process +// +// Deprecated: use [os.Getuid] and [os.Getegid] instead. func CurrentIdentity() Identity { return Identity{UID: os.Getuid(), GID: os.Getegid()} } diff --git a/pkg/idtools/idtools_unix_test.go b/pkg/idtools/idtools_unix_test.go index 381a1d7a62..916c2811e9 100644 --- a/pkg/idtools/idtools_unix_test.go +++ b/pkg/idtools/idtools_unix_test.go @@ -334,7 +334,7 @@ func TestGetRootUIDGID(t *testing.T) { }, } - uid, gid, err := GetRootUIDGID(uidMap, gidMap) + uid, gid, err := getRootUIDGID(uidMap, gidMap) assert.Check(t, err) assert.Check(t, is.Equal(os.Geteuid(), uid)) assert.Check(t, is.Equal(os.Getegid(), gid)) @@ -346,7 +346,7 @@ func TestGetRootUIDGID(t *testing.T) { Size: 1, }, } - _, _, err = GetRootUIDGID(uidMapError, gidMap) + _, _, err = getRootUIDGID(uidMapError, gidMap) assert.Check(t, is.Error(err, "Container ID 0 cannot be mapped to a host ID")) } @@ -364,20 +364,6 @@ func TestToContainer(t *testing.T) { assert.Check(t, is.Equal(uidMap[0].ContainerID, containerID)) } -// TestMkdirIsNotDir checks that mkdirAs() function (used by MkdirAll...) -// returns a correct error in case a directory which it is about to create -// already exists but is a file (rather than a directory). -func TestMkdirIsNotDir(t *testing.T) { - file, err := os.CreateTemp("", t.Name()) - if err != nil { - t.Fatalf("Couldn't create temp dir: %v", err) - } - defer os.Remove(file.Name()) - - err = mkdirAs(file.Name(), 0o755, Identity{UID: 0, GID: 0}, false, false) - assert.Check(t, is.Error(err, "mkdir "+file.Name()+": not a directory")) -} - func RequiresRoot(t *testing.T) { skip.If(t, os.Getuid() != 0, "skipping test that requires root") } diff --git a/pkg/idtools/idtools_windows.go b/pkg/idtools/idtools_windows.go index a12b14040a..f83f59f30c 100644 --- a/pkg/idtools/idtools_windows.go +++ b/pkg/idtools/idtools_windows.go @@ -1,9 +1,5 @@ package idtools -import ( - "os" -) - const ( SeTakeOwnershipPrivilege = "SeTakeOwnershipPrivilege" ) @@ -14,11 +10,3 @@ const ( ContainerUserSidString = "S-1-5-93-2-2" ) - -// This is currently a wrapper around [os.MkdirAll] since currently -// 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, _ os.FileMode, _ Identity, _, _ bool) error { - return os.MkdirAll(path, 0) -} diff --git a/vendor.mod b/vendor.mod index b3391db995..a5241f6898 100644 --- a/vendor.mod +++ b/vendor.mod @@ -76,7 +76,7 @@ require ( github.com/moby/sys/sequential v0.6.0 github.com/moby/sys/signal v0.7.1 github.com/moby/sys/symlink v0.3.0 - github.com/moby/sys/user v0.3.0 + github.com/moby/sys/user v0.4.0 github.com/moby/sys/userns v0.1.0 github.com/moby/term v0.5.2 github.com/morikuni/aec v1.0.0 diff --git a/vendor.sum b/vendor.sum index 3a957d5bb4..42a8de83c7 100644 --- a/vendor.sum +++ b/vendor.sum @@ -409,8 +409,8 @@ github.com/moby/sys/signal v0.7.1 h1:PrQxdvxcGijdo6UXXo/lU/TvHUWyPhj7UOpSo8tuvk0 github.com/moby/sys/signal v0.7.1/go.mod h1:Se1VGehYokAkrSQwL4tDzHvETwUZlnY7S5XtQ50mQp8= github.com/moby/sys/symlink v0.3.0 h1:GZX89mEZ9u53f97npBy4Rc3vJKj7JBDj/PN2I22GrNU= github.com/moby/sys/symlink v0.3.0/go.mod h1:3eNdhduHmYPcgsJtZXW1W4XUJdZGBIkttZ8xKqPUJq0= -github.com/moby/sys/user v0.3.0 h1:9ni5DlcW5an3SvRSx4MouotOygvzaXbaSrc/wGDFWPo= -github.com/moby/sys/user v0.3.0/go.mod h1:bG+tYYYJgaMtRKgEmuueC0hJEAZWwtIbZTB+85uoHjs= +github.com/moby/sys/user v0.4.0 h1:jhcMKit7SA80hivmFJcbB1vqmw//wU61Zdui2eQXuMs= +github.com/moby/sys/user v0.4.0/go.mod h1:bG+tYYYJgaMtRKgEmuueC0hJEAZWwtIbZTB+85uoHjs= github.com/moby/sys/userns v0.1.0 h1:tVLXkFOxVu9A64/yh59slHVv9ahO9UIev4JZusOLG/g= github.com/moby/sys/userns v0.1.0/go.mod h1:IHUYgu/kao6N8YZlp9Cf444ySSvCmDlmzUcYfDHOl28= github.com/moby/term v0.5.2 h1:6qk3FJAFDs6i/q3W/pQ97SX192qKfZgGjCQqfCJkgzQ= diff --git a/vendor/github.com/moby/sys/user/idtools.go b/vendor/github.com/moby/sys/user/idtools.go new file mode 100644 index 0000000000..595b7a9272 --- /dev/null +++ b/vendor/github.com/moby/sys/user/idtools.go @@ -0,0 +1,141 @@ +package user + +import ( + "fmt" + "os" +) + +// MkdirOpt is a type for options to pass to Mkdir calls +type MkdirOpt func(*mkdirOptions) + +type mkdirOptions struct { + onlyNew bool +} + +// WithOnlyNew is an option for MkdirAllAndChown that will only change ownership and permissions +// on newly created directories. If the directory already exists, it will not be modified +func WithOnlyNew(o *mkdirOptions) { + o.onlyNew = true +} + +// MkdirAllAndChown creates a directory (include any along the path) and then modifies +// ownership to the requested uid/gid. By default, if the directory already exists, this +// function will still change ownership and permissions. If WithOnlyNew is passed as an +// option, then only the newly created directories will have ownership and permissions changed. +func MkdirAllAndChown(path string, mode os.FileMode, uid, gid int, opts ...MkdirOpt) error { + var options mkdirOptions + for _, opt := range opts { + opt(&options) + } + + return mkdirAs(path, mode, uid, gid, true, options.onlyNew) +} + +// MkdirAndChown creates a directory and then modifies ownership to the requested uid/gid. +// By default, if the directory already exists, this function still changes ownership and permissions. +// If WithOnlyNew is passed as an option, then only the newly created directory will have ownership +// and permissions changed. +// Note that unlike os.Mkdir(), this function does not return IsExist error +// in case path already exists. +func MkdirAndChown(path string, mode os.FileMode, uid, gid int, opts ...MkdirOpt) error { + var options mkdirOptions + for _, opt := range opts { + opt(&options) + } + return mkdirAs(path, mode, uid, gid, false, options.onlyNew) +} + +// getRootUIDGID retrieves the remapped root uid/gid pair from the set of maps. +// If the maps are empty, then the root uid/gid will default to "real" 0/0 +func getRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) { + uid, err := toHost(0, uidMap) + if err != nil { + return -1, -1, err + } + gid, err := toHost(0, gidMap) + if err != nil { + return -1, -1, err + } + return uid, gid, nil +} + +// toContainer takes an id mapping, and uses it to translate a +// host ID to the remapped ID. If no map is provided, then the translation +// assumes a 1-to-1 mapping and returns the passed in id +func toContainer(hostID int, idMap []IDMap) (int, error) { + if idMap == nil { + return hostID, nil + } + for _, m := range idMap { + if (int64(hostID) >= m.ParentID) && (int64(hostID) <= (m.ParentID + m.Count - 1)) { + contID := int(m.ID + (int64(hostID) - m.ParentID)) + return contID, nil + } + } + return -1, fmt.Errorf("host ID %d cannot be mapped to a container ID", hostID) +} + +// toHost takes an id mapping and a remapped ID, and translates the +// ID to the mapped host ID. If no map is provided, then the translation +// assumes a 1-to-1 mapping and returns the passed in id # +func toHost(contID int, idMap []IDMap) (int, error) { + if idMap == nil { + return contID, nil + } + for _, m := range idMap { + if (int64(contID) >= m.ID) && (int64(contID) <= (m.ID + m.Count - 1)) { + hostID := int(m.ParentID + (int64(contID) - m.ID)) + return hostID, nil + } + } + return -1, fmt.Errorf("container ID %d cannot be mapped to a host ID", contID) +} + +// IdentityMapping contains a mappings of UIDs and GIDs. +// The zero value represents an empty mapping. +type IdentityMapping struct { + UIDMaps []IDMap `json:"UIDMaps"` + GIDMaps []IDMap `json:"GIDMaps"` +} + +// RootPair returns a uid and gid pair for the root user. The error is ignored +// because a root user always exists, and the defaults are correct when the uid +// and gid maps are empty. +func (i IdentityMapping) RootPair() (int, int) { + uid, gid, _ := getRootUIDGID(i.UIDMaps, i.GIDMaps) + return uid, gid +} + +// ToHost returns the host UID and GID for the container uid, gid. +// Remapping is only performed if the ids aren't already the remapped root ids +func (i IdentityMapping) ToHost(uid, gid int) (int, int, error) { + var err error + ruid, rgid := i.RootPair() + + if uid != ruid { + ruid, err = toHost(uid, i.UIDMaps) + if err != nil { + return ruid, rgid, err + } + } + + if gid != rgid { + rgid, err = toHost(gid, i.GIDMaps) + } + return ruid, rgid, err +} + +// ToContainer returns the container UID and GID for the host uid and gid +func (i IdentityMapping) ToContainer(uid, gid int) (int, int, error) { + ruid, err := toContainer(uid, i.UIDMaps) + if err != nil { + return -1, -1, err + } + rgid, err := toContainer(gid, i.GIDMaps) + return ruid, rgid, err +} + +// Empty returns true if there are no id mappings +func (i IdentityMapping) Empty() bool { + return len(i.UIDMaps) == 0 && len(i.GIDMaps) == 0 +} diff --git a/pkg/idtools/idtools_unix.go b/vendor/github.com/moby/sys/user/idtools_unix.go similarity index 62% rename from pkg/idtools/idtools_unix.go rename to vendor/github.com/moby/sys/user/idtools_unix.go index 1f11fe4740..4e39d2446b 100644 --- a/pkg/idtools/idtools_unix.go +++ b/vendor/github.com/moby/sys/user/idtools_unix.go @@ -1,6 +1,6 @@ //go:build !windows -package idtools +package user import ( "fmt" @@ -8,11 +8,9 @@ import ( "path/filepath" "strconv" "syscall" - - "github.com/moby/sys/user" ) -func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting bool) error { +func mkdirAs(path string, mode os.FileMode, uid, gid int, mkAll, onlyNew bool) error { path, err := filepath.Abs(path) if err != nil { return err @@ -23,21 +21,21 @@ func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting if !stat.IsDir() { return &os.PathError{Op: "mkdir", Path: path, Err: syscall.ENOTDIR} } - if !chownExisting { + if onlyNew { return nil } // short-circuit -- we were called with an existing directory and chown was requested - return setPermissions(path, mode, owner, stat) + return setPermissions(path, mode, uid, 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 + // so that we can chown all of them properly at the end. If onlyNew is true, we won't // chown the full directory path if it exists var paths []string if os.IsNotExist(err) { - paths = []string{path} + paths = append(paths, path) } if mkAll { @@ -49,7 +47,7 @@ func mkdirAs(path string, mode os.FileMode, owner Identity, mkAll, chownExisting if dirPath == "/" { break } - if _, err = os.Stat(dirPath); err != nil && os.IsNotExist(err) { + if _, err = os.Stat(dirPath); os.IsNotExist(err) { paths = append(paths, dirPath) } } @@ -62,39 +60,18 @@ 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 = setPermissions(pathComponent, mode, owner, nil); err != nil { + if err = setPermissions(pathComponent, mode, uid, gid, nil); err != nil { return err } } return nil } -// LookupUser uses traditional local system files lookup (from libcontainer/user) on a username -// -// Deprecated: use [user.LookupUser] instead -func LookupUser(name string) (user.User, error) { - return user.LookupUser(name) -} - -// LookupUID uses traditional local system files lookup (from libcontainer/user) on a uid -// -// Deprecated: use [user.LookupUid] instead -func LookupUID(uid int) (user.User, error) { - return user.LookupUid(uid) -} - -// LookupGroup uses traditional local system files lookup (from libcontainer/user) on a group name, -// -// Deprecated: use [user.LookupGroup] instead -func LookupGroup(name string) (user.Group, error) { - return user.LookupGroup(name) -} - // 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. // Likewise for setting permissions. -func setPermissions(p string, mode os.FileMode, owner Identity, stat os.FileInfo) error { +func setPermissions(p string, mode os.FileMode, uid, gid int, stat os.FileInfo) error { if stat == nil { var err error stat, err = os.Stat(p) @@ -108,10 +85,10 @@ func setPermissions(p string, mode os.FileMode, owner Identity, stat os.FileInfo } } ssi := stat.Sys().(*syscall.Stat_t) - if ssi.Uid == uint32(owner.UID) && ssi.Gid == uint32(owner.GID) { + if ssi.Uid == uint32(uid) && ssi.Gid == uint32(gid) { return nil } - return os.Chown(p, owner.UID, owner.GID) + return os.Chown(p, uid, gid) } // LoadIdentityMapping takes a requested username and @@ -119,9 +96,9 @@ func setPermissions(p string, mode os.FileMode, owner Identity, stat os.FileInfo // proper uid and gid remapping ranges for that user/group pair func LoadIdentityMapping(name string) (IdentityMapping, error) { // TODO: Consider adding support for calling out to "getent" - usr, err := user.LookupUser(name) + 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: %w", name, err) } subuidRanges, err := lookupSubRangesFile("/etc/subuid", usr) @@ -139,9 +116,9 @@ func LoadIdentityMapping(name string) (IdentityMapping, error) { }, nil } -func lookupSubRangesFile(path string, usr user.User) ([]IDMap, error) { +func lookupSubRangesFile(path string, usr User) ([]IDMap, error) { uidstr := strconv.Itoa(usr.Uid) - rangeList, err := user.ParseSubIDFileFilter(path, func(sid user.SubID) bool { + rangeList, err := ParseSubIDFileFilter(path, func(sid SubID) bool { return sid.Name == usr.Name || sid.Name == uidstr }) if err != nil { @@ -153,14 +130,14 @@ func lookupSubRangesFile(path string, usr user.User) ([]IDMap, error) { idMap := []IDMap{} - containerID := 0 + var containerID int64 for _, idrange := range rangeList { idMap = append(idMap, IDMap{ - ContainerID: containerID, - HostID: int(idrange.SubID), - Size: int(idrange.Count), + ID: containerID, + ParentID: idrange.SubID, + Count: idrange.Count, }) - containerID = containerID + int(idrange.Count) + containerID = containerID + idrange.Count } return idMap, nil } diff --git a/vendor/github.com/moby/sys/user/idtools_windows.go b/vendor/github.com/moby/sys/user/idtools_windows.go new file mode 100644 index 0000000000..9de730cafb --- /dev/null +++ b/vendor/github.com/moby/sys/user/idtools_windows.go @@ -0,0 +1,13 @@ +package user + +import ( + "os" +) + +// This is currently a wrapper around [os.MkdirAll] since currently +// 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, _ os.FileMode, _, _ int, _, _ bool) error { + return os.MkdirAll(path, 0) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 197d0b8376..a750e1c4d4 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1025,7 +1025,7 @@ github.com/moby/sys/signal # github.com/moby/sys/symlink v0.3.0 ## explicit; go 1.17 github.com/moby/sys/symlink -# github.com/moby/sys/user v0.3.0 +# github.com/moby/sys/user v0.4.0 ## explicit; go 1.17 github.com/moby/sys/user # github.com/moby/sys/userns v0.1.0