Add missing AllowElevated policy check when creating a container (#1624)

* Add missing AllowElevated policy check when creating a container

When we added AllowElevated and checked it was working correctly, we
got it slightly wrong. When a container is started, we were adding in
expected mounts that only happen for privileged containers and
using those are mounts that are allowed.

During testing, if AllowElevated was left off, a privileged container
would fail to start seemingly indicating that all was good. However,
all was not good.

A malicious orchestrator with control of the API could create a container
privileged that didn't contain any extra "privileged mounts" and the
container would start as privileged with everything else that being
privileged entails except for the mounts.

This commit adds an explicit check as part of crete container to verify
that is the container is attempting to be started as privileged that it
has AllowElevated.

Maksim and I both thought that this had been implemented. I remember it
being implemented. Apparently that memory is incorrect. Either way, it
was noticed last Thursday and here's the fix.

Signed-off-by: Sean T. Allen <seanallen@microsoft.com>

internal/guest/runtime/hcsv2/uvm.go

@@ -39,6 +39,7 @@ import (
 	"github.com/Microsoft/hcsshim/pkg/annotations"
 	"github.com/Microsoft/hcsshim/pkg/securitypolicy"
 	"github.com/mattn/go-shellwords"
+	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
@@ -325,6 +326,7 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 		settings.OCISpecification.Process.Env,
 		settings.OCISpecification.Process.Cwd,
 		settings.OCISpecification.Mounts,
+		isPrivilegedContainerCreationRequest(settings.OCISpecification),
 	)
 	if err != nil {
 		return nil, errors.Wrapf(err, "container creation denied due to policy")
@@ -990,3 +992,9 @@ func processOCIEnvToParam(envs []string) map[string]string {
 
 	return paramEnv
 }
+
+// isPrivilegedContainerCreationRequest returns if a given container
+// creation request would create a privileged container
+func isPrivilegedContainerCreationRequest(spec *specs.Spec) bool {
+	return spec.Annotations[annotations.LCOWPrivileged] == "true"
+}

pkg/securitypolicy/framework.rego

@@ -203,6 +203,15 @@ workingDirectory_ok(working_dir) {
     input.workingDir == working_dir
 }
 
+privileged_ok(elevation_allowed) {
+    not input.privileged
+}
+
+privileged_ok(elevation_allowed) {
+    input.privileged
+    input.privileged == elevation_allowed
+}
+
 default container_started := false
 
 container_started {
@@ -221,6 +230,7 @@ create_container := {"metadata": [updateMatches, addStarted],
     # mount list
     possible_containers := [container |
         container := data.metadata.matches[input.containerID][_]
+        privileged_ok(container.allow_elevated)
         workingDirectory_ok(container.working_dir)
         command_ok(container.command)
         mountList_ok(container.mounts, container.allow_elevated)
@@ -706,6 +716,19 @@ errors["no matching containers for overlay"] {
     not overlay_matches
 }
 
+default privileged_matches := false
+
+privileged_matches {
+    input.rule == "create_container"
+    some container in data.metadata.matches[input.containerID]
+    privileged_ok(container.allow_elevated)
+}
+
+errors["privileged escalation not allowed"] {
+    input.rule in ["create_container"]
+    not privileged_matches
+}
+
 default command_matches := false
 
 command_matches {