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)) + } + }) + } +}