jasder
/
runc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merge pull request #708 from cyphar/treat-numric-ids-properly 

libcontainer: user: always treat numeric ids numerically
This commit is contained in:
Aleksa Sarai 2016-03-31 07:52:26 +11:00
parent 8fa5343b00 3cfff676b1
commit 0a5293fa4e
2 changed files with 116 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

153
libcontainer/user/user.go
View File

@ -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
@ -235,12 +232,12 @@ 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 {
defaults = new(ExecUser)
}
@ -258,87 +255,113 @@ 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
// Atoi *twice* for each iteration over lines.
uidArg, uidErr := strconv.Atoi(userArg)
gidArg, gidErr := strconv.Atoi(groupArg)
// Find the matching user.
users, err := ParsePasswdFilter(passwd, func(u User) bool {
if userArg == "" {
// Default to current state of the user.
return u.Uid == user.Uid
}
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 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)
}
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
}
// Check if user is a member.
for _, u := range g.List {
if u == name {
return true
// 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
}
return false
if gidErr == nil {
// If the groupArg is numeric, always treat it as a GID.
return gidArg == g.Gid
}
return g.Name == groupArg
})
if err != nil && group != nil {
return nil, fmt.Errorf("Unable to find groups for 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
// 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 {
// 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.
// Ensure gid is inside gid range.
if gidErr != nil {
// Not numeric.
return nil, fmt.Errorf("unable to find group %s: %v", groupArg, ErrNoGroupEntries)
}
user.Gid = gidArg
// Must be inside valid 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

28
libcontainer/user/user_test.go
View File

@ -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 {