From f9b72b1b46b7fcb991a022da9e3c877f1f33de1d Mon Sep 17 00:00:00 2001 From: Petar Petrov Date: Fri, 10 Jun 2016 10:35:13 +0000 Subject: [PATCH] Allow additional groups to be overridden in exec Signed-off-by: Julian Friedman Signed-off-by: Petar Petrov Signed-off-by: Georgi Sabev --- libcontainer/configs/config.go | 4 -- libcontainer/container_linux.go | 1 + libcontainer/init_linux.go | 5 ++- libcontainer/integration/exec_test.go | 12 ++--- libcontainer/integration/execin_test.go | 58 +++++++++++++++++++++++++ libcontainer/process.go | 4 ++ libcontainer/specconv/spec_linux.go | 4 -- utils_linux.go | 4 ++ 8 files changed, 76 insertions(+), 16 deletions(-) diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go index 806e0be9..3c38191b 100644 --- a/libcontainer/configs/config.go +++ b/libcontainer/configs/config.go @@ -148,10 +148,6 @@ type Config struct { // More information about kernel oom score calculation here: https://lwn.net/Articles/317814/ OomScoreAdj int `json:"oom_score_adj"` - // AdditionalGroups specifies the gids that should be added to supplementary groups - // in addition to those that the user belongs to. - AdditionalGroups []string `json:"additional_groups"` - // UidMappings is an array of User ID mappings for User Namespaces UidMappings []IDMap `json:"uid_mappings"` diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 294109cd..322d6eaf 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -385,6 +385,7 @@ func (c *linuxContainer) newInitConfig(process *Process) *initConfig { Args: process.Args, Env: process.Env, User: process.User, + AdditionalGroups: process.AdditionalGroups, Cwd: process.Cwd, Console: process.consolePath, Capabilities: process.Capabilities, diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index d6d9901c..01ff0d13 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -52,6 +52,7 @@ type initConfig struct { AppArmorProfile string `json:"apparmor_profile"` NoNewPrivileges bool `json:"no_new_privileges"` User string `json:"user"` + AdditionalGroups []string `json:"additional_groups"` Config *configs.Config `json:"config"` Console string `json:"console"` Networks []*network `json:"network"` @@ -213,8 +214,8 @@ func setupUser(config *initConfig) error { } var addGroups []int - if len(config.Config.AdditionalGroups) > 0 { - addGroups, err = user.GetAdditionalGroupsPath(config.Config.AdditionalGroups, groupPath) + if len(config.AdditionalGroups) > 0 { + addGroups, err = user.GetAdditionalGroupsPath(config.AdditionalGroups, groupPath) if err != nil { return err } diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 6d2db4c7..d1f929a3 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -431,7 +431,6 @@ func TestAdditionalGroups(t *testing.T) { defer remove(rootfs) config := newTemplateConfig(rootfs) - config.AdditionalGroups = []string{"plugdev", "audio"} factory, err := libcontainer.New(root, libcontainer.Cgroupfs) ok(t, err) @@ -442,11 +441,12 @@ func TestAdditionalGroups(t *testing.T) { var stdout bytes.Buffer pconfig := libcontainer.Process{ - Cwd: "/", - Args: []string{"sh", "-c", "id", "-Gn"}, - Env: standardEnvironment, - Stdin: nil, - Stdout: &stdout, + Cwd: "/", + Args: []string{"sh", "-c", "id", "-Gn"}, + Env: standardEnvironment, + Stdin: nil, + Stdout: &stdout, + AdditionalGroups: []string{"plugdev", "audio"}, } err = container.Run(&pconfig) ok(t, err) diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 2568aaa2..174b5f06 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -134,6 +134,64 @@ func testExecInRlimit(t *testing.T, userns bool) { } } +func TestExecInAdditionalGroups(t *testing.T) { + if testing.Short() { + return + } + + rootfs, err := newRootfs() + ok(t, err) + defer remove(rootfs) + + config := newTemplateConfig(rootfs) + container, err := newContainer(config) + ok(t, err) + defer container.Destroy() + + // Execute a first process in the container + stdinR, stdinW, err := os.Pipe() + ok(t, err) + process := &libcontainer.Process{ + Cwd: "/", + Args: []string{"cat"}, + Env: standardEnvironment, + Stdin: stdinR, + } + err = container.Run(process) + stdinR.Close() + defer stdinW.Close() + ok(t, err) + + var stdout bytes.Buffer + pconfig := libcontainer.Process{ + Cwd: "/", + Args: []string{"sh", "-c", "id", "-Gn"}, + Env: standardEnvironment, + Stdin: nil, + Stdout: &stdout, + AdditionalGroups: []string{"plugdev", "audio"}, + } + err = container.Run(&pconfig) + ok(t, err) + + // Wait for process + waitProcess(&pconfig, t) + + stdinW.Close() + waitProcess(process, 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 TestExecInError(t *testing.T) { if testing.Short() { return diff --git a/libcontainer/process.go b/libcontainer/process.go index 46fb2749..334add57 100644 --- a/libcontainer/process.go +++ b/libcontainer/process.go @@ -28,6 +28,10 @@ type Process struct { // local to the container's user and group configuration. User string + // AdditionalGroups specifies the gids that should be added to supplementary groups + // in addition to those that the user belongs to. + AdditionalGroups []string + // Cwd will change the processes current working directory inside the container's rootfs. Cwd string diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index 8c0540e8..596c5086 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -8,7 +8,6 @@ import ( "fmt" "os" "path/filepath" - "strconv" "strings" "syscall" "time" @@ -229,9 +228,6 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) { if spec.Linux.Resources != nil && spec.Linux.Resources.OOMScoreAdj != nil { config.OomScoreAdj = *spec.Linux.Resources.OOMScoreAdj } - for _, g := range spec.Process.User.AdditionalGids { - config.AdditionalGroups = append(config.AdditionalGroups, strconv.FormatUint(uint64(g), 10)) - } createHooks(spec, config) config.MountLabel = spec.Linux.MountLabel config.Version = specs.Version diff --git a/utils_linux.go b/utils_linux.go index aa98ebbf..a36f99a2 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "syscall" "github.com/Sirupsen/logrus" @@ -83,6 +84,9 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) { NoNewPrivileges: &p.NoNewPrivileges, AppArmorProfile: p.ApparmorProfile, } + for _, gid := range p.User.AdditionalGids { + lp.AdditionalGroups = append(lp.AdditionalGroups, strconv.FormatUint(uint64(gid), 10)) + } for _, rlimit := range p.Rlimits { rl, err := createLibContainerRlimit(rlimit) if err != nil {