From b5c99c0e95c331647e9c4352ac6adae862c05b2e Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Sat, 1 Mar 2025 00:11:49 -0800 Subject: [PATCH 1/3] 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 From 3fc36bcac49cb881abe16a0bd4a469a02fd6af11 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Sat, 1 Mar 2025 01:57:51 -0800 Subject: [PATCH 2/3] Update daemon to use moby sys/user identity mapping Signed-off-by: Derek McGowan --- daemon/archive_tarcopyoptions.go | 3 ++- daemon/container_operations_unix.go | 27 +++++++++++---------- daemon/create_unix.go | 9 +++---- daemon/daemon.go | 31 ++++++++++++------------ daemon/daemon_unix.go | 36 +++++++++++++++------------- daemon/daemon_windows.go | 10 ++++---- daemon/export.go | 3 ++- daemon/info.go | 2 +- daemon/oci_linux.go | 11 +++++---- daemon/volumes_unix.go | 8 ++++--- daemon/workdir.go | 5 +++- internal/usergroup/add_linux_test.go | 7 +++--- internal/usergroup/lookup_unix.go | 27 ++++++++++----------- pkg/idtools/idtools.go | 25 +++++++++++++++++++ 14 files changed, 119 insertions(+), 85 deletions(-) diff --git a/daemon/archive_tarcopyoptions.go b/daemon/archive_tarcopyoptions.go index 2e2141042e..42bd1ace2b 100644 --- a/daemon/archive_tarcopyoptions.go +++ b/daemon/archive_tarcopyoptions.go @@ -2,6 +2,7 @@ package daemon // import "github.com/docker/docker/daemon" import ( "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/idtools" ) // defaultTarCopyOptions is the setting that is used when unpacking an archive @@ -9,6 +10,6 @@ import ( func (daemon *Daemon) defaultTarCopyOptions(noOverwriteDirNonDir bool) *archive.TarOptions { return &archive.TarOptions{ NoOverwriteDirNonDir: noOverwriteDirNonDir, - IDMap: daemon.idMapping, + IDMap: idtools.FromUserIdentityMapping(daemon.idMapping), } } diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index cda101673d..dd8351dd57 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -23,6 +23,7 @@ import ( "github.com/docker/docker/pkg/process" "github.com/docker/docker/pkg/stringid" "github.com/moby/sys/mount" + "github.com/moby/sys/user" "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" "go.opentelemetry.io/otel" @@ -251,14 +252,14 @@ func (daemon *Daemon) setupIPCDirs(ctr *container.Container) error { fallthrough case ipcMode.IsShareable(): - rootIDs := daemon.idMapping.RootPair() + uid, gid := daemon.idMapping.RootPair() if !ctr.HasMountFor("/dev/shm") { shmPath, err := ctr.ShmResourcePath() if err != nil { return err } - if err := idtools.MkdirAllAndChown(shmPath, 0o700, rootIDs); err != nil { + if err := user.MkdirAllAndChown(shmPath, 0o700, uid, gid); err != nil { return err } @@ -266,7 +267,7 @@ func (daemon *Daemon) setupIPCDirs(ctr *container.Container) error { if err := unix.Mount("shm", shmPath, "tmpfs", uintptr(unix.MS_NOEXEC|unix.MS_NOSUID|unix.MS_NODEV), label.FormatMountLabel(shmproperty, ctr.GetMountLabel())); err != nil { return fmt.Errorf("mounting shm tmpfs: %s", err) } - if err := os.Chown(shmPath, rootIDs.UID, rootIDs.GID); err != nil { + if err := os.Chown(shmPath, uid, gid); err != nil { return err } ctr.ShmPath = shmPath @@ -298,7 +299,7 @@ func (daemon *Daemon) setupSecretDir(ctr *container.Container) (setupErr error) } // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMapping.RootPair() + ruid, rgid := daemon.idMapping.RootPair() for _, s := range ctr.SecretReferences { // TODO (ehazlett): use type switch when more are supported @@ -313,7 +314,7 @@ func (daemon *Daemon) setupSecretDir(ctr *container.Container) (setupErr error) if err != nil { return errors.Wrap(err, "error getting secret file path") } - if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0o700, rootIDs); err != nil { + if err := user.MkdirAllAndChown(filepath.Dir(fPath), 0o700, ruid, rgid); err != nil { return errors.Wrap(err, "error creating secret mount path") } @@ -338,7 +339,7 @@ func (daemon *Daemon) setupSecretDir(ctr *container.Container) (setupErr error) return err } - if err := os.Chown(fPath, rootIDs.UID+uid, rootIDs.GID+gid); err != nil { + if err := os.Chown(fPath, ruid+uid, rgid+gid); err != nil { return errors.Wrap(err, "error setting ownership for secret") } if err := os.Chmod(fPath, s.File.Mode); err != nil { @@ -364,7 +365,7 @@ func (daemon *Daemon) setupSecretDir(ctr *container.Container) (setupErr error) if err != nil { return errors.Wrap(err, "error getting config file path for container") } - if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0o700, rootIDs); err != nil { + if err := user.MkdirAllAndChown(filepath.Dir(fPath), 0o700, ruid, rgid); err != nil { return errors.Wrap(err, "error creating config mount path") } @@ -389,7 +390,7 @@ func (daemon *Daemon) setupSecretDir(ctr *container.Container) (setupErr error) return err } - if err := os.Chown(fPath, rootIDs.UID+uid, rootIDs.GID+gid); err != nil { + if err := os.Chown(fPath, ruid+uid, rgid+gid); err != nil { return errors.Wrap(err, "error setting ownership for config") } if err := os.Chmod(fPath, configRef.File.Mode); err != nil { @@ -404,18 +405,18 @@ func (daemon *Daemon) setupSecretDir(ctr *container.Container) (setupErr error) // In practice this is using a tmpfs mount and is used for both "configs" and "secrets" func (daemon *Daemon) createSecretsDir(ctr *container.Container) error { // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMapping.RootPair() + uid, gid := daemon.idMapping.RootPair() dir, err := ctr.SecretMountPath() if err != nil { return errors.Wrap(err, "error getting container secrets dir") } // create tmpfs - if err := idtools.MkdirAllAndChown(dir, 0o700, rootIDs); err != nil { + if err := user.MkdirAllAndChown(dir, 0o700, uid, gid); err != nil { return errors.Wrap(err, "error creating secret local mount path") } - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", uid, gid) if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { return errors.Wrap(err, "unable to setup secret mount") } @@ -430,8 +431,8 @@ func (daemon *Daemon) remountSecretDir(ctr *container.Container) error { if err := label.Relabel(dir, ctr.MountLabel, false); err != nil { log.G(context.TODO()).WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label") } - rootIDs := daemon.idMapping.RootPair() - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + uid, gid := daemon.idMapping.RootPair() + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", uid, gid) // remount secrets ro if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { diff --git a/daemon/create_unix.go b/daemon/create_unix.go index d91bf94fec..7853a9a5c6 100644 --- a/daemon/create_unix.go +++ b/daemon/create_unix.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker/container" "github.com/docker/docker/errdefs" "github.com/docker/docker/oci" + "github.com/docker/docker/pkg/idtools" volumemounts "github.com/docker/docker/volume/mounts" volumeopts "github.com/docker/docker/volume/service/opts" "github.com/opencontainers/selinux/go-selinux/label" @@ -27,8 +28,8 @@ func (daemon *Daemon) createContainerOSSpecificSettings(ctx context.Context, con } defer daemon.Unmount(container) - rootIDs := daemon.idMapping.RootPair() - if err := container.SetupWorkingDirectory(rootIDs); err != nil { + uid, gid := daemon.idMapping.RootPair() + if err := container.SetupWorkingDirectory(idtools.Identity{UID: uid, GID: gid}); err != nil { return err } @@ -107,8 +108,8 @@ func (daemon *Daemon) populateVolume(ctx context.Context, c *container.Container } return err } - - volumePath, cleanup, err := mnt.Setup(ctx, c.MountLabel, daemon.idMapping.RootPair(), nil) + uid, gid := daemon.idMapping.RootPair() + volumePath, cleanup, err := mnt.Setup(ctx, c.MountLabel, idtools.Identity{UID: uid, GID: gid}, nil) if err != nil { if errdefs.IsNotFound(err) { return nil diff --git a/daemon/daemon.go b/daemon/daemon.go index 395f4cc351..50bb480d3b 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -75,6 +75,7 @@ import ( "github.com/moby/buildkit/util/grpcerrors" "github.com/moby/buildkit/util/tracing" "github.com/moby/locker" + "github.com/moby/sys/user" "github.com/moby/sys/userns" "github.com/pkg/errors" "go.etcd.io/bbolt" @@ -113,7 +114,7 @@ type Daemon struct { sysInfoOnce sync.Once sysInfo *sysinfo.SysInfo shutdown bool - idMapping idtools.IdentityMapping + idMapping user.IdentityMapping PluginStore *plugin.Store // TODO: remove pluginManager *plugin.Manager linkIndex *linkIndex @@ -791,7 +792,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S if err != nil { return nil, err } - rootIDs := idMapping.RootPair() + uid, gid := idMapping.RootPair() // set up the tmpDir to use a canonical path tmp, err := prepareTempDir(config.Root) @@ -878,10 +879,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S } daemonRepo := filepath.Join(cfgStore.Root, "containers") - if err := idtools.MkdirAllAndChown(daemonRepo, 0o710, idtools.Identity{ - UID: idtools.CurrentIdentity().UID, - GID: rootIDs.GID, - }); err != nil { + if err := user.MkdirAllAndChown(daemonRepo, 0o710, os.Getuid(), gid); err != nil { return nil, err } @@ -999,7 +997,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S } log.G(ctx).Debugf("Using default logging driver %s", d.defaultLogConfig.Type) - d.volumes, err = volumesservice.NewVolumeService(cfgStore.Root, d.PluginStore, rootIDs, d) + d.volumes, err = volumesservice.NewVolumeService(cfgStore.Root, d.PluginStore, idtools.Identity{UID: uid, GID: gid}, d) if err != nil { return nil, err } @@ -1074,15 +1072,15 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S RegistryHosts: d.RegistryHosts, Registry: d.registryService, EventsService: d.EventsService, - IDMapping: idMapping, - RefCountMounter: snapshotter.NewMounter(config.Root, driverName, idMapping), + IDMapping: idtools.FromUserIdentityMapping(idMapping), + RefCountMounter: snapshotter.NewMounter(config.Root, driverName, idtools.FromUserIdentityMapping(idMapping)), }) } else { layerStore, err := layer.NewStoreFromOptions(layer.StoreOptions{ Root: cfgStore.Root, GraphDriver: driverName, GraphDriverOptions: cfgStore.GraphOptions, - IDMapping: idMapping, + IDMapping: idtools.FromUserIdentityMapping(idMapping), }) if err != nil { return nil, err @@ -1423,7 +1421,7 @@ func prepareTempDir(rootDir string) (string, error) { } } } - return tmpDir, idtools.MkdirAllAndChown(tmpDir, 0o700, idtools.CurrentIdentity()) + return tmpDir, user.MkdirAllAndChown(tmpDir, 0o700, os.Getuid(), os.Getegid()) } func (daemon *Daemon) setGenericResources(conf *config.Config) error { @@ -1545,7 +1543,8 @@ func CreateDaemonRoot(config *config.Config) error { if err != nil { return err } - return setupDaemonRoot(config, realRoot, idMapping.RootPair()) + uid, gid := idMapping.RootPair() + return setupDaemonRoot(config, realRoot, uid, gid) } // RemapContainerdNamespaces returns the right containerd namespaces to use: @@ -1561,16 +1560,16 @@ func RemapContainerdNamespaces(config *config.Config) (ns string, pluginNs strin if idMapping.Empty() { return config.ContainerdNamespace, config.ContainerdPluginNamespace, nil } - root := idMapping.RootPair() + uid, gid := idMapping.RootPair() ns = config.ContainerdNamespace if _, ok := config.ValuesSet["containerd-namespace"]; !ok { - ns = fmt.Sprintf("%s-%d.%d", config.ContainerdNamespace, root.UID, root.GID) + ns = fmt.Sprintf("%s-%d.%d", config.ContainerdNamespace, uid, gid) } pluginNs = config.ContainerdPluginNamespace if _, ok := config.ValuesSet["containerd-plugin-namespace"]; !ok { - pluginNs = fmt.Sprintf("%s-%d.%d", config.ContainerdPluginNamespace, root.UID, root.GID) + pluginNs = fmt.Sprintf("%s-%d.%d", config.ContainerdPluginNamespace, uid, gid) } return ns, pluginNs, nil @@ -1601,7 +1600,7 @@ func (daemon *Daemon) GetAttachmentStore() *network.AttachmentStore { // IdentityMapping returns uid/gid mapping or a SID (in the case of Windows) for the builder func (daemon *Daemon) IdentityMapping() idtools.IdentityMapping { - return daemon.idMapping + return idtools.FromUserIdentityMapping(daemon.idMapping) } // ImageService returns the Daemon's ImageService diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index b988b07a8e..a36bdee907 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -45,6 +45,7 @@ import ( "github.com/docker/docker/runconfig" volumemounts "github.com/docker/docker/volume/mounts" "github.com/moby/sys/mount" + "github.com/moby/sys/user" "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux" "github.com/opencontainers/selinux/go-selinux/label" @@ -1250,9 +1251,10 @@ func removeDefaultBridgeInterface() { } } -func setupInitLayer(idMapping idtools.IdentityMapping) func(string) error { +func setupInitLayer(idMapping user.IdentityMapping) func(string) error { return func(initPath string) error { - return initlayer.Setup(initPath, idMapping.RootPair()) + uid, gid := idMapping.RootPair() + return initlayer.Setup(initPath, idtools.Identity{UID: uid, GID: gid}) } } @@ -1349,9 +1351,9 @@ func parseRemappedRoot(usergrp string) (string, string, error) { return username, groupname, nil } -func setupRemappedRoot(config *config.Config) (idtools.IdentityMapping, error) { +func setupRemappedRoot(config *config.Config) (user.IdentityMapping, error) { if runtime.GOOS != "linux" && config.RemappedRoot != "" { - return idtools.IdentityMapping{}, fmt.Errorf("User namespaces are only supported on Linux") + return user.IdentityMapping{}, fmt.Errorf("User namespaces are only supported on Linux") } // if the daemon was started with remapped root option, parse @@ -1359,13 +1361,13 @@ func setupRemappedRoot(config *config.Config) (idtools.IdentityMapping, error) { if config.RemappedRoot != "" { username, groupname, err := parseRemappedRoot(config.RemappedRoot) if err != nil { - return idtools.IdentityMapping{}, err + return user.IdentityMapping{}, err } if username == "root" { // Cannot setup user namespaces with a 1-to-1 mapping; "--root=0:0" is a no-op // effectively log.G(context.TODO()).Warn("User namespaces: root cannot be remapped with itself; user namespaces are OFF") - return idtools.IdentityMapping{}, nil + return user.IdentityMapping{}, nil } log.G(context.TODO()).Infof("User namespaces: ID ranges will be mapped to subuid/subgid ranges of: %s", username) // update remapped root setting now that we have resolved them to actual names @@ -1373,14 +1375,14 @@ func setupRemappedRoot(config *config.Config) (idtools.IdentityMapping, error) { mappings, err := usergroup.LoadIdentityMapping(username) if err != nil { - return idtools.IdentityMapping{}, errors.Wrap(err, "Can't create ID mappings") + return user.IdentityMapping{}, errors.Wrap(err, "Can't create ID mappings") } return mappings, nil } - return idtools.IdentityMapping{}, nil + return user.IdentityMapping{}, nil } -func setupDaemonRoot(config *config.Config, rootDir string, remappedRoot idtools.Identity) error { +func setupDaemonRoot(config *config.Config, rootDir string, uid, gid int) 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 @@ -1400,9 +1402,9 @@ func setupDaemonRoot(config *config.Config, rootDir string, remappedRoot idtools } } - id := idtools.Identity{UID: idtools.CurrentIdentity().UID, GID: remappedRoot.GID} + curuid := os.Getuid() // First make sure the current root dir has the correct perms. - if err := idtools.MkdirAllAndChown(config.Root, 0o710, id); err != nil { + if err := user.MkdirAllAndChown(config.Root, 0o710, curuid, gid); err != nil { return errors.Wrapf(err, "could not create or set daemon root permissions: %s", config.Root) } @@ -1411,10 +1413,10 @@ func setupDaemonRoot(config *config.Config, rootDir string, remappedRoot 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", remappedRoot.UID, remappedRoot.GID)) + config.Root = filepath.Join(rootDir, fmt.Sprintf("%d.%d", uid, gid)) log.G(context.TODO()).Debugf("Creating user namespaced daemon root: %s", config.Root) // Create the root directory if it doesn't exist - if err := idtools.MkdirAllAndChown(config.Root, 0o710, id); err != nil { + if err := user.MkdirAllAndChown(config.Root, 0o710, curuid, gid); 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 @@ -1427,7 +1429,7 @@ func setupDaemonRoot(config *config.Config, rootDir string, remappedRoot idtools if dirPath == "/" { break } - if !canAccess(dirPath, remappedRoot) { + if !canAccess(dirPath, uid, gid) { 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) } } @@ -1445,7 +1447,7 @@ func setupDaemonRoot(config *config.Config, rootDir string, remappedRoot idtools // 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 { +func canAccess(path string, uid, gid int) bool { statInfo, err := os.Stat(path) if err != nil { return false @@ -1456,11 +1458,11 @@ func canAccess(path string, pair idtools.Identity) bool { return true } ssi := statInfo.Sys().(*syscall.Stat_t) - if ssi.Uid == uint32(pair.UID) && (perms&0o100 == 0o100) { + if ssi.Uid == uint32(uid) && (perms&0o100 == 0o100) { // owner access. return true } - if ssi.Gid == uint32(pair.GID) && (perms&0o010 == 0o010) { + if ssi.Gid == uint32(gid) && (perms&0o010 == 0o010) { // group access. return true } diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index 5a099c1800..20d173c08c 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -24,10 +24,10 @@ import ( "github.com/docker/docker/libnetwork/netlabel" "github.com/docker/docker/libnetwork/options" "github.com/docker/docker/libnetwork/scope" - "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/parsers/operatingsystem" "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/pkg/system" + "github.com/moby/sys/user" "github.com/pkg/errors" "golang.org/x/sys/windows" "golang.org/x/sys/windows/svc/mgr" @@ -56,7 +56,7 @@ func (daemon *Daemon) parseSecurityOpt(daemonCfg *config.Config, securityOptions return nil } -func setupInitLayer(idMapping idtools.IdentityMapping) func(string) error { +func setupInitLayer(idMapping user.IdentityMapping) func(string) error { return nil } @@ -462,11 +462,11 @@ func recursiveUnmount(_ string) error { return nil } -func setupRemappedRoot(config *config.Config) (idtools.IdentityMapping, error) { - return idtools.IdentityMapping{}, nil +func setupRemappedRoot(config *config.Config) (user.IdentityMapping, error) { + return user.IdentityMapping{}, nil } -func setupDaemonRoot(config *config.Config, rootDir string, rootIdentity idtools.Identity) error { +func setupDaemonRoot(config *config.Config, rootDir string, uid, gid int) error { config.Root = rootDir // Create the root directory if it doesn't exists if err := system.MkdirAllWithACL(config.Root, 0, system.SddlAdministratorsLocalSystem); err != nil { diff --git a/daemon/export.go b/daemon/export.go index d01057cd26..b3aaca8477 100644 --- a/daemon/export.go +++ b/daemon/export.go @@ -11,6 +11,7 @@ import ( "github.com/docker/docker/errdefs" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/chrootarchive" + "github.com/docker/docker/pkg/idtools" ) // ContainerExport writes the contents of the container to the given @@ -65,7 +66,7 @@ func (daemon *Daemon) containerExport(ctx context.Context, ctr *container.Contai archv, err := chrootarchive.Tar(basefs, &archive.TarOptions{ Compression: archive.Uncompressed, - IDMap: daemon.idMapping, + IDMap: idtools.FromUserIdentityMapping(daemon.idMapping), }, basefs) if err != nil { return err diff --git a/daemon/info.go b/daemon/info.go index 339d9664ef..ebc3fa280c 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -191,7 +191,7 @@ func (daemon *Daemon) fillSecurityOptions(v *system.Info, sysInfo *sysinfo.SysIn if selinux.GetEnabled() { securityOptions = append(securityOptions, "name=selinux") } - if rootIDs := daemon.idMapping.RootPair(); rootIDs.UID != 0 || rootIDs.GID != 0 { + if uid, gid := daemon.idMapping.RootPair(); uid != 0 || gid != 0 { securityOptions = append(securityOptions, "name=userns") } if Rootless(cfg) { diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index 1529dee046..28c7543527 100644 --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -360,13 +360,13 @@ func WithNamespaces(daemon *Daemon, c *container.Container) coci.SpecOpts { } } -func specMapping(s []idtools.IDMap) []specs.LinuxIDMapping { +func specMapping(s []user.IDMap) []specs.LinuxIDMapping { var ids []specs.LinuxIDMapping for _, item := range s { ids = append(ids, specs.LinuxIDMapping{ - HostID: uint32(item.HostID), - ContainerID: uint32(item.ContainerID), - Size: uint32(item.Size), + HostID: uint32(item.ParentID), + ContainerID: uint32(item.ID), + Size: uint32(item.Count), }) } return ids @@ -712,7 +712,8 @@ func withCommonOptions(daemon *Daemon, daemonCfg *dconfig.Config, c *container.C Path: c.BaseFS, Readonly: c.HostConfig.ReadonlyRootfs, } - if err := c.SetupWorkingDirectory(daemon.idMapping.RootPair()); err != nil { + uid, gid := daemon.idMapping.RootPair() + if err := c.SetupWorkingDirectory(idtools.Identity{UID: uid, GID: gid}); err != nil { return err } cwd := c.Config.WorkingDir diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index 0a04fe2369..05f4d56fea 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -14,6 +14,7 @@ import ( mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/container" "github.com/docker/docker/internal/cleanups" + "github.com/docker/docker/pkg/idtools" volumemounts "github.com/docker/docker/volume/mounts" "github.com/pkg/errors" ) @@ -61,7 +62,8 @@ func (daemon *Daemon) setupMounts(ctx context.Context, c *container.Container) ( return nil } - path, clean, err := m.Setup(ctx, c.MountLabel, daemon.idMapping.RootPair(), checkfunc) + uid, gid := daemon.idMapping.RootPair() + path, clean, err := m.Setup(ctx, c.MountLabel, idtools.Identity{UID: uid, GID: gid}, checkfunc) if err != nil { return nil, nil, err } @@ -106,13 +108,13 @@ func (daemon *Daemon) setupMounts(ctx context.Context, c *container.Container) ( // if we are going to mount any of the network files from container // metadata, the ownership must be set properly for potential container // remapped root (user namespaces) - rootIDs := daemon.idMapping.RootPair() + uid, gid := daemon.idMapping.RootPair() for _, mnt := range netMounts { // we should only modify ownership of network files within our own container // metadata repository. If the user specifies a mount path external, it is // up to the user to make sure the file has proper ownership for userns if strings.Index(mnt.Source, daemon.repository) == 0 { - if err := os.Chown(mnt.Source, rootIDs.UID, rootIDs.GID); err != nil { + if err := os.Chown(mnt.Source, uid, gid); err != nil { return nil, nil, err } } diff --git a/daemon/workdir.go b/daemon/workdir.go index 4c0184998a..0e4885f54f 100644 --- a/daemon/workdir.go +++ b/daemon/workdir.go @@ -1,5 +1,7 @@ package daemon // import "github.com/docker/docker/daemon" +import "github.com/docker/docker/pkg/idtools" + // ContainerCreateWorkdir creates the working directory. This solves the // issue arising from https://github.com/docker/docker/issues/27545, // which was initially fixed by https://github.com/docker/docker/pull/27884. But that fix @@ -16,5 +18,6 @@ func (daemon *Daemon) ContainerCreateWorkdir(cID string) error { return err } defer daemon.Unmount(container) - return container.SetupWorkingDirectory(daemon.idMapping.RootPair()) + uid, gid := daemon.idMapping.RootPair() + return container.SetupWorkingDirectory(idtools.Identity{UID: uid, GID: gid}) } diff --git a/internal/usergroup/add_linux_test.go b/internal/usergroup/add_linux_test.go index 9308965d15..6e6cc5e430 100644 --- a/internal/usergroup/add_linux_test.go +++ b/internal/usergroup/add_linux_test.go @@ -7,7 +7,7 @@ import ( "syscall" "testing" - "github.com/docker/docker/pkg/idtools" + mobyuser "github.com/moby/sys/user" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/skip" @@ -29,14 +29,13 @@ func TestNewIDMappings(t *testing.T) { idMapping, err := LoadIdentityMapping(tempUser.Username) assert.Check(t, err) - rootUID, rootGID, err := idtools.GetRootUIDGID(idMapping.UIDMaps, idMapping.GIDMaps) - assert.Check(t, err) + rootUID, rootGID := idMapping.RootPair() dirName, err := os.MkdirTemp("", "mkdirall") assert.Check(t, err, "Couldn't create temp directory") defer os.RemoveAll(dirName) - err = idtools.MkdirAllAndChown(dirName, 0o700, idtools.Identity{UID: rootUID, GID: rootGID}) + err = mobyuser.MkdirAllAndChown(dirName, 0o700, rootUID, rootGID) assert.Check(t, err, "Couldn't change ownership of file path. Got error") cmd := exec.Command("ls", "-la", dirName) cmd.SysProcAttr = &syscall.SysProcAttr{ diff --git a/internal/usergroup/lookup_unix.go b/internal/usergroup/lookup_unix.go index 79e9535c04..4ac10ecc52 100644 --- a/internal/usergroup/lookup_unix.go +++ b/internal/usergroup/lookup_unix.go @@ -10,7 +10,6 @@ import ( "strconv" "syscall" - "github.com/docker/docker/pkg/idtools" "github.com/moby/sys/user" ) @@ -140,28 +139,28 @@ func getExitCode(err error) (int, error) { // LoadIdentityMapping takes a requested username and // using the data from /etc/sub{uid,gid} ranges, creates the // proper uid and gid remapping ranges for that user/group pair -func LoadIdentityMapping(name string) (idtools.IdentityMapping, error) { +func LoadIdentityMapping(name string) (user.IdentityMapping, error) { usr, err := LookupUser(name) if err != nil { - return idtools.IdentityMapping{}, fmt.Errorf("could not get user for username %s: %v", name, err) + return user.IdentityMapping{}, fmt.Errorf("could not get user for username %s: %v", name, err) } subuidRanges, err := lookupSubRangesFile("/etc/subuid", usr) if err != nil { - return idtools.IdentityMapping{}, err + return user.IdentityMapping{}, err } subgidRanges, err := lookupSubRangesFile("/etc/subgid", usr) if err != nil { - return idtools.IdentityMapping{}, err + return user.IdentityMapping{}, err } - return idtools.IdentityMapping{ + return user.IdentityMapping{ UIDMaps: subuidRanges, GIDMaps: subgidRanges, }, nil } -func lookupSubRangesFile(path string, usr user.User) ([]idtools.IDMap, error) { +func lookupSubRangesFile(path string, usr user.User) ([]user.IDMap, error) { uidstr := strconv.Itoa(usr.Uid) rangeList, err := user.ParseSubIDFileFilter(path, func(sid user.SubID) bool { return sid.Name == usr.Name || sid.Name == uidstr @@ -173,16 +172,16 @@ func lookupSubRangesFile(path string, usr user.User) ([]idtools.IDMap, error) { return nil, fmt.Errorf("no subuid ranges found for user %q", usr.Name) } - idMap := []idtools.IDMap{} + idMap := []user.IDMap{} - containerID := 0 + containerID := int64(0) for _, idrange := range rangeList { - idMap = append(idMap, idtools.IDMap{ - ContainerID: containerID, - HostID: int(idrange.SubID), - Size: int(idrange.Count), + idMap = append(idMap, user.IDMap{ + ID: containerID, + ParentID: idrange.SubID, + Count: idrange.Count, }) - containerID = containerID + int(idrange.Count) + containerID = containerID + idrange.Count } return idMap, nil } diff --git a/pkg/idtools/idtools.go b/pkg/idtools/idtools.go index 37b184cc62..dcb09c2377 100644 --- a/pkg/idtools/idtools.go +++ b/pkg/idtools/idtools.go @@ -119,6 +119,31 @@ type IdentityMapping struct { GIDMaps []IDMap `json:"GIDMaps"` } +// FromUserIdentityMapping converts a [user.IdentityMapping] to an [idtools.IdentityMapping]. +// +// Deprecated: use [user.IdentityMapping] directly, this is transitioning to user package. +func FromUserIdentityMapping(u user.IdentityMapping) IdentityMapping { + return IdentityMapping{ + UIDMaps: fromUserIDMap(u.UIDMaps), + GIDMaps: fromUserIDMap(u.GIDMaps), + } +} + +func fromUserIDMap(u []user.IDMap) []IDMap { + if u == nil { + return nil + } + m := make([]IDMap, len(u)) + for i := range u { + m[i] = IDMap{ + ContainerID: int(u[i].ID), + HostID: int(u[i].ParentID), + Size: int(u[i].Count), + } + } + return m +} + // 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. From f1ec5bf14f42a2da15ac19fcf651065faf251131 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 4 Apr 2025 10:19:43 +0200 Subject: [PATCH 3/3] pkg/idtools: remove tests already covered in moby/sys/user Removes all tests, except for TestGetRootUIDGID and TestToContainer, which are the only once that have a local implementation that's not covered. Signed-off-by: Sebastiaan van Stijn --- pkg/idtools/idtools_unix_test.go | 312 ------------------------------- 1 file changed, 312 deletions(-) diff --git a/pkg/idtools/idtools_unix_test.go b/pkg/idtools/idtools_unix_test.go index 916c2811e9..6270d66924 100644 --- a/pkg/idtools/idtools_unix_test.go +++ b/pkg/idtools/idtools_unix_test.go @@ -3,321 +3,13 @@ package idtools import ( - "fmt" "os" - "path/filepath" "testing" - "golang.org/x/sys/unix" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" - "gotest.tools/v3/skip" ) -type node struct { - uid int - gid int -} - -func TestMkdirAllAndChown(t *testing.T) { - RequiresRoot(t) - dirName, err := os.MkdirTemp("", "mkdirall") - if err != nil { - t.Fatalf("Couldn't create temp dir: %v", err) - } - defer os.RemoveAll(dirName) - - testTree := map[string]node{ - "usr": {0, 0}, - "usr/bin": {0, 0}, - "lib": {33, 33}, - "lib/x86_64": {45, 45}, - "lib/x86_64/share": {1, 1}, - } - - if err := buildTree(dirName, testTree); err != nil { - t.Fatal(err) - } - - // 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"), 0o755, Identity{UID: 99, GID: 99}); err != nil { - t.Fatal(err) - } - testTree["usr/share"] = node{99, 99} - verifyTree, err := readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } - - // test 2-deep new directories--both should be owned by the uid/gid pair - 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} - testTree["lib/some/other"] = node{101, 101} - verifyTree, err = readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } - - // test a directory that already exists; should be chowned, but nothing else - if err := MkdirAllAndChown(filepath.Join(dirName, "usr"), 0o755, Identity{UID: 102, GID: 102}); err != nil { - t.Fatal(err) - } - testTree["usr"] = node{102, 102} - verifyTree, err = readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } -} - -func TestMkdirAllAndChownNew(t *testing.T) { - RequiresRoot(t) - dirName, err := os.MkdirTemp("", "mkdirnew") - assert.NilError(t, err) - defer os.RemoveAll(dirName) - - testTree := map[string]node{ - "usr": {0, 0}, - "usr/bin": {0, 0}, - "lib": {33, 33}, - "lib/x86_64": {45, 45}, - "lib/x86_64/share": {1, 1}, - } - 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"), 0o755, Identity{UID: 99, GID: 99}) - assert.NilError(t, err) - - testTree["usr/share"] = node{99, 99} - verifyTree, err := readTree(dirName, "") - assert.NilError(t, err) - 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"), 0o755, Identity{UID: 101, GID: 101}) - assert.NilError(t, err) - testTree["lib/some"] = node{101, 101} - testTree["lib/some/other"] = node{101, 101} - verifyTree, err = readTree(dirName, "") - assert.NilError(t, err) - assert.NilError(t, compareTrees(testTree, verifyTree)) - - // test a directory that already exists; should NOT be chowned - err = MkdirAllAndChownNew(filepath.Join(dirName, "usr"), 0o755, Identity{UID: 102, GID: 102}) - assert.NilError(t, err) - verifyTree, err = readTree(dirName, "") - assert.NilError(t, err) - assert.NilError(t, compareTrees(testTree, verifyTree)) -} - -func TestMkdirAllAndChownNewRelative(t *testing.T) { - RequiresRoot(t) - - tests := []struct { - in string - out []string - }{ - { - in: "dir1", - out: []string{"dir1"}, - }, - { - in: "dir2/subdir2", - out: []string{"dir2", "dir2/subdir2"}, - }, - { - in: "dir3/subdir3/", - out: []string{"dir3", "dir3/subdir3"}, - }, - { - in: "dir4/subdir4/.", - out: []string{"dir4", "dir4/subdir4"}, - }, - { - in: "dir5/././subdir5/", - out: []string{"dir5", "dir5/subdir5"}, - }, - { - in: "./dir6", - out: []string{"dir6"}, - }, - { - in: "./dir7/subdir7", - out: []string{"dir7", "dir7/subdir7"}, - }, - { - in: "./dir8/subdir8/", - out: []string{"dir8", "dir8/subdir8"}, - }, - { - in: "./dir9/subdir9/.", - out: []string{"dir9", "dir9/subdir9"}, - }, - { - in: "./dir10/././subdir10/", - out: []string{"dir10", "dir10/subdir10"}, - }, - } - - // Set the current working directory to the temp-dir, as we're - // testing relative paths. - tmpDir := t.TempDir() - setWorkingDirectory(t, tmpDir) - - const expectedUIDGID = 101 - - for _, tc := range tests { - t.Run(tc.in, func(t *testing.T) { - for _, p := range tc.out { - _, err := os.Stat(p) - assert.ErrorIs(t, err, os.ErrNotExist) - } - - err := MkdirAllAndChownNew(tc.in, 0o755, Identity{UID: expectedUIDGID, GID: expectedUIDGID}) - assert.Check(t, err) - - for _, p := range tc.out { - s := &unix.Stat_t{} - err = unix.Stat(p, s) - if assert.Check(t, err) { - assert.Check(t, is.Equal(uint64(s.Uid), uint64(expectedUIDGID))) - assert.Check(t, is.Equal(uint64(s.Gid), uint64(expectedUIDGID))) - } - } - }) - } -} - -// Change the current working directory for the duration of the test. This may -// break if tests are run in parallel. -func setWorkingDirectory(t *testing.T, dir string) { - t.Helper() - cwd, err := os.Getwd() - assert.NilError(t, err) - t.Cleanup(func() { - assert.NilError(t, os.Chdir(cwd)) - }) - err = os.Chdir(dir) - assert.NilError(t, err) -} - -func TestMkdirAndChown(t *testing.T) { - RequiresRoot(t) - dirName, err := os.MkdirTemp("", "mkdir") - if err != nil { - t.Fatalf("Couldn't create temp dir: %v", err) - } - defer os.RemoveAll(dirName) - - testTree := map[string]node{ - "usr": {0, 0}, - } - if err := buildTree(dirName, testTree); err != nil { - t.Fatal(err) - } - - // test a directory that already exists; should just chown to the requested uid/gid - if err := MkdirAndChown(filepath.Join(dirName, "usr"), 0o755, Identity{UID: 99, GID: 99}); err != nil { - t.Fatal(err) - } - testTree["usr"] = node{99, 99} - verifyTree, err := readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } - - // create a subdir under a dir which doesn't exist--should fail - 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"), 0o755, Identity{UID: 102, GID: 102}); err != nil { - t.Fatal(err) - } - testTree["usr/bin"] = node{102, 102} - verifyTree, err = readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } -} - -func buildTree(base string, tree map[string]node) error { - for path, node := range tree { - fullPath := filepath.Join(base, path) - 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 nil -} - -func readTree(base, root string) (map[string]node, error) { - tree := make(map[string]node) - - dirInfos, err := os.ReadDir(base) - if err != nil { - 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) - } - tree[filepath.Join(root, info.Name())] = node{int(s.Uid), int(s.Gid)} - if info.IsDir() { - // read the subdirectory - subtree, err := readTree(filepath.Join(base, info.Name()), filepath.Join(root, info.Name())) - if err != nil { - return nil, err - } - for path, nodeinfo := range subtree { - tree[path] = nodeinfo - } - } - } - return tree, nil -} - -func compareTrees(left, right map[string]node) error { - if len(left) != len(right) { - return fmt.Errorf("trees aren't the same size") - } - for path, nodeLeft := range left { - if nodeRight, ok := right[path]; ok { - if nodeRight.uid != nodeLeft.uid || nodeRight.gid != nodeLeft.gid { - // mismatch - return fmt.Errorf("mismatched ownership for %q: expected: %d:%d, got: %d:%d", path, - nodeLeft.uid, nodeLeft.gid, nodeRight.uid, nodeRight.gid) - } - continue - } - return fmt.Errorf("right tree didn't contain path %q", path) - } - return nil -} - func TestGetRootUIDGID(t *testing.T) { uidMap := []IDMap{ { @@ -363,7 +55,3 @@ func TestToContainer(t *testing.T) { assert.Check(t, err) assert.Check(t, is.Equal(uidMap[0].ContainerID, containerID)) } - -func RequiresRoot(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") -}