Add new gcs hooks, add expected mounts to security policy (microsoft#…

…1258)

Introduce a new `wait-paths` binary, which polls file system
until requested paths are available or a timeout is reached.

Security policy has been updated to have `ExpectedMounts` entries,
which will be used in conjunction with "wait-paths" hook for
synchronization purposes.

Refactor oci-hook logic into its own internal package and update
existing code to use that package. Copy runc HookName and constants
definitions to break dependency on runc

Introduce `ExpectedMounts` as part of security policy language and
the logic to enforce the policy, which resolves the expected mounts
in the UVM and adds a wait-paths hook to the spec.

Add positive and negative CRI tests.

Signed-off-by: Maksim An <maksiman@microsoft.com>

devices/drivers.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 
 	"github.com/Microsoft/hcsshim/internal/cmd"
+	"github.com/Microsoft/hcsshim/internal/guestpath"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/resources"
 	"github.com/Microsoft/hcsshim/internal/uvm"
@@ -45,7 +46,7 @@ func InstallKernelDriver(ctx context.Context, vm *uvm.UtilityVM, driver string)
 		}
 		return closer, execPnPInstallDriver(ctx, vm, uvmPath)
 	}
-	uvmPathForShare := fmt.Sprintf(uvm.LCOWGlobalMountPrefix, vm.UVMMountCounter())
+	uvmPathForShare := fmt.Sprintf(guestpath.LCOWGlobalMountPrefixFmt, vm.UVMMountCounter())
 	scsiCloser, err := vm.AddSCSI(ctx, driver, uvmPathForShare, true, false, []string{}, uvm.VMAccessTypeIndividual)
 	if err != nil {
 		return closer, fmt.Errorf("failed to add SCSI disk to utility VM for path %+v: %s", driver, err)

guest/runtime/hcsv2/nvidia_utils.go

@@ -10,17 +10,16 @@ import (
 	"os/exec"
 	"strings"
 
+	oci "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+
 	"github.com/Microsoft/hcsshim/cmd/gcstools/generichook"
 	"github.com/Microsoft/hcsshim/internal/guest/storage/pci"
+	"github.com/Microsoft/hcsshim/internal/guestpath"
+	"github.com/Microsoft/hcsshim/internal/hooks"
 	"github.com/Microsoft/hcsshim/pkg/annotations"
-	oci "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/pkg/errors"
 )
 
-// path that the shim mounts the nvidia gpu vhd to in the uvm
-// this MUST match the path mapped to in the shim
-const lcowNvidiaMountPath = "/run/nvidia"
-
 const nvidiaDebugFilePath = "/nvidia-container.log"
 
 const nvidiaToolBinary = "nvidia-container-cli"
@@ -70,35 +69,33 @@ func addNvidiaDevicePreHook(ctx context.Context, spec *oci.Spec) error {
 	// add template for pid argument to be injected later by the generic hook binary
 	args = append(args, "--no-cgroups", "--pid={{pid}}", spec.Root.Path)
 
-	if spec.Hooks == nil {
-		spec.Hooks = &oci.Hooks{}
-	}
-
 	hookLogDebugFileEnvOpt := fmt.Sprintf("%s=%s", generichook.LogDebugFileEnvKey, nvidiaDebugFilePath)
 	hookEnv := append(updateEnvWithNvidiaVariables(), hookLogDebugFileEnvOpt)
-	nvidiaHook := oci.Hook{
-		Path: genericHookPath,
-		Args: args,
-		Env:  hookEnv,
-	}
-
-	spec.Hooks.Prestart = append(spec.Hooks.Prestart, nvidiaHook)
-	return nil
+	nvidiaHook := hooks.NewOCIHook(genericHookPath, args, hookEnv)
+	return hooks.AddOCIHook(spec, hooks.Prestart, nvidiaHook)
 }
 
 // updateEnvWithNvidiaVariables creates an env with the nvidia gpu vhd in PATH and insecure mode set
 func updateEnvWithNvidiaVariables() []string {
+	nvidiaBin := fmt.Sprintf("%s/bin", guestpath.LCOWNvidiaMountPath)
+	env := updatePathEnv(nvidiaBin)
+	// NVC_INSECURE_MODE allows us to run nvidia-container-cli without seccomp
+	// we don't currently use seccomp in the uvm, so avoid using it here for now as well
+	env = append(env, "NVC_INSECURE_MODE=1")
+	return env
+}
+
+// updatePathEnv adds specified `dirs` to PATH variable and returns the result environment variables.
+func updatePathEnv(dirs ...string) []string {
 	pathPrefix := "PATH="
-	nvidiaBin := fmt.Sprintf("%s/bin", lcowNvidiaMountPath)
+	additionalDirs := strings.Join(dirs, ":")
 	env := os.Environ()
 	for i, v := range env {
 		if strings.HasPrefix(v, pathPrefix) {
-			newPath := fmt.Sprintf("%s:%s", v, nvidiaBin)
+			newPath := fmt.Sprintf("%s:%s", v, additionalDirs)
 			env[i] = newPath
+			return env
 		}
 	}
-	// NVC_INSECURE_MODE allows us to run nvidia-container-cli without seccomp
-	// we don't currently use seccomp in the uvm, so avoid using it here for now as well
-	env = append(env, "NVC_INSECURE_MODE=1")
-	return env
+	return append(env, fmt.Sprintf("PATH=%s", additionalDirs))
 }

guest/runtime/hcsv2/sandbox_container.go

@@ -10,16 +10,18 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/Microsoft/hcsshim/internal/guest/network"
-	"github.com/Microsoft/hcsshim/internal/oc"
-	"github.com/Microsoft/hcsshim/pkg/annotations"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"go.opencensus.io/trace"
+
+	"github.com/Microsoft/hcsshim/internal/guest/network"
+	"github.com/Microsoft/hcsshim/internal/guestpath"
+	"github.com/Microsoft/hcsshim/internal/oc"
+	"github.com/Microsoft/hcsshim/pkg/annotations"
 )
 
 func getSandboxRootDir(id string) string {
-	return filepath.Join("/run/gcs/c", id)
+	return filepath.Join(guestpath.LCOWRootPrefixInUVM, id)
 }
 
 func getSandboxHugePageMountsDir(id string) string {

guest/runtime/hcsv2/spec.go

@@ -10,12 +10,14 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/Microsoft/hcsshim/internal/log"
-	"github.com/Microsoft/hcsshim/pkg/annotations"
 	"github.com/opencontainers/runc/libcontainer/devices"
 	"github.com/opencontainers/runc/libcontainer/user"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
+
+	"github.com/Microsoft/hcsshim/internal/hooks"
+	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/Microsoft/hcsshim/pkg/annotations"
 )
 
 // getNetworkNamespaceID returns the `ToLower` of
@@ -257,17 +259,7 @@ func applyAnnotationsToSpec(ctx context.Context, spec *oci.Spec) error {
 }
 
 // Helper function to create an oci prestart hook to run ldconfig
-func addLDConfigHook(ctx context.Context, spec *oci.Spec, args, env []string) error {
-	if spec.Hooks == nil {
-		spec.Hooks = &oci.Hooks{}
-	}
-
-	ldConfigHook := oci.Hook{
-		Path: "/sbin/ldconfig",
-		Args: args,
-		Env:  env,
-	}
-
-	spec.Hooks.Prestart = append(spec.Hooks.Prestart, ldConfigHook)
-	return nil
+func addLDConfigHook(_ context.Context, spec *oci.Spec, args, env []string) error {
+	ldConfigHook := hooks.NewOCIHook("/sbin/ldconfig", args, env)
+	return hooks.AddOCIHook(spec, hooks.Prestart, ldConfigHook)
 }

guest/runtime/hcsv2/standalone_container.go

@@ -10,15 +10,17 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/Microsoft/hcsshim/internal/guest/network"
-	"github.com/Microsoft/hcsshim/internal/oc"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"go.opencensus.io/trace"
+
+	"github.com/Microsoft/hcsshim/internal/guest/network"
+	"github.com/Microsoft/hcsshim/internal/guestpath"
+	"github.com/Microsoft/hcsshim/internal/oc"
 )
 
 func getStandaloneRootDir(id string) string {
-	return filepath.Join("/run/gcs/c", id)
+	return filepath.Join(guestpath.LCOWRootPrefixInUVM, id)
 }
 
 func getStandaloneHostnamePath(id string) string {

guest/runtime/hcsv2/uvm.go

@@ -225,12 +225,16 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 	// We append the variable after the security policy enforcing logic completes so as to bypass it; the
 	// security policy variable cannot be included in the security policy as its value is not available
 	// security policy construction time.
-
 	if policyEnforcer, ok := (h.securityPolicyEnforcer).(*securitypolicy.StandardSecurityPolicyEnforcer); ok {
 		secPolicyEnv := fmt.Sprintf("SECURITY_POLICY=%s", policyEnforcer.EncodedSecurityPolicy)
 		settings.OCISpecification.Process.Env = append(settings.OCISpecification.Process.Env, secPolicyEnv)
 	}
 
+	// Sandbox mount paths need to be resolved in the spec before expected mounts policy can be enforced.
+	if err = h.securityPolicyEnforcer.EnforceExpectedMountsPolicy(id, settings.OCISpecification); err != nil {
+		return nil, errors.Wrapf(err, "container creation denied due to policy")
+	}
+
 	// Create the BundlePath
 	if err := os.MkdirAll(settings.OCIBundlePath, 0700); err != nil {
 		return nil, errors.Wrapf(err, "failed to create OCIBundlePath: '%s'", settings.OCIBundlePath)

guest/runtime/hcsv2/workload_container.go

@@ -9,16 +9,18 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/Microsoft/hcsshim/internal/oc"
-	"github.com/Microsoft/hcsshim/pkg/annotations"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"go.opencensus.io/trace"
 	"golang.org/x/sys/unix"
+
+	"github.com/Microsoft/hcsshim/internal/guestpath"
+	"github.com/Microsoft/hcsshim/internal/oc"
+	"github.com/Microsoft/hcsshim/pkg/annotations"
 )
 
 func getWorkloadRootDir(id string) string {
-	return filepath.Join("/run/gcs/c", id)
+	return filepath.Join(guestpath.LCOWRootPrefixInUVM, id)
 }
 
 // os.MkdirAll combines the given permissions with the running process's
@@ -32,11 +34,10 @@ func mkdirAllModePerm(target string) error {
 }
 
 func updateSandboxMounts(sbid string, spec *oci.Spec) error {
-	sandboxMountPrefix := "sandbox://"
 	for i, m := range spec.Mounts {
-		if strings.HasPrefix(m.Source, sandboxMountPrefix) {
+		if strings.HasPrefix(m.Source, guestpath.SandboxMountPrefix) {
 			mountsDir := getSandboxMountsDir(sbid)
-			subPath := strings.TrimPrefix(m.Source, sandboxMountPrefix)
+			subPath := strings.TrimPrefix(m.Source, guestpath.SandboxMountPrefix)
 			sandboxSource := filepath.Join(mountsDir, subPath)
 
 			// filepath.Join cleans the resulting path before returning so it would resolve the relative path if one was given.
@@ -59,11 +60,10 @@ func updateSandboxMounts(sbid string, spec *oci.Spec) error {
 }
 
 func updateHugePageMounts(sbid string, spec *oci.Spec) error {
-	mountPrefix := "hugepages://"
 	for i, m := range spec.Mounts {
-		if strings.HasPrefix(m.Source, mountPrefix) {
+		if strings.HasPrefix(m.Source, guestpath.HugePagesMountPrefix) {
 			mountsDir := getSandboxHugePageMountsDir(sbid)
-			subPath := strings.TrimPrefix(m.Source, mountPrefix)
+			subPath := strings.TrimPrefix(m.Source, guestpath.HugePagesMountPrefix)
 			pageSize := strings.Split(subPath, string(os.PathSeparator))[0]
 			hugePageMountSource := filepath.Join(mountsDir, subPath)
 

guest/storage/test/policy/mountmonitoringsecuritypolicyenforcer.go

@@ -1,6 +1,8 @@
 package policy
 
 import (
+	oci "github.com/opencontainers/runtime-spec/specs-go"
+
 	"github.com/Microsoft/hcsshim/pkg/securitypolicy"
 )
 
@@ -32,3 +34,7 @@ func (p *MountMonitoringSecurityPolicyEnforcer) EnforceOverlayMountPolicy(contai
 func (p *MountMonitoringSecurityPolicyEnforcer) EnforceCreateContainerPolicy(_ string, _ []string, _ []string, _ string) (err error) {
 	return nil
 }
+
+func (p *MountMonitoringSecurityPolicyEnforcer) EnforceExpectedMountsPolicy(_ string, _ *oci.Spec) error {
+	return nil
+}

guestpath/paths.go

@@ -0,0 +1,24 @@
+package guestpath
+
+const (
+	// LCOWNvidiaMountPath is the path format in LCOW UVM where nvidia tools are mounted
+	// keep this value in sync with opengcs
+	LCOWNvidiaMountPath = "/run/nvidia"
+	// LCOWRootPrefixInUVM is the path inside UVM where LCOW container's root file system will be mounted
+	LCOWRootPrefixInUVM = "/run/gcs/c"
+	// WCOWRootPrefixInUVM is the path inside UVM where WCOW container's root file system will be mounted
+	WCOWRootPrefixInUVM = `C:\c`
+	// SandboxMountPrefix is mount prefix used in container spec to mark a sandbox-mount
+	SandboxMountPrefix = "sandbox://"
+	// HugePagesMountPrefix is mount prefix used in container spec to mark a huge-pages mount
+	HugePagesMountPrefix = "hugepages://"
+	// LCOWMountPathPrefixFmt is the path format in the LCOW UVM where non global mounts, such
+	// as Plan9 mounts are added
+	LCOWMountPathPrefixFmt = "/mounts/m%d"
+	// LCOWGlobalMountPrefixFmt is the path format in the LCOW UVM where global mounts are added
+	LCOWGlobalMountPrefixFmt = "/run/mounts/m%d"
+	// WCOWGlobalMountPrefixFmt is the path prefix format in the WCOW UVM where mounts are added
+	WCOWGlobalMountPrefixFmt = "C:\\mounts\\m%d"
+	// RootfsPath is part of the container's rootfs path
+	RootfsPath = "rootfs"
+)

hcsoci/create.go

@@ -14,6 +14,7 @@ import (
 	"github.com/Microsoft/go-winio/pkg/guid"
 	"github.com/Microsoft/hcsshim/internal/clone"
 	"github.com/Microsoft/hcsshim/internal/cow"
+	"github.com/Microsoft/hcsshim/internal/guestpath"
 	"github.com/Microsoft/hcsshim/internal/hcs"
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 	"github.com/Microsoft/hcsshim/internal/log"
@@ -27,8 +28,8 @@ import (
 )
 
 var (
-	lcowRootInUVM = "/run/gcs/c/%s"
-	wcowRootInUVM = `C:\c\%s`
+	lcowRootInUVM = guestpath.LCOWRootPrefixInUVM + "/%s"
+	wcowRootInUVM = guestpath.WCOWRootPrefixInUVM + "/%s"
 )
 
 // CreateOptions are the set of fields used to call CreateContainer().

hcsoci/devices.go

@@ -9,16 +9,18 @@ import (
 	"path/filepath"
 	"strconv"
 
+	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+
 	"github.com/Microsoft/hcsshim/internal/devices"
+	"github.com/Microsoft/hcsshim/internal/guestpath"
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/oci"
 	"github.com/Microsoft/hcsshim/internal/resources"
 	"github.com/Microsoft/hcsshim/internal/uvm"
 	"github.com/Microsoft/hcsshim/osversion"
 	"github.com/Microsoft/hcsshim/pkg/annotations"
-	specs "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/pkg/errors"
 )
 
 const deviceUtilExeName = "device-util.exe"
@@ -222,14 +224,14 @@ func handleAssignedDevicesLCOW(
 		scsiMount, err := vm.AddSCSI(
 			ctx,
 			gpuSupportVhdPath,
-			uvm.LCOWNvidiaMountPath,
+			guestpath.LCOWNvidiaMountPath,
 			true,
 			false,
 			options,
 			uvm.VMAccessTypeNoop,
 		)
 		if err != nil {
-			return resultDevs, closers, errors.Wrapf(err, "failed to add scsi device %s in the UVM %s at %s", gpuSupportVhdPath, vm.ID(), uvm.LCOWNvidiaMountPath)
+			return resultDevs, closers, errors.Wrapf(err, "failed to add scsi device %s in the UVM %s at %s", gpuSupportVhdPath, vm.ID(), guestpath.LCOWNvidiaMountPath)
 		}
 		closers = append(closers, scsiMount)
 	}

hcsoci/hcsdoc_wcow.go

@@ -11,6 +11,10 @@ import (
 	"regexp"
 	"strings"
 
+	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/sirupsen/logrus"
+
+	"github.com/Microsoft/hcsshim/internal/guestpath"
 	"github.com/Microsoft/hcsshim/internal/hcs/schema1"
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 	"github.com/Microsoft/hcsshim/internal/layers"
@@ -22,8 +26,6 @@ import (
 	"github.com/Microsoft/hcsshim/internal/wclayer"
 	"github.com/Microsoft/hcsshim/osversion"
 	"github.com/Microsoft/hcsshim/pkg/annotations"
-	specs "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/sirupsen/logrus"
 )
 
 // A simple wrapper struct around the container mount configs that should be added to the
@@ -78,7 +80,7 @@ func createMountsConfig(ctx context.Context, coi *createOptionsInternal) (*mount
 					return nil, err
 				}
 				mdv2.HostPath = uvmPath
-			} else if strings.HasPrefix(mount.Source, "sandbox://") {
+			} else if strings.HasPrefix(mount.Source, guestpath.SandboxMountPrefix) {
 				// Convert to the path in the guest that was asked for.
 				mdv2.HostPath = convertToWCOWSandboxMountPath(mount.Source)
 			} else {