From 9331e29314d18d9f98184307b2649a89d092342c Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Fri, 27 Dec 2013 10:47:42 -0700 Subject: [PATCH 01/50] Move UserLookup functionality into a separate pkg/user submodule that implements proper parsing of /etc/passwd and /etc/group, and use that to add support for "docker run -u user:group" and for getting supplementary groups (if ":group" is not specified) Docker-DCO-1.1-Signed-off-by: Andrew Page (github: tianon) --- user/MAINTAINERS | 1 + user/user.go | 245 ++++++++++++++++++++++++++++++++++++++++++++++ user/user_test.go | 94 ++++++++++++++++++ 3 files changed, 340 insertions(+) create mode 100644 user/MAINTAINERS create mode 100644 user/user.go create mode 100644 user/user_test.go diff --git a/user/MAINTAINERS b/user/MAINTAINERS new file mode 100644 index 00000000..18e05a30 --- /dev/null +++ b/user/MAINTAINERS @@ -0,0 +1 @@ +Tianon Gravi (@tianon) diff --git a/user/user.go b/user/user.go new file mode 100644 index 00000000..30fc90f0 --- /dev/null +++ b/user/user.go @@ -0,0 +1,245 @@ +package user + +import ( + "bufio" + "fmt" + "io" + "os" + "reflect" + "strconv" + "strings" +) + +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 +} + +func parseLine(line string, v ...interface{}) { + if line == "" { + return + } + + parts := strings.Split(line, ":") + for i, p := range parts { + if len(v) <= i { + // if we have more "parts" than we have places to put them, bail for great "tolerance" of naughty configuration files + break + } + + t := reflect.TypeOf(v[i]) + if t.Kind() != reflect.Ptr { + // panic, because this is a programming/logic error, not a runtime one + panic("parseLine expects only pointers! argument " + strconv.Itoa(i) + " is not a pointer!") + } + + switch t.Elem().Kind() { + case reflect.String: + // "root", "adm", "/bin/bash" + *v[i].(*string) = p + case reflect.Int: + // "0", "4", "1000" + *v[i].(*int), _ = strconv.Atoi(p) + // ignore string to int conversion errors, for great "tolerance" of naughty configuration files + case reflect.Slice, reflect.Array: + // "", "root", "root,adm,daemon" + list := []string{} + if p != "" { + list = strings.Split(p, ",") + } + *v[i].(*[]string) = list + } + } +} + +func ParsePasswd() ([]*User, error) { + return ParsePasswdFilter(nil) +} + +func ParsePasswdFilter(filter func(*User) bool) ([]*User, error) { + f, err := os.Open("/etc/passwd") + if err != nil { + return nil, err + } + defer f.Close() + return parsePasswdFile(f, filter) +} + +func parsePasswdFile(r io.Reader, filter func(*User) bool) ([]*User, error) { + var ( + s = bufio.NewScanner(r) + out = []*User{} + ) + + for s.Scan() { + if err := s.Err(); err != nil { + return nil, err + } + + text := strings.TrimSpace(s.Text()) + if text == "" { + 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( + text, + &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell, + ) + + if filter == nil || filter(p) { + out = append(out, p) + } + } + + return out, nil +} + +func ParseGroup() ([]*Group, error) { + return ParseGroupFilter(nil) +} + +func ParseGroupFilter(filter func(*Group) bool) ([]*Group, error) { + f, err := os.Open("/etc/group") + if err != nil { + return nil, err + } + defer f.Close() + return parseGroupFile(f, filter) +} + +func parseGroupFile(r io.Reader, filter func(*Group) bool) ([]*Group, error) { + var ( + s = bufio.NewScanner(r) + out = []*Group{} + ) + + for s.Scan() { + if err := s.Err(); err != nil { + return nil, err + } + + text := s.Text() + if text == "" { + 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( + text, + &p.Name, &p.Pass, &p.Gid, &p.List, + ) + + if filter == nil || filter(p) { + out = append(out, p) + } + } + + return out, nil +} + +// Given a string like "user", "1000", "user:group", "1000:1000", returns the uid, gid, and list of supplementary group IDs, if possible. +func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) (int, int, []int, error) { + var ( + uid = defaultUid + gid = defaultGid + suppGids = []int{} + + userArg, groupArg string + ) + + // allow for userArg to have either "user" syntax, or optionally "user:group" syntax + parseLine(userSpec, &userArg, &groupArg) + + users, err := ParsePasswdFilter(func(u *User) bool { + if userArg == "" { + return u.Uid == uid + } + return u.Name == userArg || strconv.Itoa(u.Uid) == userArg + }) + if err != nil && !os.IsNotExist(err) { + if userArg == "" { + userArg = strconv.Itoa(uid) + } + return 0, 0, nil, fmt.Errorf("Unable to find user %v: %v", userArg, err) + } + + haveUser := users != nil && len(users) > 0 + if haveUser { + // if we found any user entries that matched our filter, let's take the first one as "correct" + uid = users[0].Uid + gid = users[0].Gid + } else if userArg != "" { + // we asked for a user but didn't find them... let's check to see if we wanted a numeric user + uid, err = strconv.Atoi(userArg) + if err != nil { + // not numeric - we have to bail + return 0, 0, nil, fmt.Errorf("Unable to find user %v", userArg) + } + + // if userArg couldn't be found in /etc/passwd but is numeric, just roll with it - this is legit + } + + if groupArg != "" || (haveUser && users[0].Name != "") { + groups, err := ParseGroupFilter(func(g *Group) bool { + if groupArg != "" { + return g.Name == groupArg || strconv.Itoa(g.Gid) == groupArg + } + for _, u := range g.List { + if u == users[0].Name { + return true + } + } + return false + }) + if err != nil && !os.IsNotExist(err) { + return 0, 0, nil, fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err) + } + + haveGroup := groups != nil && len(groups) > 0 + if groupArg != "" { + if haveGroup { + // if we found any group entries that matched our filter, let's take the first one as "correct" + gid = groups[0].Gid + } else { + // we asked for a group but didn't find id... let's check to see if we wanted a numeric group + gid, err = strconv.Atoi(groupArg) + if err != nil { + // not numeric - we have to bail + return 0, 0, nil, fmt.Errorf("Unable to find group %v", groupArg) + } + + // if groupArg couldn't be found in /etc/group but is numeric, just roll with it - this is legit + } + } else if haveGroup { + suppGids = make([]int, len(groups)) + for i, group := range groups { + suppGids[i] = group.Gid + } + } + } + + return uid, gid, suppGids, nil +} diff --git a/user/user_test.go b/user/user_test.go new file mode 100644 index 00000000..136632c2 --- /dev/null +++ b/user/user_test.go @@ -0,0 +1,94 @@ +package user + +import ( + "strings" + "testing" +) + +func TestUserParseLine(t *testing.T) { + var ( + a, b string + c []string + d int + ) + + parseLine("", &a, &b) + if a != "" || b != "" { + t.Fatalf("a and b should be empty ('%v', '%v')", a, b) + } + + parseLine("a", &a, &b) + if a != "a" || b != "" { + t.Fatalf("a should be 'a' and b should be empty ('%v', '%v')", a, b) + } + + parseLine("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("", &c) + if len(c) != 0 { + t.Fatalf("c should be empty (%#v)", c) + } + + parseLine("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("::::::::::", &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("not a number", &d) + if d != 0 { + t.Fatalf("d should be 0 (%v)", d) + } + + parseLine("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 := parsePasswdFile(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 := parseGroupFile(strings.NewReader(` +root:x:0:root +adm:x:4:root,adm,daemon +this is just some garbage data +`), nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(groups) != 3 { + t.Fatalf("Expected 3 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)) + } +} From 5e76f74ebc082108ed80728b7b30b484c2d117f2 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 17 Jan 2014 13:41:38 -0800 Subject: [PATCH 02/50] Use type switch instead of reflection Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) --- user/user.go | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/user/user.go b/user/user.go index 30fc90f0..1672f7e6 100644 --- a/user/user.go +++ b/user/user.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "reflect" "strconv" "strings" ) @@ -39,27 +38,24 @@ func parseLine(line string, v ...interface{}) { break } - t := reflect.TypeOf(v[i]) - if t.Kind() != reflect.Ptr { - // panic, because this is a programming/logic error, not a runtime one - panic("parseLine expects only pointers! argument " + strconv.Itoa(i) + " is not a pointer!") - } - - switch t.Elem().Kind() { - case reflect.String: + switch e := v[i].(type) { + case *string: // "root", "adm", "/bin/bash" - *v[i].(*string) = p - case reflect.Int: + *e = p + case *int: // "0", "4", "1000" - *v[i].(*int), _ = strconv.Atoi(p) // ignore string to int conversion errors, for great "tolerance" of naughty configuration files - case reflect.Slice, reflect.Array: + *e, _ = strconv.Atoi(p) + case *[]string: // "", "root", "root,adm,daemon" - list := []string{} if p != "" { - list = strings.Split(p, ",") + *e = strings.Split(p, ",") + } else { + *e = []string{} } - *v[i].(*[]string) = list + default: + // panic, because this is a programming/logic error, not a runtime one + panic("parseLine expects only pointers! argument " + strconv.Itoa(i) + " is not a pointer!") } } } From b64c71e6b777ab333b1f8b8d481a9bf1a9ac8adf Mon Sep 17 00:00:00 2001 From: Alexandr Morozov Date: Sat, 17 May 2014 22:43:31 +0400 Subject: [PATCH 03/50] Check uid ranges Fixes #5647 Docker-DCO-1.1-Signed-off-by: Alexandr Morozov (github: LK4D4) --- user/user.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/user/user.go b/user/user.go index 1672f7e6..df471012 100644 --- a/user/user.go +++ b/user/user.go @@ -9,6 +9,15 @@ import ( "strings" ) +const ( + minId = 0 + maxId = 1<<31 - 1 //for 32-bit systems compatibility +) + +var ( + ErrRange = fmt.Errorf("Uids and gids must be in range %d-%d", minId, maxId) +) + type User struct { Name string Pass string @@ -194,6 +203,9 @@ func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) // not numeric - we have to bail return 0, 0, nil, fmt.Errorf("Unable to find user %v", userArg) } + if uid < minId || uid > maxId { + return 0, 0, nil, ErrRange + } // if userArg couldn't be found in /etc/passwd but is numeric, just roll with it - this is legit } @@ -226,6 +238,9 @@ func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) // not numeric - we have to bail return 0, 0, nil, fmt.Errorf("Unable to find group %v", groupArg) } + if gid < minId || gid > maxId { + return 0, 0, nil, ErrRange + } // if groupArg couldn't be found in /etc/group but is numeric, just roll with it - this is legit } From e70ad147b6db97ef5bac2c226e3c92a00330fa0a Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Tue, 29 Jul 2014 11:08:10 -0600 Subject: [PATCH 04/50] Move "pkg/user" into libcontainer and add support for GetUserGroupSupplementary to return "Home" too Docker-DCO-1.1-Signed-off-by: Andrew Page (github: tianon) --- user/user.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/user/user.go b/user/user.go index df471012..493dd86f 100644 --- a/user/user.go +++ b/user/user.go @@ -165,12 +165,13 @@ func parseGroupFile(r io.Reader, filter func(*Group) bool) ([]*Group, error) { return out, nil } -// Given a string like "user", "1000", "user:group", "1000:1000", returns the uid, gid, and list of supplementary group IDs, if possible. -func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) (int, int, []int, error) { +// Given a string like "user", "1000", "user:group", "1000:1000", returns the uid, gid, list of supplementary group IDs, and home directory, if available and/or applicable. +func GetUserGroupSupplementaryHome(userSpec string, defaultUid, defaultGid int, defaultHome string) (int, int, []int, string, error) { var ( uid = defaultUid gid = defaultGid suppGids = []int{} + home = defaultHome userArg, groupArg string ) @@ -188,7 +189,7 @@ func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) if userArg == "" { userArg = strconv.Itoa(uid) } - return 0, 0, nil, fmt.Errorf("Unable to find user %v: %v", userArg, err) + return 0, 0, nil, "", fmt.Errorf("Unable to find user %v: %v", userArg, err) } haveUser := users != nil && len(users) > 0 @@ -196,15 +197,16 @@ func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) // if we found any user entries that matched our filter, let's take the first one as "correct" uid = users[0].Uid gid = users[0].Gid + home = users[0].Home } else if userArg != "" { // we asked for a user but didn't find them... let's check to see if we wanted a numeric user uid, err = strconv.Atoi(userArg) if err != nil { // not numeric - we have to bail - return 0, 0, nil, fmt.Errorf("Unable to find user %v", userArg) + return 0, 0, nil, "", fmt.Errorf("Unable to find user %v", userArg) } if uid < minId || uid > maxId { - return 0, 0, nil, ErrRange + return 0, 0, nil, "", ErrRange } // if userArg couldn't be found in /etc/passwd but is numeric, just roll with it - this is legit @@ -223,7 +225,7 @@ func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) return false }) if err != nil && !os.IsNotExist(err) { - return 0, 0, nil, fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err) + return 0, 0, nil, "", fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err) } haveGroup := groups != nil && len(groups) > 0 @@ -236,10 +238,10 @@ func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) gid, err = strconv.Atoi(groupArg) if err != nil { // not numeric - we have to bail - return 0, 0, nil, fmt.Errorf("Unable to find group %v", groupArg) + return 0, 0, nil, "", fmt.Errorf("Unable to find group %v", groupArg) } if gid < minId || gid > maxId { - return 0, 0, nil, ErrRange + return 0, 0, nil, "", ErrRange } // if groupArg couldn't be found in /etc/group but is numeric, just roll with it - this is legit @@ -252,5 +254,5 @@ func GetUserGroupSupplementary(userSpec string, defaultUid int, defaultGid int) } } - return uid, gid, suppGids, nil + return uid, gid, suppGids, home, nil } From ef3397c7ab9e3ce392d20d73c776afbbfc0eb20a Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 15 Aug 2014 04:19:17 +1000 Subject: [PATCH 05/50] user: *: refactor and expand libcontainer/user API This patch refactors most of GetUserGroupSupplementaryHome and its signature, to make using it much simpler. The private parsing ftunctions have also been exposed (parsePasswdFile, parseGroupFile) to allow custom data source to be used (increasing the versatility of the user/ tools). In addition, file path wrappers around the formerly private API functions have been added to make usage of the API for callers easier if the files that are being parsed are on the filesystem (while the io.Reader APIs are exposed for non-traditional usecases). Signed-off-by: Aleksa Sarai (github: cyphar) --- user/user.go | 192 ++++++++++++++++++++++++++++++++++------------ user/user_test.go | 4 +- 2 files changed, 144 insertions(+), 52 deletions(-) diff --git a/user/user.go b/user/user.go index 493dd86f..69387f2e 100644 --- a/user/user.go +++ b/user/user.go @@ -69,23 +69,36 @@ func parseLine(line string, v ...interface{}) { } } -func ParsePasswd() ([]*User, error) { - return ParsePasswdFilter(nil) +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 ParsePasswdFilter(filter func(*User) bool) ([]*User, error) { - f, err := os.Open("/etc/passwd") +func ParsePasswdFileFilter(path string, filter func(User) bool) ([]User, error) { + passwd, err := os.Open(path) if err != nil { return nil, err } - defer f.Close() - return parsePasswdFile(f, filter) + defer passwd.Close() + return ParsePasswdFilter(passwd, filter) } -func parsePasswdFile(r io.Reader, filter func(*User) bool) ([]*User, error) { +func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { + if r == nil { + return nil, fmt.Errorf("nil source for passwd-formatted data") + } + var ( s = bufio.NewScanner(r) - out = []*User{} + out = []User{} ) for s.Scan() { @@ -103,7 +116,7 @@ func parsePasswdFile(r io.Reader, filter func(*User) bool) ([]*User, error) { // 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{} + p := User{} parseLine( text, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell, @@ -117,23 +130,36 @@ func parsePasswdFile(r io.Reader, filter func(*User) bool) ([]*User, error) { return out, nil } -func ParseGroup() ([]*Group, error) { - return ParseGroupFilter(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 ParseGroupFilter(filter func(*Group) bool) ([]*Group, error) { - f, err := os.Open("/etc/group") +func ParseGroupFileFilter(path string, filter func(Group) bool) ([]Group, error) { + group, err := os.Open(path) if err != nil { return nil, err } - defer f.Close() - return parseGroupFile(f, filter) + defer group.Close() + return ParseGroupFilter(group, filter) } -func parseGroupFile(r io.Reader, filter func(*Group) bool) ([]*Group, error) { +func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { + if r == nil { + return nil, fmt.Errorf("nil source for group-formatted data") + } + var ( s = bufio.NewScanner(r) - out = []*Group{} + out = []Group{} ) for s.Scan() { @@ -151,7 +177,7 @@ func parseGroupFile(r io.Reader, filter func(*Group) bool) ([]*Group, error) { // Name:Pass:Gid:List // root:x:0:root // adm:x:4:root,adm,daemon - p := &Group{} + p := Group{} parseLine( text, &p.Name, &p.Pass, &p.Gid, &p.List, @@ -165,94 +191,160 @@ func parseGroupFile(r io.Reader, filter func(*Group) bool) ([]*Group, error) { return out, nil } -// Given a string like "user", "1000", "user:group", "1000:1000", returns the uid, gid, list of supplementary group IDs, and home directory, if available and/or applicable. -func GetUserGroupSupplementaryHome(userSpec string, defaultUid, defaultGid int, defaultHome string) (int, int, []int, string, error) { - var ( - uid = defaultUid - gid = defaultGid - suppGids = []int{} - home = defaultHome +type ExecUser struct { + Uid, Gid int + Sgids []int + Home string +} +// GetExecUserFile 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 GetExecUserFile(userSpec string, defaults *ExecUser, passwdPath, groupPath string) (*ExecUser, error) { + passwd, err := os.Open(passwdPath) + if err != nil { + passwd = nil + } else { + defer passwd.Close() + } + + group, err := os.Open(groupPath) + if err != nil { + group = nil + } else { + defer group.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" +func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (*ExecUser, error) { + var ( userArg, groupArg string + name string ) + 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 parseLine(userSpec, &userArg, &groupArg) - users, err := ParsePasswdFilter(func(u *User) bool { + users, err := ParsePasswdFilter(passwd, func(u User) bool { if userArg == "" { - return u.Uid == uid + return u.Uid == user.Uid } return u.Name == userArg || strconv.Itoa(u.Uid) == userArg }) - if err != nil && !os.IsNotExist(err) { + if err != nil && passwd != nil { if userArg == "" { - userArg = strconv.Itoa(uid) + userArg = strconv.Itoa(user.Uid) } - return 0, 0, nil, "", fmt.Errorf("Unable to find user %v: %v", userArg, err) + return nil, fmt.Errorf("Unable to find user %v: %v", userArg, err) } haveUser := users != nil && len(users) > 0 if haveUser { // if we found any user entries that matched our filter, let's take the first one as "correct" - uid = users[0].Uid - gid = users[0].Gid - home = users[0].Home + name = users[0].Name + user.Uid = users[0].Uid + user.Gid = users[0].Gid + user.Home = users[0].Home } else if userArg != "" { // we asked for a user but didn't find them... let's check to see if we wanted a numeric user - uid, err = strconv.Atoi(userArg) + user.Uid, err = strconv.Atoi(userArg) if err != nil { // not numeric - we have to bail - return 0, 0, nil, "", fmt.Errorf("Unable to find user %v", userArg) + return nil, fmt.Errorf("Unable to find user %v", userArg) } - if uid < minId || uid > maxId { - return 0, 0, nil, "", ErrRange + + // Must be inside valid uid range. + if user.Uid < minId || user.Uid > maxId { + return nil, ErrRange } // if userArg couldn't be found in /etc/passwd but is numeric, just roll with it - this is legit } - if groupArg != "" || (haveUser && users[0].Name != "") { - groups, err := ParseGroupFilter(func(g *Group) bool { + if groupArg != "" || name != "" { + groups, err := ParseGroupFilter(group, func(g Group) bool { + // Explicit group format takes precedence. if groupArg != "" { return g.Name == groupArg || strconv.Itoa(g.Gid) == groupArg } + + // Check if user is a member. for _, u := range g.List { - if u == users[0].Name { + if u == name { return true } } + return false }) - if err != nil && !os.IsNotExist(err) { - return 0, 0, nil, "", fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err) + if err != nil && group != nil { + return nil, fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err) } haveGroup := groups != nil && len(groups) > 0 if groupArg != "" { if haveGroup { // if we found any group entries that matched our filter, let's take the first one as "correct" - gid = groups[0].Gid + user.Gid = groups[0].Gid } else { // we asked for a group but didn't find id... let's check to see if we wanted a numeric group - gid, err = strconv.Atoi(groupArg) + user.Gid, err = strconv.Atoi(groupArg) if err != nil { // not numeric - we have to bail - return 0, 0, nil, "", fmt.Errorf("Unable to find group %v", groupArg) + return nil, fmt.Errorf("Unable to find group %v", groupArg) } - if gid < minId || gid > maxId { - return 0, 0, nil, "", ErrRange + + // Ensure gid is inside gid range. + if user.Gid < minId || user.Gid > maxId { + return nil, ErrRange } // if groupArg couldn't be found in /etc/group but is numeric, just roll with it - this is legit } } else if haveGroup { - suppGids = make([]int, len(groups)) + // If implicit group format, fill supplementary gids. + user.Sgids = make([]int, len(groups)) for i, group := range groups { - suppGids[i] = group.Gid + user.Sgids[i] = group.Gid } } } - return uid, gid, suppGids, home, nil + return user, nil } diff --git a/user/user_test.go b/user/user_test.go index 136632c2..2dc963f4 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -54,7 +54,7 @@ func TestUserParseLine(t *testing.T) { } func TestUserParsePasswd(t *testing.T) { - users, err := parsePasswdFile(strings.NewReader(` + 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 @@ -74,7 +74,7 @@ this is just some garbage data } func TestUserParseGroup(t *testing.T) { - groups, err := parseGroupFile(strings.NewReader(` + groups, err := ParseGroupFilter(strings.NewReader(` root:x:0:root adm:x:4:root,adm,daemon this is just some garbage data From 03c82a3b689dde7960ee7094cff4aa5620a0bedd Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 29 Aug 2014 18:43:40 +1000 Subject: [PATCH 06/50] user: lookup: added os/user-like lookup API This patch adds an os/user-like user lookup API, implemented in pure Go. It also has some features not present in the standard library implementation (such as group lookups). Signed-off-by: Aleksa Sarai (github: cyphar) --- user/lookup.go | 108 +++++++++++++++++++++++++++++++++++++ user/lookup_unix.go | 30 +++++++++++ user/lookup_unsupported.go | 21 ++++++++ 3 files changed, 159 insertions(+) create mode 100644 user/lookup.go create mode 100644 user/lookup_unix.go create mode 100644 user/lookup_unsupported.go diff --git a/user/lookup.go b/user/lookup.go new file mode 100644 index 00000000..6f8a982f --- /dev/null +++ b/user/lookup.go @@ -0,0 +1,108 @@ +package user + +import ( + "errors" + "fmt" + "syscall" +) + +var ( + // The current operating system does not provide the required data for user lookups. + ErrUnsupported = errors.New("user lookup: operating system does not provide passwd-formatted data") +) + +func lookupUser(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{}, fmt.Errorf("no matching entries in passwd file") + } + + // Assume the first entry is the "correct" one. + return users[0], nil +} + +// 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(syscall.Getuid()) +} + +// 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 lookupUser(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 lookupUser(func(u User) bool { + return u.Uid == uid + }) +} + +func lookupGroup(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{}, fmt.Errorf("no matching entries in group file") + } + + // Assume the first entry is the "correct" one. + return groups[0], nil +} + +// 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(syscall.Getgid()) +} + +// 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 lookupGroup(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 lookupGroup(func(g Group) bool { + return g.Gid == gid + }) +} diff --git a/user/lookup_unix.go b/user/lookup_unix.go new file mode 100644 index 00000000..409c114e --- /dev/null +++ b/user/lookup_unix.go @@ -0,0 +1,30 @@ +// +build darwin dragonfly freebsd linux netbsd openbsd solaris + +package user + +import ( + "io" + "os" +) + +// Unix-specific path to the passwd and group formatted files. +const ( + unixPasswdFile = "/etc/passwd" + unixGroupFile = "/etc/group" +) + +func GetPasswdFile() (string, error) { + return unixPasswdFile, nil +} + +func GetPasswd() (io.ReadCloser, error) { + return os.Open(unixPasswdFile) +} + +func GetGroupFile() (string, error) { + return unixGroupFile, nil +} + +func GetGroup() (io.ReadCloser, error) { + return os.Open(unixGroupFile) +} diff --git a/user/lookup_unsupported.go b/user/lookup_unsupported.go new file mode 100644 index 00000000..0f15c57d --- /dev/null +++ b/user/lookup_unsupported.go @@ -0,0 +1,21 @@ +// +build !darwin,!dragonfly,!freebsd,!linux,!netbsd,!openbsd,!solaris + +package user + +import "io" + +func GetPasswdFile() (string, error) { + return "", ErrUnsupported +} + +func GetPasswd() (io.ReadCloser, error) { + return nil, ErrUnsupported +} + +func GetGroupFile() (string, error) { + return "", ErrUnsupported +} + +func GetGroup() (io.ReadCloser, error) { + return nil, ErrUnsupported +} From 3b425ccb30e896ac24a1dad8c0b8e4aba34fc57c Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 15 Aug 2014 05:36:04 +1000 Subject: [PATCH 07/50] user: add unit tests for GetExecUser Signed-off-by: Aleksa Sarai (github: cyphar) --- user/user_test.go | 258 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 258 insertions(+) diff --git a/user/user_test.go b/user/user_test.go index 2dc963f4..4fe008fb 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -1,6 +1,8 @@ package user import ( + "io" + "reflect" "strings" "testing" ) @@ -92,3 +94,259 @@ this is just some garbage data 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 +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 + 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, + }, + }, + } + + 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("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 +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", + } + + 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 + } + } +} From 52e6df1e539d06d7bc09b5d174db71c49c02d96e Mon Sep 17 00:00:00 2001 From: Andrey Vagin Date: Tue, 27 Jan 2015 15:54:19 +0300 Subject: [PATCH 08/50] Merge remote-tracking branch 'origin/master' into api Signed-off-by: Andrey Vagin --- user/MAINTAINERS | 1 + user/lookup_unix.go | 16 ++++++++-------- user/lookup_unsupported.go | 4 ++-- user/user.go | 4 ++-- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/user/MAINTAINERS b/user/MAINTAINERS index 18e05a30..edbe2006 100644 --- a/user/MAINTAINERS +++ b/user/MAINTAINERS @@ -1 +1,2 @@ Tianon Gravi (@tianon) +Aleksa Sarai (@cyphar) diff --git a/user/lookup_unix.go b/user/lookup_unix.go index 409c114e..758b734c 100644 --- a/user/lookup_unix.go +++ b/user/lookup_unix.go @@ -9,22 +9,22 @@ import ( // Unix-specific path to the passwd and group formatted files. const ( - unixPasswdFile = "/etc/passwd" - unixGroupFile = "/etc/group" + unixPasswdPath = "/etc/passwd" + unixGroupPath = "/etc/group" ) -func GetPasswdFile() (string, error) { - return unixPasswdFile, nil +func GetPasswdPath() (string, error) { + return unixPasswdPath, nil } func GetPasswd() (io.ReadCloser, error) { - return os.Open(unixPasswdFile) + return os.Open(unixPasswdPath) } -func GetGroupFile() (string, error) { - return unixGroupFile, nil +func GetGroupPath() (string, error) { + return unixGroupPath, nil } func GetGroup() (io.ReadCloser, error) { - return os.Open(unixGroupFile) + return os.Open(unixGroupPath) } diff --git a/user/lookup_unsupported.go b/user/lookup_unsupported.go index 0f15c57d..72179488 100644 --- a/user/lookup_unsupported.go +++ b/user/lookup_unsupported.go @@ -4,7 +4,7 @@ package user import "io" -func GetPasswdFile() (string, error) { +func GetPasswdPath() (string, error) { return "", ErrUnsupported } @@ -12,7 +12,7 @@ func GetPasswd() (io.ReadCloser, error) { return nil, ErrUnsupported } -func GetGroupFile() (string, error) { +func GetGroupPath() (string, error) { return "", ErrUnsupported } diff --git a/user/user.go b/user/user.go index 69387f2e..d7439f12 100644 --- a/user/user.go +++ b/user/user.go @@ -197,11 +197,11 @@ type ExecUser struct { Home string } -// GetExecUserFile is a wrapper for GetExecUser. It reads data from each of the +// 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 GetExecUserFile(userSpec string, defaults *ExecUser, passwdPath, groupPath string) (*ExecUser, error) { +func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath string) (*ExecUser, error) { passwd, err := os.Open(passwdPath) if err != nil { passwd = nil From 6f2328c5d1c87f8b8ba2cb5790a592d962a4b43f Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Thu, 30 Apr 2015 19:02:31 -0400 Subject: [PATCH 09/50] Lookup additional groups in the container. Signed-off-by: Mrunal Patel --- user/user.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/user/user.go b/user/user.go index d7439f12..baff970c 100644 --- a/user/user.go +++ b/user/user.go @@ -348,3 +348,52 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( return user, nil } + +// GetAdditionalGroupsPath is a wrapper for GetAdditionalGroups. It reads data from the +// given file path and uses that data as the arguments to GetAdditionalGroups. +func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int, error) { + var groupIds []int + + for _, ag := range additionalGroups { + groupReader, err := os.Open(groupPath) + if err != nil { + return nil, fmt.Errorf("Failed to open group file: %v", err) + } + defer groupReader.Close() + + groupId, err := GetAdditionalGroup(ag, groupReader) + if err != nil { + return nil, err + } + groupIds = append(groupIds, groupId) + } + + return groupIds, nil +} + +// GetAdditionalGroup looks up the specified group in the passed groupReader. +func GetAdditionalGroup(additionalGroup string, groupReader io.Reader) (int, error) { + groups, err := ParseGroupFilter(groupReader, func(g Group) bool { + return g.Name == additionalGroup || strconv.Itoa(g.Gid) == additionalGroup + }) + if err != nil { + return -1, fmt.Errorf("Unable to find additional groups %v: %v", additionalGroup, err) + } + if groups != nil && len(groups) > 0 { + // if we found any group entries that matched our filter, let's take the first one as "correct" + return groups[0].Gid, nil + } else { + // we asked for a group but didn't find id... let's check to see if we wanted a numeric group + addGroup, err := strconv.Atoi(additionalGroup) + if err != nil { + // not numeric - we have to bail + return -1, fmt.Errorf("Unable to find group %v", additionalGroup) + } + + // Ensure gid is inside gid range. + if addGroup < minId || addGroup > maxId { + return -1, ErrRange + } + return addGroup, nil + } +} From c1a87e39b1807ad00a435633917fbf3ca11070ec Mon Sep 17 00:00:00 2001 From: "Daniel, Dao Quang Minh" Date: Mon, 25 May 2015 19:02:34 +0000 Subject: [PATCH 10/50] refactor GetAdditionalGroupsPath This parses group file only once to process a list of groups instead of parsing once for each group. Also added an unit test for GetAdditionalGroupsPath Signed-off-by: Daniel, Dao Quang Minh --- user/user.go | 82 +++++++++++++++++++++++------------------- user/user_test.go | 91 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 37 deletions(-) diff --git a/user/user.go b/user/user.go index baff970c..13226dbf 100644 --- a/user/user.go +++ b/user/user.go @@ -349,51 +349,59 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( return user, nil } -// GetAdditionalGroupsPath is a wrapper for GetAdditionalGroups. It reads data from the -// given file path and uses that data as the arguments to GetAdditionalGroups. +// GetAdditionalGroupsPath looks up a list of groups by name or group id +// against the group file. If a group name cannot be found, an error will be +// returned. If a group id cannot be found, it will be returned as-is. func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int, error) { - var groupIds []int - - for _, ag := range additionalGroups { - groupReader, err := os.Open(groupPath) - if err != nil { - return nil, fmt.Errorf("Failed to open group file: %v", err) - } - defer groupReader.Close() - - groupId, err := GetAdditionalGroup(ag, groupReader) - if err != nil { - return nil, err - } - groupIds = append(groupIds, groupId) + groupReader, err := os.Open(groupPath) + if err != nil { + return nil, fmt.Errorf("Failed to open group file: %v", err) } + defer groupReader.Close() - return groupIds, nil -} - -// GetAdditionalGroup looks up the specified group in the passed groupReader. -func GetAdditionalGroup(additionalGroup string, groupReader io.Reader) (int, error) { groups, err := ParseGroupFilter(groupReader, func(g Group) bool { - return g.Name == additionalGroup || strconv.Itoa(g.Gid) == additionalGroup + for _, ag := range additionalGroups { + if g.Name == ag || strconv.Itoa(g.Gid) == ag { + return true + } + } + return false }) if err != nil { - return -1, fmt.Errorf("Unable to find additional groups %v: %v", additionalGroup, err) + return nil, fmt.Errorf("Unable to find additional groups %v: %v", additionalGroups, err) } - if groups != nil && len(groups) > 0 { - // if we found any group entries that matched our filter, let's take the first one as "correct" - return groups[0].Gid, nil - } else { - // we asked for a group but didn't find id... let's check to see if we wanted a numeric group - addGroup, err := strconv.Atoi(additionalGroup) - if err != nil { - // not numeric - we have to bail - return -1, fmt.Errorf("Unable to find group %v", additionalGroup) - } - // Ensure gid is inside gid range. - if addGroup < minId || addGroup > maxId { - return -1, ErrRange + 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.Atoi(ag) + if err != nil { + return nil, fmt.Errorf("Unable to find group %s", ag) + } + // Ensure gid is inside gid range. + if gid < minId || gid > maxId { + return nil, ErrRange + } + gidMap[gid] = struct{}{} } - return addGroup, nil } + gids := []int{} + for gid := range gidMap { + gids = append(gids, gid) + } + return gids, nil } diff --git a/user/user_test.go b/user/user_test.go index 4fe008fb..ffb0760e 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -1,8 +1,12 @@ package user import ( + "fmt" "io" + "io/ioutil" "reflect" + "sort" + "strconv" "strings" "testing" ) @@ -350,3 +354,90 @@ this is just some garbage data } } } + +func TestGetAdditionalGroupsPath(t *testing.T) { + const 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 +` + tests := []struct { + groups []string + expected []int + hasError bool + }{ + { + // 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.Itoa(1 << 31)}, + expected: nil, + hasError: true, + }, + } + + for _, test := range tests { + tmpFile, err := ioutil.TempFile("", "get-additional-groups-path") + if err != nil { + t.Error(err) + } + fmt.Fprint(tmpFile, groupContent) + tmpFile.Close() + + gids, err := GetAdditionalGroupsPath(test.groups, tmpFile.Name()) + 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.Sort(sort.IntSlice(gids)) + if !reflect.DeepEqual(gids, test.expected) { + t.Errorf("Gids(%v), expect %v from groups %v", gids, test.expected, test.groups) + } + } +} From f975a5101df52511b8dbd682db0470f644620949 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sun, 28 Jun 2015 11:14:24 +1000 Subject: [PATCH 11/50] libcontainer: user: fix GetAdditionalGroupsPath to match API The old GetAdditionalGroups* API didn't match the rest of libcontainer/user, we make functions that take io.Readers and then make wrappers around them. Otherwise we have to do dodgy stuff when testing our code. Fixes: b6df94ee65436 ("refactor GetAdditionalGroupsPath") Signed-off-by: Aleksa Sarai --- user/user.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/user/user.go b/user/user.go index 13226dbf..964e31bf 100644 --- a/user/user.go +++ b/user/user.go @@ -349,17 +349,12 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( return user, nil } -// GetAdditionalGroupsPath looks up a list of groups by name or group id -// against the group file. If a group name cannot be found, an error will be -// returned. If a group id cannot be found, it will be returned as-is. -func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int, error) { - groupReader, err := os.Open(groupPath) - if err != nil { - return nil, fmt.Errorf("Failed to open group file: %v", err) - } - defer groupReader.Close() - - groups, err := ParseGroupFilter(groupReader, func(g Group) bool { +// GetAdditionalGroups looks up a list of groups by name or group id against +// 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, it will be returned +// as-is. +func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, error) { + groups, err := ParseGroupFilter(group, func(g Group) bool { for _, ag := range additionalGroups { if g.Name == ag || strconv.Itoa(g.Gid) == ag { return true @@ -405,3 +400,14 @@ func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int } return gids, nil } + +// Wrapper around GetAdditionalGroups that opens the groupPath given and gives +// it as an argument to GetAdditionalGroups. +func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int, error) { + group, err := os.Open(groupPath) + if err != nil { + return nil, fmt.Errorf("Failed to open group file: %v", err) + } + defer group.Close() + return GetAdditionalGroups(additionalGroups, group) +} From 78033bda257ce3269be066a4ea64e2992b2b1817 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sun, 28 Jun 2015 11:18:20 +1000 Subject: [PATCH 12/50] libcontainer: user: update tests for GetAdditionalGroups Update the tests to use the test-friendly GetAdditionalGroups API, rather than making random files for no good reason. Signed-off-by: Aleksa Sarai --- user/user_test.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/user/user_test.go b/user/user_test.go index ffb0760e..0e37ac3d 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -1,9 +1,7 @@ package user import ( - "fmt" "io" - "io/ioutil" "reflect" "sort" "strconv" @@ -355,7 +353,7 @@ this is just some garbage data } } -func TestGetAdditionalGroupsPath(t *testing.T) { +func TestGetAdditionalGroups(t *testing.T) { const groupContent = ` root:x:0:root adm:x:43: @@ -419,14 +417,9 @@ this is just some garbage data } for _, test := range tests { - tmpFile, err := ioutil.TempFile("", "get-additional-groups-path") - if err != nil { - t.Error(err) - } - fmt.Fprint(tmpFile, groupContent) - tmpFile.Close() + group := strings.NewReader(groupContent) - gids, err := GetAdditionalGroupsPath(test.groups, tmpFile.Name()) + gids, err := GetAdditionalGroups(test.groups, group) if test.hasError && err == nil { t.Errorf("Parse(%#v) expects error but has none", test) continue From 652fbaa2111b243d6c2a8692072d28b7ae2f4135 Mon Sep 17 00:00:00 2001 From: Sami Wagiaalla Date: Sun, 4 Oct 2015 19:02:30 -0400 Subject: [PATCH 13/50] Allow numeric groups for containers without /etc/group /etc/groups is not needed when specifying numeric group ids. This change allows containers without /etc/groups to specify numeric supplemental groups. Signed-off-by: Sami Wagiaalla --- user/user.go | 39 ++++++++++++++++++++++----------------- user/user_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/user/user.go b/user/user.go index 964e31bf..e6375ea4 100644 --- a/user/user.go +++ b/user/user.go @@ -349,21 +349,26 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( return user, nil } -// GetAdditionalGroups looks up a list of groups by name or group id against -// 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, it will be returned -// as-is. +// 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, err := ParseGroupFilter(group, func(g Group) bool { - for _, ag := range additionalGroups { - if g.Name == ag || strconv.Itoa(g.Gid) == ag { - return true + var 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: %v", additionalGroups, err) } - return false - }) - if err != nil { - return nil, fmt.Errorf("Unable to find additional groups %v: %v", additionalGroups, err) } gidMap := make(map[int]struct{}) @@ -401,13 +406,13 @@ func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, err return gids, nil } -// Wrapper around GetAdditionalGroups that opens the groupPath given and gives -// it as an argument to GetAdditionalGroups. +// 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) { group, err := os.Open(groupPath) - if err != nil { - return nil, fmt.Errorf("Failed to open group file: %v", err) + if err == nil { + defer group.Close() } - defer group.Close() return GetAdditionalGroups(additionalGroups, group) } diff --git a/user/user_test.go b/user/user_test.go index 0e37ac3d..53b2289b 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -434,3 +434,39 @@ this is just some garbage data } } } + +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.Sort(sort.IntSlice(gids)) + if !reflect.DeepEqual(gids, test.expected) { + t.Errorf("Gids(%v), expect %v from groups %v", gids, test.expected, test.groups) + } + } +} From 23eb47f7fe2227b61775e00c37378f1c3c7678b2 Mon Sep 17 00:00:00 2001 From: Thomas LE ROUX Date: Thu, 17 Mar 2016 18:23:12 +0100 Subject: [PATCH 14/50] Export user and group lookup errors as variables. Export errors as variables when no matching entries are found in passwd or group file. Signed-off-by: Thomas LE ROUX --- user/lookup.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/user/lookup.go b/user/lookup.go index 6f8a982f..ab1439f3 100644 --- a/user/lookup.go +++ b/user/lookup.go @@ -2,13 +2,15 @@ package user import ( "errors" - "fmt" "syscall" ) var ( // The current operating system does not provide the required data for user lookups. ErrUnsupported = errors.New("user lookup: operating system does not provide passwd-formatted data") + // No matching entries found in file. + ErrNoPasswdEntries = errors.New("no matching entries in passwd file") + ErrNoGroupEntries = errors.New("no matching entries in group file") ) func lookupUser(filter func(u User) bool) (User, error) { @@ -27,7 +29,7 @@ func lookupUser(filter func(u User) bool) (User, error) { // No user entries found. if len(users) == 0 { - return User{}, fmt.Errorf("no matching entries in passwd file") + return User{}, ErrNoPasswdEntries } // Assume the first entry is the "correct" one. @@ -75,7 +77,7 @@ func lookupGroup(filter func(g Group) bool) (Group, error) { // No user entries found. if len(groups) == 0 { - return Group{}, fmt.Errorf("no matching entries in group file") + return Group{}, ErrNoGroupEntries } // Assume the first entry is the "correct" one. From 92e1773b9a037d4180e2917e8525463abc06a796 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 30 Mar 2016 22:48:07 +1100 Subject: [PATCH 15/50] libcontainer: user: always treat numeric ids numerically Most shadow-related tools don't treat numeric ids as potential usernames, so change our behaviour to match that. Previously, using an explicit specification like 111:222 could result in the UID and GID not being 111 and 222 respectively (which is confusing). Signed-off-by: Aleksa Sarai --- user/user.go | 89 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 32 deletions(-) diff --git a/user/user.go b/user/user.go index e6375ea4..9f4c2cb5 100644 --- a/user/user.go +++ b/user/user.go @@ -235,10 +235,14 @@ func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath // * "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) { var ( userArg, groupArg string - name string ) if defaults == nil { @@ -261,11 +265,22 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( // allow for userArg to have either "user" syntax, or optionally "user:group" syntax parseLine(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) + users, err := ParsePasswdFilter(passwd, func(u User) bool { if userArg == "" { return u.Uid == user.Uid } - return u.Name == userArg || strconv.Itoa(u.Uid) == userArg + + if uidErr == nil { + // If the userArg is numeric, always treat it as a UID. + return uidArg == u.Uid + } + + return u.Name == userArg }) if err != nil && passwd != nil { if userArg == "" { @@ -274,47 +289,55 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( return nil, fmt.Errorf("Unable to find user %v: %v", userArg, err) } - haveUser := users != nil && len(users) > 0 - if haveUser { - // if we found any user entries that matched our filter, let's take the first one as "correct" - name = users[0].Name + 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 != "" { - // we asked for a user but didn't find them... let's check to see if we wanted a numeric user - user.Uid, err = strconv.Atoi(userArg) - if err != nil { - // not numeric - we have to bail - return nil, fmt.Errorf("Unable to find user %v", 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: %v", userArg, ErrNoPasswdEntries) } + user.Uid = uidArg // Must be inside valid uid range. if user.Uid < minId || user.Uid > maxId { return nil, ErrRange } - // if userArg couldn't be found in /etc/passwd but is numeric, just roll with it - this is legit + // Okay, so it's numeric. We can just roll with this. } - if groupArg != "" || name != "" { + // 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 { - // Explicit group format takes precedence. - if groupArg != "" { - return g.Name == groupArg || strconv.Itoa(g.Gid) == groupArg + // 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 } - // Check if user is a member. - for _, u := range g.List { - if u == name { - return true - } + if gidErr == nil { + // If the groupArg is numeric, always treat it as a GID. + return gidArg == g.Gid } - return false + return g.Name == groupArg }) if err != nil && group != nil { - return nil, fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err) + return nil, fmt.Errorf("unable to find groups for spec %v: %v", matchedUserName, err) } haveGroup := groups != nil && len(groups) > 0 @@ -322,23 +345,25 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( if haveGroup { // if we found any group entries that matched our filter, let's take the first one as "correct" user.Gid = groups[0].Gid - } else { - // we asked for a group but didn't find id... let's check to see if we wanted a numeric group - user.Gid, err = strconv.Atoi(groupArg) - if err != nil { - // not numeric - we have to bail - return nil, fmt.Errorf("Unable to find group %v", groupArg) + } else if groupArg != "" { + // 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: %v", groupArg, ErrNoGroupEntries) } + user.Gid = gidArg // Ensure gid is inside gid range. if user.Gid < minId || user.Gid > maxId { return nil, ErrRange } - // if groupArg couldn't be found in /etc/group but is numeric, just roll with it - this is legit + // Okay, so it's numeric. We can just roll with this. } - } else if haveGroup { - // If implicit group format, fill supplementary gids. + } 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 From a4b5eb7d4ddb47b23ac46f9571643987a6d10d26 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 30 Mar 2016 22:44:04 +1100 Subject: [PATCH 16/50] libcontainer: user: add tests for numeric user specifications Signed-off-by: Aleksa Sarai --- user/user_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/user/user_test.go b/user/user_test.go index 53b2289b..8cad2f55 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -101,12 +101,16 @@ 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::::: this is just some garbage data ` const groupContent = ` root:x:0:root adm:x:43: grp:x:1234:root,adm +444:x:555:111 +odd:x:444: this is just some garbage data ` defaultExecUser := ExecUser{ @@ -192,6 +196,26 @@ this is just some garbage data 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", + }, + }, } for _, test := range tests { @@ -206,6 +230,7 @@ this is just some garbage data } 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() @@ -218,6 +243,7 @@ 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 = ` @@ -240,6 +266,8 @@ this is just some garbage data "-1:0", "0:-3", "-5:-2", + "-42", + "-43", } for _, test := range tests { From 496e469b54b676df53550dd0e667af7ab184f7e6 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 30 Mar 2016 22:50:14 +1100 Subject: [PATCH 17/50] libcontainer: user: general cleanups Some of the code was quite confusing inside libcontainer/user, so refactor and comment it so future maintainers can understand what's going and what edge cases we have to deal with. Signed-off-by: Aleksa Sarai --- user/user.go | 60 +++++++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/user/user.go b/user/user.go index 9f4c2cb5..43fd39ef 100644 --- a/user/user.go +++ b/user/user.go @@ -15,7 +15,7 @@ const ( ) var ( - ErrRange = fmt.Errorf("Uids and gids must be in range %d-%d", minId, maxId) + ErrRange = fmt.Errorf("uids and gids must be in range %d-%d", minId, maxId) ) type User struct { @@ -42,29 +42,30 @@ func parseLine(line string, v ...interface{}) { parts := strings.Split(line, ":") for i, p := range parts { + // Ignore cases where we don't have enough fields to populate the arguments. + // Some configuration files like to misbehave. if len(v) <= i { - // if we have more "parts" than we have places to put them, bail for great "tolerance" of naughty configuration files break } + // 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: - // "root", "adm", "/bin/bash" *e = p case *int: - // "0", "4", "1000" - // ignore string to int conversion errors, for great "tolerance" of naughty configuration files + // "numbers", with conversion errors ignored because of some misbehaving configuration files. *e, _ = strconv.Atoi(p) case *[]string: - // "", "root", "root,adm,daemon" + // Comma-separated lists. if p != "" { *e = strings.Split(p, ",") } else { *e = []string{} } default: - // panic, because this is a programming/logic error, not a runtime one - panic("parseLine expects only pointers! argument " + strconv.Itoa(i) + " is not a pointer!") + // Someone goof'd when writing code using this function. Scream so they can hear us. + panic(fmt.Sprintf("parseLine only accepts {*string, *int, *[]string} as arguments! %#v is not a pointer!", e)) } } } @@ -106,8 +107,8 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { return nil, err } - text := strings.TrimSpace(s.Text()) - if text == "" { + line := strings.TrimSpace(s.Text()) + if line == "" { continue } @@ -117,10 +118,7 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { // root:x:0:0:root:/root:/bin/bash // adm:x:3:4:adm:/var/adm:/bin/false p := User{} - parseLine( - text, - &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell, - ) + 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) @@ -135,6 +133,7 @@ func ParseGroupFile(path string) ([]Group, error) { if err != nil { return nil, err } + defer group.Close() return ParseGroup(group) } @@ -178,10 +177,7 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { // root:x:0:root // adm:x:4:root,adm,daemon p := Group{} - parseLine( - text, - &p.Name, &p.Pass, &p.Gid, &p.List, - ) + parseLine(text, &p.Name, &p.Pass, &p.Gid, &p.List) if filter == nil || filter(p) { out = append(out, p) @@ -192,9 +188,10 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { } type ExecUser struct { - Uid, Gid int - Sgids []int - Home string + Uid int + Gid int + Sgids []int + Home string } // GetExecUserPath is a wrapper for GetExecUser. It reads data from each of the @@ -241,10 +238,6 @@ func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath // 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) { - var ( - userArg, groupArg string - ) - if defaults == nil { defaults = new(ExecUser) } @@ -262,7 +255,8 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( user.Sgids = []int{} } - // allow for userArg to have either "user" syntax, or optionally "user:group" syntax + // Allow for userArg to have either "user" syntax, or optionally "user:group" syntax + var userArg, groupArg string parseLine(userSpec, &userArg, &groupArg) // Convert userArg and groupArg to be numeric, so we don't have to execute @@ -270,8 +264,10 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( 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 } @@ -282,11 +278,13 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( 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 %v: %v", userArg, err) + return nil, fmt.Errorf("unable to find user %s: %v", userArg, err) } var matchedUserName string @@ -340,10 +338,10 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( return nil, fmt.Errorf("unable to find groups for spec %v: %v", matchedUserName, err) } - haveGroup := groups != nil && len(groups) > 0 + // Only start modifying user.Gid if it is in explicit form. if groupArg != "" { - if haveGroup { - // if we found any group entries that matched our filter, let's take the first one as "correct" + if len(groups) > 0 { + // First match wins, even if there's more than one matching entry. user.Gid = groups[0].Gid } else if groupArg != "" { // If we can't find a group with the given name, the only other valid @@ -355,7 +353,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( } user.Gid = gidArg - // Ensure gid is inside gid range. + // Must be inside valid gid range. if user.Gid < minId || user.Gid > maxId { return nil, ErrRange } From 91968cce75057421cbf934d12767fe0d0a36dcf1 Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Tue, 27 Sep 2016 06:02:22 -0400 Subject: [PATCH 18/50] Fix TestGetAdditionalGroups on i686 Fixes: #941 Signed-off-by: Qiang Huang --- user/user_test.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/user/user_test.go b/user/user_test.go index 8cad2f55..24ee559e 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" "testing" + + "github.com/opencontainers/runc/libcontainer/utils" ) func TestUserParseLine(t *testing.T) { @@ -382,6 +384,12 @@ this is just some garbage data } func TestGetAdditionalGroups(t *testing.T) { + type foo struct { + groups []string + expected []int + hasError bool + } + const groupContent = ` root:x:0:root adm:x:43: @@ -389,11 +397,7 @@ grp:x:1234:root,adm adm:x:4343:root,adm-duplicate this is just some garbage data ` - tests := []struct { - groups []string - expected []int - hasError bool - }{ + tests := []foo{ { // empty group groups: []string{}, @@ -436,12 +440,15 @@ this is just some garbage data expected: nil, hasError: true, }, - { + } + + if utils.GetIntSize() > 4 { + tests = append(tests, foo{ // groups with too large id groups: []string{strconv.Itoa(1 << 31)}, expected: nil, hasError: true, - }, + }) } for _, test := range tests { From c006cd274f6e79601cd74bab7579ba8072e3806f Mon Sep 17 00:00:00 2001 From: Lei Jitang Date: Mon, 9 Jan 2017 01:56:14 -0500 Subject: [PATCH 19/50] Cleanup: remove redundant code Signed-off-by: Lei Jitang --- user/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/user.go b/user/user.go index 43fd39ef..f258d624 100644 --- a/user/user.go +++ b/user/user.go @@ -343,7 +343,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( if len(groups) > 0 { // First match wins, even if there's more than one matching entry. user.Gid = groups[0].Gid - } else if groupArg != "" { + } 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. From 6351002943aca38ca2bf73d3d84c82b3f89cadd4 Mon Sep 17 00:00:00 2001 From: Wang Long Date: Wed, 18 Jan 2017 17:18:22 +0800 Subject: [PATCH 20/50] user: fix the parameter error The parameters passed to `GetExecUser` is not correct. Consider the following code: ``` package main import ( "fmt" "io" "os" ) func main() { passwd, err := os.Open("/etc/passwd1") if err != nil { passwd = nil } else { defer passwd.Close() } err = GetUserPasswd(passwd) if err != nil { fmt.Printf("%#v\n", err) } } func GetUserPasswd(r io.Reader) error { if r == nil { return fmt.Errorf("nil source for passwd-formatted data") } else { fmt.Printf("r = %#v\n", r) } return nil } ``` If the file `/etc/passwd1` is not exist, we expect to return `nil source for passwd-formatted data` error, and in fact, the func `GetUserPasswd` return nil. The same logic exists in runc code. this patch fix it. Signed-off-by: Wang Long --- user/user.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/user/user.go b/user/user.go index f258d624..8962cab3 100644 --- a/user/user.go +++ b/user/user.go @@ -199,18 +199,16 @@ type ExecUser struct { // 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) { - passwd, err := os.Open(passwdPath) - if err != nil { - passwd = nil - } else { - defer passwd.Close() + var passwd, group io.Reader + + if passwdFile, err := os.Open(passwdPath); err == nil { + passwd = passwdFile + defer passwdFile.Close() } - group, err := os.Open(groupPath) - if err != nil { - group = nil - } else { - defer group.Close() + if groupFile, err := os.Open(groupPath); err == nil { + group = groupFile + defer groupFile.Close() } return GetExecUser(userSpec, defaults, passwd, group) @@ -433,9 +431,11 @@ func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, err // that opens the groupPath given and gives it as an argument to // GetAdditionalGroups. func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int, error) { - group, err := os.Open(groupPath) - if err == nil { - defer group.Close() + var group io.Reader + + if groupFile, err := os.Open(groupPath); err == nil { + group = groupFile + defer groupFile.Close() } return GetAdditionalGroups(additionalGroups, group) } From b0f54babf150adb33ffdf11aad0043cd84781e54 Mon Sep 17 00:00:00 2001 From: Christy Perez Date: Tue, 9 May 2017 17:38:27 -0400 Subject: [PATCH 21/50] Move libcontainer to x/sys/unix Since syscall is outdated and broken for some architectures, use x/sys/unix instead. There are still some dependencies on the syscall package that will remain in syscall for the forseeable future: Errno Signal SysProcAttr Additionally: - os still uses syscall, so it needs to be kept for anything returning *os.ProcessState, such as process.Wait. Signed-off-by: Christy Perez --- user/lookup.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/user/lookup.go b/user/lookup.go index ab1439f3..bf491c89 100644 --- a/user/lookup.go +++ b/user/lookup.go @@ -2,7 +2,8 @@ package user import ( "errors" - "syscall" + + "golang.org/x/sys/unix" ) var ( @@ -40,7 +41,7 @@ func lookupUser(filter func(u User) bool) (User, error) { // 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(syscall.Getuid()) + return LookupUid(unix.Getuid()) } // LookupUser looks up a user by their username in /etc/passwd. If the user @@ -88,7 +89,7 @@ func lookupGroup(filter func(g Group) bool) (Group, error) { // 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(syscall.Getgid()) + return LookupGid(unix.Getgid()) } // LookupGroup looks up a group by its name in /etc/group. If the group cannot From 8cb7a3ac48049e918e3374b42d71834ac10bec7f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 16 May 2017 13:45:21 +0200 Subject: [PATCH 22/50] libcontainer/user: add supplementary groups only for non-numeric users Signed-off-by: Valentin Rothberg --- user/user.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/user/user.go b/user/user.go index 8962cab3..2471535a 100644 --- a/user/user.go +++ b/user/user.go @@ -358,8 +358,8 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( // 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. + } else if len(groups) > 0 && uidErr != nil { + // Supplementary group ids only make sense if in the implicit form for non-numeric users. user.Sgids = make([]int, len(groups)) for i, group := range groups { user.Sgids[i] = group.Gid From b69bd1bb1adc99701491b009cd82f4b40f1727c9 Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Thu, 3 Aug 2017 11:28:56 -0700 Subject: [PATCH 23/50] Move user pkg unix specific calls to unix file Signed-off-by: Kenfe-Mickael Laventure --- user/lookup.go | 16 ---------------- user/lookup_unix.go | 16 ++++++++++++++++ user/lookup_unsupported.go | 19 ++++++++++++++++++- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/user/lookup.go b/user/lookup.go index bf491c89..95e9eebc 100644 --- a/user/lookup.go +++ b/user/lookup.go @@ -2,8 +2,6 @@ package user import ( "errors" - - "golang.org/x/sys/unix" ) var ( @@ -37,13 +35,6 @@ func lookupUser(filter func(u User) bool) (User, error) { return users[0], nil } -// 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()) -} - // 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. @@ -85,13 +76,6 @@ func lookupGroup(filter func(g Group) bool) (Group, error) { return groups[0], nil } -// 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()) -} - // 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. diff --git a/user/lookup_unix.go b/user/lookup_unix.go index 758b734c..c2bb9ec9 100644 --- a/user/lookup_unix.go +++ b/user/lookup_unix.go @@ -5,6 +5,8 @@ package user import ( "io" "os" + + "golang.org/x/sys/unix" ) // Unix-specific path to the passwd and group formatted files. @@ -28,3 +30,17 @@ func GetGroupPath() (string, error) { 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()) +} diff --git a/user/lookup_unsupported.go b/user/lookup_unsupported.go index 72179488..4a8d00ac 100644 --- a/user/lookup_unsupported.go +++ b/user/lookup_unsupported.go @@ -2,7 +2,10 @@ package user -import "io" +import ( + "io" + "syscall" +) func GetPasswdPath() (string, error) { return "", ErrUnsupported @@ -19,3 +22,17 @@ func GetGroupPath() (string, error) { func GetGroup() (io.ReadCloser, error) { return nil, ErrUnsupported } + +// 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(syscall.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(syscall.Getgid()) +} From 83ce7637863459c805c64b55865f04a7320b4d4d Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Fri, 4 Aug 2017 14:28:21 -0700 Subject: [PATCH 24/50] Revert "Merge pull request #1450 from vrothberg/sgid-non-numeric" This reverts commit be16efd31c1748d9203905dffe449c655791c6a9, reversing changes made to 51b501dab1889ca609db9c536ac976f0f53e7021. Signed-off-by: Kenfe-Mickael Laventure --- user/user.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/user/user.go b/user/user.go index 2471535a..8962cab3 100644 --- a/user/user.go +++ b/user/user.go @@ -358,8 +358,8 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( // Okay, so it's numeric. We can just roll with this. } - } else if len(groups) > 0 && uidErr != nil { - // Supplementary group ids only make sense if in the implicit form for non-numeric users. + } 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 From 5123b0af91adda71a874d7f892b2381bc430165d Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Thu, 16 Nov 2017 17:29:21 +0000 Subject: [PATCH 25/50] remove placeholder for non-linux platforms runc currently only support Linux platform, and since we dont intend to expose the support to other platform, removing all other platforms placeholder code. `libcontainer/configs` still being used in https://github.com/moby/moby/blob/master/daemon/daemon_windows.go so keeping it for now. After this, we probably should also rename files to drop linux suffices if possible. Signed-off-by: Daniel Dao --- user/lookup_unsupported.go | 38 -------------------------------------- 1 file changed, 38 deletions(-) delete mode 100644 user/lookup_unsupported.go diff --git a/user/lookup_unsupported.go b/user/lookup_unsupported.go deleted file mode 100644 index 4a8d00ac..00000000 --- a/user/lookup_unsupported.go +++ /dev/null @@ -1,38 +0,0 @@ -// +build !darwin,!dragonfly,!freebsd,!linux,!netbsd,!openbsd,!solaris - -package user - -import ( - "io" - "syscall" -) - -func GetPasswdPath() (string, error) { - return "", ErrUnsupported -} - -func GetPasswd() (io.ReadCloser, error) { - return nil, ErrUnsupported -} - -func GetGroupPath() (string, error) { - return "", ErrUnsupported -} - -func GetGroup() (io.ReadCloser, error) { - return nil, ErrUnsupported -} - -// 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(syscall.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(syscall.Getgid()) -} From 3d7cc23c209ef6427ee2e71226bde1b8c85f7273 Mon Sep 17 00:00:00 2001 From: Vincent Batts Date: Wed, 28 Feb 2018 14:14:24 -0500 Subject: [PATCH 26/50] libcontainer/user: platform dependent calls This rearranges a bit of the user and group lookup, such that only a basic subset is exposed. Signed-off-by: Vincent Batts --- user/lookup.go | 62 +++---------------------------------- user/lookup_unix.go | 70 ++++++++++++++++++++++++++++++++++++++++++ user/lookup_windows.go | 40 ++++++++++++++++++++++++ user/user.go | 40 ++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 58 deletions(-) create mode 100644 user/lookup_windows.go diff --git a/user/lookup.go b/user/lookup.go index 95e9eebc..6fd8dd0d 100644 --- a/user/lookup.go +++ b/user/lookup.go @@ -12,84 +12,30 @@ var ( ErrNoGroupEntries = errors.New("no matching entries in group file") ) -func lookupUser(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 -} - // 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 lookupUser(func(u User) bool { - return u.Name == username - }) + return lookupUser(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 lookupUser(func(u User) bool { - return u.Uid == uid - }) -} - -func lookupGroup(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 + return lookupUid(uid) } // 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 lookupGroup(func(g Group) bool { - return g.Name == groupname - }) + return lookupGroup(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 lookupGroup(func(g Group) bool { - return g.Gid == gid - }) + return lookupGid(gid) } diff --git a/user/lookup_unix.go b/user/lookup_unix.go index c2bb9ec9..c45e3004 100644 --- a/user/lookup_unix.go +++ b/user/lookup_unix.go @@ -15,6 +15,76 @@ const ( unixGroupPath = "/etc/group" ) +func lookupUser(username string) (User, error) { + return lookupUserFunc(func(u User) bool { + return u.Name == username + }) +} + +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 +} + +func lookupGroup(groupname string) (Group, error) { + return lookupGroupFunc(func(g Group) bool { + return g.Name == groupname + }) +} + +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 } diff --git a/user/lookup_windows.go b/user/lookup_windows.go new file mode 100644 index 00000000..65cd40e9 --- /dev/null +++ b/user/lookup_windows.go @@ -0,0 +1,40 @@ +// +build windows + +package user + +import ( + "fmt" + "os/user" +) + +func lookupUser(username string) (User, error) { + u, err := user.Lookup(username) + if err != nil { + return User{}, err + } + return userFromOS(u) +} + +func lookupUid(uid int) (User, error) { + u, err := user.LookupId(fmt.Sprintf("%d", uid)) + if err != nil { + return User{}, err + } + return userFromOS(u) +} + +func lookupGroup(groupname string) (Group, error) { + g, err := user.LookupGroup(groupname) + if err != nil { + return Group{}, err + } + return groupFromOS(g) +} + +func lookupGid(gid int) (Group, error) { + g, err := user.LookupGroupId(fmt.Sprintf("%d", gid)) + if err != nil { + return Group{}, err + } + return groupFromOS(g) +} diff --git a/user/user.go b/user/user.go index 8962cab3..93414516 100644 --- a/user/user.go +++ b/user/user.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "os/user" "strconv" "strings" ) @@ -28,6 +29,28 @@ type User struct { Shell string } +// userFromOS converts an os/user.(*User) to local User +// +// (This does not include Pass, Shell or Gecos) +func userFromOS(u *user.User) (User, error) { + newUser := User{ + Name: u.Username, + Home: u.HomeDir, + } + id, err := strconv.Atoi(u.Uid) + if err != nil { + return newUser, err + } + newUser.Uid = id + + id, err = strconv.Atoi(u.Gid) + if err != nil { + return newUser, err + } + newUser.Gid = id + return newUser, nil +} + type Group struct { Name string Pass string @@ -35,6 +58,23 @@ type Group struct { List []string } +// groupFromOS converts an os/user.(*Group) to local Group +// +// (This does not include Pass, Shell or Gecos) +func groupFromOS(g *user.Group) (Group, error) { + newGroup := Group{ + Name: g.Name, + } + + id, err := strconv.Atoi(g.Gid) + if err != nil { + return newGroup, err + } + newGroup.Gid = id + + return newGroup, nil +} + func parseLine(line string, v ...interface{}) { if line == "" { return From 0eb4b05b2927fcf983abd0c763d65f81e8d55194 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 16 Jan 2018 15:11:56 +0900 Subject: [PATCH 27/50] libcontainer: add parser for /etc/sub{u,g}id and /proc/PID/{u,g}id_map Signed-off-by: Akihiro Suda --- user/lookup_unix.go | 26 +++++++++ user/user.go | 129 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 153 insertions(+), 2 deletions(-) diff --git a/user/lookup_unix.go b/user/lookup_unix.go index c45e3004..c1e634c9 100644 --- a/user/lookup_unix.go +++ b/user/lookup_unix.go @@ -114,3 +114,29 @@ func CurrentUser() (User, error) { func CurrentGroup() (Group, error) { return LookupGid(unix.Getgid()) } + +func CurrentUserSubUIDs() ([]SubID, error) { + u, err := CurrentUser() + if err != nil { + return nil, err + } + return ParseSubIDFileFilter("/etc/subuid", + func(entry SubID) bool { return entry.Name == u.Name }) +} + +func CurrentGroupSubGIDs() ([]SubID, error) { + g, err := CurrentGroup() + if err != nil { + return nil, err + } + return ParseSubIDFileFilter("/etc/subgid", + func(entry SubID) bool { return entry.Name == g.Name }) +} + +func CurrentProcessUIDMap() ([]IDMap, error) { + return ParseIDMapFile("/proc/self/uid_map") +} + +func CurrentProcessGIDMap() ([]IDMap, error) { + return ParseIDMapFile("/proc/self/gid_map") +} diff --git a/user/user.go b/user/user.go index 93414516..37993da8 100644 --- a/user/user.go +++ b/user/user.go @@ -75,12 +75,29 @@ func groupFromOS(g *user.Group) (Group, error) { return newGroup, nil } +// SubID represents an entry in /etc/sub{u,g}id +type SubID struct { + Name string + SubID int + Count int +} + +// IDMap represents an entry in /proc/PID/{u,g}id_map +type IDMap struct { + ID int + ParentID int + Count int +} + func parseLine(line string, v ...interface{}) { - if line == "" { + parseParts(strings.Split(line, ":"), v...) +} + +func parseParts(parts []string, v ...interface{}) { + if len(parts) == 0 { return } - parts := strings.Split(line, ":") for i, p := range parts { // Ignore cases where we don't have enough fields to populate the arguments. // Some configuration files like to misbehave. @@ -479,3 +496,111 @@ func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int } 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, fmt.Errorf("nil source for subid-formatted data") + } + + var ( + s = bufio.NewScanner(r) + out = []SubID{} + ) + + for s.Scan() { + if err := s.Err(); err != nil { + return nil, err + } + + line := strings.TrimSpace(s.Text()) + if line == "" { + continue + } + + // see: man 5 subuid + p := SubID{} + parseLine(line, &p.Name, &p.SubID, &p.Count) + + if filter == nil || filter(p) { + out = append(out, p) + } + } + + return out, nil +} + +func ParseIDMapFile(path string) ([]IDMap, error) { + r, err := os.Open(path) + if err != nil { + return nil, err + } + defer r.Close() + return ParseIDMap(r) +} + +func ParseIDMap(r io.Reader) ([]IDMap, error) { + return ParseIDMapFilter(r, nil) +} + +func ParseIDMapFileFilter(path string, filter func(IDMap) bool) ([]IDMap, error) { + r, err := os.Open(path) + if err != nil { + return nil, err + } + defer r.Close() + return ParseIDMapFilter(r, filter) +} + +func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) { + if r == nil { + return nil, fmt.Errorf("nil source for idmap-formatted data") + } + + var ( + s = bufio.NewScanner(r) + out = []IDMap{} + ) + + for s.Scan() { + if err := s.Err(); err != nil { + return nil, err + } + + line := strings.TrimSpace(s.Text()) + if line == "" { + continue + } + + // see: man 7 user_namespaces + p := IDMap{} + parseParts(strings.Fields(line), &p.ID, &p.ParentID, &p.Count) + + if filter == nil || filter(p) { + out = append(out, p) + } + } + + return out, nil +} From bd84305fd930da6cbc1ca3a76dd69b15ce931e24 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 14 Jun 2018 17:59:28 +0000 Subject: [PATCH 28/50] libcontainer: fix compilation on GOARCH=arm GOARM=6 (32 bits) This fixes the following compilation error on 32bit ARM: ``` $ GOARCH=arm GOARCH=6 go build ./libcontainer/system/ libcontainer/system/linux.go:119:89: constant 4294967295 overflows int ``` Signed-off-by: Tibor Vass --- user/user.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/user/user.go b/user/user.go index 37993da8..7b912bbf 100644 --- a/user/user.go +++ b/user/user.go @@ -78,15 +78,15 @@ func groupFromOS(g *user.Group) (Group, error) { // SubID represents an entry in /etc/sub{u,g}id type SubID struct { Name string - SubID int - Count int + SubID int64 + Count int64 } // IDMap represents an entry in /proc/PID/{u,g}id_map type IDMap struct { - ID int - ParentID int - Count int + ID int64 + ParentID int64 + Count int64 } func parseLine(line string, v ...interface{}) { @@ -113,6 +113,8 @@ func parseParts(parts []string, v ...interface{}) { case *int: // "numbers", with conversion errors ignored because of some misbehaving configuration files. *e, _ = strconv.Atoi(p) + case *int64: + *e, _ = strconv.ParseInt(p, 10, 64) case *[]string: // Comma-separated lists. if p != "" { @@ -122,7 +124,7 @@ func parseParts(parts []string, v ...interface{}) { } default: // Someone goof'd when writing code using this function. Scream so they can hear us. - panic(fmt.Sprintf("parseLine only accepts {*string, *int, *[]string} as arguments! %#v is not a pointer!", e)) + panic(fmt.Sprintf("parseLine only accepts {*string, *int, *int64, *[]string} as arguments! %#v is not a pointer!", e)) } } } From c608377df2901063dd760e4034da7af5bbc59b7f Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Wed, 29 Aug 2018 07:32:54 +0900 Subject: [PATCH 29/50] libcontainer: CurrentGroupSubGIDs -> CurrentUserSubGIDs subgid is defined per user, not group (see subgid(5)) This commit also adds support for specifying subuid owner with a numeric UID. Signed-off-by: Akihiro Suda --- user/lookup_unix.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/user/lookup_unix.go b/user/lookup_unix.go index c1e634c9..92b5ae8d 100644 --- a/user/lookup_unix.go +++ b/user/lookup_unix.go @@ -5,6 +5,7 @@ package user import ( "io" "os" + "strconv" "golang.org/x/sys/unix" ) @@ -115,22 +116,23 @@ func CurrentGroup() (Group, error) { return LookupGid(unix.Getgid()) } -func CurrentUserSubUIDs() ([]SubID, error) { +func currentUserSubIDs(fileName string) ([]SubID, error) { u, err := CurrentUser() if err != nil { return nil, err } - return ParseSubIDFileFilter("/etc/subuid", - func(entry SubID) bool { return entry.Name == u.Name }) + filter := func(entry SubID) bool { + return entry.Name == u.Name || entry.Name == strconv.Itoa(u.Uid) + } + return ParseSubIDFileFilter(fileName, filter) } -func CurrentGroupSubGIDs() ([]SubID, error) { - g, err := CurrentGroup() - if err != nil { - return nil, err - } - return ParseSubIDFileFilter("/etc/subgid", - func(entry SubID) bool { return entry.Name == g.Name }) +func CurrentUserSubUIDs() ([]SubID, error) { + return currentUserSubIDs("/etc/subuid") +} + +func CurrentUserSubGIDs() ([]SubID, error) { + return currentUserSubIDs("/etc/subgid") } func CurrentProcessUIDMap() ([]IDMap, error) { From afa09178ce93690c177bd932d843ddbb7bec276b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 27 Mar 2020 00:10:53 -0700 Subject: [PATCH 30/50] Nit: fix use of bufio.Scanner.Err The Err() method should be called after the Scan() loop, not inside it. Found by git grep -A3 -F '.Scan()' | grep Err Signed-off-by: Kir Kolyshkin --- user/user.go | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/user/user.go b/user/user.go index 7b912bbf..de30982b 100644 --- a/user/user.go +++ b/user/user.go @@ -162,10 +162,6 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { ) for s.Scan() { - if err := s.Err(); err != nil { - return nil, err - } - line := strings.TrimSpace(s.Text()) if line == "" { continue @@ -183,6 +179,9 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { out = append(out, p) } } + if err := s.Err(); err != nil { + return nil, err + } return out, nil } @@ -221,10 +220,6 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { ) for s.Scan() { - if err := s.Err(); err != nil { - return nil, err - } - text := s.Text() if text == "" { continue @@ -242,6 +237,9 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { out = append(out, p) } } + if err := s.Err(); err != nil { + return nil, err + } return out, nil } @@ -532,10 +530,6 @@ func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) { ) for s.Scan() { - if err := s.Err(); err != nil { - return nil, err - } - line := strings.TrimSpace(s.Text()) if line == "" { continue @@ -549,6 +543,9 @@ func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) { out = append(out, p) } } + if err := s.Err(); err != nil { + return nil, err + } return out, nil } @@ -586,10 +583,6 @@ func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) { ) for s.Scan() { - if err := s.Err(); err != nil { - return nil, err - } - line := strings.TrimSpace(s.Text()) if line == "" { continue @@ -603,6 +596,9 @@ func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) { out = append(out, p) } } + if err := s.Err(); err != nil { + return nil, err + } return out, nil } From aeea88e1e0fd5f2b7e0cc5c71c7087bac89f0368 Mon Sep 17 00:00:00 2001 From: tjucoder Date: Sun, 5 Jul 2020 12:10:47 +0800 Subject: [PATCH 31/50] make sure pty.Close() will be called and fix comment Signed-off-by: tjucoder --- user/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/user.go b/user/user.go index de30982b..4b89dad7 100644 --- a/user/user.go +++ b/user/user.go @@ -60,7 +60,7 @@ type Group struct { // groupFromOS converts an os/user.(*Group) to local Group // -// (This does not include Pass, Shell or Gecos) +// (This does not include Pass or List) func groupFromOS(g *user.Group) (Group, error) { newGroup := Group{ Name: g.Name, From b3750afa0b2ae576d8c5d23e62b3f77db62f117e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 29 Sep 2020 13:38:06 +0200 Subject: [PATCH 32/50] use string-concatenation instead of sprintf for simple cases Signed-off-by: Sebastiaan van Stijn --- user/lookup_windows.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/user/lookup_windows.go b/user/lookup_windows.go index 65cd40e9..f19333e6 100644 --- a/user/lookup_windows.go +++ b/user/lookup_windows.go @@ -3,8 +3,8 @@ package user import ( - "fmt" "os/user" + "strconv" ) func lookupUser(username string) (User, error) { @@ -16,7 +16,7 @@ func lookupUser(username string) (User, error) { } func lookupUid(uid int) (User, error) { - u, err := user.LookupId(fmt.Sprintf("%d", uid)) + u, err := user.LookupId(strconv.Itoa(uid)) if err != nil { return User{}, err } @@ -32,7 +32,7 @@ func lookupGroup(groupname string) (Group, error) { } func lookupGid(gid int) (Group, error) { - g, err := user.LookupGroupId(fmt.Sprintf("%d", gid)) + g, err := user.LookupGroupId(strconv.Itoa(gid)) if err != nil { return Group{}, err } From 581a9d45ff8d629636d727cf9c15137ed288713b Mon Sep 17 00:00:00 2001 From: Amim Knabben Date: Fri, 2 Oct 2020 23:12:29 -0400 Subject: [PATCH 33/50] Fixing some lint issues Signed-off-by: Amim Knabben --- user/user_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/user/user_test.go b/user/user_test.go index 24ee559e..058ce64a 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -463,7 +463,7 @@ this is just some garbage data t.Errorf("Parse(%#v) has error %v", test, err) continue } - sort.Sort(sort.IntSlice(gids)) + sort.Ints(gids) if !reflect.DeepEqual(gids, test.expected) { t.Errorf("Gids(%v), expect %v from groups %v", gids, test.expected, test.groups) } @@ -499,7 +499,7 @@ func TestGetAdditionalGroupsNumeric(t *testing.T) { t.Errorf("Parse(%#v) has error %v", test, err) continue } - sort.Sort(sort.IntSlice(gids)) + sort.Ints(gids) if !reflect.DeepEqual(gids, test.expected) { t.Errorf("Gids(%v), expect %v from groups %v", gids, test.expected, test.groups) } From efcad6e77eec2ef1a8664e0a7a6ab6aae68b1502 Mon Sep 17 00:00:00 2001 From: Shengjing Zhu Date: Sat, 23 Jan 2021 22:53:32 +0800 Subject: [PATCH 34/50] Fix int overflow in test on 32 bit system Signed-off-by: Shengjing Zhu --- user/user.go | 4 ++-- user/user_test.go | 11 +++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/user/user.go b/user/user.go index 4b89dad7..a533bf5e 100644 --- a/user/user.go +++ b/user/user.go @@ -466,7 +466,7 @@ func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, err // 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.Atoi(ag) + gid, err := strconv.ParseInt(ag, 10, 64) if err != nil { return nil, fmt.Errorf("Unable to find group %s", ag) } @@ -474,7 +474,7 @@ func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, err if gid < minId || gid > maxId { return nil, ErrRange } - gidMap[gid] = struct{}{} + gidMap[int(gid)] = struct{}{} } } gids := []int{} diff --git a/user/user_test.go b/user/user_test.go index 058ce64a..23bf4667 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -7,8 +7,6 @@ import ( "strconv" "strings" "testing" - - "github.com/opencontainers/runc/libcontainer/utils" ) func TestUserParseLine(t *testing.T) { @@ -440,15 +438,12 @@ this is just some garbage data expected: nil, hasError: true, }, - } - - if utils.GetIntSize() > 4 { - tests = append(tests, foo{ + { // groups with too large id - groups: []string{strconv.Itoa(1 << 31)}, + groups: []string{strconv.FormatInt(1<<31, 10)}, expected: nil, hasError: true, - }) + }, } for _, test := range tests { From c49050c1cd06b4e379a50ee9d038d90fbd78e89f Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Mon, 8 Mar 2021 21:41:52 +0000 Subject: [PATCH 35/50] Move fuzzers upstream Signed-off-by: AdamKorcz --- user/user_fuzzer.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 user/user_fuzzer.go diff --git a/user/user_fuzzer.go b/user/user_fuzzer.go new file mode 100644 index 00000000..8c9bb5df --- /dev/null +++ b/user/user_fuzzer.go @@ -0,0 +1,42 @@ +// +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 +} From 7736478fa044d1d1d7a15afcc760448fd575007a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 30 Nov 2020 10:33:47 -0800 Subject: [PATCH 36/50] libct/user: rm windows code Commit f1e605bf added these two functions, but they are only used from Windows code. The v1 of this patch moved these functions to _windows.go file, but after some discussion we decided to drop windows code altogether, so this is what this patch now does. This fixes > libcontainer/user/user.go:64:6: func `groupFromOS` is unused (unused) > libcontainer/user/user.go:35:6: func `userFromOS` is unused (unused) Signed-off-by: Kir Kolyshkin --- user/lookup_windows.go | 40 ---------------------------------------- user/user.go | 40 ---------------------------------------- 2 files changed, 80 deletions(-) delete mode 100644 user/lookup_windows.go diff --git a/user/lookup_windows.go b/user/lookup_windows.go deleted file mode 100644 index f19333e6..00000000 --- a/user/lookup_windows.go +++ /dev/null @@ -1,40 +0,0 @@ -// +build windows - -package user - -import ( - "os/user" - "strconv" -) - -func lookupUser(username string) (User, error) { - u, err := user.Lookup(username) - if err != nil { - return User{}, err - } - return userFromOS(u) -} - -func lookupUid(uid int) (User, error) { - u, err := user.LookupId(strconv.Itoa(uid)) - if err != nil { - return User{}, err - } - return userFromOS(u) -} - -func lookupGroup(groupname string) (Group, error) { - g, err := user.LookupGroup(groupname) - if err != nil { - return Group{}, err - } - return groupFromOS(g) -} - -func lookupGid(gid int) (Group, error) { - g, err := user.LookupGroupId(strconv.Itoa(gid)) - if err != nil { - return Group{}, err - } - return groupFromOS(g) -} diff --git a/user/user.go b/user/user.go index a533bf5e..90eb4b0b 100644 --- a/user/user.go +++ b/user/user.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "os/user" "strconv" "strings" ) @@ -29,28 +28,6 @@ type User struct { Shell string } -// userFromOS converts an os/user.(*User) to local User -// -// (This does not include Pass, Shell or Gecos) -func userFromOS(u *user.User) (User, error) { - newUser := User{ - Name: u.Username, - Home: u.HomeDir, - } - id, err := strconv.Atoi(u.Uid) - if err != nil { - return newUser, err - } - newUser.Uid = id - - id, err = strconv.Atoi(u.Gid) - if err != nil { - return newUser, err - } - newUser.Gid = id - return newUser, nil -} - type Group struct { Name string Pass string @@ -58,23 +35,6 @@ type Group struct { List []string } -// groupFromOS converts an os/user.(*Group) to local Group -// -// (This does not include Pass or List) -func groupFromOS(g *user.Group) (Group, error) { - newGroup := Group{ - Name: g.Name, - } - - id, err := strconv.Atoi(g.Gid) - if err != nil { - return newGroup, err - } - newGroup.Gid = id - - return newGroup, nil -} - // SubID represents an entry in /etc/sub{u,g}id type SubID struct { Name string From 50e9abca92a5eb7fab039bd681c5c89a2bde17b7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 14 Mar 2021 19:38:37 +0100 Subject: [PATCH 37/50] libcontainer/user: fix windows compile error Move the unix-specific code to a file that's not compiled on Windows. Some of the errors (ErrUnsupported, ErrNoPasswdEntries, ErrNoGroupEntries) are used in other parts of the code, so are moved to a non-platform specific file. Most of "user" is probably not useful on Windows, although it's possible that Windows code may have to parse a passwd file, so leaving that code for now. Signed-off-by: Sebastiaan van Stijn --- user/lookup.go | 41 ----------------------------------------- user/lookup_unix.go | 20 ++++++++++++++++---- user/user.go | 8 ++++++++ 3 files changed, 24 insertions(+), 45 deletions(-) delete mode 100644 user/lookup.go diff --git a/user/lookup.go b/user/lookup.go deleted file mode 100644 index 6fd8dd0d..00000000 --- a/user/lookup.go +++ /dev/null @@ -1,41 +0,0 @@ -package user - -import ( - "errors" -) - -var ( - // The current operating system does not provide the required data for user lookups. - ErrUnsupported = errors.New("user lookup: operating system does not provide passwd-formatted data") - // No matching entries found in file. - ErrNoPasswdEntries = errors.New("no matching entries in passwd file") - ErrNoGroupEntries = errors.New("no matching entries in group file") -) - -// 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 lookupUser(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 lookupUid(uid) -} - -// 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 lookupGroup(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 lookupGid(gid) -} diff --git a/user/lookup_unix.go b/user/lookup_unix.go index 92b5ae8d..967717a1 100644 --- a/user/lookup_unix.go +++ b/user/lookup_unix.go @@ -16,13 +16,19 @@ const ( unixGroupPath = "/etc/group" ) -func lookupUser(username string) (User, error) { +// 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 }) } -func lookupUid(uid int) (User, error) { +// 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 }) @@ -51,13 +57,19 @@ func lookupUserFunc(filter func(u User) bool) (User, error) { return users[0], nil } -func lookupGroup(groupname string) (Group, error) { +// 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 }) } -func lookupGid(gid int) (Group, error) { +// 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 }) diff --git a/user/user.go b/user/user.go index 90eb4b0b..68da4400 100644 --- a/user/user.go +++ b/user/user.go @@ -2,6 +2,7 @@ package user import ( "bufio" + "errors" "fmt" "io" "os" @@ -15,6 +16,13 @@ const ( ) var ( + // The current operating system does not provide the required data for user lookups. + ErrUnsupported = errors.New("user lookup: operating system does not provide passwd-formatted data") + + // No matching entries found in file. + ErrNoPasswdEntries = errors.New("no matching entries in passwd file") + ErrNoGroupEntries = errors.New("no matching entries in group file") + ErrRange = fmt.Errorf("uids and gids must be in range %d-%d", minId, maxId) ) From 48fed46daf591a785d8298ceb289957dca10a48a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 14 Mar 2021 19:41:55 +0100 Subject: [PATCH 38/50] libcontainer/user: remove outdated MAINTAINERS file Signed-off-by: Sebastiaan van Stijn --- user/MAINTAINERS | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 user/MAINTAINERS diff --git a/user/MAINTAINERS b/user/MAINTAINERS deleted file mode 100644 index edbe2006..00000000 --- a/user/MAINTAINERS +++ /dev/null @@ -1,2 +0,0 @@ -Tianon Gravi (@tianon) -Aleksa Sarai (@cyphar) From 3e3208cdc20af609c5516faff18a4ac49dfd9da2 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 1 Jun 2021 11:56:21 -0700 Subject: [PATCH 39/50] Use gofumpt to format code gofumpt (mvdan.cc/gofumpt) is a fork of gofmt with stricter rules. Brought to you by git ls-files \*.go | grep -v ^vendor/ | xargs gofumpt -s -w Looking at the diff, all these changes make sense. Also, replace gofmt with gofumpt in golangci.yml. Signed-off-by: Kir Kolyshkin --- user/user.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/user/user.go b/user/user.go index 68da4400..110860b4 100644 --- a/user/user.go +++ b/user/user.go @@ -12,7 +12,7 @@ import ( const ( minId = 0 - maxId = 1<<31 - 1 //for 32-bit systems compatibility + maxId = 1<<31 - 1 // for 32-bit systems compatibility ) var ( @@ -401,7 +401,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( // 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) { - var groups = []Group{} + groups := []Group{} if group != nil { var err error groups, err = ParseGroupFilter(group, func(g Group) bool { From da08f0283c7df6f5638527b4b22264a7b3927065 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 27 May 2021 17:00:14 +0200 Subject: [PATCH 40/50] libcontainer/user: remove unused ErrUnsupported Signed-off-by: Sebastiaan van Stijn --- user/user.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/user/user.go b/user/user.go index 110860b4..5b013eb3 100644 --- a/user/user.go +++ b/user/user.go @@ -16,9 +16,6 @@ const ( ) var ( - // The current operating system does not provide the required data for user lookups. - ErrUnsupported = errors.New("user lookup: operating system does not provide passwd-formatted data") - // No matching entries found in file. ErrNoPasswdEntries = errors.New("no matching entries in passwd file") ErrNoGroupEntries = errors.New("no matching entries in group file") From edeacd3f08898c2b116b9c8f4688095cdb275f1c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 1 Jun 2021 13:09:03 +0200 Subject: [PATCH 41/50] libcontainer/user: fix capitalization (golint) Signed-off-by: Sebastiaan van Stijn --- user/user.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/user/user.go b/user/user.go index 5b013eb3..d2c16f7f 100644 --- a/user/user.go +++ b/user/user.go @@ -11,16 +11,17 @@ import ( ) const ( - minId = 0 - maxId = 1<<31 - 1 // for 32-bit systems compatibility + minID = 0 + maxID = 1<<31 - 1 // for 32-bit systems compatibility ) var ( - // No matching entries found in file. + // ErrNoPasswdEntries is returned if no matching entries were found in /etc/group. ErrNoPasswdEntries = errors.New("no matching entries in passwd file") - ErrNoGroupEntries = errors.New("no matching entries in group file") - - ErrRange = fmt.Errorf("uids and gids must be in range %d-%d", minId, maxId) + // 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 { @@ -325,7 +326,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( user.Uid = uidArg // Must be inside valid uid range. - if user.Uid < minId || user.Uid > maxId { + if user.Uid < minID || user.Uid > maxID { return nil, ErrRange } @@ -374,7 +375,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( user.Gid = gidArg // Must be inside valid gid range. - if user.Gid < minId || user.Gid > maxId { + if user.Gid < minID || user.Gid > maxID { return nil, ErrRange } @@ -436,7 +437,7 @@ func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, err return nil, fmt.Errorf("Unable to find group %s", ag) } // Ensure gid is inside gid range. - if gid < minId || gid > maxId { + if gid < minID || gid > maxID { return nil, ErrRange } gidMap[int(gid)] = struct{}{} From da5b9b2c862c4596f94e2c1cc4d2ec2bdf4a4647 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 8 Jun 2021 11:24:00 -0700 Subject: [PATCH 42/50] Replace fmt.Errorf w/o %-style to errors.New Using fmt.Errorf for errors that do not have %-style formatting directives is an overkill. Switch to errors.New. Found by git grep fmt.Errorf | grep -v ^vendor | grep -v '%' Signed-off-by: Kir Kolyshkin --- user/user.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/user/user.go b/user/user.go index d2c16f7f..78695801 100644 --- a/user/user.go +++ b/user/user.go @@ -119,7 +119,7 @@ func ParsePasswdFileFilter(path string, filter func(User) bool) ([]User, error) func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { if r == nil { - return nil, fmt.Errorf("nil source for passwd-formatted data") + return nil, errors.New("nil source for passwd-formatted data") } var ( @@ -177,7 +177,7 @@ func ParseGroupFileFilter(path string, filter func(Group) bool) ([]Group, error) func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { if r == nil { - return nil, fmt.Errorf("nil source for group-formatted data") + return nil, errors.New("nil source for group-formatted data") } var ( @@ -487,7 +487,7 @@ func ParseSubIDFileFilter(path string, filter func(SubID) bool) ([]SubID, error) func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) { if r == nil { - return nil, fmt.Errorf("nil source for subid-formatted data") + return nil, errors.New("nil source for subid-formatted data") } var ( @@ -540,7 +540,7 @@ func ParseIDMapFileFilter(path string, filter func(IDMap) bool) ([]IDMap, error) func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) { if r == nil { - return nil, fmt.Errorf("nil source for idmap-formatted data") + return nil, errors.New("nil source for idmap-formatted data") } var ( From 013c24434dd5fa94dee61fd68c70e783e25979d7 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 8 Jun 2021 20:05:54 -0700 Subject: [PATCH 43/50] *: fmt.Errorf: use %w when appropriate This should result in no change when the error is printed, but make the errors returned unwrappable, meaning errors.As and errors.Is will work. Signed-off-by: Kir Kolyshkin --- user/user.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/user/user.go b/user/user.go index 78695801..998daab0 100644 --- a/user/user.go +++ b/user/user.go @@ -305,7 +305,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( if userArg == "" { userArg = strconv.Itoa(user.Uid) } - return nil, fmt.Errorf("unable to find user %s: %v", userArg, err) + return nil, fmt.Errorf("unable to find user %s: %w", userArg, err) } var matchedUserName string @@ -321,7 +321,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( if uidErr != nil { // Not numeric. - return nil, fmt.Errorf("unable to find user %s: %v", userArg, ErrNoPasswdEntries) + return nil, fmt.Errorf("unable to find user %s: %w", userArg, ErrNoPasswdEntries) } user.Uid = uidArg @@ -356,7 +356,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( return g.Name == groupArg }) if err != nil && group != nil { - return nil, fmt.Errorf("unable to find groups for spec %v: %v", matchedUserName, err) + 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. @@ -370,7 +370,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( if gidErr != nil { // Not numeric. - return nil, fmt.Errorf("unable to find group %s: %v", groupArg, ErrNoGroupEntries) + return nil, fmt.Errorf("unable to find group %s: %w", groupArg, ErrNoGroupEntries) } user.Gid = gidArg @@ -411,7 +411,7 @@ func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, err return false }) if err != nil { - return nil, fmt.Errorf("Unable to find additional groups %v: %v", additionalGroups, err) + return nil, fmt.Errorf("Unable to find additional groups %v: %w", additionalGroups, err) } } @@ -434,7 +434,8 @@ func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, err if !found { gid, err := strconv.ParseInt(ag, 10, 64) if err != nil { - return nil, fmt.Errorf("Unable to find group %s", ag) + // 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 { From 02cfd3c0e26944398c477c11c0544800383abf42 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 2 Jul 2021 12:12:25 -0700 Subject: [PATCH 44/50] libct/user: use []byte more, avoid allocations Every []byte to string conversion results in a new allocation. Avoid some by using []byte more. Signed-off-by: Kir Kolyshkin --- user/user.go | 37 +++++++++++++++++++------------------ user/user_test.go | 16 ++++++++-------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/user/user.go b/user/user.go index 998daab0..dad473d8 100644 --- a/user/user.go +++ b/user/user.go @@ -2,6 +2,7 @@ package user import ( "bufio" + "bytes" "errors" "fmt" "io" @@ -55,11 +56,11 @@ type IDMap struct { Count int64 } -func parseLine(line string, v ...interface{}) { - parseParts(strings.Split(line, ":"), v...) +func parseLine(line []byte, v ...interface{}) { + parseParts(bytes.Split(line, []byte(":")), v...) } -func parseParts(parts []string, v ...interface{}) { +func parseParts(parts [][]byte, v ...interface{}) { if len(parts) == 0 { return } @@ -75,16 +76,16 @@ func parseParts(parts []string, v ...interface{}) { // This is legit. switch e := v[i].(type) { case *string: - *e = p + *e = string(p) case *int: // "numbers", with conversion errors ignored because of some misbehaving configuration files. - *e, _ = strconv.Atoi(p) + *e, _ = strconv.Atoi(string(p)) case *int64: - *e, _ = strconv.ParseInt(p, 10, 64) + *e, _ = strconv.ParseInt(string(p), 10, 64) case *[]string: // Comma-separated lists. - if p != "" { - *e = strings.Split(p, ",") + if len(p) != 0 { + *e = strings.Split(string(p), ",") } else { *e = []string{} } @@ -128,8 +129,8 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { ) for s.Scan() { - line := strings.TrimSpace(s.Text()) - if line == "" { + line := bytes.TrimSpace(s.Bytes()) + if len(line) == 0 { continue } @@ -186,8 +187,8 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { ) for s.Scan() { - text := s.Text() - if text == "" { + text := s.Bytes() + if len(text) == 0 { continue } @@ -278,7 +279,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( // Allow for userArg to have either "user" syntax, or optionally "user:group" syntax var userArg, groupArg string - parseLine(userSpec, &userArg, &groupArg) + 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. @@ -497,8 +498,8 @@ func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) { ) for s.Scan() { - line := strings.TrimSpace(s.Text()) - if line == "" { + line := bytes.TrimSpace(s.Bytes()) + if len(line) == 0 { continue } @@ -550,14 +551,14 @@ func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) { ) for s.Scan() { - line := strings.TrimSpace(s.Text()) - if line == "" { + line := bytes.TrimSpace(s.Bytes()) + if len(line) == 0 { continue } // see: man 7 user_namespaces p := IDMap{} - parseParts(strings.Fields(line), &p.ID, &p.ParentID, &p.Count) + parseParts(bytes.Fields(line), &p.ID, &p.ParentID, &p.Count) if filter == nil || filter(p) { out = append(out, p) diff --git a/user/user_test.go b/user/user_test.go index 23bf4667..e1501c85 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -16,42 +16,42 @@ func TestUserParseLine(t *testing.T) { d int ) - parseLine("", &a, &b) + parseLine([]byte(""), &a, &b) if a != "" || b != "" { t.Fatalf("a and b should be empty ('%v', '%v')", a, b) } - parseLine("a", &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("bad boys:corny cows", &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("", &c) + parseLine([]byte(""), &c) if len(c) != 0 { t.Fatalf("c should be empty (%#v)", c) } - parseLine("d,e,f:g:h:i,j,k", &c, &a, &b, &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("::::::::::", &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("not a number", &d) + parseLine([]byte("not a number"), &d) if d != 0 { t.Fatalf("d should be 0 (%v)", d) } - parseLine("b:12:c", &a, &d, &b) + 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) } From a04d4c788aa8cda1404428c8b1c019e566c5f1e5 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 2 Jul 2021 12:15:44 -0700 Subject: [PATCH 45/50] libct/user: ParseGroupFilter: use TrimSpace Same as in other places (other parsers here, as well as golang os/user parser and glibc parser all tolerate extra space at BOL and EOL). Signed-off-by: Kir Kolyshkin --- user/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/user.go b/user/user.go index dad473d8..d1601d67 100644 --- a/user/user.go +++ b/user/user.go @@ -187,7 +187,7 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { ) for s.Scan() { - text := s.Bytes() + text := bytes.TrimSpace(s.Bytes()) if len(text) == 0 { continue } From 330eb4cf33e9fe3b801a8bd2e6e9cc2bd686db77 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 2 Jul 2021 12:52:23 -0700 Subject: [PATCH 46/50] libct/user: fix parsing long /etc/group lines Lines in /etc/group longer than 64 characters breaks the current implementation of group parser. This is caused by bufio.Scanner buffer limit. Fix by re-using the fix for a similar problem in golang os/user, namely https://go-review.googlesource.com/c/go/+/283601. Add some tests. Co-authored-by: Andrey Bokhanko Signed-off-by: Kir Kolyshkin --- user/user.go | 59 ++++++++++++++++++++++++++++++++++++----------- user/user_test.go | 44 ++++++++++++++++++++++++++++------- 2 files changed, 82 insertions(+), 21 deletions(-) diff --git a/user/user.go b/user/user.go index d1601d67..2473c5ea 100644 --- a/user/user.go +++ b/user/user.go @@ -180,15 +180,53 @@ 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() - var ( - s = bufio.NewScanner(r) - out = []Group{} - ) + if err != nil { + // We should return no error if EOF is reached + // without a match. + if err == io.EOF { //nolint:errorlint // comparison with io.EOF is legit, https://github.com/polyfloyd/go-errorlint/pull/12 + err = nil + } + return out, err + } - for s.Scan() { - text := bytes.TrimSpace(s.Bytes()) - if len(text) == 0 { + // 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 } @@ -198,17 +236,12 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { // root:x:0:root // adm:x:4:root,adm,daemon p := Group{} - parseLine(text, &p.Name, &p.Pass, &p.Gid, &p.List) + parseLine(wholeLine, &p.Name, &p.Pass, &p.Gid, &p.List) if filter == nil || filter(p) { out = append(out, p) } } - if err := s.Err(); err != nil { - return nil, err - } - - return out, nil } type ExecUser struct { diff --git a/user/user_test.go b/user/user_test.go index e1501c85..c0c762d6 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -1,6 +1,7 @@ package user import ( + "fmt" "io" "reflect" "sort" @@ -82,12 +83,12 @@ func TestUserParseGroup(t *testing.T) { root:x:0:root adm:x:4:root,adm,daemon this is just some garbage data -`), nil) +`+largeGroup()), nil) if err != nil { t.Fatalf("Unexpected error: %v", err) } - if len(groups) != 3 { - t.Fatalf("Expected 3 groups, got %v", len(groups)) + 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)) @@ -103,16 +104,18 @@ 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 ` - const groupContent = ` + groupContent := ` root:x:0:root adm:x:43: -grp:x:1234:root,adm +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, @@ -216,6 +219,16 @@ this is just some garbage data 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 { @@ -388,13 +401,13 @@ func TestGetAdditionalGroups(t *testing.T) { hasError bool } - const groupContent = ` + 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 @@ -444,6 +457,11 @@ this is just some garbage data expected: nil, hasError: true, }, + { + // group with very long list of users + groups: []string{"largegroup"}, + expected: []int{1000}, + }, } for _, test := range tests { @@ -500,3 +518,13 @@ func TestGetAdditionalGroupsNumeric(t *testing.T) { } } } + +// 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() +} From 9f62d16eb4ec713fad31c46482bbc99f4400d955 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 30 Aug 2021 20:58:22 -0700 Subject: [PATCH 47/50] *: add go-1.17+ go:build tags Go 1.17 introduce this new (and better) way to specify build tags. For more info, see https://golang.org/design/draft-gobuild. As a way to seamlessly switch from old to new build tags, gofmt (and gopls) from go 1.17 adds the new tags along with the old ones. Later, when go < 1.17 is no longer supported, the old build tags can be removed. Now, as I started to use latest gopls (v0.7.1), it adds these tags while I edit. Rather than to randomly add new build tags, I guess it is better to do it once for all files. Mind that previous commits removed some tags that were useless, so this one only touches packages that can at least be built on non-linux. Brought to you by go1.17 fmt ./... Signed-off-by: Kir Kolyshkin --- user/lookup_unix.go | 1 + user/user_fuzzer.go | 1 + 2 files changed, 2 insertions(+) diff --git a/user/lookup_unix.go b/user/lookup_unix.go index 967717a1..f95c1409 100644 --- a/user/lookup_unix.go +++ b/user/lookup_unix.go @@ -1,3 +1,4 @@ +//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris // +build darwin dragonfly freebsd linux netbsd openbsd solaris package user diff --git a/user/user_fuzzer.go b/user/user_fuzzer.go index 8c9bb5df..e018eae6 100644 --- a/user/user_fuzzer.go +++ b/user/user_fuzzer.go @@ -1,3 +1,4 @@ +//go:build gofuzz // +build gofuzz package user From bad3316ea9836608d05d829fbdb3baab480e7b90 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 3 Aug 2022 15:10:25 -0700 Subject: [PATCH 48/50] libct: fixes for godoc 1.19 Since Go 1.19, godoc recognizes lists, code blocks, headings etc. It also reformats the sources making it more apparent that these features are used. Fix a few places where it misinterpreted the formatting (such as indented vs unindented), and format the result using the gofumpt from HEAD, which already incorporates gofmt 1.19 changes. Some more fixes (and enhancements) might be required. Signed-off-by: Kir Kolyshkin --- user/user.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/user/user.go b/user/user.go index 2473c5ea..a1e21668 100644 --- a/user/user.go +++ b/user/user.go @@ -280,13 +280,13 @@ func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath // 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" +// - "" +// - "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 From 9820685ec1f7369857e2d13c0c3bcb7e2efb6568 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 28 Jun 2023 13:20:09 -0700 Subject: [PATCH 49/50] ci: bump golangci-lint, remove fixed exception The exception was fixed by https://github.com/polyfloyd/go-errorlint/pull/12 which eventually made its way into golangci-lint. Signed-off-by: Kir Kolyshkin --- user/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/user.go b/user/user.go index a1e21668..984466d1 100644 --- a/user/user.go +++ b/user/user.go @@ -201,7 +201,7 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { if err != nil { // We should return no error if EOF is reached // without a match. - if err == io.EOF { //nolint:errorlint // comparison with io.EOF is legit, https://github.com/polyfloyd/go-errorlint/pull/12 + if err == io.EOF { err = nil } return out, err From 0123f325138006548a9c17ccd7e09dd7cd1cc860 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 16 Sep 2023 12:42:18 +0200 Subject: [PATCH 50/50] user: add go.mod and integrate in CI Signed-off-by: Sebastiaan van Stijn --- Makefile | 2 +- user/go.mod | 5 +++++ user/go.sum | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 user/go.mod create mode 100644 user/go.sum diff --git a/Makefile b/Makefile index 80c6ee9d..ba4ddc4c 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -PACKAGES ?= mountinfo mount sequential signal symlink +PACKAGES ?= mountinfo mount sequential signal symlink user BINDIR ?= _build/bin CROSS ?= linux/arm linux/arm64 linux/ppc64le linux/s390x \ freebsd/amd64 openbsd/amd64 darwin/amd64 darwin/arm64 windows/amd64 diff --git a/user/go.mod b/user/go.mod new file mode 100644 index 00000000..1c62fac9 --- /dev/null +++ b/user/go.mod @@ -0,0 +1,5 @@ +module github.com/moby/sys/user + +go 1.17 + +require golang.org/x/sys v0.1.0 diff --git a/user/go.sum b/user/go.sum new file mode 100644 index 00000000..b69ea857 --- /dev/null +++ b/user/go.sum @@ -0,0 +1,2 @@ +golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=