From 4866cd23de2e799e0d0a1b3f5893d17a1b1ad1c5 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 28 Aug 2023 20:05:56 -0700 Subject: [PATCH] libct: do not parse passwd and group on every run/exec OCI runtime spec states [1] that the UID, primary GID, and additional GIDs are all specified as numbers, and also adds that symbolic names resolution "are left to upper levels to derive". Meaning, runc should not care about user and group names. Yet, runc tries to be clever than that, always parsing container's /etc/passwd and /etc/group. It results in a few things: 1. If UID (or GID) specified can't be found inside container's /etc/passwd (or /etc/group), runc (run or exec) errors out. 2. Any additional GIDs specified in container's /etc/group are automatically prepended to the list for setgroups(2). Meaning, a user can either specify additional GIDs in OCI runtime spec, or container's /etc/group entry for a given user. Looks like (1) is questionable (on a normal Linux system, I can run programs under any UID (GID), not limited to those listed in /etc/passwd (/etc/group), and (2) is just an extra mechanism of specifying additional GIDs. Let's remove those, hopefully increasing runc performance as well as OCI spec conformance. With that, also remove most of libcontainer/user parsers. The only remaining need to parse /etc/passwd is to set HOME environment variable for a specified UID, in case it is not. For that, we can use standard os/user package, which has both libc-based and own ("pure Go") /etc/passwd parsers. PS Note that the structures being changed (initConfig and Process) are never saved to disk as JSON by runc, so there is no compatibility issue for runc users. This is a breaking change in libcontainer, but we never promised that libcontainer API will be stable. [1] https://github.com/opencontainers/runtime-spec/blob/v1.0.2/config.md#posix-platform-user Signed-off-by: Kir Kolyshkin --- libcontainer/container_linux.go | 3 +- libcontainer/init_linux.go | 75 +--- libcontainer/integration/exec_test.go | 14 +- libcontainer/integration/execin_test.go | 14 +- libcontainer/process.go | 6 +- libcontainer/system/syscall_linux_32.go | 4 +- libcontainer/system/syscall_linux_64.go | 4 +- libcontainer/user/lookup_unix.go | 145 ------- libcontainer/user/user.go | 510 ----------------------- libcontainer/user/user_fuzzer.go | 43 -- libcontainer/user/user_test.go | 530 ------------------------ utils_linux.go | 15 +- 12 files changed, 53 insertions(+), 1310 deletions(-) delete mode 100644 libcontainer/user/user_fuzzer.go delete mode 100644 libcontainer/user/user_test.go diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index c941239b841..fcf645a891b 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -711,7 +711,8 @@ func (c *Container) newInitConfig(process *Process) *initConfig { Config: c.config, Args: process.Args, Env: process.Env, - User: process.User, + UID: process.UID, + GID: process.GID, AdditionalGroups: process.AdditionalGroups, Cwd: process.Cwd, Capabilities: process.Capabilities, diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 30152696f72..d474d43d7b2 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -7,6 +7,7 @@ import ( "fmt" "net" "os" + "os/user" "runtime" "runtime/debug" "strconv" @@ -22,7 +23,6 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/system" - "github.com/opencontainers/runc/libcontainer/user" "github.com/opencontainers/runc/libcontainer/utils" ) @@ -68,8 +68,9 @@ type initConfig struct { ProcessLabel string `json:"process_label"` AppArmorProfile string `json:"apparmor_profile"` NoNewPrivileges bool `json:"no_new_privileges"` - User string `json:"user"` - AdditionalGroups []string `json:"additional_groups"` + UID uint32 `json:"uid"` + GID uint32 `json:"gid"` + AdditionalGroups []int `json:"additional_groups"` Config *configs.Config `json:"config"` Networks []*network `json:"network"` PassedFilesCount int `json:"passed_files_count"` @@ -428,58 +429,26 @@ func syncParentSeccomp(pipe *os.File, seccompFd *os.File) error { // setupUser changes the groups, gid, and uid for the user inside the container func setupUser(config *initConfig) error { - // Set up defaults. - defaultExecUser := user.ExecUser{ - Uid: 0, - Gid: 0, - Home: "/", - } - - passwdPath, err := user.GetPasswdPath() - if err != nil { - return err - } - - groupPath, err := user.GetGroupPath() - if err != nil { - return err - } - - execUser, err := user.GetExecUserPath(config.User, &defaultExecUser, passwdPath, groupPath) - if err != nil { - return err - } - - var addGroups []int - if len(config.AdditionalGroups) > 0 { - addGroups, err = user.GetAdditionalGroupsPath(config.AdditionalGroups, groupPath) - if err != nil { - return err - } - } - // Rather than just erroring out later in setuid(2) and setgid(2), check // that the user is mapped here. - if _, err := config.Config.HostUID(execUser.Uid); err != nil { + if _, err := config.Config.HostUID(int(config.UID)); err != nil { return errors.New("cannot set uid to unmapped user in user namespace") } - if _, err := config.Config.HostGID(execUser.Gid); err != nil { + if _, err := config.Config.HostGID(int(config.GID)); err != nil { return errors.New("cannot set gid to unmapped user in user namespace") } - if config.RootlessEUID { + if config.RootlessEUID && len(config.AdditionalGroups) > 0 { // We cannot set any additional groups in a rootless container and thus // we bail if the user asked us to do so. TODO: We currently can't do // this check earlier, but if libcontainer.Process.User was typesafe // this might work. - if len(addGroups) > 0 { - return errors.New("cannot set any additional groups in a rootless container") - } + return errors.New("cannot set any additional groups in a rootless container") } // Before we change to the container's user make sure that the processes // STDIO is correctly owned by the user that we are switching to. - if err := fixStdioPermissions(execUser); err != nil { + if err := fixStdioPermissions(config.UID); err != nil { return err } @@ -495,32 +464,34 @@ func setupUser(config *initConfig) error { allowSupGroups := !config.RootlessEUID && string(bytes.TrimSpace(setgroups)) != "deny" if allowSupGroups { - suppGroups := append(execUser.Sgids, addGroups...) - if err := unix.Setgroups(suppGroups); err != nil { + if err := unix.Setgroups(config.AdditionalGroups); err != nil { return &os.SyscallError{Syscall: "setgroups", Err: err} } } - if err := system.Setgid(execUser.Gid); err != nil { + if err := system.Setgid(config.GID); err != nil { return err } - if err := system.Setuid(execUser.Uid); err != nil { + if err := system.Setuid(config.UID); err != nil { return err } - // if we didn't get HOME already, set it based on the user's HOME - if envHome := os.Getenv("HOME"); envHome == "" { - if err := os.Setenv("HOME", execUser.Home); err != nil { - return err + // If we didn't get HOME already, try to set it based on the user's HOME. + if _, ok := os.LookupEnv("HOME"); !ok { + u, err := user.LookupId(strconv.Itoa(int(config.UID))) + if err == nil { + if err := os.Setenv("HOME", u.HomeDir); err != nil { + return err + } } } return nil } -// fixStdioPermissions fixes the permissions of PID 1's STDIO within the container to the specified user. +// fixStdioPermissions fixes the permissions of PID 1's STDIO within the container to the specified uid. // The ownership needs to match because it is created outside of the container and needs to be // localized. -func fixStdioPermissions(u *user.ExecUser) error { +func fixStdioPermissions(uid uint32) error { var null unix.Stat_t if err := unix.Stat("/dev/null", &null); err != nil { return &os.PathError{Op: "stat", Path: "/dev/null", Err: err} @@ -533,7 +504,7 @@ func fixStdioPermissions(u *user.ExecUser) error { // Skip chown if uid is already the one we want or any of the STDIO descriptors // were redirected to /dev/null. - if int(s.Uid) == u.Uid || s.Rdev == null.Rdev { + if s.Uid == uid || s.Rdev == null.Rdev { continue } @@ -543,7 +514,7 @@ func fixStdioPermissions(u *user.ExecUser) error { // that users expect to be able to actually use their console. Without // this code, you couldn't effectively run as a non-root user inside a // container and also have a console set up. - if err := file.Chown(u.Uid, int(s.Gid)); err != nil { + if err := file.Chown(int(uid), int(s.Gid)); err != nil { // If we've hit an EINVAL then s.Gid isn't mapped in the user // namespace. If we've hit an EPERM then the inode's current owner // is not mapped in our user namespace (in particular, diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 9795964caf2..4453f7fb7c5 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -395,7 +395,7 @@ func TestAdditionalGroups(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, - AdditionalGroups: []string{"plugdev", "audio"}, + AdditionalGroups: []int{3333, 99999}, Init: true, } err = container.Run(&pconfig) @@ -406,13 +406,11 @@ func TestAdditionalGroups(t *testing.T) { outputGroups := stdout.String() - // Check that the groups output has the groups that we specified - if !strings.Contains(outputGroups, "audio") { - t.Fatalf("Listed groups do not contain the audio group as expected: %v", outputGroups) - } - - if !strings.Contains(outputGroups, "plugdev") { - t.Fatalf("Listed groups do not contain the plugdev group as expected: %v", outputGroups) + // Check that the groups output has the groups that we specified. + for _, gid := range pconfig.AdditionalGroups { + if !strings.Contains(outputGroups, strconv.Itoa(gid)) { + t.Errorf("Listed groups do not contain gid %d as expected: %v", gid, outputGroups) + } } } diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index c5c324130c6..ef455bde6e0 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -162,7 +162,7 @@ func TestExecInAdditionalGroups(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, - AdditionalGroups: []string{"plugdev", "audio"}, + AdditionalGroups: []int{4444, 87654}, } err = container.Run(&pconfig) ok(t, err) @@ -175,13 +175,11 @@ func TestExecInAdditionalGroups(t *testing.T) { outputGroups := stdout.String() - // Check that the groups output has the groups that we specified - if !strings.Contains(outputGroups, "audio") { - t.Fatalf("Listed groups do not contain the audio group as expected: %v", outputGroups) - } - - if !strings.Contains(outputGroups, "plugdev") { - t.Fatalf("Listed groups do not contain the plugdev group as expected: %v", outputGroups) + // Check that the groups output has the groups that we specified. + for _, gid := range pconfig.AdditionalGroups { + if !strings.Contains(outputGroups, strconv.Itoa(gid)) { + t.Errorf("Listed groups do not contain gid %d as expected: %v", gid, outputGroups) + } } } diff --git a/libcontainer/process.go b/libcontainer/process.go index 4de4a9e75c2..d57a2cad299 100644 --- a/libcontainer/process.go +++ b/libcontainer/process.go @@ -26,13 +26,13 @@ type Process struct { // Env specifies the environment variables for the process. Env []string - // User will set the uid and gid of the executing process running inside the container + // UID and GID of the executing process running inside the container // local to the container's user and group configuration. - User string + UID, GID uint32 // AdditionalGroups specifies the gids that should be added to supplementary groups // in addition to those that the user belongs to. - AdditionalGroups []string + AdditionalGroups []int // Cwd will change the processes current working directory inside the container's rootfs. Cwd string diff --git a/libcontainer/system/syscall_linux_32.go b/libcontainer/system/syscall_linux_32.go index 1acc5cb03d0..f61d7423a53 100644 --- a/libcontainer/system/syscall_linux_32.go +++ b/libcontainer/system/syscall_linux_32.go @@ -9,7 +9,7 @@ import ( ) // Setuid sets the uid of the calling thread to the specified uid. -func Setuid(uid int) (err error) { +func Setuid(uid uint32) (err error) { _, _, e1 := unix.RawSyscall(unix.SYS_SETUID32, uintptr(uid), 0, 0) if e1 != 0 { err = e1 @@ -18,7 +18,7 @@ func Setuid(uid int) (err error) { } // Setgid sets the gid of the calling thread to the specified gid. -func Setgid(gid int) (err error) { +func Setgid(gid uint32) (err error) { _, _, e1 := unix.RawSyscall(unix.SYS_SETGID32, uintptr(gid), 0, 0) if e1 != 0 { err = e1 diff --git a/libcontainer/system/syscall_linux_64.go b/libcontainer/system/syscall_linux_64.go index 1ed0dba1709..63936459b24 100644 --- a/libcontainer/system/syscall_linux_64.go +++ b/libcontainer/system/syscall_linux_64.go @@ -9,7 +9,7 @@ import ( ) // Setuid sets the uid of the calling thread to the specified uid. -func Setuid(uid int) (err error) { +func Setuid(uid uint32) (err error) { _, _, e1 := unix.RawSyscall(unix.SYS_SETUID, uintptr(uid), 0, 0) if e1 != 0 { err = e1 @@ -18,7 +18,7 @@ func Setuid(uid int) (err error) { } // Setgid sets the gid of the calling thread to the specified gid. -func Setgid(gid int) (err error) { +func Setgid(gid uint32) (err error) { _, _, e1 := unix.RawSyscall(unix.SYS_SETGID, uintptr(gid), 0, 0) if e1 != 0 { err = e1 diff --git a/libcontainer/user/lookup_unix.go b/libcontainer/user/lookup_unix.go index f95c1409fce..ad3f00f5c0b 100644 --- a/libcontainer/user/lookup_unix.go +++ b/libcontainer/user/lookup_unix.go @@ -3,151 +3,6 @@ package user -import ( - "io" - "os" - "strconv" - - "golang.org/x/sys/unix" -) - -// Unix-specific path to the passwd and group formatted files. -const ( - unixPasswdPath = "/etc/passwd" - unixGroupPath = "/etc/group" -) - -// LookupUser looks up a user by their username in /etc/passwd. If the user -// cannot be found (or there is no /etc/passwd file on the filesystem), then -// LookupUser returns an error. -func LookupUser(username string) (User, error) { - return lookupUserFunc(func(u User) bool { - return u.Name == username - }) -} - -// LookupUid looks up a user by their user id in /etc/passwd. If the user cannot -// be found (or there is no /etc/passwd file on the filesystem), then LookupId -// returns an error. -func LookupUid(uid int) (User, error) { - return lookupUserFunc(func(u User) bool { - return u.Uid == uid - }) -} - -func lookupUserFunc(filter func(u User) bool) (User, error) { - // Get operating system-specific passwd reader-closer. - passwd, err := GetPasswd() - if err != nil { - return User{}, err - } - defer passwd.Close() - - // Get the users. - users, err := ParsePasswdFilter(passwd, filter) - if err != nil { - return User{}, err - } - - // No user entries found. - if len(users) == 0 { - return User{}, ErrNoPasswdEntries - } - - // Assume the first entry is the "correct" one. - return users[0], nil -} - -// LookupGroup looks up a group by its name in /etc/group. If the group cannot -// be found (or there is no /etc/group file on the filesystem), then LookupGroup -// returns an error. -func LookupGroup(groupname string) (Group, error) { - return lookupGroupFunc(func(g Group) bool { - return g.Name == groupname - }) -} - -// LookupGid looks up a group by its group id in /etc/group. If the group cannot -// be found (or there is no /etc/group file on the filesystem), then LookupGid -// returns an error. -func LookupGid(gid int) (Group, error) { - return lookupGroupFunc(func(g Group) bool { - return g.Gid == gid - }) -} - -func lookupGroupFunc(filter func(g Group) bool) (Group, error) { - // Get operating system-specific group reader-closer. - group, err := GetGroup() - if err != nil { - return Group{}, err - } - defer group.Close() - - // Get the users. - groups, err := ParseGroupFilter(group, filter) - if err != nil { - return Group{}, err - } - - // No user entries found. - if len(groups) == 0 { - return Group{}, ErrNoGroupEntries - } - - // Assume the first entry is the "correct" one. - return groups[0], nil -} - -func GetPasswdPath() (string, error) { - return unixPasswdPath, nil -} - -func GetPasswd() (io.ReadCloser, error) { - return os.Open(unixPasswdPath) -} - -func GetGroupPath() (string, error) { - return unixGroupPath, nil -} - -func GetGroup() (io.ReadCloser, error) { - return os.Open(unixGroupPath) -} - -// CurrentUser looks up the current user by their user id in /etc/passwd. If the -// user cannot be found (or there is no /etc/passwd file on the filesystem), -// then CurrentUser returns an error. -func CurrentUser() (User, error) { - return LookupUid(unix.Getuid()) -} - -// CurrentGroup looks up the current user's group by their primary group id's -// entry in /etc/passwd. If the group cannot be found (or there is no -// /etc/group file on the filesystem), then CurrentGroup returns an error. -func CurrentGroup() (Group, error) { - return LookupGid(unix.Getgid()) -} - -func currentUserSubIDs(fileName string) ([]SubID, error) { - u, err := CurrentUser() - if err != nil { - return nil, err - } - filter := func(entry SubID) bool { - return entry.Name == u.Name || entry.Name == strconv.Itoa(u.Uid) - } - return ParseSubIDFileFilter(fileName, filter) -} - -func CurrentUserSubUIDs() ([]SubID, error) { - return currentUserSubIDs("/etc/subuid") -} - -func CurrentUserSubGIDs() ([]SubID, error) { - return currentUserSubIDs("/etc/subgid") -} - func CurrentProcessUIDMap() ([]IDMap, error) { return ParseIDMapFile("/proc/self/uid_map") } diff --git a/libcontainer/user/user.go b/libcontainer/user/user.go index 984466d1ab5..29fcde2af60 100644 --- a/libcontainer/user/user.go +++ b/libcontainer/user/user.go @@ -8,47 +8,8 @@ import ( "io" "os" "strconv" - "strings" ) -const ( - minID = 0 - maxID = 1<<31 - 1 // for 32-bit systems compatibility -) - -var ( - // ErrNoPasswdEntries is returned if no matching entries were found in /etc/group. - ErrNoPasswdEntries = errors.New("no matching entries in passwd file") - // ErrNoGroupEntries is returned if no matching entries were found in /etc/passwd. - ErrNoGroupEntries = errors.New("no matching entries in group file") - // ErrRange is returned if a UID or GID is outside of the valid range. - ErrRange = fmt.Errorf("uids and gids must be in range %d-%d", minID, maxID) -) - -type User struct { - Name string - Pass string - Uid int - Gid int - Gecos string - Home string - Shell string -} - -type Group struct { - Name string - Pass string - Gid int - List []string -} - -// SubID represents an entry in /etc/sub{u,g}id -type SubID struct { - Name string - SubID int64 - Count int64 -} - // IDMap represents an entry in /proc/PID/{u,g}id_map type IDMap struct { ID int64 @@ -56,10 +17,6 @@ type IDMap struct { Count int64 } -func parseLine(line []byte, v ...interface{}) { - parseParts(bytes.Split(line, []byte(":")), v...) -} - func parseParts(parts [][]byte, v ...interface{}) { if len(parts) == 0 { return @@ -75,20 +32,8 @@ func parseParts(parts [][]byte, v ...interface{}) { // Use the type of the argument to figure out how to parse it, scanf() style. // This is legit. switch e := v[i].(type) { - case *string: - *e = string(p) - case *int: - // "numbers", with conversion errors ignored because of some misbehaving configuration files. - *e, _ = strconv.Atoi(string(p)) case *int64: *e, _ = strconv.ParseInt(string(p), 10, 64) - case *[]string: - // Comma-separated lists. - if len(p) != 0 { - *e = strings.Split(string(p), ",") - } else { - *e = []string{} - } default: // Someone goof'd when writing code using this function. Scream so they can hear us. panic(fmt.Sprintf("parseLine only accepts {*string, *int, *int64, *[]string} as arguments! %#v is not a pointer!", e)) @@ -96,461 +41,6 @@ func parseParts(parts [][]byte, v ...interface{}) { } } -func ParsePasswdFile(path string) ([]User, error) { - passwd, err := os.Open(path) - if err != nil { - return nil, err - } - defer passwd.Close() - return ParsePasswd(passwd) -} - -func ParsePasswd(passwd io.Reader) ([]User, error) { - return ParsePasswdFilter(passwd, nil) -} - -func ParsePasswdFileFilter(path string, filter func(User) bool) ([]User, error) { - passwd, err := os.Open(path) - if err != nil { - return nil, err - } - defer passwd.Close() - return ParsePasswdFilter(passwd, filter) -} - -func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { - if r == nil { - return nil, errors.New("nil source for passwd-formatted data") - } - - var ( - s = bufio.NewScanner(r) - out = []User{} - ) - - for s.Scan() { - line := bytes.TrimSpace(s.Bytes()) - if len(line) == 0 { - continue - } - - // see: man 5 passwd - // name:password:UID:GID:GECOS:directory:shell - // Name:Pass:Uid:Gid:Gecos:Home:Shell - // root:x:0:0:root:/root:/bin/bash - // adm:x:3:4:adm:/var/adm:/bin/false - p := User{} - parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell) - - if filter == nil || filter(p) { - out = append(out, p) - } - } - if err := s.Err(); err != nil { - return nil, err - } - - return out, nil -} - -func ParseGroupFile(path string) ([]Group, error) { - group, err := os.Open(path) - if err != nil { - return nil, err - } - - defer group.Close() - return ParseGroup(group) -} - -func ParseGroup(group io.Reader) ([]Group, error) { - return ParseGroupFilter(group, nil) -} - -func ParseGroupFileFilter(path string, filter func(Group) bool) ([]Group, error) { - group, err := os.Open(path) - if err != nil { - return nil, err - } - defer group.Close() - return ParseGroupFilter(group, filter) -} - -func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { - if r == nil { - return nil, errors.New("nil source for group-formatted data") - } - rd := bufio.NewReader(r) - out := []Group{} - - // Read the file line-by-line. - for { - var ( - isPrefix bool - wholeLine []byte - err error - ) - - // Read the next line. We do so in chunks (as much as reader's - // buffer is able to keep), check if we read enough columns - // already on each step and store final result in wholeLine. - for { - var line []byte - line, isPrefix, err = rd.ReadLine() - - if err != nil { - // We should return no error if EOF is reached - // without a match. - if err == io.EOF { - err = nil - } - return out, err - } - - // Simple common case: line is short enough to fit in a - // single reader's buffer. - if !isPrefix && len(wholeLine) == 0 { - wholeLine = line - break - } - - wholeLine = append(wholeLine, line...) - - // Check if we read the whole line already. - if !isPrefix { - break - } - } - - // There's no spec for /etc/passwd or /etc/group, but we try to follow - // the same rules as the glibc parser, which allows comments and blank - // space at the beginning of a line. - wholeLine = bytes.TrimSpace(wholeLine) - if len(wholeLine) == 0 || wholeLine[0] == '#' { - continue - } - - // see: man 5 group - // group_name:password:GID:user_list - // Name:Pass:Gid:List - // root:x:0:root - // adm:x:4:root,adm,daemon - p := Group{} - parseLine(wholeLine, &p.Name, &p.Pass, &p.Gid, &p.List) - - if filter == nil || filter(p) { - out = append(out, p) - } - } -} - -type ExecUser struct { - Uid int - Gid int - Sgids []int - Home string -} - -// GetExecUserPath is a wrapper for GetExecUser. It reads data from each of the -// given file paths and uses that data as the arguments to GetExecUser. If the -// files cannot be opened for any reason, the error is ignored and a nil -// io.Reader is passed instead. -func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath string) (*ExecUser, error) { - var passwd, group io.Reader - - if passwdFile, err := os.Open(passwdPath); err == nil { - passwd = passwdFile - defer passwdFile.Close() - } - - if groupFile, err := os.Open(groupPath); err == nil { - group = groupFile - defer groupFile.Close() - } - - return GetExecUser(userSpec, defaults, passwd, group) -} - -// GetExecUser parses a user specification string (using the passwd and group -// readers as sources for /etc/passwd and /etc/group data, respectively). In -// the case of blank fields or missing data from the sources, the values in -// defaults is used. -// -// GetExecUser will return an error if a user or group literal could not be -// found in any entry in passwd and group respectively. -// -// Examples of valid user specifications are: -// - "" -// - "user" -// - "uid" -// - "user:group" -// - "uid:gid -// - "user:gid" -// - "uid:group" -// -// It should be noted that if you specify a numeric user or group id, they will -// not be evaluated as usernames (only the metadata will be filled). So attempting -// to parse a user with user.Name = "1337" will produce the user with a UID of -// 1337. -func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (*ExecUser, error) { - if defaults == nil { - defaults = new(ExecUser) - } - - // Copy over defaults. - user := &ExecUser{ - Uid: defaults.Uid, - Gid: defaults.Gid, - Sgids: defaults.Sgids, - Home: defaults.Home, - } - - // Sgids slice *cannot* be nil. - if user.Sgids == nil { - user.Sgids = []int{} - } - - // Allow for userArg to have either "user" syntax, or optionally "user:group" syntax - var userArg, groupArg string - parseLine([]byte(userSpec), &userArg, &groupArg) - - // Convert userArg and groupArg to be numeric, so we don't have to execute - // Atoi *twice* for each iteration over lines. - uidArg, uidErr := strconv.Atoi(userArg) - gidArg, gidErr := strconv.Atoi(groupArg) - - // Find the matching user. - users, err := ParsePasswdFilter(passwd, func(u User) bool { - if userArg == "" { - // Default to current state of the user. - return u.Uid == user.Uid - } - - if uidErr == nil { - // If the userArg is numeric, always treat it as a UID. - return uidArg == u.Uid - } - - return u.Name == userArg - }) - - // If we can't find the user, we have to bail. - if err != nil && passwd != nil { - if userArg == "" { - userArg = strconv.Itoa(user.Uid) - } - return nil, fmt.Errorf("unable to find user %s: %w", userArg, err) - } - - var matchedUserName string - if len(users) > 0 { - // First match wins, even if there's more than one matching entry. - matchedUserName = users[0].Name - user.Uid = users[0].Uid - user.Gid = users[0].Gid - user.Home = users[0].Home - } else if userArg != "" { - // If we can't find a user with the given username, the only other valid - // option is if it's a numeric username with no associated entry in passwd. - - if uidErr != nil { - // Not numeric. - return nil, fmt.Errorf("unable to find user %s: %w", userArg, ErrNoPasswdEntries) - } - user.Uid = uidArg - - // Must be inside valid uid range. - if user.Uid < minID || user.Uid > maxID { - return nil, ErrRange - } - - // Okay, so it's numeric. We can just roll with this. - } - - // On to the groups. If we matched a username, we need to do this because of - // the supplementary group IDs. - if groupArg != "" || matchedUserName != "" { - groups, err := ParseGroupFilter(group, func(g Group) bool { - // If the group argument isn't explicit, we'll just search for it. - if groupArg == "" { - // Check if user is a member of this group. - for _, u := range g.List { - if u == matchedUserName { - return true - } - } - return false - } - - if gidErr == nil { - // If the groupArg is numeric, always treat it as a GID. - return gidArg == g.Gid - } - - return g.Name == groupArg - }) - if err != nil && group != nil { - return nil, fmt.Errorf("unable to find groups for spec %v: %w", matchedUserName, err) - } - - // Only start modifying user.Gid if it is in explicit form. - if groupArg != "" { - if len(groups) > 0 { - // First match wins, even if there's more than one matching entry. - user.Gid = groups[0].Gid - } else { - // If we can't find a group with the given name, the only other valid - // option is if it's a numeric group name with no associated entry in group. - - if gidErr != nil { - // Not numeric. - return nil, fmt.Errorf("unable to find group %s: %w", groupArg, ErrNoGroupEntries) - } - user.Gid = gidArg - - // Must be inside valid gid range. - if user.Gid < minID || user.Gid > maxID { - return nil, ErrRange - } - - // Okay, so it's numeric. We can just roll with this. - } - } else if len(groups) > 0 { - // Supplementary group ids only make sense if in the implicit form. - user.Sgids = make([]int, len(groups)) - for i, group := range groups { - user.Sgids[i] = group.Gid - } - } - } - - return user, nil -} - -// GetAdditionalGroups looks up a list of groups by name or group id -// against the given /etc/group formatted data. If a group name cannot -// be found, an error will be returned. If a group id cannot be found, -// or the given group data is nil, the id will be returned as-is -// provided it is in the legal range. -func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, error) { - groups := []Group{} - if group != nil { - var err error - groups, err = ParseGroupFilter(group, func(g Group) bool { - for _, ag := range additionalGroups { - if g.Name == ag || strconv.Itoa(g.Gid) == ag { - return true - } - } - return false - }) - if err != nil { - return nil, fmt.Errorf("Unable to find additional groups %v: %w", additionalGroups, err) - } - } - - gidMap := make(map[int]struct{}) - for _, ag := range additionalGroups { - var found bool - for _, g := range groups { - // if we found a matched group either by name or gid, take the - // first matched as correct - if g.Name == ag || strconv.Itoa(g.Gid) == ag { - if _, ok := gidMap[g.Gid]; !ok { - gidMap[g.Gid] = struct{}{} - found = true - break - } - } - } - // we asked for a group but didn't find it. let's check to see - // if we wanted a numeric group - if !found { - gid, err := strconv.ParseInt(ag, 10, 64) - if err != nil { - // Not a numeric ID either. - return nil, fmt.Errorf("Unable to find group %s: %w", ag, ErrNoGroupEntries) - } - // Ensure gid is inside gid range. - if gid < minID || gid > maxID { - return nil, ErrRange - } - gidMap[int(gid)] = struct{}{} - } - } - gids := []int{} - for gid := range gidMap { - gids = append(gids, gid) - } - return gids, nil -} - -// GetAdditionalGroupsPath is a wrapper around GetAdditionalGroups -// that opens the groupPath given and gives it as an argument to -// GetAdditionalGroups. -func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int, error) { - var group io.Reader - - if groupFile, err := os.Open(groupPath); err == nil { - group = groupFile - defer groupFile.Close() - } - return GetAdditionalGroups(additionalGroups, group) -} - -func ParseSubIDFile(path string) ([]SubID, error) { - subid, err := os.Open(path) - if err != nil { - return nil, err - } - defer subid.Close() - return ParseSubID(subid) -} - -func ParseSubID(subid io.Reader) ([]SubID, error) { - return ParseSubIDFilter(subid, nil) -} - -func ParseSubIDFileFilter(path string, filter func(SubID) bool) ([]SubID, error) { - subid, err := os.Open(path) - if err != nil { - return nil, err - } - defer subid.Close() - return ParseSubIDFilter(subid, filter) -} - -func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) { - if r == nil { - return nil, errors.New("nil source for subid-formatted data") - } - - var ( - s = bufio.NewScanner(r) - out = []SubID{} - ) - - for s.Scan() { - line := bytes.TrimSpace(s.Bytes()) - if len(line) == 0 { - continue - } - - // see: man 5 subuid - p := SubID{} - parseLine(line, &p.Name, &p.SubID, &p.Count) - - if filter == nil || filter(p) { - out = append(out, p) - } - } - if err := s.Err(); err != nil { - return nil, err - } - - return out, nil -} - func ParseIDMapFile(path string) ([]IDMap, error) { r, err := os.Open(path) if err != nil { diff --git a/libcontainer/user/user_fuzzer.go b/libcontainer/user/user_fuzzer.go deleted file mode 100644 index e018eae614e..00000000000 --- a/libcontainer/user/user_fuzzer.go +++ /dev/null @@ -1,43 +0,0 @@ -//go:build gofuzz -// +build gofuzz - -package user - -import ( - "io" - "strings" -) - -func IsDivisbleBy(n int, divisibleby int) bool { - return (n % divisibleby) == 0 -} - -func FuzzUser(data []byte) int { - if len(data) == 0 { - return -1 - } - if !IsDivisbleBy(len(data), 5) { - return -1 - } - - var divided [][]byte - - chunkSize := len(data) / 5 - - for i := 0; i < len(data); i += chunkSize { - end := i + chunkSize - - divided = append(divided, data[i:end]) - } - - _, _ = ParsePasswdFilter(strings.NewReader(string(divided[0])), nil) - - var passwd, group io.Reader - - group = strings.NewReader(string(divided[1])) - _, _ = GetAdditionalGroups([]string{string(divided[2])}, group) - - passwd = strings.NewReader(string(divided[3])) - _, _ = GetExecUser(string(divided[4]), nil, passwd, group) - return 1 -} diff --git a/libcontainer/user/user_test.go b/libcontainer/user/user_test.go deleted file mode 100644 index c0c762d6a89..00000000000 --- a/libcontainer/user/user_test.go +++ /dev/null @@ -1,530 +0,0 @@ -package user - -import ( - "fmt" - "io" - "reflect" - "sort" - "strconv" - "strings" - "testing" -) - -func TestUserParseLine(t *testing.T) { - var ( - a, b string - c []string - d int - ) - - parseLine([]byte(""), &a, &b) - if a != "" || b != "" { - t.Fatalf("a and b should be empty ('%v', '%v')", a, b) - } - - parseLine([]byte("a"), &a, &b) - if a != "a" || b != "" { - t.Fatalf("a should be 'a' and b should be empty ('%v', '%v')", a, b) - } - - parseLine([]byte("bad boys:corny cows"), &a, &b) - if a != "bad boys" || b != "corny cows" { - t.Fatalf("a should be 'bad boys' and b should be 'corny cows' ('%v', '%v')", a, b) - } - - parseLine([]byte(""), &c) - if len(c) != 0 { - t.Fatalf("c should be empty (%#v)", c) - } - - parseLine([]byte("d,e,f:g:h:i,j,k"), &c, &a, &b, &c) - if a != "g" || b != "h" || len(c) != 3 || c[0] != "i" || c[1] != "j" || c[2] != "k" { - t.Fatalf("a should be 'g', b should be 'h', and c should be ['i','j','k'] ('%v', '%v', '%#v')", a, b, c) - } - - parseLine([]byte("::::::::::"), &a, &b, &c) - if a != "" || b != "" || len(c) != 0 { - t.Fatalf("a, b, and c should all be empty ('%v', '%v', '%#v')", a, b, c) - } - - parseLine([]byte("not a number"), &d) - if d != 0 { - t.Fatalf("d should be 0 (%v)", d) - } - - parseLine([]byte("b:12:c"), &a, &d, &b) - if a != "b" || b != "c" || d != 12 { - t.Fatalf("a should be 'b' and b should be 'c', and d should be 12 ('%v', '%v', %v)", a, b, d) - } -} - -func TestUserParsePasswd(t *testing.T) { - users, err := ParsePasswdFilter(strings.NewReader(` -root:x:0:0:root:/root:/bin/bash -adm:x:3:4:adm:/var/adm:/bin/false -this is just some garbage data -`), nil) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - if len(users) != 3 { - t.Fatalf("Expected 3 users, got %v", len(users)) - } - if users[0].Uid != 0 || users[0].Name != "root" { - t.Fatalf("Expected users[0] to be 0 - root, got %v - %v", users[0].Uid, users[0].Name) - } - if users[1].Uid != 3 || users[1].Name != "adm" { - t.Fatalf("Expected users[1] to be 3 - adm, got %v - %v", users[1].Uid, users[1].Name) - } -} - -func TestUserParseGroup(t *testing.T) { - groups, err := ParseGroupFilter(strings.NewReader(` -root:x:0:root -adm:x:4:root,adm,daemon -this is just some garbage data -`+largeGroup()), nil) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - if len(groups) != 4 { - t.Fatalf("Expected 4 groups, got %v", len(groups)) - } - if groups[0].Gid != 0 || groups[0].Name != "root" || len(groups[0].List) != 1 { - t.Fatalf("Expected groups[0] to be 0 - root - 1 member, got %v - %v - %v", groups[0].Gid, groups[0].Name, len(groups[0].List)) - } - if groups[1].Gid != 4 || groups[1].Name != "adm" || len(groups[1].List) != 3 { - t.Fatalf("Expected groups[1] to be 4 - adm - 3 members, got %v - %v - %v", groups[1].Gid, groups[1].Name, len(groups[1].List)) - } -} - -func TestValidGetExecUser(t *testing.T) { - const passwdContent = ` -root:x:0:0:root user:/root:/bin/bash -adm:x:42:43:adm:/var/adm:/bin/false -111:x:222:333::/var/garbage -odd:x:111:112::/home/odd::::: -user7456:x:7456:100:Vasya:/home/user7456 -this is just some garbage data -` - groupContent := ` -root:x:0:root -adm:x:43: -grp:x:1234:root,adm,user7456 -444:x:555:111 -odd:x:444: -this is just some garbage data -` + largeGroup() - - defaultExecUser := ExecUser{ - Uid: 8888, - Gid: 8888, - Sgids: []int{8888}, - Home: "/8888", - } - - tests := []struct { - ref string - expected ExecUser - }{ - { - ref: "root", - expected: ExecUser{ - Uid: 0, - Gid: 0, - Sgids: []int{0, 1234}, - Home: "/root", - }, - }, - { - ref: "adm", - expected: ExecUser{ - Uid: 42, - Gid: 43, - Sgids: []int{1234}, - Home: "/var/adm", - }, - }, - { - ref: "root:adm", - expected: ExecUser{ - Uid: 0, - Gid: 43, - Sgids: defaultExecUser.Sgids, - Home: "/root", - }, - }, - { - ref: "adm:1234", - expected: ExecUser{ - Uid: 42, - Gid: 1234, - Sgids: defaultExecUser.Sgids, - Home: "/var/adm", - }, - }, - { - ref: "42:1234", - expected: ExecUser{ - Uid: 42, - Gid: 1234, - Sgids: defaultExecUser.Sgids, - Home: "/var/adm", - }, - }, - { - ref: "1337:1234", - expected: ExecUser{ - Uid: 1337, - Gid: 1234, - Sgids: defaultExecUser.Sgids, - Home: defaultExecUser.Home, - }, - }, - { - ref: "1337", - expected: ExecUser{ - Uid: 1337, - Gid: defaultExecUser.Gid, - Sgids: defaultExecUser.Sgids, - Home: defaultExecUser.Home, - }, - }, - { - ref: "", - expected: ExecUser{ - Uid: defaultExecUser.Uid, - Gid: defaultExecUser.Gid, - Sgids: defaultExecUser.Sgids, - Home: defaultExecUser.Home, - }, - }, - - // Regression tests for #695. - { - ref: "111", - expected: ExecUser{ - Uid: 111, - Gid: 112, - Sgids: defaultExecUser.Sgids, - Home: "/home/odd", - }, - }, - { - ref: "111:444", - expected: ExecUser{ - Uid: 111, - Gid: 444, - Sgids: defaultExecUser.Sgids, - Home: "/home/odd", - }, - }, - // Test for #3036. - { - ref: "7456", - expected: ExecUser{ - Uid: 7456, - Gid: 100, - Sgids: []int{1234, 1000}, // 1000 is largegroup GID - Home: "/home/user7456", - }, - }, - } - - for _, test := range tests { - passwd := strings.NewReader(passwdContent) - group := strings.NewReader(groupContent) - - execUser, err := GetExecUser(test.ref, &defaultExecUser, passwd, group) - if err != nil { - t.Logf("got unexpected error when parsing '%s': %s", test.ref, err.Error()) - t.Fail() - continue - } - - if !reflect.DeepEqual(test.expected, *execUser) { - t.Logf("ref: %v", test.ref) - t.Logf("got: %#v", execUser) - t.Logf("expected: %#v", test.expected) - t.Fail() - continue - } - } -} - -func TestInvalidGetExecUser(t *testing.T) { - const passwdContent = ` -root:x:0:0:root user:/root:/bin/bash -adm:x:42:43:adm:/var/adm:/bin/false --42:x:12:13:broken:/very/broken -this is just some garbage data -` - const groupContent = ` -root:x:0:root -adm:x:43: -grp:x:1234:root,adm -this is just some garbage data -` - - tests := []string{ - // No such user/group. - "notuser", - "notuser:notgroup", - "root:notgroup", - "notuser:adm", - "8888:notgroup", - "notuser:8888", - - // Invalid user/group values. - "-1:0", - "0:-3", - "-5:-2", - "-42", - "-43", - } - - for _, test := range tests { - passwd := strings.NewReader(passwdContent) - group := strings.NewReader(groupContent) - - execUser, err := GetExecUser(test, nil, passwd, group) - if err == nil { - t.Logf("got unexpected success when parsing '%s': %#v", test, execUser) - t.Fail() - continue - } - } -} - -func TestGetExecUserNilSources(t *testing.T) { - const passwdContent = ` -root:x:0:0:root user:/root:/bin/bash -adm:x:42:43:adm:/var/adm:/bin/false -this is just some garbage data -` - const groupContent = ` -root:x:0:root -adm:x:43: -grp:x:1234:root,adm -this is just some garbage data -` - - defaultExecUser := ExecUser{ - Uid: 8888, - Gid: 8888, - Sgids: []int{8888}, - Home: "/8888", - } - - tests := []struct { - ref string - passwd, group bool - expected ExecUser - }{ - { - ref: "", - passwd: false, - group: false, - expected: ExecUser{ - Uid: 8888, - Gid: 8888, - Sgids: []int{8888}, - Home: "/8888", - }, - }, - { - ref: "root", - passwd: true, - group: false, - expected: ExecUser{ - Uid: 0, - Gid: 0, - Sgids: []int{8888}, - Home: "/root", - }, - }, - { - ref: "0", - passwd: false, - group: false, - expected: ExecUser{ - Uid: 0, - Gid: 8888, - Sgids: []int{8888}, - Home: "/8888", - }, - }, - { - ref: "0:0", - passwd: false, - group: false, - expected: ExecUser{ - Uid: 0, - Gid: 0, - Sgids: []int{8888}, - Home: "/8888", - }, - }, - } - - for _, test := range tests { - var passwd, group io.Reader - - if test.passwd { - passwd = strings.NewReader(passwdContent) - } - - if test.group { - group = strings.NewReader(groupContent) - } - - execUser, err := GetExecUser(test.ref, &defaultExecUser, passwd, group) - if err != nil { - t.Logf("got unexpected error when parsing '%s': %s", test.ref, err.Error()) - t.Fail() - continue - } - - if !reflect.DeepEqual(test.expected, *execUser) { - t.Logf("got: %#v", execUser) - t.Logf("expected: %#v", test.expected) - t.Fail() - continue - } - } -} - -func TestGetAdditionalGroups(t *testing.T) { - type foo struct { - groups []string - expected []int - hasError bool - } - - groupContent := ` -root:x:0:root -adm:x:43: -grp:x:1234:root,adm -adm:x:4343:root,adm-duplicate -this is just some garbage data -` + largeGroup() - tests := []foo{ - { - // empty group - groups: []string{}, - expected: []int{}, - }, - { - // single group - groups: []string{"adm"}, - expected: []int{43}, - }, - { - // multiple groups - groups: []string{"adm", "grp"}, - expected: []int{43, 1234}, - }, - { - // invalid group - groups: []string{"adm", "grp", "not-exist"}, - expected: nil, - hasError: true, - }, - { - // group with numeric id - groups: []string{"43"}, - expected: []int{43}, - }, - { - // group with unknown numeric id - groups: []string{"adm", "10001"}, - expected: []int{43, 10001}, - }, - { - // groups specified twice with numeric and name - groups: []string{"adm", "43"}, - expected: []int{43}, - }, - { - // groups with too small id - groups: []string{"-1"}, - expected: nil, - hasError: true, - }, - { - // groups with too large id - groups: []string{strconv.FormatInt(1<<31, 10)}, - expected: nil, - hasError: true, - }, - { - // group with very long list of users - groups: []string{"largegroup"}, - expected: []int{1000}, - }, - } - - for _, test := range tests { - group := strings.NewReader(groupContent) - - gids, err := GetAdditionalGroups(test.groups, group) - if test.hasError && err == nil { - t.Errorf("Parse(%#v) expects error but has none", test) - continue - } - if !test.hasError && err != nil { - t.Errorf("Parse(%#v) has error %v", test, err) - continue - } - sort.Ints(gids) - if !reflect.DeepEqual(gids, test.expected) { - t.Errorf("Gids(%v), expect %v from groups %v", gids, test.expected, test.groups) - } - } -} - -func TestGetAdditionalGroupsNumeric(t *testing.T) { - tests := []struct { - groups []string - expected []int - hasError bool - }{ - { - // numeric groups only - groups: []string{"1234", "5678"}, - expected: []int{1234, 5678}, - }, - { - // numeric and alphabetic - groups: []string{"1234", "fake"}, - expected: nil, - hasError: true, - }, - } - - for _, test := range tests { - gids, err := GetAdditionalGroups(test.groups, nil) - if test.hasError && err == nil { - t.Errorf("Parse(%#v) expects error but has none", test) - continue - } - if !test.hasError && err != nil { - t.Errorf("Parse(%#v) has error %v", test, err) - continue - } - sort.Ints(gids) - if !reflect.DeepEqual(gids, test.expected) { - t.Errorf("Gids(%v), expect %v from groups %v", gids, test.expected, test.groups) - } - } -} - -// Generate a proper "largegroup" entry for group tests. -func largeGroup() (res string) { - var b strings.Builder - b.WriteString("largegroup:x:1000:user1") - for i := 2; i <= 7500; i++ { - fmt.Fprintf(&b, ",user%d", i) - } - return b.String() -} diff --git a/utils_linux.go b/utils_linux.go index 0f787cb3387..b8d25c38758 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -46,10 +46,10 @@ func getDefaultImagePath() string { // spec and stdio from the current process. func newProcess(p specs.Process) (*libcontainer.Process, error) { lp := &libcontainer.Process{ - Args: p.Args, - Env: p.Env, - // TODO: fix libcontainer's API to better support uid/gid in a typesafe way. - User: fmt.Sprintf("%d:%d", p.User.UID, p.User.GID), + Args: p.Args, + Env: p.Env, + UID: p.User.UID, + GID: p.User.GID, Cwd: p.Cwd, Label: p.SelinuxLabel, NoNewPrivileges: &p.NoNewPrivileges, @@ -69,8 +69,11 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) { lp.Capabilities.Permitted = p.Capabilities.Permitted lp.Capabilities.Ambient = p.Capabilities.Ambient } - for _, gid := range p.User.AdditionalGids { - lp.AdditionalGroups = append(lp.AdditionalGroups, strconv.FormatUint(uint64(gid), 10)) + if l := len(p.User.AdditionalGids); l > 0 { + lp.AdditionalGroups = make([]int, 0, l) + for i, g := range p.User.AdditionalGids { + lp.AdditionalGroups[i] = int(g) + } } for _, rlimit := range p.Rlimits { rl, err := createLibContainerRlimit(rlimit)