Adding policy enforcement for User.

This PR adds policy enforcement for the User property of container processes.
Policy authors can now explicitly allow and deny users, groups, and umasks
associated with the init process and exec processes that they define on
containers.

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

internal/guest/runtime/hcsv2/uvm.go

@@ -15,6 +15,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"sync"
 	"syscall"
@@ -43,6 +44,7 @@ import (
 	"github.com/Microsoft/hcsshim/pkg/annotations"
 	"github.com/Microsoft/hcsshim/pkg/securitypolicy"
 	"github.com/mattn/go-shellwords"
+	"github.com/opencontainers/runc/libcontainer/user"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -292,6 +294,66 @@ func setupSandboxHugePageMountsPath(id string) error {
 	return storage.MountRShared(mountPath)
 }
 
+func getUserNameForID(spec *specs.Spec, uid uint32) (string, error) {
+	users, err := user.ParsePasswdFileFilter(filepath.Join(spec.Root.Path, "/etc/passwd"), func(user user.User) bool {
+		return uint32(user.Uid) == uid
+	})
+	if err != nil {
+		return "", err
+	}
+
+	if len(users) != 1 {
+		return "", errors.Errorf("expected exactly 1 user matched '%d'", len(users))
+	}
+	return users[0].Name, nil
+}
+
+func getGroupNameForID(spec *specs.Spec, gid uint32) (string, error) {
+	groups, err := user.ParseGroupFileFilter(filepath.Join(spec.Root.Path, "/etc/group"), func(group user.Group) bool {
+		return uint32(group.Gid) == gid
+	})
+	if err != nil {
+		return "", err
+	}
+	if len(groups) != 1 {
+		return "", errors.Errorf("expected exactly 1 group matched '%d'", len(groups))
+	}
+	return groups[0].Name, nil
+}
+
+func getUserInfo(spec *specs.Spec) (securitypolicy.IDName, []securitypolicy.IDName, string, error) {
+	userName, err := getUserNameForID(spec, spec.Process.User.UID)
+	if err != nil {
+		return securitypolicy.IDName{}, nil, "", err
+	}
+
+	user := securitypolicy.IDName{ID: strconv.FormatUint(uint64(spec.Process.User.UID), 10), Name: userName}
+
+	groupName, err := getGroupNameForID(spec, spec.Process.User.GID)
+	if err != nil {
+		return securitypolicy.IDName{}, nil, "", err
+	}
+	groups := []securitypolicy.IDName{{ID: strconv.FormatUint(uint64(spec.Process.User.GID), 10), Name: groupName}}
+	additionalGIDs := spec.Process.User.AdditionalGids
+	if len(additionalGIDs) > 0 {
+		for _, gid := range additionalGIDs {
+			groupName, err = getGroupNameForID(spec, gid)
+			if err != nil {
+				return securitypolicy.IDName{}, nil, "", err
+			}
+			groups = append(groups, securitypolicy.IDName{ID: strconv.FormatUint(uint64(gid), 10), Name: groupName})
+		}
+	}
+
+	// this default value is used in the Linux kernel if no umask is specified
+	umask := "0022"
+	if spec.Process.User.Umask != nil {
+		umask = fmt.Sprintf("%04o", *spec.Process.User.Umask)
+	}
+
+	return user, groups, umask, nil
+}
+
 func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VMHostedContainerSettingsV2) (_ *Container, err error) {
 	criType, isCRI := settings.OCISpecification.Annotations[annotations.KubernetesContainerType]
 	c := &Container{
@@ -384,6 +446,11 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 		}()
 	}
 
+	user, groups, umask, err := getUserInfo(settings.OCISpecification)
+	if err != nil {
+		return nil, err
+	}
+
 	envToKeep, allowStdio, err := h.securityPolicyEnforcer.EnforceCreateContainerPolicy(
 		sandboxID,
 		id,
@@ -393,6 +460,9 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 		settings.OCISpecification.Mounts,
 		isPrivilegedContainerCreationRequest(ctx, settings.OCISpecification),
 		settings.OCISpecification.Process.NoNewPrivileges,
+		user,
+		groups,
+		umask,
 	)
 	if err != nil {
 		return nil, errors.Wrapf(err, "container creation denied due to policy")
@@ -648,6 +718,12 @@ func (h *Host) ExecProcess(ctx context.Context, containerID string, params prot.
 	var pid int
 	var c *Container
 	var envToKeep securitypolicy.EnvList
+
+	userName, groupNames, umask, err := getUserInfo(params.OCISpecification)
+	if err != nil {
+		return 0, err
+	}
+
 	if params.IsExternal || containerID == UVMContainerID {
 		var allowStdioAccess bool
 		envToKeep, allowStdioAccess, err = h.securityPolicyEnforcer.EnforceExecExternalProcessPolicy(
@@ -689,7 +765,11 @@ func (h *Host) ExecProcess(ctx context.Context, containerID string, params prot.
 				params.OCIProcess.Args,
 				params.OCIProcess.Env,
 				params.OCIProcess.Cwd,
-				params.OCIProcess.NoNewPrivileges)
+				params.OCIProcess.NoNewPrivileges,
+				userName,
+				groupNames,
+				umask,
+			)
 			if err != nil {
 				return pid, errors.Wrapf(err, "exec in container denied due to policy")
 			}

internal/tools/securitypolicy/helpers/helpers.go

@@ -2,14 +2,16 @@ package helpers
 
 import (
 	"fmt"
+	"strconv"
+	"strings"
 
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 
 	"github.com/Microsoft/hcsshim/ext4/tar2ext4"
-	"github.com/Microsoft/hcsshim/pkg/securitypolicy"
+	sp "github.com/Microsoft/hcsshim/pkg/securitypolicy"
 )
 
 // RemoteImageFromImageName parses a given imageName reference and creates a v1.Image with
@@ -63,12 +65,12 @@ func ParseEnvFromImage(img v1.Image) ([]string, error) {
 // DefaultContainerConfigs returns a hardcoded slice of container configs, which should
 // be included by default in the security policy.
 // The slice includes only a sandbox pause container.
-func DefaultContainerConfigs() []securitypolicy.ContainerConfig {
-	pause := securitypolicy.ContainerConfig{
+func DefaultContainerConfigs() []sp.ContainerConfig {
+	pause := sp.ContainerConfig{
 		ImageName: "k8s.gcr.io/pause:3.1",
 		Command:   []string{"/pause"},
 	}
-	return []securitypolicy.ContainerConfig{pause}
+	return []sp.ContainerConfig{pause}
 }
 
 // ParseWorkingDirFromImage inspects the image spec and returns working directory if
@@ -98,10 +100,63 @@ func ParseCommandFromImage(img v1.Image) ([]string, error) {
 	return cmdArgs, nil
 }
 
-// PolicyContainersFromConfigs returns a slice of securitypolicy.Container generated
-// from a slice of securitypolicy.ContainerConfig's
-func PolicyContainersFromConfigs(containerConfigs []securitypolicy.ContainerConfig) ([]*securitypolicy.Container, error) {
-	var policyContainers []*securitypolicy.Container
+// ParseUserFromImage inspects the image and returns the user and group
+func ParseUserFromImage(img v1.Image) (sp.IDNameConfig, sp.IDNameConfig, error) {
+	imgConfig, err := img.ConfigFile()
+	var user sp.IDNameConfig
+	var group sp.IDNameConfig
+	if err != nil {
+		return user, group, err
+	}
+
+	userString := imgConfig.Config.User
+	// valid values are "user", "user:group", "uid", "uid:gid", "user:gid", "uid:group"
+	// "" is also valid, and means any user
+	// we assume that any string that is not a number is a username, and thus
+	// that any string that is a number is a uid. It is possible to have a username
+	// that is a number, but that is not supported here.
+	if userString == "" {
+		// not specified, use any
+		user.Strategy = sp.IDNameStrategyAny
+		group.Strategy = sp.IDNameStrategyAny
+	} else {
+		parts := strings.Split(userString, ":")
+		if len(parts) == 1 {
+			// only user specified, use any
+			group.Strategy = sp.IDNameStrategyAny
+			user.Rule = parts[0]
+			_, err := strconv.ParseUint(parts[0], 10, 32)
+			if err == nil {
+				user.Strategy = sp.IDNameStrategyID
+			} else {
+				user.Strategy = sp.IDNameStrategyName
+			}
+		} else if len(parts) == 2 {
+			_, err := strconv.ParseUint(parts[0], 10, 32)
+			user.Rule = parts[0]
+			if err == nil {
+				user.Strategy = sp.IDNameStrategyID
+			} else {
+				user.Strategy = sp.IDNameStrategyName
+			}
+
+			_, err = strconv.ParseUint(parts[1], 10, 32)
+			group.Rule = parts[1]
+			if err == nil {
+				group.Strategy = sp.IDNameStrategyID
+			} else {
+				group.Strategy = sp.IDNameStrategyName
+			}
+		}
+	}
+
+	return user, group, nil
+}
+
+// PolicyContainersFromConfigs returns a slice of sp.Container generated
+// from a slice of sp.ContainerConfig's
+func PolicyContainersFromConfigs(containerConfigs []sp.ContainerConfig) ([]*sp.Container, error) {
+	var policyContainers []*sp.Container
 	for _, containerConfig := range containerConfigs {
 		var imageOptions []remote.Option
 
@@ -140,13 +195,13 @@ func PolicyContainersFromConfigs(containerConfigs []securitypolicy.ContainerConf
 
 		// we want all environment variables which we've extracted from the
 		// image to be required
-		envRules := securitypolicy.NewEnvVarRules(envVars, true)
+		envRules := sp.NewEnvVarRules(envVars, true)
 
 		// cri adds TERM=xterm for all workload containers. we add to all containers
 		// to prevent any possible error
-		envRules = append(envRules, securitypolicy.EnvRuleConfig{
+		envRules = append(envRules, sp.EnvRuleConfig{
 			Rule:     "TERM=xterm",
-			Strategy: securitypolicy.EnvVarRuleString,
+			Strategy: sp.EnvVarRuleString,
 			Required: false,
 		})
 
@@ -161,7 +216,12 @@ func PolicyContainersFromConfigs(containerConfigs []securitypolicy.ContainerConf
 			workingDir = containerConfig.WorkingDir
 		}
 
-		container, err := securitypolicy.CreateContainerPolicy(
+		user, group, err := ParseUserFromImage(img)
+		if err != nil {
+			return nil, err
+		}
+
+		container, err := sp.CreateContainerPolicy(
 			commandArgs,
 			layerHashes,
 			envRules,
@@ -172,6 +232,7 @@ func PolicyContainersFromConfigs(containerConfigs []securitypolicy.ContainerConf
 			containerConfig.Signals,
 			containerConfig.AllowStdioAccess,
 			!containerConfig.AllowPrivilegeEscalation,
+			setDefaultUser(containerConfig.User, user, group),
 		)
 		if err != nil {
 			return nil, err
@@ -181,3 +242,16 @@ func PolicyContainersFromConfigs(containerConfigs []securitypolicy.ContainerConf
 
 	return policyContainers, nil
 }
+
+func setDefaultUser(config *sp.UserConfig, user, group sp.IDNameConfig) sp.UserConfig {
+	if config != nil {
+		return *config
+	}
+
+	// 0022 is the default umask for containers in docker
+	return sp.UserConfig{
+		UserIDName:   user,
+		GroupIDNames: []sp.IDNameConfig{group},
+		Umask:        "0022",
+	}
+}

pkg/securitypolicy/framework.rego

@@ -235,6 +235,31 @@ noNewPrivileges_ok(no_new_privileges) {
     input.noNewPrivileges == no_new_privileges
 }
 
+idName_ok(pattern, "any", value) {
+    true
+}
+
+idName_ok(pattern, "id", value) {
+    pattern == value.id
+}
+
+idName_ok(pattern, "name", value) {
+    pattern == value.name
+}
+
+idName_ok(pattern, "re2", value) {
+    regex.match(pattern, value.name)
+}
+
+user_ok(user) {
+    user.umask == input.umask
+    idName_ok(user.user_idname.pattern, user.user_idname.strategy, input.user)
+    every group in input.groups {
+        some group_idname in user.group_idnames
+        idName_ok(group_idname.pattern, group_idname.strategy, group)
+    }
+}
+
 default container_started := false
 
 container_started {
@@ -257,6 +282,7 @@ create_container := {"metadata": [updateMatches, addStarted],
         # the error handling, such that error messaging correctly reflects
         # the narrowing process.
         noNewPrivileges_ok(container.no_new_privileges)
+        user_ok(container.user)
         privileged_ok(container.allow_elevated)
         workingDirectory_ok(container.working_dir)
         command_ok(container.command)
@@ -380,6 +406,7 @@ exec_in_container := {"metadata": [updateMatches],
         # the narrowing process.
         workingDirectory_ok(container.working_dir)
         noNewPrivileges_ok(container.no_new_privileges)
+        user_ok(container.user)
         some process in container.exec_processes
         command_ok(process.command)
     ]
@@ -921,6 +948,7 @@ errors["missing required environment variable"] {
     possible_containers := [container |
         container := data.metadata.matches[input.containerID][_]
         noNewPrivileges_ok(container.no_new_privileges)
+        user_ok(container.user)
         privileged_ok(container.allow_elevated)
         workingDirectory_ok(container.working_dir)
         command_ok(container.command)
@@ -952,6 +980,7 @@ errors["missing required environment variable"] {
     possible_containers := [container |
         container := data.metadata.matches[input.containerID][_]
         noNewPrivileges_ok(container.no_new_privileges)
+        user_ok(container.user)
         workingDirectory_ok(container.working_dir)
         some process in container.exec_processes
         command_ok(process.command)
@@ -1163,6 +1192,7 @@ errors["containers only distinguishable by allow_stdio_access"] {
     possible_containers := [container |
         container := data.metadata.matches[input.containerID][_]
         noNewPrivileges_ok(container.no_new_privileges)
+        user_ok(container.user)
         privileged_ok(container.allow_elevated)
         workingDirectory_ok(container.working_dir)
         command_ok(container.command)
@@ -1234,3 +1264,25 @@ errors["invalid noNewPrivileges"] {
     input.rule in ["create_container", "exec_in_container"]
     not noNewPrivileges_matches
 }
+
+default user_matches := false
+
+user_matches {
+    input.rule == "create_container"
+    some container in data.metadata.matches[input.containerID]
+    user_ok(container.user)
+}
+
+user_matches {
+    input.rule == "exec_in_container"
+    some container in data.metadata.matches[input.containerID]
+    some process in container.exec_processes
+    command_ok(process.command)
+    workingDirectory_ok(process.working_dir)
+    user_ok(process.user)
+}
+
+errors["invalid user"] {
+    input.rule in ["create_container", "exec_in_container"]
+    not user_matches
+}