Merge pull request #753 from YaoZengzeng/security-context

feature: namespace options and supplemental groups for cri manager

cli/common_flags.go

@@ -71,6 +71,8 @@ func addCommonFlags(flagSet *pflag.FlagSet) *container {
 	// user
 	flagSet.StringVarP(&c.user, "user", "u", "", "UID")
 
+	flagSet.StringSliceVar(&c.groupAdd, "group-add", nil, "Add additional groups to join")
+
 	flagSet.StringVar(&c.utsMode, "uts", "", "UTS namespace to use")
 
 	flagSet.StringSliceVarP(&c.volume, "volume", "v", nil, "Bind mount volumes to container")

cli/container.go

@@ -24,6 +24,7 @@ type container struct {
 	entrypoint       string
 	workdir          string
 	user             string
+	groupAdd         []string
 	hostname         string
 	cpushare         int64
 	cpusetcpus       string
@@ -228,6 +229,7 @@ func (c *container) config() (*types.ContainerCreateConfig, error) {
 			IpcMode:       c.ipcMode,
 			PidMode:       c.pidMode,
 			UTSMode:       c.utsMode,
+			GroupAdd:      c.groupAdd,
 			Sysctls:       sysctls,
 			SecurityOpt:   c.securityOpt,
 			NetworkMode:   networkMode,

daemon/mgr/cri.go

@@ -48,6 +48,9 @@ const (
 	// TODO: specify them in the parameters of pouchd.
 	streamServerAddress = ""
 	streamServerPort    = "10010"
+
+	namespaceModeHost = "host"
+	namespaceModeNone = "none"
 )
 
 var (

daemon/mgr/cri_utils.go

@@ -157,12 +157,59 @@ func parseSandboxName(name string) (*runtime.PodSandboxMetadata, error) {
 	}, nil
 }
 
+func modifySandboxNamespaceOptions(nsOpts *runtime.NamespaceOption, hostConfig *apitypes.HostConfig) {
+	if nsOpts == nil {
+		return
+	}
+	if nsOpts.HostPid {
+		hostConfig.PidMode = namespaceModeHost
+	}
+	if nsOpts.HostIpc {
+		hostConfig.IpcMode = namespaceModeHost
+	}
+	if nsOpts.HostNetwork {
+		hostConfig.NetworkMode = namespaceModeHost
+	}
+}
+
+func applySandboxSecurityContext(lc *runtime.LinuxPodSandboxConfig, config *apitypes.ContainerConfig, hc *apitypes.HostConfig) error {
+	if lc == nil {
+		return nil
+	}
+
+	var sc *runtime.LinuxContainerSecurityContext
+	if lc.SecurityContext != nil {
+		sc = &runtime.LinuxContainerSecurityContext{
+			SupplementalGroups: lc.SecurityContext.SupplementalGroups,
+			RunAsUser:          lc.SecurityContext.RunAsUser,
+			ReadonlyRootfs:     lc.SecurityContext.ReadonlyRootfs,
+			SelinuxOptions:     lc.SecurityContext.SelinuxOptions,
+			NamespaceOptions:   lc.SecurityContext.NamespaceOptions,
+		}
+	}
+
+	modifyContainerConfig(sc, config)
+	err := modifyHostConfig(sc, hc)
+	if err != nil {
+		return err
+	}
+	modifySandboxNamespaceOptions(sc.GetNamespaceOptions(), hc)
+
+	return nil
+}
+
 // applySandboxLinuxOptions applies LinuxPodSandboxConfig to pouch's HostConfig and ContainerCreateConfig.
 func applySandboxLinuxOptions(hc *apitypes.HostConfig, lc *runtime.LinuxPodSandboxConfig, createConfig *apitypes.ContainerCreateConfig, image string) error {
 	if lc == nil {
 		return nil
 	}
 
+	// Apply security context.
+	err := applySandboxSecurityContext(lc, &createConfig.ContainerConfig, hc)
+	if err != nil {
+		return err
+	}
+
 	// Set sysctls.
 	hc.Sysctls = lc.Sysctls
 	return nil
@@ -292,10 +339,36 @@ func parseContainerName(name string) (*runtime.ContainerMetadata, error) {
 func modifyContainerNamespaceOptions(nsOpts *runtime.NamespaceOption, podSandboxID string, hostConfig *apitypes.HostConfig) {
 	sandboxNSMode := fmt.Sprintf("container:%v", podSandboxID)
 
-	hostConfig.PidMode = sandboxNSMode
-	hostConfig.NetworkMode = sandboxNSMode
-	hostConfig.IpcMode = sandboxNSMode
-	hostConfig.UTSMode = sandboxNSMode
+	if nsOpts == nil {
+		hostConfig.PidMode = sandboxNSMode
+		hostConfig.IpcMode = sandboxNSMode
+		hostConfig.NetworkMode = sandboxNSMode
+		return
+	}
+
+	for _, n := range []struct {
+		hostMode bool
+		nsMode   *string
+	}{
+		{
+			hostMode: nsOpts.HostPid,
+			nsMode:   &hostConfig.PidMode,
+		},
+		{
+			hostMode: nsOpts.HostIpc,
+			nsMode:   &hostConfig.IpcMode,
+		},
+		{
+			hostMode: nsOpts.HostNetwork,
+			nsMode:   &hostConfig.NetworkMode,
+		},
+	} {
+		if n.hostMode {
+			*n.nsMode = namespaceModeHost
+		} else {
+			*n.nsMode = sandboxNSMode
+		}
+	}
 }
 
 // getAppArmorSecurityOpts gets appArmor options from container config.
@@ -346,6 +419,11 @@ func modifyHostConfig(sc *runtime.LinuxContainerSecurityContext, hostConfig *api
 		return nil
 	}
 
+	// Apply supplemental groups.
+	for _, group := range sc.SupplementalGroups {
+		hostConfig.GroupAdd = append(hostConfig.GroupAdd, strconv.FormatInt(group, 10))
+	}
+
 	// TODO: apply other security options.
 
 	// Apply capability options.

daemon/mgr/spec_process.go

@@ -63,31 +63,29 @@ func setupProcessTTY(ctx context.Context, c *ContainerMeta, spec *SpecWrapper) e
 }
 
 func setupProcessUser(ctx context.Context, c *ContainerMeta, spec *SpecWrapper) (err error) {
-	// The user option is complicated, now we only handle case "uid".
-	// resolve uid:gid and user:group.
-	if c.Config.User == "" {
-		return nil
-	}
-
 	// container rootfs is created by containerd, pouch just creates a snapshot
 	// id and keeps it in memory. If container is in start process, we can not
 	// find if user if exist in container image, so we do some simple check.
 	var uid, gid uint32
 
-	if _, err := os.Stat(c.BaseFS); err != nil {
-		logrus.Infof("snapshot %s is not exist, maybe in start process.", c.BaseFS)
-		uid, gid = user.GetIntegerID(c.Config.User)
-	} else {
-		uid, gid, err = user.Get(c.BaseFS, c.Config.User)
-		if err != nil {
-			return err
+	if c.Config.User != "" {
+		if _, err := os.Stat(c.BaseFS); err != nil {
+			logrus.Infof("snapshot %s is not exist, maybe in start process.", c.BaseFS)
+			uid, gid = user.GetIntegerID(c.Config.User)
+		} else {
+			uid, gid, err = user.Get(c.BaseFS, c.Config.User)
+			if err != nil {
+				return err
+			}
 		}
 	}
 
+	additionalGids := user.GetAdditionalGids(c.HostConfig.GroupAdd)
+
 	spec.s.Process.User = specs.User{
-		UID: uid,
-		GID: gid,
+		UID:            uid,
+		GID:            gid,
+		AdditionalGids: additionalGids,
 	}
-
 	return nil
 }

hack/cri-test/test-cri.sh

@@ -23,10 +23,10 @@ POUCH_SOCK="/var/run/pouchcri.sock"
 
 # CRI_FOCUS focuses the test to run.
 # With the CRI manager completes its function, we may need to expand this field.
-CRI_FOCUS=${CRI_FOCUS:-"PodSandbox|AppArmor|Privileged is true|basic operations on container|Runtime info|mount propagation|volume and device|RunAsUser|container port|Streaming"}
+CRI_FOCUS=${CRI_FOCUS:-"PodSandbox|AppArmor|Privileged is true|basic operations on container|Runtime info|mount propagation|volume and device|RunAsUser|container port|Streaming|NamespaceOption|SupplementalGroups"}
 
 # CRI_SKIP skips the test to skip.
-CRI_SKIP=${CRI_SKIP:-"RunAsUserName|attach"}
+CRI_SKIP=${CRI_SKIP:-"RunAsUserName|attach|HostNetwork"}
 # REPORT_DIR is the the directory to store test logs.
 REPORT_DIR=${REPORT_DIR:-"/tmp/test-cri"}
 

pkg/user/user.go

@@ -119,6 +119,22 @@ func GetIntegerID(user string) (uint32, uint32) {
 	return uint32(uid), uint32(gid)
 }
 
+// GetAdditionalGids parse supplementary gids from slice groups.
+func GetAdditionalGids(groups []string) []uint32 {
+	var additionalGids []uint32
+
+	// TODO: check whether group is valid and support group name format like "nobody".
+	for _, group := range groups {
+		gid, err := strconv.ParseUint(group, 10, 32)
+		if err != nil {
+			continue
+		}
+		additionalGids = append(additionalGids, uint32(gid))
+	}
+
+	return additionalGids
+}
+
 // parseID parses uid from /etc/passwd.
 func parseID(file, str string, parserFilter filterFunc) (uint32, error) {
 	idInt, idErr := strconv.Atoi(str)

pkg/user/user_test.go

@@ -2,6 +2,7 @@ package user
 
 import (
 	"fmt"
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -20,3 +21,12 @@ func TestParseString(t *testing.T) {
 		assert.Equal(fmt.Sprintf("%s:%s:%d:%d", u1, u2, i1, i2), l)
 	}
 }
+
+func TestGetAdditionalGids(t *testing.T) {
+	groups := []string{"1234", "5678"}
+	expected := []uint32{1234, 5678}
+
+	result := GetAdditionalGids(groups)
+	assert := assert.New(t)
+	assert.True(reflect.DeepEqual(expected, result), true)
+}