From 9996c76761bcc0f2068a99df2f25fccfcdf9a7c9 Mon Sep 17 00:00:00 2001 From: Maksim An Date: Thu, 10 Feb 2022 13:45:25 -0800 Subject: [PATCH 1/7] Add current working directory enforcement. The working directory can be set as part of container image by WORKINGDIR Dockerfile directive or could be explicitly set inside CRI container config. Changing the CWD of container can change the expected behavior of a container. If the working_dir config is not present or empty inside the policy config, this information will be gathered from container image spec. Add logic to enforce CWD enforcement inside GCS and extend policy dev tool and policy structure to support this scenario. The enforcement is an exact string match between what's in the policy and what's in the generated container spec. If the paths don't match, the container will fail to start. Signed-off-by: Maksim An --- internal/guest/runtime/hcsv2/uvm.go | 7 ++- .../mountmonitoringsecuritypolicyenforcer.go | 2 +- internal/tools/securitypolicy/main.go | 7 ++- pkg/securitypolicy/securitypolicy.go | 45 ++++++++++++------- pkg/securitypolicy/securitypolicy_test.go | 2 +- pkg/securitypolicy/securitypolicyenforcer.go | 41 +++++++++++++---- .../pkg/securitypolicy/securitypolicy.go | 45 ++++++++++++------- .../securitypolicy/securitypolicyenforcer.go | 41 +++++++++++++---- 8 files changed, 138 insertions(+), 52 deletions(-) diff --git a/internal/guest/runtime/hcsv2/uvm.go b/internal/guest/runtime/hcsv2/uvm.go index bd359da0e1..7de89dc9e3 100644 --- a/internal/guest/runtime/hcsv2/uvm.go +++ b/internal/guest/runtime/hcsv2/uvm.go @@ -154,7 +154,12 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM return nil, gcserr.NewHresultError(gcserr.HrVmcomputeSystemAlreadyExists) } - err = h.securityPolicyEnforcer.EnforceCreateContainerPolicy(id, settings.OCISpecification.Process.Args, settings.OCISpecification.Process.Env) + err = h.securityPolicyEnforcer.EnforceCreateContainerPolicy( + id, + settings.OCISpecification.Process.Args, + settings.OCISpecification.Process.Env, + settings.OCISpecification.Process.Cwd, + ) if err != nil { return nil, errors.Wrapf(err, "container creation denied due to policy") diff --git a/internal/guest/storage/test/policy/mountmonitoringsecuritypolicyenforcer.go b/internal/guest/storage/test/policy/mountmonitoringsecuritypolicyenforcer.go index 3ba20684b4..bb06d465a8 100644 --- a/internal/guest/storage/test/policy/mountmonitoringsecuritypolicyenforcer.go +++ b/internal/guest/storage/test/policy/mountmonitoringsecuritypolicyenforcer.go @@ -29,6 +29,6 @@ func (p *MountMonitoringSecurityPolicyEnforcer) EnforceOverlayMountPolicy(contai return nil } -func (p *MountMonitoringSecurityPolicyEnforcer) EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) { +func (p *MountMonitoringSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_ string, _ []string, _ []string, _ string) (err error) { return nil } diff --git a/internal/tools/securitypolicy/main.go b/internal/tools/securitypolicy/main.go index 4c03e0e793..37ae31ff53 100644 --- a/internal/tools/securitypolicy/main.go +++ b/internal/tools/securitypolicy/main.go @@ -80,6 +80,7 @@ func createPolicyFromConfig(config *securitypolicy.PolicyConfig) (*securitypolic []string{"/pause"}, []securitypolicy.EnvRule{}, securitypolicy.AuthConfig{}, + "", ) config.Containers = append(config.Containers, pause) @@ -147,7 +148,11 @@ func createPolicyFromConfig(config *securitypolicy.PolicyConfig) (*securitypolic } envRules = append(envRules, rule) - container, err := securitypolicy.NewContainer(containerConfig.Command, layerHashes, envRules) + workingDir := containerConfig.WorkingDir + if workingDir == "" { + workingDir = imgConfig.Config.WorkingDir + } + container, err := securitypolicy.NewContainer(containerConfig.Command, layerHashes, envRules, workingDir) if err != nil { return nil, err } diff --git a/pkg/securitypolicy/securitypolicy.go b/pkg/securitypolicy/securitypolicy.go index 7a78eb7b42..21983d60bf 100644 --- a/pkg/securitypolicy/securitypolicy.go +++ b/pkg/securitypolicy/securitypolicy.go @@ -38,19 +38,27 @@ type EnvRuleConfig struct { // ContainerConfig contains toml or JSON config for container described // in security policy. type ContainerConfig struct { - ImageName string `json:"image_name" toml:"image_name"` - Command []string `json:"command" toml:"command"` - Auth AuthConfig `json:"auth" toml:"auth"` - EnvRules []EnvRule `json:"env_rules" toml:"env_rule"` + ImageName string `json:"image_name" toml:"image_name"` + Command []string `json:"command" toml:"command"` + Auth AuthConfig `json:"auth" toml:"auth"` + EnvRules []EnvRule `json:"env_rules" toml:"env_rule"` + WorkingDir string `json:"working_dir" toml:"working_dir"` } // NewContainerConfig creates a new ContainerConfig from the given values. -func NewContainerConfig(imageName string, command []string, envRules []EnvRule, auth AuthConfig) ContainerConfig { +func NewContainerConfig( + imageName string, + command []string, + envRules []EnvRule, + auth AuthConfig, + workingDir string, +) ContainerConfig { return ContainerConfig{ - ImageName: imageName, - Command: command, - EnvRules: envRules, - Auth: auth, + ImageName: imageName, + Command: command, + EnvRules: envRules, + Auth: auth, + WorkingDir: workingDir, } } @@ -72,6 +80,9 @@ type securityPolicyContainer struct { // order that the layers are overlayed is important and needs to be enforced // as part of policy. Layers []string `json:"layers"` + // WorkingDir is a path to container's working directory, which all the processes + // will default to. + WorkingDir string `json:"working_dir"` } // SecurityPolicyState is a structure that holds user supplied policy to enforce @@ -141,9 +152,10 @@ type Containers struct { } type Container struct { - Command CommandArgs `json:"command"` - EnvRules EnvRules `json:"env_rules"` - Layers Layers `json:"layers"` + Command CommandArgs `json:"command"` + EnvRules EnvRules `json:"env_rules"` + Layers Layers `json:"layers"` + WorkingDir string `json:"working_dir"` } type Layers struct { @@ -170,14 +182,15 @@ type EnvRule struct { // NewContainer creates a new Container instance from the provided values // or an error if envRules validation fails. -func NewContainer(command, layers []string, envRules []EnvRule) (*Container, error) { +func NewContainer(command, layers []string, envRules []EnvRule, workingDir string) (*Container, error) { if err := validateEnvRules(envRules); err != nil { return nil, err } return &Container{ - Command: newCommandArgs(command), - Layers: newLayers(layers), - EnvRules: newEnvRules(envRules), + Command: newCommandArgs(command), + Layers: newLayers(layers), + EnvRules: newEnvRules(envRules), + WorkingDir: workingDir, }, nil } diff --git a/pkg/securitypolicy/securitypolicy_test.go b/pkg/securitypolicy/securitypolicy_test.go index f92019afb1..faf54588c9 100644 --- a/pkg/securitypolicy/securitypolicy_test.go +++ b/pkg/securitypolicy/securitypolicy_test.go @@ -799,7 +799,7 @@ func (*generatedContainers) Generate(r *rand.Rand, size int) reflect.Value { } func generateContainers(r *rand.Rand, upTo int32) *generatedContainers { - containers := []securityPolicyContainer{} + var containers []securityPolicyContainer numContainers := (int)(atLeastOneAtMost(r, upTo)) for i := 0; i < numContainers; i++ { diff --git a/pkg/securitypolicy/securitypolicyenforcer.go b/pkg/securitypolicy/securitypolicyenforcer.go index 21605509f3..9986dd7209 100644 --- a/pkg/securitypolicy/securitypolicyenforcer.go +++ b/pkg/securitypolicy/securitypolicyenforcer.go @@ -14,7 +14,7 @@ type SecurityPolicyEnforcer interface { EnforceDeviceMountPolicy(target string, deviceHash string) (err error) EnforceDeviceUnmountPolicy(unmountTarget string) (err error) EnforceOverlayMountPolicy(containerID string, layerPaths []string) (err error) - EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) + EnforceCreateContainerPolicy(containerID string, argList []string, envList []string, workingDir string) (err error) } func NewSecurityPolicyEnforcer(state SecurityPolicyState) (SecurityPolicyEnforcer, error) { @@ -298,7 +298,12 @@ func (pe *StandardSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID return nil } -func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) { +func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy( + containerID string, + argList []string, + envList []string, + workingDir string, +) (err error) { pe.mutex.Lock() defer pe.mutex.Unlock() @@ -310,13 +315,15 @@ func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy(container return errors.New("container has already been started") } - err = pe.enforceCommandPolicy(containerID, argList) - if err != nil { + if err = pe.enforceCommandPolicy(containerID, argList); err != nil { return err } - err = pe.enforceEnvironmentVariablePolicy(containerID, envList) - if err != nil { + if err = pe.enforceEnvironmentVariablePolicy(containerID, envList); err != nil { + return err + } + + if err = pe.enforceWorkingDirPolicy(containerID, workingDir); err != nil { return err } @@ -385,6 +392,24 @@ func (pe *StandardSecurityPolicyEnforcer) enforceEnvironmentVariablePolicy(conta return nil } +func (pe *StandardSecurityPolicyEnforcer) enforceWorkingDirPolicy(containerID string, workingDir string) error { + possibleIndices := possibleIndexesForID(containerID, pe.ContainerIndexToContainerIds) + + matched := false + for _, pIndex := range possibleIndices { + pWorkingDir := pe.Containers[pIndex].WorkingDir + if pWorkingDir == workingDir { + matched = true + continue + } + pe.narrowMatchesForContainerIndex(pIndex, containerID) + } + if !matched { + return fmt.Errorf("working_dir %s unmached by policy rule", workingDir) + } + return nil +} + func envIsMatchedByRule(envVariable string, rules []EnvRule) bool { for _, rule := range rules { switch rule.Strategy { @@ -463,7 +488,7 @@ func (p *OpenDoorSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID s return nil } -func (p *OpenDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) { +func (p *OpenDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_ string, _ []string, _ []string, _ string) (err error) { return nil } @@ -483,6 +508,6 @@ func (p *ClosedDoorSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID return errors.New("creating an overlay fs is denied by policy") } -func (p *ClosedDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) { +func (p *ClosedDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_ string, _ []string, _ []string, _ string) (err error) { return errors.New("running commands is denied by policy") } diff --git a/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicy.go b/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicy.go index 7a78eb7b42..21983d60bf 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicy.go +++ b/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicy.go @@ -38,19 +38,27 @@ type EnvRuleConfig struct { // ContainerConfig contains toml or JSON config for container described // in security policy. type ContainerConfig struct { - ImageName string `json:"image_name" toml:"image_name"` - Command []string `json:"command" toml:"command"` - Auth AuthConfig `json:"auth" toml:"auth"` - EnvRules []EnvRule `json:"env_rules" toml:"env_rule"` + ImageName string `json:"image_name" toml:"image_name"` + Command []string `json:"command" toml:"command"` + Auth AuthConfig `json:"auth" toml:"auth"` + EnvRules []EnvRule `json:"env_rules" toml:"env_rule"` + WorkingDir string `json:"working_dir" toml:"working_dir"` } // NewContainerConfig creates a new ContainerConfig from the given values. -func NewContainerConfig(imageName string, command []string, envRules []EnvRule, auth AuthConfig) ContainerConfig { +func NewContainerConfig( + imageName string, + command []string, + envRules []EnvRule, + auth AuthConfig, + workingDir string, +) ContainerConfig { return ContainerConfig{ - ImageName: imageName, - Command: command, - EnvRules: envRules, - Auth: auth, + ImageName: imageName, + Command: command, + EnvRules: envRules, + Auth: auth, + WorkingDir: workingDir, } } @@ -72,6 +80,9 @@ type securityPolicyContainer struct { // order that the layers are overlayed is important and needs to be enforced // as part of policy. Layers []string `json:"layers"` + // WorkingDir is a path to container's working directory, which all the processes + // will default to. + WorkingDir string `json:"working_dir"` } // SecurityPolicyState is a structure that holds user supplied policy to enforce @@ -141,9 +152,10 @@ type Containers struct { } type Container struct { - Command CommandArgs `json:"command"` - EnvRules EnvRules `json:"env_rules"` - Layers Layers `json:"layers"` + Command CommandArgs `json:"command"` + EnvRules EnvRules `json:"env_rules"` + Layers Layers `json:"layers"` + WorkingDir string `json:"working_dir"` } type Layers struct { @@ -170,14 +182,15 @@ type EnvRule struct { // NewContainer creates a new Container instance from the provided values // or an error if envRules validation fails. -func NewContainer(command, layers []string, envRules []EnvRule) (*Container, error) { +func NewContainer(command, layers []string, envRules []EnvRule, workingDir string) (*Container, error) { if err := validateEnvRules(envRules); err != nil { return nil, err } return &Container{ - Command: newCommandArgs(command), - Layers: newLayers(layers), - EnvRules: newEnvRules(envRules), + Command: newCommandArgs(command), + Layers: newLayers(layers), + EnvRules: newEnvRules(envRules), + WorkingDir: workingDir, }, nil } diff --git a/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go b/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go index 21605509f3..9986dd7209 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go +++ b/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go @@ -14,7 +14,7 @@ type SecurityPolicyEnforcer interface { EnforceDeviceMountPolicy(target string, deviceHash string) (err error) EnforceDeviceUnmountPolicy(unmountTarget string) (err error) EnforceOverlayMountPolicy(containerID string, layerPaths []string) (err error) - EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) + EnforceCreateContainerPolicy(containerID string, argList []string, envList []string, workingDir string) (err error) } func NewSecurityPolicyEnforcer(state SecurityPolicyState) (SecurityPolicyEnforcer, error) { @@ -298,7 +298,12 @@ func (pe *StandardSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID return nil } -func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) { +func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy( + containerID string, + argList []string, + envList []string, + workingDir string, +) (err error) { pe.mutex.Lock() defer pe.mutex.Unlock() @@ -310,13 +315,15 @@ func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy(container return errors.New("container has already been started") } - err = pe.enforceCommandPolicy(containerID, argList) - if err != nil { + if err = pe.enforceCommandPolicy(containerID, argList); err != nil { return err } - err = pe.enforceEnvironmentVariablePolicy(containerID, envList) - if err != nil { + if err = pe.enforceEnvironmentVariablePolicy(containerID, envList); err != nil { + return err + } + + if err = pe.enforceWorkingDirPolicy(containerID, workingDir); err != nil { return err } @@ -385,6 +392,24 @@ func (pe *StandardSecurityPolicyEnforcer) enforceEnvironmentVariablePolicy(conta return nil } +func (pe *StandardSecurityPolicyEnforcer) enforceWorkingDirPolicy(containerID string, workingDir string) error { + possibleIndices := possibleIndexesForID(containerID, pe.ContainerIndexToContainerIds) + + matched := false + for _, pIndex := range possibleIndices { + pWorkingDir := pe.Containers[pIndex].WorkingDir + if pWorkingDir == workingDir { + matched = true + continue + } + pe.narrowMatchesForContainerIndex(pIndex, containerID) + } + if !matched { + return fmt.Errorf("working_dir %s unmached by policy rule", workingDir) + } + return nil +} + func envIsMatchedByRule(envVariable string, rules []EnvRule) bool { for _, rule := range rules { switch rule.Strategy { @@ -463,7 +488,7 @@ func (p *OpenDoorSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID s return nil } -func (p *OpenDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) { +func (p *OpenDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_ string, _ []string, _ []string, _ string) (err error) { return nil } @@ -483,6 +508,6 @@ func (p *ClosedDoorSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID return errors.New("creating an overlay fs is denied by policy") } -func (p *ClosedDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(containerID string, argList []string, envList []string) (err error) { +func (p *ClosedDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_ string, _ []string, _ []string, _ string) (err error) { return errors.New("running commands is denied by policy") } From 383cc9309befd83ebf474f9b15ad491f31148241 Mon Sep 17 00:00:00 2001 From: Maksim An Date: Fri, 18 Feb 2022 16:44:33 -0800 Subject: [PATCH 2/7] Minor refactor of securitypolicy_test Add a utility funcion that picks a random container from an array and generates a valid/invalid overlay for that container. Refactor tests to use the new utility function. Signed-off-by: Maksim An --- pkg/securitypolicy/securitypolicy_test.go | 210 +++++++++++----------- 1 file changed, 102 insertions(+), 108 deletions(-) diff --git a/pkg/securitypolicy/securitypolicy_test.go b/pkg/securitypolicy/securitypolicy_test.go index faf54588c9..7eb1b0f093 100644 --- a/pkg/securitypolicy/securitypolicy_test.go +++ b/pkg/securitypolicy/securitypolicy_test.go @@ -1,6 +1,7 @@ package securitypolicy import ( + "fmt" "math/rand" "reflect" "strconv" @@ -30,6 +31,12 @@ const ( ignoredEncodedPolicyString = "" ) +var testRand *rand.Rand + +func init() { + testRand = rand.New(rand.NewSource(time.Now().Unix())) +} + // Validate that our conversion from the external SecurityPolicy representation // to our internal format is done correctly. func Test_StandardSecurityPolicyEnforcer_From_Security_Policy_Conversion(t *testing.T) { @@ -215,19 +222,13 @@ func Test_EnforceDeviceUmountPolicy_Removes_Device_Entries(t *testing.T) { // return an error when there's no matching overlay targets. func Test_EnforceOverlayMountPolicy_No_Matches(t *testing.T) { f := func(p *generatedContainers) bool { - - policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) - - r := rand.New(rand.NewSource(time.Now().UnixNano())) - containerID := generateContainerID(r) - container := selectContainerFromContainers(p, r) - - layerPaths, err := createInvalidOverlayForContainer(policy, container, r) + tc, err := setupContainerWithOverlay(p, false) if err != nil { + t.Error(err) return false } - err = policy.EnforceOverlayMountPolicy(containerID, layerPaths) + err = tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers) // not getting an error means something is broken return err != nil @@ -242,19 +243,13 @@ func Test_EnforceOverlayMountPolicy_No_Matches(t *testing.T) { // return an error when there's a valid overlay target. func Test_EnforceOverlayMountPolicy_Matches(t *testing.T) { f := func(p *generatedContainers) bool { - - policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) - - r := rand.New(rand.NewSource(time.Now().UnixNano())) - containerID := generateContainerID(r) - container := selectContainerFromContainers(p, r) - - layerPaths, err := createValidOverlayForContainer(policy, container, r) + tc, err := setupContainerWithOverlay(p, true) if err != nil { + t.Error(err) return false } - err = policy.EnforceOverlayMountPolicy(containerID, layerPaths) + err = tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers) // getting an error means something is broken return err == nil @@ -267,26 +262,17 @@ func Test_EnforceOverlayMountPolicy_Matches(t *testing.T) { // Tests the specific case of trying to mount the same overlay twice using the /// same container id. This should be disallowed. func Test_EnforceOverlayMountPolicy_Overlay_Single_Container_Twice(t *testing.T) { - r := rand.New(rand.NewSource(time.Now().UnixNano())) - p := generateContainers(r, 1) - - policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) - - containerID := generateContainerID(r) - container := selectContainerFromContainers(p, r) - - layerPaths, err := createValidOverlayForContainer(policy, container, r) + gc := generateContainers(testRand, 1) + tc, err := setupContainerWithOverlay(gc, true) if err != nil { t.Fatalf("expected nil error got: %v", err) } - err = policy.EnforceOverlayMountPolicy(containerID, layerPaths) - if err != nil { + if err := tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err != nil { t.Fatalf("expected nil error got: %v", err) } - err = policy.EnforceOverlayMountPolicy(containerID, layerPaths) - if err == nil { + if err := tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err == nil { t.Fatalf("able to create overlay for the same container twice") } } @@ -297,10 +283,9 @@ func Test_EnforceOverlayMountPolicy_Overlay_Single_Container_Twice(t *testing.T) // all 13 should be allowed. func Test_EnforceOverlayMountPolicy_Multiple_Instances_Same_Container(t *testing.T) { for containersToCreate := 2; containersToCreate <= maxContainersInGeneratedPolicy; containersToCreate++ { - r := rand.New(rand.NewSource(time.Now().UnixNano())) var containers []securityPolicyContainer - for i := 1; i <= int(containersToCreate); i++ { + for i := 1; i <= containersToCreate; i++ { arg := "command " + strconv.Itoa(i) c := securityPolicyContainer{ Command: []string{arg}, @@ -310,11 +295,11 @@ func Test_EnforceOverlayMountPolicy_Multiple_Instances_Same_Container(t *testing containers = append(containers, c) } - policy := NewStandardSecurityPolicyEnforcer(containers, "") + sp := NewStandardSecurityPolicyEnforcer(containers, "") idsUsed := map[string]bool{} for i := 0; i < len(containers); i++ { - layerPaths, err := createValidOverlayForContainer(policy, containers[i], r) + layerPaths, err := createValidOverlayForContainer(sp, containers[i], testRand) if err != nil { t.Fatal("unexpected error on test setup") } @@ -322,12 +307,12 @@ func Test_EnforceOverlayMountPolicy_Multiple_Instances_Same_Container(t *testing idUnique := false var id string for idUnique == false { - id = generateContainerID(r) + id = generateContainerID(testRand) _, found := idsUsed[id] idUnique = !found idsUsed[id] = true } - err = policy.EnforceOverlayMountPolicy(id, layerPaths) + err = sp.EnforceOverlayMountPolicy(id, layerPaths) if err != nil { t.Fatalf("failed with %d containers", containersToCreate) } @@ -342,30 +327,28 @@ func Test_EnforceOverlayMountPolicy_Multiple_Instances_Same_Container(t *testing // policy, we should be able to create a single container for that overlay // but no more than that one. func Test_EnforceOverlayMountPolicy_Overlay_Single_Container_Twice_With_Different_IDs(t *testing.T) { - r := rand.New(rand.NewSource(time.Now().UnixNano())) - p := generateContainers(r, 1) - - policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) + p := generateContainers(testRand, 1) + sp := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) var containerIDOne, containerIDTwo string for containerIDOne == containerIDTwo { - containerIDOne = generateContainerID(r) - containerIDTwo = generateContainerID(r) + containerIDOne = generateContainerID(testRand) + containerIDTwo = generateContainerID(testRand) } - container := selectContainerFromContainers(p, r) + container := selectContainerFromContainers(p, testRand) - layerPaths, err := createValidOverlayForContainer(policy, container, r) + layerPaths, err := createValidOverlayForContainer(sp, container, testRand) if err != nil { t.Fatalf("expected nil error got: %v", err) } - err = policy.EnforceOverlayMountPolicy(containerIDOne, layerPaths) + err = sp.EnforceOverlayMountPolicy(containerIDOne, layerPaths) if err != nil { t.Fatalf("expected nil error got: %v", err) } - err = policy.EnforceOverlayMountPolicy(containerIDTwo, layerPaths) + err = sp.EnforceOverlayMountPolicy(containerIDTwo, layerPaths) if err == nil { t.Fatalf("able to reuse an overlay across containers") } @@ -373,23 +356,18 @@ func Test_EnforceOverlayMountPolicy_Overlay_Single_Container_Twice_With_Differen func Test_EnforceCommandPolicy_Matches(t *testing.T) { f := func(p *generatedContainers) bool { - policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) - - r := rand.New(rand.NewSource(time.Now().UnixNano())) - containerID := generateContainerID(r) - container := selectContainerFromContainers(p, r) - - layerPaths, err := createValidOverlayForContainer(policy, container, r) + tc, err := setupContainerWithOverlay(p, true) if err != nil { + t.Error(err) return false } - err = policy.EnforceOverlayMountPolicy(containerID, layerPaths) - if err != nil { + if err := tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err != nil { + t.Errorf("failed to enforce overlay mount policy: %s", err) return false } - err = policy.enforceCommandPolicy(containerID, container.Command) + err = tc.policy.enforceCommandPolicy(tc.containerID, tc.container.Command) // getting an error means something is broken return err == nil @@ -402,23 +380,18 @@ func Test_EnforceCommandPolicy_Matches(t *testing.T) { func Test_EnforceCommandPolicy_NoMatches(t *testing.T) { f := func(p *generatedContainers) bool { - policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) - - r := rand.New(rand.NewSource(time.Now().UnixNano())) - containerID := generateContainerID(r) - container := selectContainerFromContainers(p, r) - - layerPaths, err := createValidOverlayForContainer(policy, container, r) + tc, err := setupContainerWithOverlay(p, true) if err != nil { + t.Error(err) return false } - err = policy.EnforceOverlayMountPolicy(containerID, layerPaths) - if err != nil { + if err := tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err != nil { + t.Errorf("failed to enforce overlay mount policy: %s", err) return false } - err = policy.enforceCommandPolicy(containerID, generateCommand(r)) + err = tc.policy.enforceCommandPolicy(tc.containerID, generateCommand(testRand)) // not getting an error means something is broken return err != nil @@ -441,12 +414,11 @@ func Test_EnforceCommandPolicy_NoMatches(t *testing.T) { // the container in our policy" functionality works correctly. func Test_EnforceCommandPolicy_NarrowingMatches(t *testing.T) { f := func(p *generatedContainers) bool { - r := rand.New(rand.NewSource(time.Now().UnixNano())) // create two additional containers that "share everything" // except that they have different commands - testContainerOne := generateContainersContainer(r, 5) + testContainerOne := generateContainersContainer(testRand, 5) testContainerTwo := testContainerOne - testContainerTwo.Command = generateCommand(r) + testContainerTwo.Command = generateCommand(testRand) // add new containers to policy before creating enforcer p.containers = append(p.containers, testContainerOne, testContainerTwo) @@ -459,9 +431,9 @@ func Test_EnforceCommandPolicy_NarrowingMatches(t *testing.T) { // mount and overlay all our containers for index, container := range p.containers { - containerID := generateContainerID(r) + containerID := generateContainerID(testRand) - layerPaths, err := createValidOverlayForContainer(policy, container, r) + layerPaths, err := createValidOverlayForContainer(policy, container, testRand) if err != nil { return false } @@ -533,24 +505,18 @@ func Test_EnforceCommandPolicy_NarrowingMatches(t *testing.T) { func Test_EnforceEnvironmentVariablePolicy_Matches(t *testing.T) { f := func(p *generatedContainers) bool { - policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) - - r := rand.New(rand.NewSource(time.Now().UnixNano())) - containerID := generateContainerID(r) - container := selectContainerFromContainers(p, r) - - layerPaths, err := createValidOverlayForContainer(policy, container, r) + tc, err := setupContainerWithOverlay(p, true) if err != nil { + t.Error(err) return false } - - err = policy.EnforceOverlayMountPolicy(containerID, layerPaths) - if err != nil { + if err = tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err != nil { + t.Errorf("failed to enforce overlay mount policy: %s", err) return false } - envVars := buildEnvironmentVariablesFromContainerRules(container, r) - err = policy.enforceEnvironmentVariablePolicy(containerID, envVars) + envVars := buildEnvironmentVariablesFromContainerRules(tc.container, testRand) + err = tc.policy.enforceEnvironmentVariablePolicy(tc.containerID, envVars) // getting an error means something is broken return err == nil @@ -562,22 +528,22 @@ func Test_EnforceEnvironmentVariablePolicy_Matches(t *testing.T) { } func Test_EnforceEnvironmentVariablePolicy_Re2Match(t *testing.T) { - r := rand.New(rand.NewSource(time.Now().UnixNano())) - p := generateContainers(r, 1) + p := generateContainers(testRand, 1) - container := generateContainersContainer(r, 1) + container := generateContainersContainer(testRand, 1) // add a rule to re2 match re2MatchRule := EnvRule{ Strategy: EnvVarRuleRegex, - Rule: "PREFIX_.+=.+"} + Rule: "PREFIX_.+=.+", + } container.EnvRules = append(container.EnvRules, re2MatchRule) p.containers = append(p.containers, container) policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) - containerID := generateContainerID(r) + containerID := generateContainerID(testRand) - layerPaths, err := createValidOverlayForContainer(policy, container, r) + layerPaths, err := createValidOverlayForContainer(policy, container, testRand) if err != nil { t.Fatalf("expected nil error got: %v", err) } @@ -598,25 +564,19 @@ func Test_EnforceEnvironmentVariablePolicy_Re2Match(t *testing.T) { func Test_EnforceEnvironmentVariablePolicy_NotAllMatches(t *testing.T) { f := func(p *generatedContainers) bool { - policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString) - - r := rand.New(rand.NewSource(time.Now().UnixNano())) - containerID := generateContainerID(r) - container := selectContainerFromContainers(p, r) - - layerPaths, err := createValidOverlayForContainer(policy, container, r) + tc, err := setupContainerWithOverlay(p, true) if err != nil { + t.Error(err) return false } - - err = policy.EnforceOverlayMountPolicy(containerID, layerPaths) - if err != nil { + if err = tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err != nil { + t.Errorf("failed to enforce overlay mount policy: %s", err) return false } - envVars := generateEnvironmentVariables(r) - envVars = append(envVars, generateNeverMatchingEnvironmentVariable(r)) - err = policy.enforceEnvironmentVariablePolicy(containerID, envVars) + envVars := generateEnvironmentVariables(testRand) + envVars = append(envVars, generateNeverMatchingEnvironmentVariable(testRand)) + err = tc.policy.enforceEnvironmentVariablePolicy(tc.containerID, envVars) // not getting an error means something is broken return err != nil @@ -735,7 +695,7 @@ func Test_EnforceEnvironmentVariablePolicy_NarrowingMatches(t *testing.T) { // Setup and "fixtures" follow... // -func (*SecurityPolicy) Generate(r *rand.Rand, size int) reflect.Value { +func (*SecurityPolicy) Generate(r *rand.Rand, _ int) reflect.Value { // This fixture setup is used from 1 test. Given the limited scope it is // used from, all functionality is in this single function. That saves having // confusing fixture name functions where we have generate* for both internal @@ -793,11 +753,45 @@ func (*SecurityPolicy) Generate(r *rand.Rand, size int) reflect.Value { return reflect.ValueOf(p) } -func (*generatedContainers) Generate(r *rand.Rand, size int) reflect.Value { +func (*generatedContainers) Generate(r *rand.Rand, _ int) reflect.Value { c := generateContainers(r, maxContainersInGeneratedPolicy) return reflect.ValueOf(c) } +type testConfig struct { + container securityPolicyContainer + layers []string + containerID string + policy *StandardSecurityPolicyEnforcer +} + +func setupContainerWithOverlay(gc *generatedContainers, valid bool) (tc *testConfig, err error) { + sp := NewStandardSecurityPolicyEnforcer(gc.containers, ignoredEncodedPolicyString) + + containerID := generateContainerID(testRand) + c := selectContainerFromContainers(gc, testRand) + + var layerPaths []string + if valid { + layerPaths, err = createValidOverlayForContainer(sp, c, testRand) + if err != nil { + return nil, fmt.Errorf("error creating valid overlay: %w", err) + } + } else { + layerPaths, err = createInvalidOverlayForContainer(sp, c, testRand) + if err != nil { + return nil, fmt.Errorf("error creating invalid overlay: %w", err) + } + } + + return &testConfig{ + container: c, + layers: layerPaths, + containerID: containerID, + policy: sp, + }, nil +} + func generateContainers(r *rand.Rand, upTo int32) *generatedContainers { var containers []securityPolicyContainer @@ -828,7 +822,7 @@ func generateRootHash(r *rand.Rand) string { } func generateCommand(r *rand.Rand) []string { - args := []string{} + var args []string numArgs := atLeastOneAtMost(r, maxGeneratedCommandArgs) for i := 0; i < int(numArgs); i++ { @@ -854,7 +848,7 @@ func generateEnvironmentVariableRules(r *rand.Rand) []EnvRule { } func generateEnvironmentVariables(r *rand.Rand) []string { - envVars := []string{} + var envVars []string numVars := atLeastOneAtMost(r, maxGeneratedEnvironmentVariables) for i := 0; i < int(numVars); i++ { @@ -1034,7 +1028,7 @@ func randVariableString(r *rand.Rand, maxLen int32) string { func randString(r *rand.Rand, len int32) string { var s strings.Builder for i := 0; i < (int)(len); i++ { - s.WriteRune((rune)(0x00ff & r.Int31n(256))) + s.WriteRune(0x00ff & r.Int31n(256)) } return s.String() From fe6c2bebf0926a1088187b69015bc8c4e70d0bb5 Mon Sep 17 00:00:00 2001 From: Maksim An Date: Fri, 18 Feb 2022 17:27:02 -0800 Subject: [PATCH 3/7] Add unit tests for enforcing working directory Signed-off-by: Maksim An --- pkg/securitypolicy/securitypolicy_test.go | 45 +++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/pkg/securitypolicy/securitypolicy_test.go b/pkg/securitypolicy/securitypolicy_test.go index 7eb1b0f093..1c887a7e4c 100644 --- a/pkg/securitypolicy/securitypolicy_test.go +++ b/pkg/securitypolicy/securitypolicy_test.go @@ -691,6 +691,50 @@ func Test_EnforceEnvironmentVariablePolicy_NarrowingMatches(t *testing.T) { } } +func Test_WorkingDirectoryPolicy_Matches(t *testing.T) { + testFunc := func(gc *generatedContainers) bool { + tc, err := setupContainerWithOverlay(gc, true) + if err != nil { + t.Error(err) + return false + } + + if err := tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err != nil { + t.Errorf("failed to enforce overlay mount policy: %s", err) + return false + } + + err = tc.policy.enforceWorkingDirPolicy(tc.containerID, tc.container.WorkingDir) + return err == nil + } + + if err := quick.Check(testFunc, &quick.Config{MaxCount: 1000}); err != nil { + t.Errorf("Test_WorkingDirectoryPolicy_Matches: %v", err) + } +} + +func Test_WorkingDirectoryPolicy_NoMatches(t *testing.T) { + testFunc := func(gc *generatedContainers) bool { + tc, err := setupContainerWithOverlay(gc, true) + if err != nil { + t.Error(err) + return false + } + + if err := tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err != nil { + t.Errorf("failed to enforce overlay mount policy: %s", err) + return false + } + + err = tc.policy.enforceWorkingDirPolicy(tc.containerID, randString(testRand, 20)) + return err != nil + } + + if err := quick.Check(testFunc, &quick.Config{MaxCount: 1000}); err != nil { + t.Errorf("Test_WorkingDirectoryPolicy_NoMatches: %v", err) + } +} + // // Setup and "fixtures" follow... // @@ -809,6 +853,7 @@ func generateContainersContainer(r *rand.Rand, size int32) securityPolicyContain c := securityPolicyContainer{} c.Command = generateCommand(r) c.EnvRules = generateEnvironmentVariableRules(r) + c.WorkingDir = randVariableString(r, maxGeneratedCommandLength) layers := int(atLeastOneAtMost(r, size)) for i := 0; i < layers; i++ { c.Layers = append(c.Layers, generateRootHash(r)) From 815cced2861e509fc72b251b8307e7e7007561e9 Mon Sep 17 00:00:00 2001 From: Maksim An Date: Mon, 28 Feb 2022 14:11:32 -0800 Subject: [PATCH 4/7] pr feedback: fix loop and log seed Signed-off-by: Maksim An --- pkg/securitypolicy/securitypolicy_test.go | 5 ++++- pkg/securitypolicy/securitypolicyenforcer.go | 4 ++-- .../hcsshim/pkg/securitypolicy/securitypolicyenforcer.go | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/securitypolicy/securitypolicy_test.go b/pkg/securitypolicy/securitypolicy_test.go index 1c887a7e4c..fbc666f66a 100644 --- a/pkg/securitypolicy/securitypolicy_test.go +++ b/pkg/securitypolicy/securitypolicy_test.go @@ -3,6 +3,7 @@ package securitypolicy import ( "fmt" "math/rand" + "os" "reflect" "strconv" "strings" @@ -34,7 +35,9 @@ const ( var testRand *rand.Rand func init() { - testRand = rand.New(rand.NewSource(time.Now().Unix())) + seed := rand.NewSource(time.Now().Unix()) + testRand = rand.New(seed) + fmt.Fprintf(os.Stdout, "securitypolicy_test seed: %d\n", seed.Int63()) } // Validate that our conversion from the external SecurityPolicy representation diff --git a/pkg/securitypolicy/securitypolicyenforcer.go b/pkg/securitypolicy/securitypolicyenforcer.go index 9986dd7209..380a7be814 100644 --- a/pkg/securitypolicy/securitypolicyenforcer.go +++ b/pkg/securitypolicy/securitypolicyenforcer.go @@ -400,9 +400,9 @@ func (pe *StandardSecurityPolicyEnforcer) enforceWorkingDirPolicy(containerID st pWorkingDir := pe.Containers[pIndex].WorkingDir if pWorkingDir == workingDir { matched = true - continue + } else { + pe.narrowMatchesForContainerIndex(pIndex, containerID) } - pe.narrowMatchesForContainerIndex(pIndex, containerID) } if !matched { return fmt.Errorf("working_dir %s unmached by policy rule", workingDir) diff --git a/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go b/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go index 9986dd7209..380a7be814 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go +++ b/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go @@ -400,9 +400,9 @@ func (pe *StandardSecurityPolicyEnforcer) enforceWorkingDirPolicy(containerID st pWorkingDir := pe.Containers[pIndex].WorkingDir if pWorkingDir == workingDir { matched = true - continue + } else { + pe.narrowMatchesForContainerIndex(pIndex, containerID) } - pe.narrowMatchesForContainerIndex(pIndex, containerID) } if !matched { return fmt.Errorf("working_dir %s unmached by policy rule", workingDir) From 3906817d3610c60b7798bd1faa503784a9ce9015 Mon Sep 17 00:00:00 2001 From: Maksim An Date: Mon, 28 Feb 2022 14:27:52 -0800 Subject: [PATCH 5/7] fix linting issue Signed-off-by: Maksim An --- internal/jobcontainers/logon.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/jobcontainers/logon.go b/internal/jobcontainers/logon.go index 056d06e1b5..0ccfebd4da 100644 --- a/internal/jobcontainers/logon.go +++ b/internal/jobcontainers/logon.go @@ -31,7 +31,9 @@ func groupExists(groupName string) bool { ); err != nil { return false } - defer windows.NetApiBufferFree(p) + defer func() { + _ = windows.NetApiBufferFree(p) + }() return true } From f7d03f0e2c4ef149992656e8ca896cfd5535c56f Mon Sep 17 00:00:00 2001 From: Maksim An Date: Fri, 4 Mar 2022 12:04:26 -0800 Subject: [PATCH 6/7] pr feedback: use indices instead of indexes in variable names Signed-off-by: Maksim An --- pkg/securitypolicy/securitypolicy_test.go | 6 ++---- pkg/securitypolicy/securitypolicyenforcer.go | 14 +++++++------- .../pkg/securitypolicy/securitypolicyenforcer.go | 14 +++++++------- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/pkg/securitypolicy/securitypolicy_test.go b/pkg/securitypolicy/securitypolicy_test.go index fbc666f66a..e4e1598e2f 100644 --- a/pkg/securitypolicy/securitypolicy_test.go +++ b/pkg/securitypolicy/securitypolicy_test.go @@ -707,8 +707,7 @@ func Test_WorkingDirectoryPolicy_Matches(t *testing.T) { return false } - err = tc.policy.enforceWorkingDirPolicy(tc.containerID, tc.container.WorkingDir) - return err == nil + return tc.policy.enforceWorkingDirPolicy(tc.containerID, tc.container.WorkingDir) == nil } if err := quick.Check(testFunc, &quick.Config{MaxCount: 1000}); err != nil { @@ -729,8 +728,7 @@ func Test_WorkingDirectoryPolicy_NoMatches(t *testing.T) { return false } - err = tc.policy.enforceWorkingDirPolicy(tc.containerID, randString(testRand, 20)) - return err != nil + return tc.policy.enforceWorkingDirPolicy(tc.containerID, randString(testRand, 20)) != nil } if err := quick.Check(testFunc, &quick.Config{MaxCount: 1000}); err != nil { diff --git a/pkg/securitypolicy/securitypolicyenforcer.go b/pkg/securitypolicy/securitypolicyenforcer.go index 380a7be814..1c8ed714e0 100644 --- a/pkg/securitypolicy/securitypolicyenforcer.go +++ b/pkg/securitypolicy/securitypolicyenforcer.go @@ -334,10 +334,10 @@ func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy( } func (pe *StandardSecurityPolicyEnforcer) enforceCommandPolicy(containerID string, argList []string) (err error) { - // Get a list of all the indexes into our security policy's list of + // Get a list of all the indices into our security policy's list of // containers that are possible matches for this containerID based // on the image overlay layout - possibleIndexes := possibleIndexesForID(containerID, pe.ContainerIndexToContainerIds) + possibleIndices := possibleIndicesForID(containerID, pe.ContainerIndexToContainerIds) // Loop through every possible match and do two things: // 1- see if any command matches. we need at least one match or @@ -345,7 +345,7 @@ func (pe *StandardSecurityPolicyEnforcer) enforceCommandPolicy(containerID strin // 2- remove this containerID as a possible match for any container from the // security policy whose command line isn't a match. matchingCommandFound := false - for _, possibleIndex := range possibleIndexes { + for _, possibleIndex := range possibleIndices { cmd := pe.Containers[possibleIndex].Command if cmp.Equal(cmd, argList) { matchingCommandFound = true @@ -368,11 +368,11 @@ func (pe *StandardSecurityPolicyEnforcer) enforceEnvironmentVariablePolicy(conta // Get a list of all the indexes into our security policy's list of // containers that are possible matches for this containerID based // on the image overlay layout and command line - possibleIndexes := possibleIndexesForID(containerID, pe.ContainerIndexToContainerIds) + possibleIndices := possibleIndicesForID(containerID, pe.ContainerIndexToContainerIds) for _, envVariable := range envList { matchingRuleFoundForSomeContainer := false - for _, possibleIndex := range possibleIndexes { + for _, possibleIndex := range possibleIndices { envRules := pe.Containers[possibleIndex].EnvRules ok := envIsMatchedByRule(envVariable, envRules) if ok { @@ -393,7 +393,7 @@ func (pe *StandardSecurityPolicyEnforcer) enforceEnvironmentVariablePolicy(conta } func (pe *StandardSecurityPolicyEnforcer) enforceWorkingDirPolicy(containerID string, workingDir string) error { - possibleIndices := possibleIndexesForID(containerID, pe.ContainerIndexToContainerIds) + possibleIndices := possibleIndicesForID(containerID, pe.ContainerIndexToContainerIds) matched := false for _, pIndex := range possibleIndices { @@ -459,7 +459,7 @@ func equalForOverlay(a1 []string, a2 []string) bool { return true } -func possibleIndexesForID(containerID string, mapping map[int]map[string]struct{}) []int { +func possibleIndicesForID(containerID string, mapping map[int]map[string]struct{}) []int { possibles := []int{} for index, ids := range mapping { for id := range ids { diff --git a/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go b/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go index 380a7be814..1c8ed714e0 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go +++ b/test/vendor/github.com/Microsoft/hcsshim/pkg/securitypolicy/securitypolicyenforcer.go @@ -334,10 +334,10 @@ func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy( } func (pe *StandardSecurityPolicyEnforcer) enforceCommandPolicy(containerID string, argList []string) (err error) { - // Get a list of all the indexes into our security policy's list of + // Get a list of all the indices into our security policy's list of // containers that are possible matches for this containerID based // on the image overlay layout - possibleIndexes := possibleIndexesForID(containerID, pe.ContainerIndexToContainerIds) + possibleIndices := possibleIndicesForID(containerID, pe.ContainerIndexToContainerIds) // Loop through every possible match and do two things: // 1- see if any command matches. we need at least one match or @@ -345,7 +345,7 @@ func (pe *StandardSecurityPolicyEnforcer) enforceCommandPolicy(containerID strin // 2- remove this containerID as a possible match for any container from the // security policy whose command line isn't a match. matchingCommandFound := false - for _, possibleIndex := range possibleIndexes { + for _, possibleIndex := range possibleIndices { cmd := pe.Containers[possibleIndex].Command if cmp.Equal(cmd, argList) { matchingCommandFound = true @@ -368,11 +368,11 @@ func (pe *StandardSecurityPolicyEnforcer) enforceEnvironmentVariablePolicy(conta // Get a list of all the indexes into our security policy's list of // containers that are possible matches for this containerID based // on the image overlay layout and command line - possibleIndexes := possibleIndexesForID(containerID, pe.ContainerIndexToContainerIds) + possibleIndices := possibleIndicesForID(containerID, pe.ContainerIndexToContainerIds) for _, envVariable := range envList { matchingRuleFoundForSomeContainer := false - for _, possibleIndex := range possibleIndexes { + for _, possibleIndex := range possibleIndices { envRules := pe.Containers[possibleIndex].EnvRules ok := envIsMatchedByRule(envVariable, envRules) if ok { @@ -393,7 +393,7 @@ func (pe *StandardSecurityPolicyEnforcer) enforceEnvironmentVariablePolicy(conta } func (pe *StandardSecurityPolicyEnforcer) enforceWorkingDirPolicy(containerID string, workingDir string) error { - possibleIndices := possibleIndexesForID(containerID, pe.ContainerIndexToContainerIds) + possibleIndices := possibleIndicesForID(containerID, pe.ContainerIndexToContainerIds) matched := false for _, pIndex := range possibleIndices { @@ -459,7 +459,7 @@ func equalForOverlay(a1 []string, a2 []string) bool { return true } -func possibleIndexesForID(containerID string, mapping map[int]map[string]struct{}) []int { +func possibleIndicesForID(containerID string, mapping map[int]map[string]struct{}) []int { possibles := []int{} for index, ids := range mapping { for id := range ids { From d9dfbee02cd39230a2e12beae14d714b81840657 Mon Sep 17 00:00:00 2001 From: Maksim An Date: Fri, 4 Mar 2022 12:05:18 -0800 Subject: [PATCH 7/7] Set CWD to "/" when not set in image and policy config Signed-off-by: Maksim An --- internal/tools/securitypolicy/main.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/tools/securitypolicy/main.go b/internal/tools/securitypolicy/main.go index 37ae31ff53..cdb21c5a3f 100644 --- a/internal/tools/securitypolicy/main.go +++ b/internal/tools/securitypolicy/main.go @@ -148,10 +148,13 @@ func createPolicyFromConfig(config *securitypolicy.PolicyConfig) (*securitypolic } envRules = append(envRules, rule) - workingDir := containerConfig.WorkingDir - if workingDir == "" { + workingDir := "/" + if imgConfig.Config.WorkingDir != "" { workingDir = imgConfig.Config.WorkingDir } + if containerConfig.WorkingDir != "" { + workingDir = containerConfig.WorkingDir + } container, err := securitypolicy.NewContainer(containerConfig.Command, layerHashes, envRules, workingDir) if err != nil { return nil, err