From 9782dee21cf90d55a6fff7e8744d53603f9a2b57 Mon Sep 17 00:00:00 2001 From: Matthew A Johnson Date: Wed, 7 Dec 2022 18:26:16 +0000 Subject: [PATCH] Add ability in policy to allow/disallow access to stdio (#1594) This commit adds the ability of policy at the time of create container to allow or disallow access to standard io for that container. And on the external process side, if an external process is allowed to access standard io. This is done in the same way as dropping environment variables is implemented. At policy enforcement time, policy will indicate if standard io access is allowed as part of the create being allowed. So like with environment variables where it is "allow, but only with these environment variables" now we also have "allow, but do not allow standard io access". Turning off standard io for containers in a way that didn't break some expectation within the hcs/gcs relationship turned out to be remarkably difficult. Maksim and I tried a couple different approaches before settling on the approach of creating a new transport for handling the disallowed standard io access case. One of the things we had attempted was to have special TTY and PipeRelays. However, we abandonded that approach as it resulted in a ton of duplicated code. The "devnull transport approach" that this commit implements doesn't result in duplicated code. And most importantly, has been able to pass testing and not result in bugs somewhere else in the gcs/hcs relationship. When work was started on this, we expected this to take a few days to get correct. It turned out to take several weeks because the hcs/gcs standard io relationship is filled with expectations and invariants that aren't documented and are spread throughtout the code. Maksim and I settled on this approach as we felt it had the lowest overhead for maintenance and was the least likely going forward to introduce sublte bugs while passing current testing. Signed-off-by: Sean T. Allen --- internal/guest/runtime/hcsv2/uvm.go | 41 +- internal/guest/transport/devnull.go | 132 ++++++ .../tools/securitypolicy/helpers/helpers.go | 1 + pkg/securitypolicy/framework.rego | 37 +- pkg/securitypolicy/open_door.rego | 4 +- pkg/securitypolicy/opts.go | 8 + pkg/securitypolicy/regopolicy_test.go | 384 +++++++++++++----- pkg/securitypolicy/securitypolicy.go | 59 +-- pkg/securitypolicy/securitypolicy_internal.go | 23 +- pkg/securitypolicy/securitypolicy_marshal.go | 5 +- pkg/securitypolicy/securitypolicy_test.go | 15 +- pkg/securitypolicy/securitypolicyenforcer.go | 54 +-- .../securitypolicyenforcer_rego.go | 71 +++- test/cri-containerd/policy_test.go | 77 +++- 14 files changed, 696 insertions(+), 215 deletions(-) create mode 100644 internal/guest/transport/devnull.go diff --git a/internal/guest/runtime/hcsv2/uvm.go b/internal/guest/runtime/hcsv2/uvm.go index 778bde5740..e8edbb7441 100644 --- a/internal/guest/runtime/hcsv2/uvm.go +++ b/internal/guest/runtime/hcsv2/uvm.go @@ -58,8 +58,9 @@ type Host struct { externalProcesses map[int]*externalProcess // Rtime is the Runtime interface used by the GCS core. - rtime runtime.Runtime - vsock transport.Transport + rtime runtime.Runtime + vsock transport.Transport + devNullTransport transport.Transport // state required for the security policy enforcement policyMutex sync.Mutex @@ -80,6 +81,7 @@ func NewHost(rtime runtime.Runtime, vsock transport.Transport, initialEnforcer s externalProcesses: make(map[int]*externalProcess), rtime: rtime, vsock: vsock, + devNullTransport: &transport.DevNullTransport{}, securityPolicyEnforcerSet: false, securityPolicyEnforcer: initialEnforcer, logWriter: logWriter, @@ -315,7 +317,7 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM }() } - envToKeep, err := h.securityPolicyEnforcer.EnforceCreateContainerPolicy( + envToKeep, allowStdio, err := h.securityPolicyEnforcer.EnforceCreateContainerPolicy( sandboxID, id, settings.OCISpecification.Process.Args, @@ -327,6 +329,12 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM return nil, errors.Wrapf(err, "container creation denied due to policy") } + if !allowStdio { + // stdio access isn't allow for this container. Switch to the /dev/null + // transport that will eat all input/ouput. + c.vsock = h.devNullTransport + } + if envToKeep != nil { settings.OCISpecification.Process.Env = []string(envToKeep) } @@ -561,7 +569,8 @@ func (h *Host) ExecProcess(ctx context.Context, containerID string, params prot. var c *Container var envToKeep securitypolicy.EnvList if params.IsExternal || containerID == UVMContainerID { - envToKeep, err = h.securityPolicyEnforcer.EnforceExecExternalProcessPolicy( + var allowStdioAccess bool + envToKeep, allowStdioAccess, err = h.securityPolicyEnforcer.EnforceExecExternalProcessPolicy( params.CommandArgs, processParamEnvToOCIEnv(params.Environment), params.WorkingDirectory, @@ -570,11 +579,21 @@ func (h *Host) ExecProcess(ctx context.Context, containerID string, params prot. return pid, errors.Wrapf(err, "exec is denied due to policy") } + // It makes no sense to allow access if stdio access is denied and the + // process requires a terminal. + if params.EmulateConsole && !allowStdioAccess { + return pid, errors.New("exec of process that requires terminal access denied due to policy not allowing stdio access") + } + if envToKeep != nil { params.Environment = processOCIEnvToParam(envToKeep) } - pid, err = h.runExternalProcess(ctx, params, conSettings) + var tport = h.vsock + if !allowStdioAccess { + tport = h.devNullTransport + } + pid, err = h.runExternalProcess(ctx, params, conSettings, tport) } else if c, err = h.GetCreatedContainer(containerID); err == nil { // We found a V2 container. Treat this as a V2 process. if params.OCIProcess == nil { @@ -582,13 +601,20 @@ func (h *Host) ExecProcess(ctx context.Context, containerID string, params prot. // there's no policy enforcement to do for starting pid, err = c.Start(ctx, conSettings) } else { + var allowStdioAccess bool // Windows uses a different field for command, there's no enforcement // around this yet for Windows so this is Linux specific at the moment. - envToKeep, err = h.securityPolicyEnforcer.EnforceExecInContainerPolicy(containerID, params.OCIProcess.Args, params.OCIProcess.Env, params.OCIProcess.Cwd) + envToKeep, allowStdioAccess, err = h.securityPolicyEnforcer.EnforceExecInContainerPolicy(containerID, params.OCIProcess.Args, params.OCIProcess.Env, params.OCIProcess.Cwd) if err != nil { return pid, errors.Wrapf(err, "exec in container denied due to policy") } + // It makes no sense to allow access if stdio access is denied and the + // process requires a terminal. + if params.OCIProcess.Terminal && !allowStdioAccess { + return pid, errors.New("exec in container of process that requires terminal access denied due to policy not allowing stdio access") + } + if envToKeep != nil { params.OCIProcess.Env = envToKeep } @@ -659,9 +685,10 @@ func (h *Host) runExternalProcess( ctx context.Context, params prot.ProcessParameters, conSettings stdio.ConnectionSettings, + tport transport.Transport, ) (_ int, err error) { var stdioSet *stdio.ConnectionSet - stdioSet, err = stdio.Connect(h.vsock, conSettings) + stdioSet, err = stdio.Connect(tport, conSettings) if err != nil { return -1, err } diff --git a/internal/guest/transport/devnull.go b/internal/guest/transport/devnull.go new file mode 100644 index 0000000000..79c48a3fb3 --- /dev/null +++ b/internal/guest/transport/devnull.go @@ -0,0 +1,132 @@ +//go:build linux +// +build linux + +package transport + +import ( + "os" + + "github.com/sirupsen/logrus" +) + +// DevNullTransport is a transport that will: +// +// For reads: return either closed or EOF for as appropriate. +// For writers: return either closed or throw away the write as appropriate. +// +// The DevNullTransport is used as the container logging transport when stdio +// access is denied. It is also used for non-terminal external processes (aka +// a process run in the UVM) when stdio access is denied. +type DevNullTransport struct{} + +func (t *DevNullTransport) Dial(fd uint32) (Connection, error) { + logrus.WithFields(logrus.Fields{ + "fd": fd, + }).Info("opengcs::DevNullTransport::Dial") + + return newDevNullConnection(), nil +} + +// devNullConnection is the heart of our new transport. A devNullConnection +// contains two file descriptors. One for read, and one for write. We need to +// file descriptors as the code using a transport is written with the +// expectation of a duplex connection where read and write can be closed +// independent of one another. The protocol that uses the connection requires +// a duplex connection in order to work correctly. +// +// The original design of devNullConnection didn't use os.File and instead +// emulated the required behavior and only returned a os.File from the `File` +// function. However, the amount of state required to manage that was somewhat +// high and the logic to follow was somewhat convoluted. Using two file handles +// one read, one write that have /dev/null open results in a much cleaner +// design and easier to understand semantics. We want to mirror the sematics of +// /dev/null as a duplex connection, this is the simplest, most understandable +// way to achieve that. +type devNullConnection struct { + read *os.File + write *os.File +} + +func newDevNullConnection() *devNullConnection { + r, _ := os.OpenFile(os.DevNull, os.O_RDONLY, 0) + w, _ := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + + return &devNullConnection{read: r, write: w} +} + +func (c *devNullConnection) Close() error { + err1 := c.read.Close() + err2 := c.write.Close() + + if err1 != nil { + return err1 + } + + if err2 != nil { + return err2 + } + + return nil +} + +func (c *devNullConnection) CloseRead() error { + return c.read.Close() +} + +func (c *devNullConnection) CloseWrite() error { + return c.write.Close() +} + +func (c *devNullConnection) Read(buf []byte) (int, error) { + return c.read.Read(buf) +} + +func (c *devNullConnection) Write(buf []byte) (int, error) { + return c.write.Write(buf) +} + +// File() is where our lack of a real duplex connection is problematic. Code +// that uses a connection sidesteps the connection abstraction and asks for +// "the file" directly. With vsock, which is an actual duplex connection, this +// isn't actually a problem as a vsock connection is duplex. It dups the +// connection and returns and everything works just fine. +// +// Our emulating a duplex connection could be problematic depending on what +// code using the os.File returned from File() expects it semantics to be. +// In particular, with a dup like vsock does, if you close the os.File returned +// from File() it closes the connection. That isn't the case with our emulated +// duplex connection as closing the os.File that devNullConneciton.File() +// returns has no impact on the `read` and `write` file handles that are used +// during Read/Write calls. +// +// In the current usage in GCS, this isn't problematic but could becomes so. +// If you end up here because of a bug, here is the statement from the author +// about ways to fix this problem. +// +// 1. File() is a leaky abstraction. A transport should be required to be duplex +// so, having a single `File()` call that allows the caller to reach into the +// guts of a transport is bad and should be done away with. All usage of a +// transport/connection should go through their abstraction, not bypass it +// when bypassing is more convenient. +// 2. If File() didn't return a concrete type like os.File and instead returned +// an interface, then a more complicated version of devNullConnection that +// doesn't use a file handle could be written and the object that we return +// could be able to correctly handle the emulation of a duplex object. +// +// Both of the above are somewhat large changes and were not done at the time +// the initial version of DevNullTransport and devNullConnection were done. It +// was decided to leave the leaky abstraction in place as the code that uses +// transports is fiddly and error-prone based on not very well documentated +// assumptions. If you are considering taking on either 1 or 2 from above, you +// should have plenty of time on your hands as extensive testing will be +// required and the introduction of bugs both subtle and obvious is a likely +// possibility. Aka, reworking this code to remove the leaky abstraction is more +// than a refactoring and should be done with extreme care. +func (c *devNullConnection) File() (*os.File, error) { + f, err := os.OpenFile(os.DevNull, os.O_RDWR, 0) + if err != nil { + return nil, err + } + + return f, nil +} diff --git a/internal/tools/securitypolicy/helpers/helpers.go b/internal/tools/securitypolicy/helpers/helpers.go index 0b4d8f2b57..52539eaf1a 100644 --- a/internal/tools/securitypolicy/helpers/helpers.go +++ b/internal/tools/securitypolicy/helpers/helpers.go @@ -170,6 +170,7 @@ func PolicyContainersFromConfigs(containerConfigs []securitypolicy.ContainerConf containerConfig.AllowElevated, containerConfig.ExecProcesses, containerConfig.Signals, + containerConfig.AllowStdioAccess, ) if err != nil { return nil, err diff --git a/pkg/securitypolicy/framework.rego b/pkg/securitypolicy/framework.rego index ec6331c9bc..0b7d5053a9 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -157,7 +157,7 @@ valid_envs_for_all(items) := envs { some item in items envs := valid_envs_subset(item.env_rules) ] - + # we want to select the most specific matches, which in this # case consists of those matches which require dropping the # fewest environment variables (i.e. the longest lists) @@ -201,7 +201,11 @@ container_started { default create_container := {"allowed": false} -create_container := {"matches": matches, "env_list": env_list, "started": started, "allowed": true} { +create_container := {"matches": matches, + "env_list": env_list, + "allow_stdio_access": allow_stdio_access, + "started": started, + "allowed": true} { not container_started # narrow the matches based upon command, working directory, and # mount list @@ -223,6 +227,16 @@ create_container := {"matches": matches, "env_list": env_list, "started": starte ] count(containers) > 0 + + # we can't do narrowing based on allowing stdio access so at this point + # every container from the policy that might match this create request + # must have the same allow stdio value otherwise, we are in an undecidable + # state + allow_stdio_access := containers[0].allow_stdio_access + every c in containers { + c.allow_stdio_access == allow_stdio_access + } + matches := { "action": "update", "key": input.containerID, @@ -303,7 +317,9 @@ mountList_ok(mounts, allow_elevated) { default exec_in_container := {"allowed": false} -exec_in_container := {"matches": matches, "env_list": env_list, "allowed": true} { +exec_in_container := {"matches": matches, + "env_list": env_list, + "allowed": true} { container_started # narrow our matches based upon the process requested possible_containers := [container | @@ -432,7 +448,9 @@ external_process_ok(process) { default exec_external := {"allowed": false} -exec_external := {"allowed": true, "env_list": env_list} { +exec_external := {"allowed": true, + "allow_stdio_access": allow_stdio_access, + "env_list": env_list} { # we need to assemble a list of all possible external processes which # have a matching working directory and command policy_processes := [process | @@ -460,6 +478,11 @@ exec_external := {"allowed": true, "env_list": env_list} { ] count(processes) > 0 + + allow_stdio_access := processes[0].allow_stdio_access + every p in processes { + p.allow_stdio_access == allow_stdio_access + } } default get_properties := {"allowed": false} @@ -715,11 +738,11 @@ env_matches(env) { errors[envError] { input.rule in ["create_container", "exec_in_container", "exec_external"] - bad_envs := [env | + bad_envs := [env | env := input.envList[_] not env_matches(env)] count(bad_envs) > 0 - envError := concat(" ", ["invalid env list:", concat(",", bad_envs)]) + envError := concat(" ", ["invalid env list:", concat(",", bad_envs)]) } default workingDirectory_matches := false @@ -748,7 +771,7 @@ mount_matches(mount) { errors[mountError] { input.rule == "create_container" - bad_mounts := [mount.destination | + bad_mounts := [mount.destination | mount := input.mounts[_] not mount_matches(mount)] count(bad_mounts) > 0 diff --git a/pkg/securitypolicy/open_door.rego b/pkg/securitypolicy/open_door.rego index 9c1ae2523a..9346a32984 100644 --- a/pkg/securitypolicy/open_door.rego +++ b/pkg/securitypolicy/open_door.rego @@ -4,11 +4,11 @@ api_svn := "0.10.0" mount_device := {"allowed": true} mount_overlay := {"allowed": true} -create_container := {"allowed": true} +create_container := {"allowed": true, "allow_stdio_access": true} unmount_device := {"allowed": true} unmount_overlay := {"allowed": true} exec_in_container := {"allowed": true} -exec_external := {"allowed": true} +exec_external := {"allowed": true, "allow_stdio_access": true} shutdown_container := {"allowed": true} signal_container_process := {"allowed": true} plan9_mount := {"allowed": true} diff --git a/pkg/securitypolicy/opts.go b/pkg/securitypolicy/opts.go index d9510e1919..8587c1fb47 100644 --- a/pkg/securitypolicy/opts.go +++ b/pkg/securitypolicy/opts.go @@ -42,3 +42,11 @@ func WithCommand(cmd []string) ContainerConfigOpt { return nil } } + +// WithAllowStdioAccess enables or disables container init process stdio. +func WithAllowStdioAccess(stdio bool) ContainerConfigOpt { + return func(c *ContainerConfig) error { + c.AllowStdioAccess = stdio + return nil + } +} diff --git a/pkg/securitypolicy/regopolicy_test.go b/pkg/securitypolicy/regopolicy_test.go index 3b95f9898c..5a16fa0ea3 100644 --- a/pkg/securitypolicy/regopolicy_test.go +++ b/pkg/securitypolicy/regopolicy_test.go @@ -682,7 +682,7 @@ func Test_Rego_EnforceCommandPolicy_NoMatches(t *testing.T) { return false } - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, generateCommand(testRand), tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, generateCommand(testRand), tc.envList, tc.workingDir, tc.mounts) if err == nil { return false @@ -714,7 +714,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_Re2Match(t *testing.T) { } envList := append(tc.envList, "PREFIX_FOO=BAR") - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) // getting an error means something is broken if err != nil { @@ -739,7 +739,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_NotAllMatches(t *testing.T) { } envList := append(tc.envList, generateNeverMatchingEnvironmentVariable(testRand)) - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -774,7 +774,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_DropEnvs(t *testing.T) { extraEnvs := buildEnvironmentVariablesFromEnvRules(extraRules, testRand) envList := append(tc.envList, extraEnvs...) - actual, err := tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) + actual, _, err := tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) // getting an error means something is broken if err != nil { @@ -805,7 +805,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_DropEnvs_Multiple(t *testing.T) extraEnvs := buildEnvironmentVariablesFromEnvRules(extraRules, testRand) envList := append(tc.envList, extraEnvs...) - actual, err := tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) + actual, _, err := tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) // getting an error means something is broken if err != nil { @@ -827,7 +827,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_DropEnvs_Multiple_NoMatch(t *tes extraEnvs := buildEnvironmentVariablesFromEnvRules(extraRules, testRand) envList := append(tc.envList, extraEnvs...) - actual, err := tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) + actual, _, err := tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -847,7 +847,7 @@ func Test_Rego_WorkingDirectoryPolicy_NoMatches(t *testing.T) { return false } - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, randString(testRand, 20), tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, randString(testRand, 20), tc.mounts) // not getting an error means something is broken if err == nil { return false @@ -869,7 +869,7 @@ func Test_Rego_EnforceCreateContainer(t *testing.T) { return false } - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // getting an error means something is broken return err == nil @@ -911,7 +911,7 @@ func Test_Rego_Enforce_CreateContainer_Start_All_Containers(t *testing.T) { } mountSpec := buildMountSpecFromMountArray(mounts, sandboxID, testRand) - _, err = policy.EnforceCreateContainerPolicy(sandboxID, containerID, container.Command, envList, container.WorkingDir, mountSpec.Mounts) + _, _, err = policy.EnforceCreateContainerPolicy(sandboxID, containerID, container.Command, envList, container.WorkingDir, mountSpec.Mounts) // getting an error means something is broken if err != nil { @@ -938,7 +938,7 @@ func Test_Rego_EnforceCreateContainer_Invalid_ContainerID(t *testing.T) { } containerID := testDataGenerator.uniqueContainerID() - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // not getting an error means something is broken return err != nil @@ -957,12 +957,12 @@ func Test_Rego_EnforceCreateContainer_Same_Container_Twice(t *testing.T) { return false } - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) if err != nil { t.Error("Unable to start valid container.") return false } - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) if err == nil { t.Error("Able to start a container with already used id.") return false @@ -990,7 +990,7 @@ func Test_Rego_ExtendDefaultMounts(t *testing.T) { additionalMounts := buildMountSpecFromMountArray(defaultMounts, tc.sandboxID, testRand) tc.mounts = append(tc.mounts, additionalMounts.Mounts...) - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) if err != nil { t.Error(err) @@ -1017,7 +1017,7 @@ func Test_Rego_MountPolicy_NoMatches(t *testing.T) { additionalMounts := buildMountSpecFromMountArray(invalidMounts, tc.sandboxID, testRand) tc.mounts = append(tc.mounts, additionalMounts.Mounts...) - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -1046,7 +1046,7 @@ func Test_Rego_MountPolicy_NotAllOptionsFromConstraints(t *testing.T) { options := inputMounts[mindex].Options inputMounts[mindex].Options = options[:len(options)-1] - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -1072,7 +1072,7 @@ func Test_Rego_MountPolicy_BadSource(t *testing.T) { index := randMinMax(testRand, 0, int32(len(tc.mounts)-1)) tc.mounts[index].Source = randString(testRand, maxGeneratedMountSourceLength) - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -1098,7 +1098,7 @@ func Test_Rego_MountPolicy_BadDestination(t *testing.T) { index := randMinMax(testRand, 0, int32(len(tc.mounts)-1)) tc.mounts[index].Destination = randString(testRand, maxGeneratedMountDestinationLength) - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -1124,7 +1124,7 @@ func Test_Rego_MountPolicy_BadType(t *testing.T) { index := randMinMax(testRand, 0, int32(len(tc.mounts)-1)) tc.mounts[index].Type = randString(testRand, 4) - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -1152,7 +1152,7 @@ func Test_Rego_MountPolicy_BadOption(t *testing.T) { oindex := randMinMax(testRand, 0, int32(len(mountToChange.Options)-1)) tc.mounts[mindex].Options[oindex] = randString(testRand, maxGeneratedMountOptionLength) - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -1181,7 +1181,7 @@ func Test_Rego_MountPolicy_MountPrivilegedWhenNotAllowed(t *testing.T) { oindex := randMinMax(testRand, 0, int32(len(mountToChange.Options)-1)) tc.mounts[mindex].Options[oindex] = randString(testRand, maxGeneratedMountOptionLength) - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -1321,7 +1321,7 @@ func Test_Rego_ExecInContainerPolicy(t *testing.T) { process := selectExecProcess(container.container.ExecProcesses, testRand) envList := buildEnvironmentVariablesFromEnvRules(container.container.EnvRules, testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, container.container.WorkingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, container.container.WorkingDir) // getting an error means something is broken if err != nil { @@ -1350,7 +1350,7 @@ func Test_Rego_ExecInContainerPolicy_No_Matches(t *testing.T) { process := generateContainerExecProcess(testRand) envList := buildEnvironmentVariablesFromEnvRules(container.container.EnvRules, testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, container.container.WorkingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, container.container.WorkingDir) if err == nil { t.Error("Test unexpectedly passed") return false @@ -1376,7 +1376,7 @@ func Test_Rego_ExecInContainerPolicy_Command_No_Match(t *testing.T) { envList := buildEnvironmentVariablesFromEnvRules(container.container.EnvRules, testRand) command := generateCommand(testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, command, envList, container.container.WorkingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, command, envList, container.container.WorkingDir) // not getting an error means something is broken if err == nil { @@ -1405,7 +1405,7 @@ func Test_Rego_ExecInContainerPolicy_Some_Env_Not_Allowed(t *testing.T) { envList := generateEnvironmentVariables(testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, container.container.WorkingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, container.container.WorkingDir) // not getting an error means something is broken if err == nil { @@ -1434,7 +1434,7 @@ func Test_Rego_ExecInContainerPolicy_WorkingDir_No_Match(t *testing.T) { envList := buildEnvironmentVariablesFromEnvRules(container.container.EnvRules, testRand) workingDir := generateWorkingDir(testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, workingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, workingDir) // not getting an error means something is broken if err == nil { @@ -1467,7 +1467,7 @@ func Test_Rego_ExecInContainerPolicy_DropEnvs(t *testing.T) { extraEnvs := buildEnvironmentVariablesFromEnvRules(extraRules, testRand) envList := append(expected, extraEnvs...) - actual, err := tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, container.container.WorkingDir) + actual, _, err := tc.policy.EnforceExecInContainerPolicy(container.containerID, process.Command, envList, container.container.WorkingDir) if err != nil { t.Errorf("expected exec in container process to be allowed. It wasn't: %v", err) @@ -1528,21 +1528,21 @@ exec_external := { } envList := generateEnvs(envSet) - toKeep, err := policy.EnforceCreateContainerPolicy("", "", []string{}, envList, "", []oci.Mount{}) + toKeep, _, err := policy.EnforceCreateContainerPolicy("", "", []string{}, envList, "", []oci.Mount{}) if len(toKeep) > 0 { t.Error("invalid environment variables not filtered from list returned from create_container") return false } envList = generateEnvs(envSet) - toKeep, err = policy.EnforceExecInContainerPolicy("", []string{}, envList, "") + toKeep, _, err = policy.EnforceExecInContainerPolicy("", []string{}, envList, "") if len(toKeep) > 0 { t.Error("invalid environment variables not filtered from list returned from exec_in_container") return false } envList = generateEnvs(envSet) - toKeep, err = policy.EnforceExecExternalProcessPolicy([]string{}, envList, "") + toKeep, _, err = policy.EnforceExecExternalProcessPolicy([]string{}, envList, "") if len(toKeep) > 0 { t.Error("invalid environment variables not filtered from list returned from exec_external") return false @@ -1562,12 +1562,10 @@ func Test_Rego_InvalidEnvList(t *testing.T) { "allowed": true, "env_list": {"an_object": 1} } - exec_in_container := { "allowed": true, "env_list": "string" } - exec_external := { "allowed": true, "env_list": true @@ -1578,21 +1576,21 @@ func Test_Rego_InvalidEnvList(t *testing.T) { t.Fatalf("error creating policy: %v", err) } - _, err = policy.EnforceCreateContainerPolicy("", "", []string{}, []string{}, "", []oci.Mount{}) + _, _, err = policy.EnforceCreateContainerPolicy("", "", []string{}, []string{}, "", []oci.Mount{}) if err == nil { t.Errorf("expected call to create_container to fail") } else if err.Error() != "policy returned incorrect type for 'env_list', expected []interface{}, received map[string]interface {}" { t.Errorf("incorrected error message from call to create_container") } - _, err = policy.EnforceExecInContainerPolicy("", []string{}, []string{}, "") + _, _, err = policy.EnforceExecInContainerPolicy("", []string{}, []string{}, "") if err == nil { t.Errorf("expected call to exec_in_container to fail") } else if err.Error() != "policy returned incorrect type for 'env_list', expected []interface{}, received string" { t.Errorf("incorrected error message from call to exec_in_container") } - _, err = policy.EnforceExecExternalProcessPolicy([]string{}, []string{}, "") + _, _, err = policy.EnforceExecExternalProcessPolicy([]string{}, []string{}, "") if err == nil { t.Errorf("expected call to exec_external to fail") } else if err.Error() != "policy returned incorrect type for 'env_list', expected []interface{}, received bool" { @@ -1606,12 +1604,10 @@ func Test_Rego_InvalidEnvList_Member(t *testing.T) { "allowed": true, "env_list": ["one", "two", 3] } - exec_in_container := { "allowed": true, "env_list": ["one", true, "three"] } - exec_external := { "allowed": true, "env_list": ["one", ["two"], "three"] @@ -1622,21 +1618,21 @@ func Test_Rego_InvalidEnvList_Member(t *testing.T) { t.Fatalf("error creating policy: %v", err) } - _, err = policy.EnforceCreateContainerPolicy("", "", []string{}, []string{}, "", []oci.Mount{}) + _, _, err = policy.EnforceCreateContainerPolicy("", "", []string{}, []string{}, "", []oci.Mount{}) if err == nil { t.Errorf("expected call to create_container to fail") } else if err.Error() != "members of env_list from policy must be strings, received json.Number" { t.Errorf("incorrected error message from call to create_container") } - _, err = policy.EnforceExecInContainerPolicy("", []string{}, []string{}, "") + _, _, err = policy.EnforceExecInContainerPolicy("", []string{}, []string{}, "") if err == nil { t.Errorf("expected call to exec_in_container to fail") } else if err.Error() != "members of env_list from policy must be strings, received bool" { t.Errorf("incorrected error message from call to exec_in_container") } - _, err = policy.EnforceExecExternalProcessPolicy([]string{}, []string{}, "") + _, _, err = policy.EnforceExecExternalProcessPolicy([]string{}, []string{}, "") if err == nil { t.Errorf("expected call to exec_external to fail") } else if err.Error() != "members of env_list from policy must be strings, received []interface {}" { @@ -1669,7 +1665,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_MissingRequired(t *testing.T) { } } - _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) + _, _, err = tc.policy.EnforceCreateContainerPolicy(tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts) // not getting an error means something is broken if err == nil { @@ -1696,7 +1692,7 @@ func Test_Rego_ExecExternalProcessPolicy(t *testing.T) { process := selectExternalProcessFromConstraints(p, testRand) envList := buildEnvironmentVariablesFromEnvRules(process.envRules, testRand) - _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) + _, _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) if err != nil { t.Error("Policy enforcement unexpectedly was denied") return false @@ -1721,7 +1717,7 @@ func Test_Rego_ExecExternalProcessPolicy_No_Matches(t *testing.T) { process := generateExternalProcess(testRand) envList := buildEnvironmentVariablesFromEnvRules(process.envRules, testRand) - _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) + _, _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) if err == nil { t.Error("Policy was unexpectedly not enforced") return false @@ -1747,7 +1743,7 @@ func Test_Rego_ExecExternalProcessPolicy_Command_No_Match(t *testing.T) { envList := buildEnvironmentVariablesFromEnvRules(process.envRules, testRand) command := generateCommand(testRand) - _, err = tc.policy.EnforceExecExternalProcessPolicy(command, envList, process.workingDir) + _, _, err = tc.policy.EnforceExecExternalProcessPolicy(command, envList, process.workingDir) if err == nil { t.Error("Policy was unexpectedly not enforced") return false @@ -1772,7 +1768,7 @@ func Test_Rego_ExecExternalProcessPolicy_Some_Env_Not_Allowed(t *testing.T) { process := selectExternalProcessFromConstraints(p, testRand) envList := generateEnvironmentVariables(testRand) - _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) + _, _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) if err == nil { t.Error("Policy was unexpectedly not enforced") return false @@ -1798,7 +1794,7 @@ func Test_Rego_ExecExternalProcessPolicy_WorkingDir_No_Match(t *testing.T) { envList := buildEnvironmentVariablesFromEnvRules(process.envRules, testRand) workingDir := generateWorkingDir(testRand) - _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, workingDir) + _, _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, workingDir) if err == nil { t.Error("Policy was unexpectedly not enforced") return false @@ -1828,7 +1824,7 @@ func Test_Rego_ExecExternalProcessPolicy_DropEnvs(t *testing.T) { extraEnvs := buildEnvironmentVariablesFromEnvRules(extraRules, testRand) envList := append(expected, extraEnvs...) - actual, err := tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) + actual, _, err := tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) if err != nil { t.Errorf("expected exec in container process to be allowed. It wasn't: %v", err) @@ -1849,18 +1845,17 @@ func Test_Rego_ExecExternalProcessPolicy_DropEnvs(t *testing.T) { } func Test_Rego_ExecExternalProcessPolicy_DropEnvs_Multiple(t *testing.T) { + envRules := setupEnvRuleSets(3) + gc := generateConstraints(testRand, 1) gc.allowEnvironmentVariableDropping = true process0 := generateExternalProcess(testRand) - envRules0 := process0.envRules process1 := process0.clone() - envRules1 := generateEnvironmentVariableRules(testRand) - process1.envRules = append(envRules0, envRules1...) + process1.envRules = append(envRules[0], envRules[1]...) process2 := process0.clone() - envRules2 := generateEnvironmentVariableRules(testRand) - process2.envRules = append(process1.envRules, envRules2...) + process2.envRules = append(process1.envRules, envRules[2]...) gc.externalProcesses = []*externalProcess{process0, process1, process2} securityPolicy := gc.toPolicy() @@ -1874,13 +1869,13 @@ func Test_Rego_ExecExternalProcessPolicy_DropEnvs_Multiple(t *testing.T) { t.Fatal(err) } - envs0 := buildEnvironmentVariablesFromEnvRules(envRules0, testRand) - envs1 := buildEnvironmentVariablesFromEnvRules(envRules1, testRand) - envs2 := buildEnvironmentVariablesFromEnvRules(envRules2, testRand) + envs0 := buildEnvironmentVariablesFromEnvRules(envRules[0], testRand) + envs1 := buildEnvironmentVariablesFromEnvRules(envRules[1], testRand) + envs2 := buildEnvironmentVariablesFromEnvRules(envRules[2], testRand) envList := append(envs0, envs1...) envList = append(envList, envs2...) - actual, err := policy.EnforceExecExternalProcessPolicy(process2.command, envList, process2.workingDir) + actual, _, err := policy.EnforceExecExternalProcessPolicy(process2.command, envList, process2.workingDir) // getting an error means something is broken if err != nil { @@ -1893,18 +1888,18 @@ func Test_Rego_ExecExternalProcessPolicy_DropEnvs_Multiple(t *testing.T) { } func Test_Rego_ExecExternalProcessPolicy_DropEnvs_Multiple_NoMatch(t *testing.T) { + envRules := setupEnvRuleSets(3) + gc := generateConstraints(testRand, 1) gc.allowEnvironmentVariableDropping = true + process0 := generateExternalProcess(testRand) - envRules0 := process0.envRules process1 := process0.clone() - envRules1 := generateEnvironmentVariableRules(testRand) - process1.envRules = append(envRules0, envRules1...) + process1.envRules = append(envRules[0], envRules[1]...) process2 := process0.clone() - envRules2 := generateEnvironmentVariableRules(testRand) - process2.envRules = append(envRules0, envRules2...) + process2.envRules = append(envRules[0], envRules[2]...) gc.externalProcesses = []*externalProcess{process0, process1, process2} securityPolicy := gc.toPolicy() @@ -1918,9 +1913,9 @@ func Test_Rego_ExecExternalProcessPolicy_DropEnvs_Multiple_NoMatch(t *testing.T) t.Fatal(err) } - envs0 := buildEnvironmentVariablesFromEnvRules(envRules0, testRand) - envs1 := buildEnvironmentVariablesFromEnvRules(envRules1, testRand) - envs2 := buildEnvironmentVariablesFromEnvRules(envRules2, testRand) + envs0 := buildEnvironmentVariablesFromEnvRules(envRules[0], testRand) + envs1 := buildEnvironmentVariablesFromEnvRules(envRules[1], testRand) + envs2 := buildEnvironmentVariablesFromEnvRules(envRules[2], testRand) var extraLen int if len(envs1) > len(envs2) { extraLen = len(envs2) @@ -1930,7 +1925,7 @@ func Test_Rego_ExecExternalProcessPolicy_DropEnvs_Multiple_NoMatch(t *testing.T) envList := append(envs0, envs1[:extraLen]...) envList = append(envList, envs2[:extraLen]...) - actual, err := policy.EnforceExecExternalProcessPolicy(process2.command, envList, process2.workingDir) + actual, _, err := policy.EnforceExecExternalProcessPolicy(process2.command, envList, process2.workingDir) // not getting an error means something is broken if err == nil { @@ -2118,7 +2113,7 @@ func Test_Rego_SignalContainerProcessPolicy_ExecProcess_Allowed(t *testing.T) { envList := buildEnvironmentVariablesFromEnvRules(containerUnderTest.EnvRules, testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(containerID, processUnderTest.Command, envList, containerUnderTest.WorkingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(containerID, processUnderTest.Command, envList, containerUnderTest.WorkingDir) if err != nil { t.Errorf("Unable to exec process for test: %v", err) return false @@ -2169,7 +2164,7 @@ func Test_Rego_SignalContainerProcessPolicy_ExecProcess_Not_Allowed(t *testing.T envList := buildEnvironmentVariablesFromEnvRules(containerUnderTest.EnvRules, testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(containerID, processUnderTest.Command, envList, containerUnderTest.WorkingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(containerID, processUnderTest.Command, envList, containerUnderTest.WorkingDir) if err != nil { t.Errorf("Unable to exec process for test: %v", err) return false @@ -2220,7 +2215,7 @@ func Test_Rego_SignalContainerProcessPolicy_ExecProcess_Bad_Command(t *testing.T envList := buildEnvironmentVariablesFromEnvRules(containerUnderTest.EnvRules, testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(containerID, processUnderTest.Command, envList, containerUnderTest.WorkingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(containerID, processUnderTest.Command, envList, containerUnderTest.WorkingDir) if err != nil { t.Errorf("Unable to exec process for test: %v", err) return false @@ -2272,7 +2267,7 @@ func Test_Rego_SignalContainerProcessPolicy_ExecProcess_Bad_ContainerID(t *testi envList := buildEnvironmentVariablesFromEnvRules(containerUnderTest.EnvRules, testRand) - _, err = tc.policy.EnforceExecInContainerPolicy(containerID, processUnderTest.Command, envList, containerUnderTest.WorkingDir) + _, _, err = tc.policy.EnforceExecInContainerPolicy(containerID, processUnderTest.Command, envList, containerUnderTest.WorkingDir) if err != nil { t.Errorf("Unable to exec process for test: %v", err) return false @@ -2308,7 +2303,7 @@ func Test_Rego_Plan9MountPolicy(t *testing.T) { t.Fatalf("Policy enforcement unexpectedly was denied: %v", err) } - _, err = tc.policy.EnforceCreateContainerPolicy( + _, _, err = tc.policy.EnforceCreateContainerPolicy( tc.sandboxID, tc.containerID, tc.argList, @@ -2342,7 +2337,7 @@ func Test_Rego_Plan9MountPolicy_No_Matches(t *testing.T) { t.Fatalf("Policy enforcement unexpectedly was denied: %v", err) } - _, err = tc.policy.EnforceCreateContainerPolicy( + _, _, err = tc.policy.EnforceCreateContainerPolicy( tc.sandboxID, tc.containerID, tc.argList, @@ -2388,7 +2383,7 @@ func Test_Rego_Plan9UnmountPolicy(t *testing.T) { t.Fatalf("Policy enforcement unexpectedly was denied: %v", err) } - _, err = tc.policy.EnforceCreateContainerPolicy( + _, _, err = tc.policy.EnforceCreateContainerPolicy( tc.sandboxID, tc.containerID, tc.argList, @@ -2563,7 +2558,7 @@ func Test_Rego_LoadFragment_Container(t *testing.T) { return false } - _, err = tc.policy.EnforceCreateContainerPolicy( + _, _, err = tc.policy.EnforceCreateContainerPolicy( container.sandboxID, containerID, copyStrings(container.container.Command), @@ -2650,7 +2645,7 @@ func Test_Rego_LoadFragment_ExternalProcess(t *testing.T) { } envList := buildEnvironmentVariablesFromEnvRules(process.envRules, testRand) - _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) + _, _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) if err != nil { t.Error("unable to execute external process from fragment: %w", err) return false @@ -2793,7 +2788,7 @@ func Test_Rego_LoadFragment_SameIssuerTwoFeeds(t *testing.T) { return false } - _, err = tc.policy.EnforceCreateContainerPolicy( + _, _, err = tc.policy.EnforceCreateContainerPolicy( container.sandboxID, containerID, copyStrings(container.container.Command), @@ -2838,7 +2833,7 @@ func Test_Rego_LoadFragment_TwoFeeds(t *testing.T) { return false } - _, err = tc.policy.EnforceCreateContainerPolicy( + _, _, err = tc.policy.EnforceCreateContainerPolicy( container.sandboxID, containerID, copyStrings(container.container.Command), @@ -2887,7 +2882,7 @@ func Test_Rego_LoadFragment_SameFeedTwice(t *testing.T) { return false } - _, err = tc.policy.EnforceCreateContainerPolicy( + _, _, err = tc.policy.EnforceCreateContainerPolicy( container.sandboxID, containerID, copyStrings(container.container.Command), @@ -2989,7 +2984,7 @@ func Test_Rego_LoadFragment_ExcludedExternalProcess(t *testing.T) { } envList := buildEnvironmentVariablesFromEnvRules(process.envRules, testRand) - _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) + _, _, err = tc.policy.EnforceExecExternalProcessPolicy(process.command, envList, process.workingDir) if err == nil { t.Error("expected to be unable to execute external process from a fragment") return false @@ -3161,6 +3156,156 @@ func Test_Rego_Scratch_Unmount_Policy(t *testing.T) { } } +func Test_Rego_StdioAccess_Allowed(t *testing.T) { + gc := generateConstraints(testRand, 1) + gc.containers[0].AllowStdioAccess = true + gc.externalProcesses = generateExternalProcesses(testRand) + gc.externalProcesses[0].allowStdioAccess = true + tc, err := setupRegoCreateContainerTest(gc, gc.containers[0], false) + if err != nil { + t.Fatalf("error setting up test: %v", err) + } + + _, allow_stdio_access, err := tc.policy.EnforceCreateContainerPolicy( + tc.sandboxID, + tc.containerID, + tc.argList, + tc.envList, + tc.workingDir, + tc.mounts) + + if err != nil { + t.Errorf("create_container not allowed: %v", err) + } + + if !allow_stdio_access { + t.Errorf("expected allow_stdio_access to be true") + } + + // stdio access is inherited from the container and should be the same + _, allow_stdio_access, err = tc.policy.EnforceExecInContainerPolicy( + tc.containerID, + gc.containers[0].ExecProcesses[0].Command, + tc.envList, + tc.workingDir, + ) + + if err != nil { + t.Errorf("exec_in_container not allowed: %v", err) + } + + if !allow_stdio_access { + t.Errorf("expected allow_stdio_access to be true") + } + + envList := buildEnvironmentVariablesFromEnvRules(gc.externalProcesses[0].envRules, testRand) + _, allow_stdio_access, err = tc.policy.EnforceExecExternalProcessPolicy( + gc.externalProcesses[0].command, + envList, + gc.externalProcesses[0].workingDir, + ) + + if err != nil { + t.Errorf("exec_external not allowed: %v", err) + } + + if !allow_stdio_access { + t.Errorf("expected allow_stdio_access to be true") + } +} + +func Test_Rego_EnforeCreateContainerPolicy_StdioAccess_NotAllowed(t *testing.T) { + gc := generateConstraints(testRand, 1) + gc.containers[0].AllowStdioAccess = false + tc, err := setupRegoCreateContainerTest(gc, gc.containers[0], false) + if err != nil { + t.Fatalf("error setting up test: %v", err) + } + + _, allow_stdio_access, err := tc.policy.EnforceCreateContainerPolicy( + tc.sandboxID, + tc.containerID, + tc.argList, + tc.envList, + tc.workingDir, + tc.mounts) + + if err != nil { + t.Errorf("create_container not allowed: %v", err) + } + + if allow_stdio_access { + t.Errorf("expected allow_stdio_access to be false") + } +} + +func Test_Rego_Container_StdioAccess_NotDecidable(t *testing.T) { + gc := generateConstraints(testRand, 1) + container0 := gc.containers[0] + container0.AllowStdioAccess = true + container1, err := container0.clone() + if err != nil { + t.Fatalf("unable to clone container: %v", err) + } + + container1.AllowStdioAccess = false + gc.containers = append(gc.containers, container1) + + container0.ExecProcesses = append(container0.ExecProcesses, container0.ExecProcesses[0].clone()) + + gc.externalProcesses = generateExternalProcesses(testRand) + gc.externalProcesses = append(gc.externalProcesses, gc.externalProcesses[0].clone()) + gc.externalProcesses[0].allowStdioAccess = true + + tc, err := setupRegoCreateContainerTest(gc, gc.containers[0], false) + if err != nil { + t.Fatalf("error setting up test: %v", err) + } + + _, allow_stdio_access, err := tc.policy.EnforceCreateContainerPolicy( + tc.sandboxID, + tc.containerID, + tc.argList, + tc.envList, + tc.workingDir, + tc.mounts) + + if err == nil { + t.Errorf("expected create_container to not be allowed") + } + + if allow_stdio_access { + t.Errorf("expected allow_stdio_access to be false") + } +} + +func Test_Rego_ExecExternal_StdioAccess_NotAllowed(t *testing.T) { + gc := generateConstraints(testRand, 1) + gc.externalProcesses = generateExternalProcesses(testRand) + gc.externalProcesses = append(gc.externalProcesses, gc.externalProcesses[0].clone()) + gc.externalProcesses[0].allowStdioAccess = !gc.externalProcesses[0].allowStdioAccess + + policy, err := newRegoPolicy(gc.toPolicy().marshalRego(), []oci.Mount{}, []oci.Mount{}) + if err != nil { + t.Fatalf("error marshaling policy: %v", err) + } + + envList := buildEnvironmentVariablesFromEnvRules(gc.externalProcesses[0].envRules, testRand) + _, allow_stdio_access, err := policy.EnforceExecExternalProcessPolicy( + gc.externalProcesses[0].command, + envList, + gc.externalProcesses[0].workingDir, + ) + + if err == nil { + t.Errorf("expected exec_external to not be allowed") + } + + if allow_stdio_access { + t.Errorf("expected allow_stdio_access to be false") + } +} + // // Setup and "fixtures" follow... // @@ -3178,9 +3323,10 @@ func generateExternalProcesses(r *rand.Rand) []*externalProcess { func generateExternalProcess(r *rand.Rand) *externalProcess { return &externalProcess{ - command: generateCommand(r), - envRules: generateEnvironmentVariableRules(r), - workingDir: generateWorkingDir(r), + command: generateCommand(r), + envRules: generateEnvironmentVariableRules(r), + workingDir: generateWorkingDir(r), + allowStdioAccess: randBool(r), } } @@ -3456,13 +3602,14 @@ func runContainer(enforcer *regoEnforcer, container *securityPolicyContainer, de } mountSpec := buildMountSpecFromMountArray(mounts, sandboxID, testRand) - _, err = enforcer.EnforceCreateContainerPolicy(sandboxID, containerID, container.Command, envList, container.WorkingDir, mountSpec.Mounts) + _, _, err = enforcer.EnforceCreateContainerPolicy(sandboxID, containerID, container.Command, envList, container.WorkingDir, mountSpec.Mounts) if err != nil { return nil, err } return ®oRunningContainer{ container: container, + envList: envList, containerID: containerID, }, nil } @@ -3476,6 +3623,7 @@ type regoRunningContainerTestConfig struct { type regoRunningContainer struct { container *securityPolicyContainer + envList []string containerID string } @@ -3788,27 +3936,14 @@ type regoDropEnvsTestConfig struct { policy *regoEnforcer } -func setupRegoDropEnvsTest(disjoint bool) (*regoContainerTestConfig, error) { - gc := generateConstraints(testRand, 1) - gc.allowEnvironmentVariableDropping = true - - const numContainers int = 3 +func setupEnvRuleSets(count int) [][]EnvRuleConfig { numEnvRules := []int{int(randMinMax(testRand, 1, 4)), int(randMinMax(testRand, 1, 4)), int(randMinMax(testRand, 1, 4))} envRuleLookup := make(stringSet) - envRules := make([][]EnvRuleConfig, numContainers) - - containers := make([]*securityPolicyContainer, numContainers) - envs := make([][]string, numContainers) - - for i := 0; i < numContainers; i++ { - c, err := gc.containers[0].clone() - if err != nil { - return nil, err - } - containers[i] = c + envRules := make([][]EnvRuleConfig, count) + for i := 0; i < count; i++ { rules := envRuleLookup.randUniqueArray(testRand, func(r *rand.Rand) string { return randVariableString(r, 10) }, int32(numEnvRules[i])) @@ -3820,7 +3955,26 @@ func setupRegoDropEnvsTest(disjoint bool) (*regoContainerTestConfig, error) { Rule: rule, } } + } + + return envRules +} + +func setupRegoDropEnvsTest(disjoint bool) (*regoContainerTestConfig, error) { + gc := generateConstraints(testRand, 1) + gc.allowEnvironmentVariableDropping = true + + const numContainers int = 3 + envRules := setupEnvRuleSets(numContainers) + containers := make([]*securityPolicyContainer, numContainers) + envs := make([][]string, numContainers) + for i := 0; i < numContainers; i++ { + c, err := gc.containers[0].clone() + if err != nil { + return nil, err + } + containers[i] = c envs[i] = buildEnvironmentVariablesFromEnvRules(envRules[i], testRand) if i == 0 { c.EnvRules = envRules[i] @@ -4183,9 +4337,17 @@ func (p externalProcess) clone() *externalProcess { copy(envRules, p.envRules) return &externalProcess{ - command: copyStrings(p.command), - envRules: envRules, - workingDir: p.workingDir, + command: copyStrings(p.command), + envRules: envRules, + workingDir: p.workingDir, + allowStdioAccess: p.allowStdioAccess, + } +} + +func (p containerExecProcess) clone() containerExecProcess { + return containerExecProcess{ + Command: copyStrings(p.Command), + Signals: p.Signals, } } @@ -4196,14 +4358,15 @@ func (c *securityPolicyContainer) toContainer() *Container { } return &Container{ - Command: CommandArgs(stringArrayToStringMap(c.Command)), - EnvRules: envRuleArrayToEnvRules(c.EnvRules), - Layers: Layers(stringArrayToStringMap(c.Layers)), - WorkingDir: c.WorkingDir, - Mounts: mountArrayToMounts(c.Mounts), - AllowElevated: c.AllowElevated, - ExecProcesses: execProcesses, - Signals: c.Signals, + Command: CommandArgs(stringArrayToStringMap(c.Command)), + EnvRules: envRuleArrayToEnvRules(c.EnvRules), + Layers: Layers(stringArrayToStringMap(c.Layers)), + WorkingDir: c.WorkingDir, + Mounts: mountArrayToMounts(c.Mounts), + AllowElevated: c.AllowElevated, + ExecProcesses: execProcesses, + Signals: c.Signals, + AllowStdioAccess: c.AllowStdioAccess, } } @@ -4237,8 +4400,9 @@ func mountArrayToMounts(mounts []mountInternal) Mounts { func (p externalProcess) toConfig() ExternalProcessConfig { return ExternalProcessConfig{ - Command: p.command, - WorkingDir: p.workingDir, + Command: p.command, + WorkingDir: p.workingDir, + AllowStdioAccess: p.allowStdioAccess, } } diff --git a/pkg/securitypolicy/securitypolicy.go b/pkg/securitypolicy/securitypolicy.go index 2b3af323d6..96e39e4e74 100644 --- a/pkg/securitypolicy/securitypolicy.go +++ b/pkg/securitypolicy/securitypolicy.go @@ -40,8 +40,9 @@ type PolicyConfig struct { // ExternalProcessConfig contains toml or JSON config for running external processes in the UVM. type ExternalProcessConfig struct { - Command []string `json:"command" toml:"command"` - WorkingDir string `json:"working_dir" toml:"working_dir"` + Command []string `json:"command" toml:"command"` + WorkingDir string `json:"working_dir" toml:"working_dir"` + AllowStdioAccess bool `json:"allow_stdio_access" toml:"allow_stdio_access"` } // FragmentConfig contains toml or JSON config for including elements from fragments. @@ -69,15 +70,16 @@ 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 []EnvRuleConfig `json:"env_rules" toml:"env_rule"` - WorkingDir string `json:"working_dir" toml:"working_dir"` - Mounts []MountConfig `json:"mounts" toml:"mount"` - AllowElevated bool `json:"allow_elevated" toml:"allow_elevated"` - ExecProcesses []ExecProcessConfig `json:"exec_processes" toml:"exec_process"` - Signals []syscall.Signal `json:"signals" toml:"signals"` + ImageName string `json:"image_name" toml:"image_name"` + Command []string `json:"command" toml:"command"` + Auth AuthConfig `json:"auth" toml:"auth"` + EnvRules []EnvRuleConfig `json:"env_rules" toml:"env_rule"` + WorkingDir string `json:"working_dir" toml:"working_dir"` + Mounts []MountConfig `json:"mounts" toml:"mount"` + AllowElevated bool `json:"allow_elevated" toml:"allow_elevated"` + ExecProcesses []ExecProcessConfig `json:"exec_processes" toml:"exec_process"` + Signals []syscall.Signal `json:"signals" toml:"signals"` + AllowStdioAccess bool `json:"allow_stdio_access" toml:"allow_stdio_access"` } // MountConfig contains toml or JSON config for mount security policy @@ -164,14 +166,15 @@ type Containers struct { } type Container struct { - Command CommandArgs `json:"command"` - EnvRules EnvRules `json:"env_rules"` - Layers Layers `json:"layers"` - WorkingDir string `json:"working_dir"` - Mounts Mounts `json:"mounts"` - AllowElevated bool `json:"allow_elevated"` - ExecProcesses []ExecProcessConfig `json:"-"` - Signals []syscall.Signal `json:"-"` + Command CommandArgs `json:"command"` + EnvRules EnvRules `json:"env_rules"` + Layers Layers `json:"layers"` + WorkingDir string `json:"working_dir"` + Mounts Mounts `json:"mounts"` + AllowElevated bool `json:"allow_elevated"` + ExecProcesses []ExecProcessConfig `json:"-"` + Signals []syscall.Signal `json:"-"` + AllowStdioAccess bool `json:"_"` } // StringArrayMap wraps an array of strings as a string map. @@ -213,6 +216,7 @@ func CreateContainerPolicy( allowElevated bool, execProcesses []ExecProcessConfig, signals []syscall.Signal, + AllowStdioAccess bool, ) (*Container, error) { if err := validateEnvRules(envRules); err != nil { return nil, err @@ -221,14 +225,15 @@ func CreateContainerPolicy( return nil, err } return &Container{ - Command: newCommandArgs(command), - Layers: newLayers(layers), - EnvRules: newEnvRules(envRules), - WorkingDir: workingDir, - Mounts: newMountConstraints(mounts), - AllowElevated: allowElevated, - ExecProcesses: execProcesses, - Signals: signals, + Command: newCommandArgs(command), + Layers: newLayers(layers), + EnvRules: newEnvRules(envRules), + WorkingDir: workingDir, + Mounts: newMountConstraints(mounts), + AllowElevated: allowElevated, + ExecProcesses: execProcesses, + Signals: signals, + AllowStdioAccess: AllowStdioAccess, }, nil } diff --git a/pkg/securitypolicy/securitypolicy_internal.go b/pkg/securitypolicy/securitypolicy_internal.go index 4370ba7240..2f6ac14f37 100644 --- a/pkg/securitypolicy/securitypolicy_internal.go +++ b/pkg/securitypolicy/securitypolicy_internal.go @@ -129,6 +129,8 @@ type securityPolicyContainer struct { // A list of signals that are allowed to be sent to the container's init // process. Signals []syscall.Signal `json:"signals"` + // Whether to allow the capture of init process standard out and standard error + AllowStdioAccess bool } type containerExecProcess struct { @@ -138,9 +140,10 @@ type containerExecProcess struct { } type externalProcess struct { - command []string - envRules []EnvRuleConfig - workingDir string + command []string + envRules []EnvRuleConfig + workingDir string + allowStdioAccess bool } // Internal version of Mount @@ -190,11 +193,12 @@ func (c *Container) toInternal() (*securityPolicyContainer, error) { Layers: layers, // No need to have toInternal(), because WorkingDir is a string both // internally and in the policy. - WorkingDir: c.WorkingDir, - Mounts: mounts, - AllowElevated: c.AllowElevated, - ExecProcesses: execProcesses, - Signals: c.Signals, + WorkingDir: c.WorkingDir, + Mounts: mounts, + AllowElevated: c.AllowElevated, + ExecProcesses: execProcesses, + Signals: c.Signals, + AllowStdioAccess: c.AllowStdioAccess, }, nil } @@ -256,7 +260,8 @@ func (p ExternalProcessConfig) toInternal() externalProcess { Rule: "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", Required: true, }}, - workingDir: p.WorkingDir, + workingDir: p.WorkingDir, + allowStdioAccess: p.AllowStdioAccess, } } diff --git a/pkg/securitypolicy/securitypolicy_marshal.go b/pkg/securitypolicy/securitypolicy_marshal.go index 3c612bba38..b6c6531820 100644 --- a/pkg/securitypolicy/securitypolicy_marshal.go +++ b/pkg/securitypolicy/securitypolicy_marshal.go @@ -318,7 +318,8 @@ func writeContainer(builder *strings.Builder, container *securityPolicyContainer writeExecProcesses(builder, container.ExecProcesses, indent+indentUsing) writeSignals(builder, container.Signals, indent+indentUsing) writeLine(builder, `%s"allow_elevated": %v,`, indent+indentUsing, container.AllowElevated) - writeLine(builder, `%s"working_dir": "%s"`, indent+indentUsing, container.WorkingDir) + writeLine(builder, `%s"working_dir": "%s",`, indent+indentUsing, container.WorkingDir) + writeLine(builder, `%s"allow_stdio_access": %t`, indent+indentUsing, container.AllowStdioAccess) writeLine(builder, "%s},", indent) } @@ -337,7 +338,7 @@ func addContainers(builder *strings.Builder, containers []*securityPolicyContain func (p externalProcess) marshalRego() string { command := stringArray(p.command).marshalRego() envRules := envRuleArray(p.envRules).marshalRego() - return fmt.Sprintf(`{"command": %s, "env_rules": %s, "working_dir": "%s"}`, command, envRules, p.workingDir) + return fmt.Sprintf(`{"command": %s, "env_rules": %s, "working_dir": "%s", "allow_stdio_access": %t}`, command, envRules, p.workingDir, p.allowStdioAccess) } func addExternalProcesses(builder *strings.Builder, processes []*externalProcess) { diff --git a/pkg/securitypolicy/securitypolicy_test.go b/pkg/securitypolicy/securitypolicy_test.go index 9589ef8ca5..1979f01b5b 100644 --- a/pkg/securitypolicy/securitypolicy_test.go +++ b/pkg/securitypolicy/securitypolicy_test.go @@ -927,15 +927,17 @@ func generateConstraintsContainer(r *rand.Rand, minNumberOfLayers, maxNumberOfLa } c.ExecProcesses = generateExecProcesses(r) c.Signals = generateListOfSignals(r, 0, maxSignalNumber) + c.AllowStdioAccess = randBool(r) return &c } func generateContainerInitProcess(r *rand.Rand) containerInitProcess { return containerInitProcess{ - Command: generateCommand(r), - EnvRules: generateEnvironmentVariableRules(r), - WorkingDir: generateWorkingDir(r), + Command: generateCommand(r), + EnvRules: generateEnvironmentVariableRules(r), + WorkingDir: generateWorkingDir(r), + AllowStdioAccess: randBool(r), } } @@ -1332,7 +1334,8 @@ type generatedConstraints struct { } type containerInitProcess struct { - Command []string - EnvRules []EnvRuleConfig - WorkingDir string + Command []string + EnvRules []EnvRuleConfig + WorkingDir string + AllowStdioAccess bool } diff --git a/pkg/securitypolicy/securitypolicyenforcer.go b/pkg/securitypolicy/securitypolicyenforcer.go index 29eedeebf9..b77a6433c3 100644 --- a/pkg/securitypolicy/securitypolicyenforcer.go +++ b/pkg/securitypolicy/securitypolicyenforcer.go @@ -45,11 +45,11 @@ type SecurityPolicyEnforcer interface { EnforceOverlayMountPolicy(containerID string, layerPaths []string, target string) (err error) EnforceOverlayUnmountPolicy(target string) (err error) EnforceCreateContainerPolicy(sandboxID string, containerID string, - argList []string, envList []string, workingDir string, mounts []oci.Mount) (toKeep EnvList, err error) + argList []string, envList []string, workingDir string, mounts []oci.Mount) (EnvList, bool, error) ExtendDefaultMounts([]oci.Mount) error EncodedSecurityPolicy() string - EnforceExecInContainerPolicy(containerID string, argList []string, envList []string, workingDir string) (EnvList, error) - EnforceExecExternalProcessPolicy(argList []string, envList []string, workingDir string) (EnvList, error) + EnforceExecInContainerPolicy(containerID string, argList []string, envList []string, workingDir string) (EnvList, bool, error) + EnforceExecExternalProcessPolicy(argList []string, envList []string, workingDir string) (EnvList, bool, error) EnforceShutdownContainerPolicy(containerID string) error EnforceSignalContainerProcessPolicy(containerID string, signal syscall.Signal, isInitProcess bool, startupArgList []string) error EnforcePlan9MountPolicy(target string) (err error) @@ -434,50 +434,50 @@ func (pe *StandardSecurityPolicyEnforcer) EnforceCreateContainerPolicy( envList []string, workingDir string, mounts []oci.Mount, -) (toKeep EnvList, err error) { +) (allowedEnvs EnvList, stdioAccessAllowed bool, err error) { pe.mutex.Lock() defer pe.mutex.Unlock() if len(pe.Containers) < 1 { - return nil, errors.New("policy doesn't allow mounting containers") + return nil, true, errors.New("policy doesn't allow mounting containers") } if _, e := pe.startedContainers[containerID]; e { - return nil, errors.New("container has already been started") + return nil, true, errors.New("container has already been started") } if err = pe.enforceCommandPolicy(containerID, argList); err != nil { - return nil, err + return nil, true, err } if err = pe.enforceEnvironmentVariablePolicy(containerID, envList); err != nil { - return nil, err + return nil, true, err } if err = pe.enforceWorkingDirPolicy(containerID, workingDir); err != nil { - return nil, err + return nil, true, err } if err = pe.enforceMountPolicy(sandboxID, containerID, mounts); err != nil { - return nil, err + return nil, true, err } // record that we've allowed this container to start pe.startedContainers[containerID] = struct{}{} - return envList, nil + return envList, true, nil } // Stub. We are deprecating the standard enforcer. Newly added enforcement // points are simply allowed. -func (*StandardSecurityPolicyEnforcer) EnforceExecInContainerPolicy(_ string, _ []string, envList []string, _ string) (EnvList, error) { - return envList, nil +func (*StandardSecurityPolicyEnforcer) EnforceExecInContainerPolicy(_ string, _ []string, envList []string, _ string) (EnvList, bool, error) { + return envList, true, nil } // Stub. We are deprecating the standard enforcer. Newly added enforcement // points are simply allowed. -func (*StandardSecurityPolicyEnforcer) EnforceExecExternalProcessPolicy(_ []string, envList []string, _ string) (EnvList, error) { - return envList, nil +func (*StandardSecurityPolicyEnforcer) EnforceExecExternalProcessPolicy(_ []string, envList []string, _ string) (EnvList, bool, error) { + return envList, true, nil } // Stub. We are deprecating the standard enforcer. Newly added enforcement @@ -835,16 +835,16 @@ func (OpenDoorSecurityPolicyEnforcer) EnforceOverlayUnmountPolicy(string) error return nil } -func (OpenDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_, _ string, _ []string, envList []string, _ string, _ []oci.Mount) (EnvList, error) { - return envList, nil +func (OpenDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_, _ string, _ []string, envList []string, _ string, _ []oci.Mount) (EnvList, bool, error) { + return envList, true, nil } -func (OpenDoorSecurityPolicyEnforcer) EnforceExecInContainerPolicy(_ string, _ []string, envList []string, _ string) (EnvList, error) { - return envList, nil +func (OpenDoorSecurityPolicyEnforcer) EnforceExecInContainerPolicy(_ string, _ []string, envList []string, _ string) (EnvList, bool, error) { + return envList, true, nil } -func (OpenDoorSecurityPolicyEnforcer) EnforceExecExternalProcessPolicy(_ []string, envList []string, _ string) (EnvList, error) { - return envList, nil +func (OpenDoorSecurityPolicyEnforcer) EnforceExecExternalProcessPolicy(_ []string, envList []string, _ string) (EnvList, bool, error) { + return envList, true, nil } func (*OpenDoorSecurityPolicyEnforcer) EnforceShutdownContainerPolicy(_ string) error { @@ -917,16 +917,16 @@ func (ClosedDoorSecurityPolicyEnforcer) EnforceOverlayUnmountPolicy(string) erro return errors.New("removing an overlay fs is denied by policy") } -func (ClosedDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_, _ string, _ []string, _ []string, _ string, _ []oci.Mount) (EnvList, error) { - return nil, errors.New("running commands is denied by policy") +func (ClosedDoorSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_, _ string, _ []string, _ []string, _ string, _ []oci.Mount) (EnvList, bool, error) { + return nil, false, errors.New("running commands is denied by policy") } -func (ClosedDoorSecurityPolicyEnforcer) EnforceExecInContainerPolicy(_ string, _ []string, _ []string, _ string) (EnvList, error) { - return nil, errors.New("starting additional processes in a container is denied by policy") +func (ClosedDoorSecurityPolicyEnforcer) EnforceExecInContainerPolicy(_ string, _ []string, _ []string, _ string) (EnvList, bool, error) { + return nil, false, errors.New("starting additional processes in a container is denied by policy") } -func (ClosedDoorSecurityPolicyEnforcer) EnforceExecExternalProcessPolicy(_ []string, _ []string, _ string) (EnvList, error) { - return nil, errors.New("starting additional processes in uvm is denied by policy") +func (ClosedDoorSecurityPolicyEnforcer) EnforceExecExternalProcessPolicy(_ []string, _ []string, _ string) (EnvList, bool, error) { + return nil, false, errors.New("starting additional processes in uvm is denied by policy") } func (*ClosedDoorSecurityPolicyEnforcer) EnforceShutdownContainerPolicy(_ string) error { diff --git a/pkg/securitypolicy/securitypolicyenforcer_rego.go b/pkg/securitypolicy/securitypolicyenforcer_rego.go index 78d222e5af..86a25eae29 100644 --- a/pkg/securitypolicy/securitypolicyenforcer_rego.go +++ b/pkg/securitypolicy/securitypolicyenforcer_rego.go @@ -76,6 +76,8 @@ type regoEnforcer struct { compiledModules *ast.Compiler // Debug flag debug bool + // Stdio allowed state on a per container id basis + stdio map[string]bool } var _ SecurityPolicyEnforcer = (*regoEnforcer)(nil) @@ -205,6 +207,7 @@ func newRegoPolicy(code string, defaultMounts []oci.Mount, privilegedMounts []oc "api.rego": {namespace: "api", code: apiCode}, "framework.rego": {namespace: "framework", code: frameworkCode}, } + policy.stdio = map[string]bool{} err := policy.compile() if err != nil { @@ -517,9 +520,10 @@ func newMetadataOperation(operation interface{}) (*metadataOperation, error) { } var reservedResultKeys = map[string]struct{}{ - "allowed": {}, - "add_module": {}, - "env_list": {}, + "allowed": {}, + "add_module": {}, + "env_list": {}, + "allow_stdio_access": {}, } func (policy *regoEnforcer) getMetadata(name string) (metadataObject, error) { @@ -704,7 +708,7 @@ func (policy *regoEnforcer) EnforceCreateContainerPolicy( envList []string, workingDir string, mounts []oci.Mount, -) (toKeep EnvList, err error) { +) (toKeep EnvList, stdioAccessAllowed bool, err error) { input := inputData{ "containerID": containerID, "argList": argList, @@ -717,15 +721,35 @@ func (policy *regoEnforcer) EnforceCreateContainerPolicy( results, err := policy.enforce("create_container", input) if err != nil { - return nil, err + return nil, false, err } toKeep, err = getEnvsToKeep(envList, results) if err != nil { - return nil, err + return nil, false, err + } + + if value, ok := results["allow_stdio_access"]; ok { + if value, ok := value.(bool); ok { + stdioAccessAllowed = value + } else { + // we got a non-boolean. that's a clear error + // alert that we got an error rather than setting a default value + return nil, false, errors.New("`allow_stdio_access` needs to be a boolean") + } + } else { + // Policy writer didn't specify an `allow_studio_access` value. + // We have two options, return an error or set a default value. + // We are setting a default value: do not allow + stdioAccessAllowed = false } - return toKeep, nil + // Store the result of stdio access allowed for this container so we can use + // it if we get queried about allowing exec in container access. Stdio access + // is on a per-container, not per-process basis. + policy.stdio[containerID] = stdioAccessAllowed + + return toKeep, stdioAccessAllowed, nil } func (policy *regoEnforcer) EnforceDeviceUnmountPolicy(unmountTarget string) error { @@ -763,7 +787,7 @@ func (policy *regoEnforcer) EncodedSecurityPolicy() string { return policy.base64policy } -func (policy *regoEnforcer) EnforceExecInContainerPolicy(containerID string, argList []string, envList []string, workingDir string) (toKeep EnvList, err error) { +func (policy *regoEnforcer) EnforceExecInContainerPolicy(containerID string, argList []string, envList []string, workingDir string) (toKeep EnvList, stdioAccessAllowed bool, err error) { input := inputData{ "containerID": containerID, "argList": argList, @@ -773,19 +797,19 @@ func (policy *regoEnforcer) EnforceExecInContainerPolicy(containerID string, arg results, err := policy.enforce("exec_in_container", input) if err != nil { - return nil, err + return nil, false, err } toKeep, err = getEnvsToKeep(envList, results) if err != nil { - return nil, err + return nil, false, err } - return toKeep, nil + return toKeep, policy.stdio[containerID], nil } -func (policy *regoEnforcer) EnforceExecExternalProcessPolicy(argList []string, envList []string, workingDir string) (toKeep EnvList, err error) { - input := inputData{ +func (policy *regoEnforcer) EnforceExecExternalProcessPolicy(argList []string, envList []string, workingDir string) (toKeep EnvList, stdioAccessAllowed bool, err error) { + input := map[string]interface{}{ "argList": argList, "envList": envList, "workingDir": workingDir, @@ -793,15 +817,30 @@ func (policy *regoEnforcer) EnforceExecExternalProcessPolicy(argList []string, e results, err := policy.enforce("exec_external", input) if err != nil { - return nil, err + return nil, false, err } toKeep, err = getEnvsToKeep(envList, results) if err != nil { - return nil, err + return nil, false, err + } + + if value, ok := results["allow_stdio_access"]; ok { + if value, ok := value.(bool); ok { + stdioAccessAllowed = value + } else { + // we got a non-boolean. that's a clear error + // alert that we got an error rather than setting a default value + return nil, false, errors.New("`allow_stdio_access` needs to be a boolean") + } + } else { + // Policy writer didn't specify an `allow_studio_access` value. + // We have two options, return an error or set a default value. + // We are setting a default value: do not allow + stdioAccessAllowed = false } - return toKeep, nil + return toKeep, stdioAccessAllowed, nil } func (policy *regoEnforcer) EnforceShutdownContainerPolicy(containerID string) error { diff --git a/test/cri-containerd/policy_test.go b/test/cri-containerd/policy_test.go index 151b11d2d5..bdb6ef30c9 100644 --- a/test/cri-containerd/policy_test.go +++ b/test/cri-containerd/policy_test.go @@ -757,6 +757,10 @@ func Test_RunContainer_WithPolicy_And_SecurityPolicyEnv_Annotation(t *testing.T) // and validate later alpineCmd := []string{"ash", "-c", "env && sleep 1"} + opts := []securitypolicy.ContainerConfigOpt{ + securitypolicy.WithCommand(alpineCmd), + securitypolicy.WithAllowStdioAccess(true), + } for _, config := range []struct { name string policy string @@ -767,11 +771,11 @@ func Test_RunContainer_WithPolicy_And_SecurityPolicyEnv_Annotation(t *testing.T) }, { name: "StandardPolicy", - policy: alpineSecurityPolicy(t, "json", false, securitypolicy.WithCommand(alpineCmd)), + policy: alpineSecurityPolicy(t, "json", false, opts...), }, { name: "RegoPolicy", - policy: alpineSecurityPolicy(t, "rego", false, securitypolicy.WithCommand(alpineCmd)), + policy: alpineSecurityPolicy(t, "rego", false, opts...), }, } { for _, setPolicyEnv := range []bool{true, false} { @@ -1013,3 +1017,72 @@ func Test_RunPodSandboxNotAllowed_WithPolicy_EncryptedScratchPolicy(t *testing.T t.Fatalf("expected '%s' error, got '%s'", expectedError, err) } } + +func Test_RunContainer_WithPolicy_And_Binary_Logger_Without_Stdio(t *testing.T) { + requireFeatures(t, featureLCOWIntegrity) + + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + binaryPath := requireBinary(t, "sample-logging-driver.exe") + + logPath := "binary:///" + binaryPath + + pullRequiredLCOWImages(t, []string{imageLcowK8sPause, imageLcowAlpine}) + + for _, tc := range []struct { + stdioAllowed bool + expectedOutput string + }{ + { + true, + "hello\nworld\n", + }, + { + false, + "", + }, + } { + t.Run(fmt.Sprintf("StdioAllowed_%v", tc.stdioAllowed), func(t *testing.T) { + cmd := []string{"ash", "-c", "echo hello; sleep 1; echo world"} + policy := alpineSecurityPolicy( + t, + "rego", + true, + securitypolicy.WithAllowStdioAccess(tc.stdioAllowed), + securitypolicy.WithCommand(cmd), + ) + podReq := sandboxRequestWithPolicy(t, policy) + podID := runPodSandbox(t, client, ctx, podReq) + defer removePodSandbox(t, client, ctx, podID) + + logFileName := fmt.Sprintf(`%s\stdout.txt`, t.TempDir()) + conReq := getCreateContainerRequest( + podID, + fmt.Sprintf("alpine-stdio-allowed-%v", tc.stdioAllowed), + imageLcowAlpine, + cmd, + podReq.Config, + ) + conReq.Config.LogPath = logPath + fmt.Sprintf("?%s", logFileName) + + containerID := createContainer(t, client, ctx, conReq) + defer removeContainer(t, client, ctx, containerID) + + startContainer(t, client, ctx, containerID) + defer stopContainer(t, client, ctx, containerID) + + requireContainerState(ctx, t, client, containerID, runtime.ContainerState_CONTAINER_RUNNING) + requireContainerState(ctx, t, client, containerID, runtime.ContainerState_CONTAINER_EXITED) + + content, err := os.ReadFile(logFileName) + if err != nil { + t.Fatalf("failed to read log file: %s", err) + } + if tc.expectedOutput != string(content) { + t.Fatalf("expected output %q, got %q", tc.expectedOutput, string(content)) + } + }) + } +}