diff --git a/configs/config.go b/configs/config.go index 7275b642..04ea91ff 100644 --- a/configs/config.go +++ b/configs/config.go @@ -119,7 +119,7 @@ type Config struct { // AdditionalGroups specifies the gids that should be added to supplementary groups // in addition to those that the user belongs to. - AdditionalGroups []int `json:"additional_groups"` + AdditionalGroups []string `json:"additional_groups"` // UidMappings is an array of User ID mappings for User Namespaces UidMappings []IDMap `json:"uid_mappings"` diff --git a/init_linux.go b/init_linux.go index 3eabe3cd..521de51c 100644 --- a/init_linux.go +++ b/init_linux.go @@ -177,10 +177,17 @@ func setupUser(config *initConfig) error { if err != nil { return err } - suppGroups := append(execUser.Sgids, config.Config.AdditionalGroups...) + + addGroups, err := user.GetAdditionalGroupsPath(config.Config.AdditionalGroups, groupPath) + if err != nil { + return err + } + + suppGroups := append(execUser.Sgids, addGroups...) if err := syscall.Setgroups(suppGroups); err != nil { return err } + if err := system.Setgid(execUser.Gid); err != nil { return err } diff --git a/integration/exec_test.go b/integration/exec_test.go index 3b8a83b7..c218dbd6 100644 --- a/integration/exec_test.go +++ b/integration/exec_test.go @@ -386,6 +386,53 @@ func TestProcessCaps(t *testing.T) { } } +func TestAdditionalGroups(t *testing.T) { + if testing.Short() { + return + } + root, err := newTestRoot() + ok(t, err) + defer os.RemoveAll(root) + + rootfs, err := newRootfs() + ok(t, err) + defer remove(rootfs) + + config := newTemplateConfig(rootfs) + config.AdditionalGroups = []string{"plugdev", "audio"} + + factory, err := libcontainer.New(root, libcontainer.Cgroupfs) + ok(t, err) + + container, err := factory.Create("test", config) + ok(t, err) + defer container.Destroy() + + var stdout bytes.Buffer + pconfig := libcontainer.Process{ + Args: []string{"sh", "-c", "id", "-Gn"}, + Env: standardEnvironment, + Stdin: nil, + Stdout: &stdout, + } + err = container.Start(&pconfig) + ok(t, err) + + // Wait for process + waitProcess(&pconfig, t) + + outputGroups := string(stdout.Bytes()) + + // Check that the groups output has the groups that we specified + if !strings.Contains(outputGroups, "audio") { + t.Fatalf("Listed groups do not contain the audio group as expected: %v", outputGroups) + } + + if !strings.Contains(outputGroups, "plugdev") { + t.Fatalf("Listed groups do not contain the plugdev group as expected: %v", outputGroups) + } +} + func TestFreeze(t *testing.T) { testFreeze(t, false) } diff --git a/nsinit/config.go b/nsinit/config.go index 7fb28a58..58f71418 100644 --- a/nsinit/config.go +++ b/nsinit/config.go @@ -46,6 +46,7 @@ var createFlags = []cli.Flag{ cli.StringSliceFlag{Name: "bind", Value: &cli.StringSlice{}, Usage: "add bind mounts to the container"}, cli.StringSliceFlag{Name: "sysctl", Value: &cli.StringSlice{}, Usage: "set system properties in the container"}, cli.StringSliceFlag{Name: "tmpfs", Value: &cli.StringSlice{}, Usage: "add tmpfs mounts to the container"}, + cli.StringSliceFlag{Name: "groups", Value: &cli.StringSlice{}, Usage: "add additional groups"}, } var configCommand = cli.Command{ @@ -113,6 +114,7 @@ func modify(config *configs.Config, context *cli.Context) { node.Gid = uint32(userns_uid) } } + config.SystemProperties = make(map[string]string) for _, sysProp := range context.StringSlice("sysctl") { parts := strings.SplitN(sysProp, "=", 2) @@ -121,6 +123,11 @@ func modify(config *configs.Config, context *cli.Context) { } config.SystemProperties[parts[0]] = parts[1] } + + for _, group := range context.StringSlice("groups") { + config.AdditionalGroups = append(config.AdditionalGroups, group) + } + for _, rawBind := range context.StringSlice("bind") { mount := &configs.Mount{ Device: "bind", diff --git a/user/user.go b/user/user.go index d7439f12..13226dbf 100644 --- a/user/user.go +++ b/user/user.go @@ -348,3 +348,60 @@ 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 { + 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) + } + + 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{}{} + } + } + 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) + } + } +}