Add new gcs hooks, add expected mounts to security policy (#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>

Makefile

@@ -32,14 +32,15 @@ clean:
 test:
  cd $(SRCROOT) && go test -v ./internal/guest/...
 
-out/delta.tar.gz: bin/init bin/vsockexec bin/cmd/gcs bin/cmd/gcstools Makefile
+out/delta.tar.gz: bin/init bin/vsockexec bin/cmd/gcs bin/cmd/gcstools bin/cmd/hooks/wait-paths Makefile
  @mkdir -p out
  rm -rf rootfs
  mkdir -p rootfs/bin/
  cp bin/init rootfs/
  cp bin/vsockexec rootfs/bin/
  cp bin/cmd/gcs rootfs/bin/
  cp bin/cmd/gcstools rootfs/bin/
+ cp bin/cmd/hooks/wait-paths rootfs/bin/
  for tool in $(GCS_TOOLS); do ln -s gcstools rootfs/bin/$$tool; done
  git -C $(SRCROOT) rev-parse HEAD > rootfs/gcs.commit && \
  git -C $(SRCROOT) rev-parse --abbrev-ref HEAD > rootfs/gcs.branch
@@ -60,6 +61,7 @@ out/initrd.img: $(BASE) out/delta.tar.gz $(SRCROOT)/hack/catcpio.sh
 
 -include deps/cmd/gcs.gomake
 -include deps/cmd/gcstools.gomake
+-include deps/cmd/hooks/wait-paths.gomake
 
 # Implicit rule for includes that define Go targets.
 %.gomake: $(SRCROOT)/Makefile

cmd/gcs/main.go

@@ -14,20 +14,22 @@ import (
  "syscall"
  "time"
 
+ "github.com/containerd/cgroups"
+ cgroupstats "github.com/containerd/cgroups/stats/v1"
+ oci "github.com/opencontainers/runtime-spec/specs-go"
+ "github.com/pkg/errors"
+ "github.com/sirupsen/logrus"
+ "go.opencensus.io/trace"
+
  "github.com/Microsoft/hcsshim/internal/guest/bridge"
  "github.com/Microsoft/hcsshim/internal/guest/kmsg"
  "github.com/Microsoft/hcsshim/internal/guest/runtime/hcsv2"
  "github.com/Microsoft/hcsshim/internal/guest/runtime/runc"
  "github.com/Microsoft/hcsshim/internal/guest/transport"
+ "github.com/Microsoft/hcsshim/internal/guestpath"
  "github.com/Microsoft/hcsshim/internal/log"
  "github.com/Microsoft/hcsshim/internal/oc"
  "github.com/cenkalti/backoff/v4"
- "github.com/containerd/cgroups"
- cgroupstats "github.com/containerd/cgroups/stats/v1"
- oci "github.com/opencontainers/runtime-spec/specs-go"
- "github.com/pkg/errors"
- "github.com/sirupsen/logrus"
- "go.opencensus.io/trace"
 )
 
 func memoryLogFormat(metrics *cgroupstats.Metrics) logrus.Fields {
@@ -229,7 +231,7 @@ func main() {
 
  log.SetScrubbing(*scrubLogs)
 
- baseLogPath := "/run/gcs/c"
+ baseLogPath := guestpath.LCOWRootPrefixInUVM
 
  logrus.Info("GCS started")
 

cmd/hooks/wait-paths/main.go

@@ -0,0 +1,72 @@
+// +build linux
+
+package main
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strings"
+ "time"
+
+ "github.com/sirupsen/logrus"
+ "github.com/urfave/cli"
+)
+
+const (
+ pathsFlag = "paths"
+ timeoutFlag = "timeout"
+)
+
+// This is a hook that waits for a specific path to appear.
+// The hook has required list of comma-separated paths and a default timeout in seconds.
+
+func main() {
+ app := cli.NewApp()
+ app.Name = "wait-paths"
+ app.Usage = "Provide a list paths and an optional timeout"
+ app.Flags = []cli.Flag{
+ cli.StringFlag{
+ Name: pathsFlag + ",p",
+ Usage: "Comma-separated list of paths that should become available",
+ Required: true,
+ },
+ cli.IntFlag{
+ Name: timeoutFlag + ",t",
+ Usage: "Timeout in seconds",
+ Value: 30,
+ },
+ }
+ app.Action = run
+ if err := app.Run(os.Args); err != nil {
+ logrus.Fatalf("%s\n", err)
+ }
+ os.Exit(0)
+}
+
+func run(cCtx *cli.Context) error {
+ timeout := cCtx.GlobalInt(timeoutFlag)
+ paths := strings.Split(cCtx.GlobalString(pathsFlag), ",")
+
+ waitCtx, cancel := context.WithTimeout(context.Background(), time.Duration(timeout)*time.Second)
+ defer cancel()
+
+ for _, path := range paths {
+ for {
+ if _, err := os.Stat(path); err != nil {
+ if !os.IsNotExist(err) {
+ return err
+ }
+ select {
+ case <-waitCtx.Done():
+ return fmt.Errorf("timeout while waiting for path %q to appear", path)
+ default:
+ time.Sleep(time.Millisecond * 10)
+ continue
+ }
+ }
+ break
+ }
+ }
+ return nil
+}

internal/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)

internal/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))
 }

internal/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 {

internal/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)
 }

internal/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 {

internal/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)

internal/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)
 

internal/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
+}