diff --git a/cmd/containerd-shim-runhcs-v1/service_internal.go b/cmd/containerd-shim-runhcs-v1/service_internal.go index e00dc0c930..0571a13cd3 100644 --- a/cmd/containerd-shim-runhcs-v1/service_internal.go +++ b/cmd/containerd-shim-runhcs-v1/service_internal.go @@ -94,6 +94,13 @@ func (s *service) createInternal(ctx context.Context, req *task.CreateTaskReques f.Close() spec = oci.UpdateSpecFromOptions(spec, shimOpts) + //expand annotations after defaults have been loaded in from options + err = oci.ProcessAnnotations(ctx, &spec) + // since annotation expansion is used to toggle security features + // raise it rather than suppress and move on + if err != nil { + return nil, errors.Wrap(err, "unable to process OCI Spec annotations") + } if len(req.Rootfs) == 0 { // If no mounts are passed via the snapshotter its the callers full diff --git a/errors.go b/errors.go index f367022e71..a1a2912177 100644 --- a/errors.go +++ b/errors.go @@ -50,6 +50,9 @@ var ( // ErrUnexpectedValue is an error encountered when hcs returns an invalid value ErrUnexpectedValue = hcs.ErrUnexpectedValue + // ErrOperationDenied is an error when hcs attempts an operation that is explicitly denied + ErrOperationDenied = hcs.ErrOperationDenied + // ErrVmcomputeAlreadyStopped is an error encountered when a shutdown or terminate request is made on a stopped container ErrVmcomputeAlreadyStopped = hcs.ErrVmcomputeAlreadyStopped diff --git a/internal/hcs/errors.go b/internal/hcs/errors.go index e21354ffd6..85584f5b87 100644 --- a/internal/hcs/errors.go +++ b/internal/hcs/errors.go @@ -51,6 +51,9 @@ var ( // ErrUnexpectedValue is an error encountered when hcs returns an invalid value ErrUnexpectedValue = errors.New("unexpected value returned from hcs") + // ErrOperationDenied is an error when hcs attempts an operation that is explicitly denied + ErrOperationDenied = errors.New("operation denied") + // ErrVmcomputeAlreadyStopped is an error encountered when a shutdown or terminate request is made on a stopped container ErrVmcomputeAlreadyStopped = syscall.Errno(0xc0370110) diff --git a/internal/hcsoci/create.go b/internal/hcsoci/create.go index 6c9640f878..16821a1861 100644 --- a/internal/hcsoci/create.go +++ b/internal/hcsoci/create.go @@ -45,7 +45,7 @@ type CreateOptions struct { HostingSystem *uvm.UtilityVM // Utility or service VM in which the container is to be created. NetworkNamespace string // Host network namespace to use (overrides anything in the spec) - // This is an advanced debugging parameter. It allows for diagnosibility by leaving a containers + // This is an advanced debugging parameter. It allows for diagnosability by leaving a containers // resources allocated in case of a failure. Thus you would be able to use tools such as hcsdiag // to look at the state of a utility VM to see what resources were allocated. Obviously the caller // must a) not tear down the utility VM on failure (or pause in some way) and b) is responsible for @@ -170,6 +170,15 @@ func validateContainerConfig(ctx context.Context, coi *createOptionsInternal) er return fmt.Errorf("user specified mounts are not permitted for template containers") } } + + // check if gMSA is disabled + if coi.Spec.Windows != nil { + disableGMSA := oci.ParseAnnotationsDisableGMSA(ctx, coi.Spec) + if _, ok := coi.Spec.Windows.CredentialSpec.(string); ok && disableGMSA { + return fmt.Errorf("gMSA credentials are disabled: %w", hcs.ErrOperationDenied) + } + } + return nil } diff --git a/internal/oci/annotations.go b/internal/oci/annotations.go new file mode 100644 index 0000000000..d7274c0362 --- /dev/null +++ b/internal/oci/annotations.go @@ -0,0 +1,150 @@ +package oci + +import ( + "context" + "errors" + "strconv" + "strings" + + "github.com/Microsoft/hcsshim/internal/log" + "github.com/Microsoft/hcsshim/internal/logfields" + "github.com/Microsoft/hcsshim/pkg/annotations" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/sirupsen/logrus" +) + +var ErrAnnotationExpansionConflict = errors.New("annotation expansion conflict") + +// ProcessAnnotations expands annotations into their corresponding annotation groups +func ProcessAnnotations(ctx context.Context, s *specs.Spec) (err error) { + // Named `Process` and not `Expand` since this function may be expanded (pun intended) to + // deal with other annotation issues and validation. + + // Rathen than give up part of the way through on error, this just emits a warning (similar + // to the `parseAnnotation*` functions) and continues through, so the spec is not left in a + // (partially) unusable form. + // If multiple different errors are to be raised, they should be combined or, if they + // are logged, only the last kept, depending on their severity. + + // expand annotations + for key, exps := range annotations.AnnotationExpansions { + // check if annotation is present + if val, ok := s.Annotations[key]; ok { + // ideally, some normalization would occur here (ie, "True" -> "true") + // but strings may be case-sensitive + for _, k := range exps { + if v, ok := s.Annotations[k]; ok && val != v { + err = ErrAnnotationExpansionConflict + log.G(ctx).WithFields(logrus.Fields{ + logfields.OCIAnnotation: key, + logfields.Value: val, + logfields.OCIAnnotation + "-conflict": k, + logfields.Value + "-conflict": v, + }).WithError(err).Warning("annotation expansion would overwrite conflicting value") + continue + } + s.Annotations[k] = val + } + } + } + + return err +} + +// handle specific annotations + +// ParseAnnotationsDisableGMSA searches for the boolean value which specifies +// if providing a gMSA credential should be disallowed. Returns the value found, +// if parsable, otherwise returns false otherwise. +func ParseAnnotationsDisableGMSA(ctx context.Context, s *specs.Spec) bool { + return parseAnnotationsBool(ctx, s.Annotations, annotations.WCOWDisableGMSA, false) +} + +// ParseAnnotationsSaveAsTemplate searches for the boolean value which specifies +// if this create request should be considered as a template creation request. If value +// is found the returns the actual value, returns false otherwise. +func ParseAnnotationsSaveAsTemplate(ctx context.Context, s *specs.Spec) bool { + return parseAnnotationsBool(ctx, s.Annotations, annotations.SaveAsTemplate, false) +} + +// ParseAnnotationsTemplateID searches for the templateID in the create request. If the +// value is found then returns the value otherwise returns the empty string. +func ParseAnnotationsTemplateID(ctx context.Context, s *specs.Spec) string { + return parseAnnotationsString(s.Annotations, annotations.TemplateID, "") +} + +// general annotation parsing + +// parseAnnotationsBool searches `a` for `key` and if found verifies that the +// value is `true` or `false` in any case. If `key` is not found returns `def`. +func parseAnnotationsBool(ctx context.Context, a map[string]string, key string, def bool) bool { + if v, ok := a[key]; ok { + switch strings.ToLower(v) { + case "true": + return true + case "false": + return false + default: + logAnnotationParseError(ctx, key, v, logfields.Bool, nil) + } + } + return def +} + +// parseAnnotationsUint32 searches `a` for `key` and if found verifies that the +// value is a 32 bit unsigned integer. If `key` is not found returns `def`. +func parseAnnotationsUint32(ctx context.Context, a map[string]string, key string, def uint32) uint32 { + if v, ok := a[key]; ok { + countu, err := strconv.ParseUint(v, 10, 32) + if err == nil { + v := uint32(countu) + return v + } + logAnnotationParseError(ctx, key, v, logfields.Uint32, err) + } + return def +} + +// parseAnnotationsUint64 searches `a` for `key` and if found verifies that the +// value is a 64 bit unsigned integer. If `key` is not found returns `def`. +func parseAnnotationsUint64(ctx context.Context, a map[string]string, key string, def uint64) uint64 { + if v, ok := a[key]; ok { + countu, err := strconv.ParseUint(v, 10, 64) + if err == nil { + return countu + } + logAnnotationParseError(ctx, key, v, logfields.Uint64, err) + } + return def +} + +// parseAnnotationsString searches `a` for `key`. If `key` is not found returns `def`. +func parseAnnotationsString(a map[string]string, key string, def string) string { + if v, ok := a[key]; ok { + return v + } + return def +} + +// ParseAnnotationCommaSeparated searches `annotations` for `annotation` corresponding to a +// list of comma separated strings +func ParseAnnotationCommaSeparated(annotation string, annotations map[string]string) []string { + cs, ok := annotations[annotation] + if !ok || cs == "" { + return nil + } + results := strings.Split(cs, ",") + return results +} + +func logAnnotationParseError(ctx context.Context, k, v, et string, err error) { + entry := log.G(ctx).WithFields(logrus.Fields{ + logfields.OCIAnnotation: k, + logfields.Value: v, + logfields.ExpectedType: et, + }) + if err != nil { + entry = entry.WithError(err) + } + entry.Warning("annotation could not be parsed") +} diff --git a/internal/oci/annotations_test.go b/internal/oci/annotations_test.go new file mode 100644 index 0000000000..ef65735d2c --- /dev/null +++ b/internal/oci/annotations_test.go @@ -0,0 +1,86 @@ +package oci + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/Microsoft/hcsshim/pkg/annotations" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/sirupsen/logrus" +) + +func Test_ProccessAnnotations_Expansion(t *testing.T) { + // suppress warnings raised by process annotation + defer func(l logrus.Level) { + logrus.SetLevel(l) + }(logrus.GetLevel()) + logrus.SetLevel(logrus.ErrorLevel) + ctx := context.Background() + + tests := []struct { + name string + spec specs.Spec + }{ + { + name: "lcow", + spec: specs.Spec{ + Linux: &specs.Linux{}, + }, + }, + { + name: "wcow-hypervisor", + spec: specs.Spec{ + Windows: &specs.Windows{ + HyperV: &specs.WindowsHyperV{}, + }, + }, + }, + { + name: "wcow-process", + spec: specs.Spec{ + Windows: &specs.Windows{}, + }, + }, + } + + for _, tt := range tests { + // test correct expansion + for _, v := range []string{"true", "false"} { + t.Run(tt.name+"_disable_unsafe_"+v, func(subtest *testing.T) { + tt.spec.Annotations = map[string]string{ + annotations.DisableUnsafeOperations: v, + } + + err := ProcessAnnotations(ctx, &tt.spec) + if err != nil { + subtest.Fatalf("could not update spec from options: %v", err) + } + + for _, k := range annotations.AnnotationExpansions[annotations.DisableUnsafeOperations] { + if vv := tt.spec.Annotations[k]; vv != v { + subtest.Fatalf("annotation %q was incorrectly expanded to %q, expected %q", k, vv, v) + } + } + }) + } + + // test errors raised on conflict + t.Run(tt.name+"_disable_unsafe_error", func(subtest *testing.T) { + tt.spec.Annotations = map[string]string{ + annotations.DisableUnsafeOperations: "true", + annotations.DisableWritableFileShares: "false", + } + + errExp := fmt.Sprintf("could not expand %q into %q", + annotations.DisableUnsafeOperations, + annotations.DisableWritableFileShares) + + err := ProcessAnnotations(ctx, &tt.spec) + if !errors.Is(err, ErrAnnotationExpansionConflict) { + t.Fatalf("UpdateSpecFromOptions should have failed with %q, actual was %v", errExp, err) + } + }) + } +} diff --git a/internal/oci/uvm.go b/internal/oci/uvm.go index 97b156ba5b..b0876465f0 100644 --- a/internal/oci/uvm.go +++ b/internal/oci/uvm.go @@ -5,48 +5,17 @@ import ( "errors" "fmt" "strconv" - "strings" runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" "github.com/Microsoft/hcsshim/internal/clone" "github.com/Microsoft/hcsshim/internal/log" - "github.com/Microsoft/hcsshim/internal/logfields" "github.com/Microsoft/hcsshim/internal/uvm" "github.com/Microsoft/hcsshim/pkg/annotations" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) -// parseAnnotationsBool searches `a` for `key` and if found verifies that the -// value is `true` or `false` in any case. If `key` is not found returns `def`. -func parseAnnotationsBool(ctx context.Context, a map[string]string, key string, def bool) bool { - if v, ok := a[key]; ok { - switch strings.ToLower(v) { - case "true": - return true - case "false": - return false - default: - log.G(ctx).WithFields(logrus.Fields{ - logfields.OCIAnnotation: key, - logfields.Value: v, - logfields.ExpectedType: logfields.Bool, - }).Warning("annotation could not be parsed") - } - } - return def -} - -// ParseAnnotationCommaSeparated searches `annotations` for `annotation` corresponding to a -// list of comma separated strings -func ParseAnnotationCommaSeparated(annotation string, annotations map[string]string) []string { - cs, ok := annotations[annotation] - if !ok || cs == "" { - return nil - } - results := strings.Split(cs, ",") - return results -} +// UVM specific annotation parsing // ParseAnnotationsCPUCount searches `s.Annotations` for the CPU annotation. If // not found searches `s` for the Windows CPU section. If neither are found @@ -171,64 +140,6 @@ func parseAnnotationsPreferredRootFSType(ctx context.Context, a map[string]strin return def } -// parseAnnotationsUint32 searches `a` for `key` and if found verifies that the -// value is a 32 bit unsigned integer. If `key` is not found returns `def`. -func parseAnnotationsUint32(ctx context.Context, a map[string]string, key string, def uint32) uint32 { - if v, ok := a[key]; ok { - countu, err := strconv.ParseUint(v, 10, 32) - if err == nil { - v := uint32(countu) - return v - } - log.G(ctx).WithFields(logrus.Fields{ - logfields.OCIAnnotation: key, - logfields.Value: v, - logfields.ExpectedType: logfields.Uint32, - logrus.ErrorKey: err, - }).Warning("annotation could not be parsed") - } - return def -} - -// parseAnnotationsUint64 searches `a` for `key` and if found verifies that the -// value is a 64 bit unsigned integer. If `key` is not found returns `def`. -func parseAnnotationsUint64(ctx context.Context, a map[string]string, key string, def uint64) uint64 { - if v, ok := a[key]; ok { - countu, err := strconv.ParseUint(v, 10, 64) - if err == nil { - return countu - } - log.G(ctx).WithFields(logrus.Fields{ - logfields.OCIAnnotation: key, - logfields.Value: v, - logfields.ExpectedType: logfields.Uint64, - logrus.ErrorKey: err, - }).Warning("annotation could not be parsed") - } - return def -} - -// parseAnnotationsString searches `a` for `key`. If `key` is not found returns `def`. -func parseAnnotationsString(a map[string]string, key string, def string) string { - if v, ok := a[key]; ok { - return v - } - return def -} - -// ParseAnnotationsSaveAsTemplate searches for the boolean value which specifies -// if this create request should be considered as a template creation request. If value -// is found the returns the actual value, returns false otherwise. -func ParseAnnotationsSaveAsTemplate(ctx context.Context, s *specs.Spec) bool { - return parseAnnotationsBool(ctx, s.Annotations, annotations.SaveAsTemplate, false) -} - -// ParseAnnotationsTemplateID searches for the templateID in the create request. If the -// value is found then returns the value otherwise returns the empty string. -func ParseAnnotationsTemplateID(ctx context.Context, s *specs.Spec) string { - return parseAnnotationsString(s.Annotations, annotations.TemplateID, "") -} - func ParseCloneAnnotations(ctx context.Context, s *specs.Spec) (isTemplate bool, templateID string, err error) { templateID = ParseAnnotationsTemplateID(ctx, s) isTemplate = ParseAnnotationsSaveAsTemplate(ctx, s) @@ -327,6 +238,25 @@ func handleSecurityPolicy(ctx context.Context, a map[string]string, lopts *uvm.O } } +// sets options common to both WCOW and LCOW from annotations +func specToUVMCreateOptionsCommon(ctx context.Context, opts *uvm.Options, s *specs.Spec) { + opts.MemorySizeInMB = ParseAnnotationsMemory(ctx, s, annotations.MemorySizeInMB, opts.MemorySizeInMB) + opts.LowMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryLowMMIOGapInMB, opts.LowMMIOGapInMB) + opts.HighMMIOBaseInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOBaseInMB, opts.HighMMIOBaseInMB) + opts.HighMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOGapInMB, opts.HighMMIOGapInMB) + opts.AllowOvercommit = parseAnnotationsBool(ctx, s.Annotations, annotations.AllowOvercommit, opts.AllowOvercommit) + opts.EnableDeferredCommit = parseAnnotationsBool(ctx, s.Annotations, annotations.EnableDeferredCommit, opts.EnableDeferredCommit) + opts.ProcessorCount = ParseAnnotationsCPUCount(ctx, s, annotations.ProcessorCount, opts.ProcessorCount) + opts.ProcessorLimit = ParseAnnotationsCPULimit(ctx, s, annotations.ProcessorLimit, opts.ProcessorLimit) + opts.ProcessorWeight = ParseAnnotationsCPUWeight(ctx, s, annotations.ProcessorWeight, opts.ProcessorWeight) + opts.StorageQoSBandwidthMaximum = ParseAnnotationsStorageBps(ctx, s, annotations.StorageQoSBandwidthMaximum, opts.StorageQoSBandwidthMaximum) + opts.StorageQoSIopsMaximum = ParseAnnotationsStorageIops(ctx, s, annotations.StorageQoSIopsMaximum, opts.StorageQoSIopsMaximum) + opts.CPUGroupID = parseAnnotationsString(s.Annotations, annotations.CPUGroupID, opts.CPUGroupID) + opts.NetworkConfigProxy = parseAnnotationsString(s.Annotations, annotations.NetworkConfigProxy, opts.NetworkConfigProxy) + opts.ProcessDumpLocation = parseAnnotationsString(s.Annotations, annotations.ContainerProcessDumpLocation, opts.ProcessDumpLocation) + opts.NoWritableFileShares = parseAnnotationsBool(ctx, s.Annotations, annotations.DisableWritableFileShares, opts.NoWritableFileShares) +} + // SpecToUVMCreateOpts parses `s` and returns either `*uvm.OptionsLCOW` or // `*uvm.OptionsWCOW`. func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) (interface{}, error) { @@ -335,29 +265,17 @@ func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) ( } if IsLCOW(s) { lopts := uvm.NewDefaultOptionsLCOW(id, owner) - lopts.MemorySizeInMB = ParseAnnotationsMemory(ctx, s, annotations.MemorySizeInMB, lopts.MemorySizeInMB) - lopts.LowMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryLowMMIOGapInMB, lopts.LowMMIOGapInMB) - lopts.HighMMIOBaseInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOBaseInMB, lopts.HighMMIOBaseInMB) - lopts.HighMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOGapInMB, lopts.HighMMIOGapInMB) - lopts.AllowOvercommit = parseAnnotationsBool(ctx, s.Annotations, annotations.AllowOvercommit, lopts.AllowOvercommit) - lopts.EnableDeferredCommit = parseAnnotationsBool(ctx, s.Annotations, annotations.EnableDeferredCommit, lopts.EnableDeferredCommit) + specToUVMCreateOptionsCommon(ctx, lopts.Options, s) + lopts.EnableColdDiscardHint = parseAnnotationsBool(ctx, s.Annotations, annotations.EnableColdDiscardHint, lopts.EnableColdDiscardHint) - lopts.ProcessorCount = ParseAnnotationsCPUCount(ctx, s, annotations.ProcessorCount, lopts.ProcessorCount) - lopts.ProcessorLimit = ParseAnnotationsCPULimit(ctx, s, annotations.ProcessorLimit, lopts.ProcessorLimit) - lopts.ProcessorWeight = ParseAnnotationsCPUWeight(ctx, s, annotations.ProcessorWeight, lopts.ProcessorWeight) lopts.VPMemDeviceCount = parseAnnotationsUint32(ctx, s.Annotations, annotations.VPMemCount, lopts.VPMemDeviceCount) lopts.VPMemSizeBytes = parseAnnotationsUint64(ctx, s.Annotations, annotations.VPMemSize, lopts.VPMemSizeBytes) lopts.VPMemNoMultiMapping = parseAnnotationsBool(ctx, s.Annotations, annotations.VPMemNoMultiMapping, lopts.VPMemNoMultiMapping) - lopts.StorageQoSBandwidthMaximum = ParseAnnotationsStorageBps(ctx, s, annotations.StorageQoSBandwidthMaximum, lopts.StorageQoSBandwidthMaximum) - lopts.StorageQoSIopsMaximum = ParseAnnotationsStorageIops(ctx, s, annotations.StorageQoSIopsMaximum, lopts.StorageQoSIopsMaximum) lopts.VPCIEnabled = parseAnnotationsBool(ctx, s.Annotations, annotations.VPCIEnabled, lopts.VPCIEnabled) lopts.BootFilesPath = parseAnnotationsString(s.Annotations, annotations.BootFilesRootPath, lopts.BootFilesPath) - lopts.CPUGroupID = parseAnnotationsString(s.Annotations, annotations.CPUGroupID, lopts.CPUGroupID) - lopts.NetworkConfigProxy = parseAnnotationsString(s.Annotations, annotations.NetworkConfigProxy, lopts.NetworkConfigProxy) lopts.EnableScratchEncryption = parseAnnotationsBool(ctx, s.Annotations, annotations.EncryptedScratchDisk, lopts.EnableScratchEncryption) lopts.SecurityPolicy = parseAnnotationsString(s.Annotations, annotations.SecurityPolicy, lopts.SecurityPolicy) lopts.KernelBootOptions = parseAnnotationsString(s.Annotations, annotations.KernelBootOptions, lopts.KernelBootOptions) - lopts.ProcessDumpLocation = parseAnnotationsString(s.Annotations, annotations.ContainerProcessDumpLocation, lopts.ProcessDumpLocation) lopts.DisableTimeSyncService = parseAnnotationsBool(ctx, s.Annotations, annotations.DisableLCOWTimeSyncService, lopts.DisableTimeSyncService) handleAnnotationPreferredRootFSType(ctx, s.Annotations, lopts) handleAnnotationKernelDirectBoot(ctx, s.Annotations, lopts) @@ -375,22 +293,10 @@ func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) ( return lopts, nil } else if IsWCOW(s) { wopts := uvm.NewDefaultOptionsWCOW(id, owner) - wopts.MemorySizeInMB = ParseAnnotationsMemory(ctx, s, annotations.MemorySizeInMB, wopts.MemorySizeInMB) - wopts.LowMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryLowMMIOGapInMB, wopts.LowMMIOGapInMB) - wopts.HighMMIOBaseInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOBaseInMB, wopts.HighMMIOBaseInMB) - wopts.HighMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOGapInMB, wopts.HighMMIOGapInMB) - wopts.AllowOvercommit = parseAnnotationsBool(ctx, s.Annotations, annotations.AllowOvercommit, wopts.AllowOvercommit) - wopts.EnableDeferredCommit = parseAnnotationsBool(ctx, s.Annotations, annotations.EnableDeferredCommit, wopts.EnableDeferredCommit) - wopts.ProcessorCount = ParseAnnotationsCPUCount(ctx, s, annotations.ProcessorCount, wopts.ProcessorCount) - wopts.ProcessorLimit = ParseAnnotationsCPULimit(ctx, s, annotations.ProcessorLimit, wopts.ProcessorLimit) - wopts.ProcessorWeight = ParseAnnotationsCPUWeight(ctx, s, annotations.ProcessorWeight, wopts.ProcessorWeight) - wopts.StorageQoSBandwidthMaximum = ParseAnnotationsStorageBps(ctx, s, annotations.StorageQoSBandwidthMaximum, wopts.StorageQoSBandwidthMaximum) - wopts.StorageQoSIopsMaximum = ParseAnnotationsStorageIops(ctx, s, annotations.StorageQoSIopsMaximum, wopts.StorageQoSIopsMaximum) + specToUVMCreateOptionsCommon(ctx, wopts.Options, s) + wopts.DisableCompartmentNamespace = parseAnnotationsBool(ctx, s.Annotations, annotations.DisableCompartmentNamespace, wopts.DisableCompartmentNamespace) - wopts.CPUGroupID = parseAnnotationsString(s.Annotations, annotations.CPUGroupID, wopts.CPUGroupID) - wopts.NetworkConfigProxy = parseAnnotationsString(s.Annotations, annotations.NetworkConfigProxy, wopts.NetworkConfigProxy) wopts.NoDirectMap = parseAnnotationsBool(ctx, s.Annotations, annotations.VSMBNoDirectMap, wopts.NoDirectMap) - wopts.ProcessDumpLocation = parseAnnotationsString(s.Annotations, annotations.ContainerProcessDumpLocation, wopts.ProcessDumpLocation) wopts.NoInheritHostTimezone = parseAnnotationsBool(ctx, s.Annotations, annotations.NoInheritHostTimezone, wopts.NoInheritHostTimezone) handleAnnotationFullyPhysicallyBacked(ctx, s.Annotations, wopts) if err := handleCloneAnnotations(ctx, s.Annotations, wopts); err != nil { @@ -434,5 +340,6 @@ func UpdateSpecFromOptions(s specs.Spec, opts *runhcsopts.Options) specs.Spec { s.Annotations[key] = value } } + return s } diff --git a/internal/oci/uvm_test.go b/internal/oci/uvm_test.go index 8653d9142b..c3425f8751 100644 --- a/internal/oci/uvm_test.go +++ b/internal/oci/uvm_test.go @@ -1,15 +1,18 @@ package oci import ( + "context" + "fmt" "testing" runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" + "github.com/Microsoft/hcsshim/internal/uvm" "github.com/Microsoft/hcsshim/pkg/annotations" + "github.com/google/go-cmp/cmp" "github.com/opencontainers/runtime-spec/specs-go" ) func Test_SpecUpdate_MemorySize_WithAnnotation_WithOpts(t *testing.T) { - opts := &runhcsopts.Options{ VmMemorySizeInMb: 3072, } @@ -27,7 +30,6 @@ func Test_SpecUpdate_MemorySize_WithAnnotation_WithOpts(t *testing.T) { } func Test_SpecUpdate_MemorySize_NoAnnotation_WithOpts(t *testing.T) { - opts := &runhcsopts.Options{ VmMemorySizeInMb: 3072, } @@ -75,3 +77,114 @@ func Test_SpecUpdate_ProcessorCount_NoAnnotation_WithOpts(t *testing.T) { t.Fatal("should have updated annotation to default when annotation is not provided in the spec") } } + +func Test_SpecToUVMCreateOptions_Default_LCOW(t *testing.T) { + s := &specs.Spec{ + Linux: &specs.Linux{}, + Annotations: make(map[string]string), + } + + opts, err := SpecToUVMCreateOpts(context.Background(), s, t.Name(), "") + if err != nil { + t.Fatalf("could not generate creation options from spec: %v", err) + } + + lopts := (opts).(*uvm.OptionsLCOW) + dopts := uvm.NewDefaultOptionsLCOW(t.Name(), "") + + // output handler equality is always false, so set to nil + lopts.OutputHandler = nil + dopts.OutputHandler = nil + + if !cmp.Equal(*lopts, *dopts) { + t.Fatalf("should not have updated create options from default when no annotation are provided:\n%s", cmp.Diff(lopts, dopts)) + } +} + +func Test_SpecToUVMCreateOptions_Default_WCOW(t *testing.T) { + s := &specs.Spec{ + Windows: &specs.Windows{ + HyperV: &specs.WindowsHyperV{}, + }, + Annotations: make(map[string]string), + } + + opts, err := SpecToUVMCreateOpts(context.Background(), s, t.Name(), "") + if err != nil { + t.Fatalf("could not generate creation options from spec: %v", err) + } + + wopts := (opts).(*uvm.OptionsWCOW) + dopts := uvm.NewDefaultOptionsWCOW(t.Name(), "") + + if !cmp.Equal(*wopts, *dopts) { + t.Fatalf("should not have updated create options from default when no annotation are provided:\n%s", cmp.Diff(wopts, dopts)) + } +} + +func Test_SpecToUVMCreateOptions_Common(t *testing.T) { + cpugroupid := "1" + lowmmiogap := 1024 + as := map[string]string{ + annotations.ProcessorCount: "8", + annotations.CPUGroupID: cpugroupid, + annotations.DisableWritableFileShares: "true", + annotations.MemoryLowMMIOGapInMB: fmt.Sprint(lowmmiogap), + } + + tests := []struct { + name string + spec specs.Spec + extract func(interface{}) *uvm.Options + }{ + { + name: "lcow", + spec: specs.Spec{ + Linux: &specs.Linux{}, + }, + // generics would be nice ... + extract: func(i interface{}) *uvm.Options { + o := (i).(*uvm.OptionsLCOW) + return o.Options + }, + }, + { + name: "wcow-hypervisor", + spec: specs.Spec{ + Windows: &specs.Windows{ + HyperV: &specs.WindowsHyperV{}, + }, + }, + extract: func(i interface{}) *uvm.Options { + o := (i).(*uvm.OptionsWCOW) + return o.Options + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.spec.Annotations = as + opts, err := SpecToUVMCreateOpts(context.Background(), &tt.spec, t.Name(), "") + if err != nil { + t.Fatalf("could not generate creation options from spec: %v", err) + } + + // get the underlying uvm.Options from uvm.Options[LW]COW + copts := tt.extract(opts) + if copts.LowMMIOGapInMB != uint64(lowmmiogap) { + t.Fatalf("should have updated creation options low MMIO Gap when annotation is provided: %v != %v", copts.LowMMIOGapInMB, lowmmiogap) + } + if copts.ProcessorCount != 8 { + t.Fatalf("should have updated creation options processor count when annotation is provided: %v != 8", copts.ProcessorCount) + } + if copts.CPUGroupID != cpugroupid { + t.Fatalf("should have updated creation options CPU group Id when annotation is provided: %v != %v", copts.CPUGroupID, cpugroupid) + } + if !copts.NoWritableFileShares { + t.Fatal("should have disabled writable in shares creation when annotation is provided") + } + }) + } + +} diff --git a/internal/uvm/create.go b/internal/uvm/create.go index 8f293f67fe..a3d9fcd4e6 100644 --- a/internal/uvm/create.go +++ b/internal/uvm/create.go @@ -91,6 +91,9 @@ type Options struct { // applied to all containers. On Windows it's configurable per container, but we can mimic this for // Windows by just applying the location specified here per container. ProcessDumpLocation string + + // NoWritableFileShares disables adding any writable vSMB and Plan9 shares to the UVM + NoWritableFileShares bool } // compares the create opts used during template creation with the create opts @@ -184,6 +187,7 @@ func newDefaultOptions(id, owner string) *Options { EnableDeferredCommit: false, ProcessorCount: defaultProcessorCount(), FullyPhysicallyBacked: false, + NoWritableFileShares: false, } if opts.Owner == "" { @@ -382,6 +386,10 @@ func (uvm *UtilityVM) VSMBNoDirectMap() bool { return uvm.vsmbNoDirectMap } +func (uvm *UtilityVM) NoWritableFileShares() bool { + return uvm.noWritableFileShares +} + // Closes the external GCS connection if it is being used and also closes the // listener for GCS connection. func (uvm *UtilityVM) CloseGCSConnection() (err error) { diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 5a5628230d..629d080e59 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -729,6 +729,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error createOpts: opts, vpmemMultiMapping: !opts.VPMemNoMultiMapping, encryptScratch: opts.EnableScratchEncryption, + noWritableFileShares: opts.NoWritableFileShares, } defer func() { diff --git a/internal/uvm/create_wcow.go b/internal/uvm/create_wcow.go index c09b49d9f0..1d083ffda2 100644 --- a/internal/uvm/create_wcow.go +++ b/internal/uvm/create_wcow.go @@ -257,6 +257,7 @@ func CreateWCOW(ctx context.Context, opts *OptionsWCOW) (_ *UtilityVM, err error physicallyBacked: !opts.AllowOvercommit, devicesPhysicallyBacked: opts.FullyPhysicallyBacked, vsmbNoDirectMap: opts.NoDirectMap, + noWritableFileShares: opts.NoWritableFileShares, createOpts: *opts, } diff --git a/internal/uvm/plan9.go b/internal/uvm/plan9.go index 3b86b392ae..475a9dbc1d 100644 --- a/internal/uvm/plan9.go +++ b/internal/uvm/plan9.go @@ -6,6 +6,7 @@ import ( "fmt" "strconv" + "github.com/Microsoft/hcsshim/internal/hcs" "github.com/Microsoft/hcsshim/internal/hcs/resourcepaths" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" @@ -41,6 +42,9 @@ func (uvm *UtilityVM) AddPlan9(ctx context.Context, hostPath string, uvmPath str if uvmPath == "" { return nil, fmt.Errorf("uvmPath must be passed to AddPlan9") } + if !readOnly && uvm.NoWritableFileShares() { + return nil, fmt.Errorf("adding writable shares is denied: %w", hcs.ErrOperationDenied) + } // TODO: JTERRY75 - These are marked private in the schema. For now use them // but when there are public variants we need to switch to them. diff --git a/internal/uvm/scsi.go b/internal/uvm/scsi.go index 216da8dc43..5cac727387 100644 --- a/internal/uvm/scsi.go +++ b/internal/uvm/scsi.go @@ -630,7 +630,7 @@ func (sm *SCSIMount) GobDecode(data []byte) error { // Clone function creates a clone of the SCSIMount `sm` and adds the cloned SCSIMount to // the uvm `vm`. If `sm` is read only then it is simply added to the `vm`. But if it is a -// writeable mount(e.g a scratch layer) then a copy of it is made and that copy is added +// writable mount(e.g a scratch layer) then a copy of it is made and that copy is added // to the `vm`. func (sm *SCSIMount) Clone(ctx context.Context, vm *UtilityVM, cd *cloneData) error { var ( @@ -642,7 +642,7 @@ func (sm *SCSIMount) Clone(ctx context.Context, vm *UtilityVM, cd *cloneData) er ) if !sm.readOnly { - // This is a writeable SCSI mount. It must be either the + // This is a writable SCSI mount. It must be either the // 1. scratch VHD of the UVM or // 2. scratch VHD of the container. // A user provided writable SCSI mount is not allowed on the template UVM diff --git a/internal/uvm/types.go b/internal/uvm/types.go index 065f61c0ba..b87fe45f7f 100644 --- a/internal/uvm/types.go +++ b/internal/uvm/types.go @@ -60,6 +60,13 @@ type UtilityVM struct { // NOTE: All accesses to this MUST be done atomically. containerCounter uint64 + // noWritableFileShares disables mounting any writable vSMB or Plan9 shares + // on the uVM. This prevents containers in the uVM modifying files and directories + // made available via the "mounts" options in the container spec, or shared + // to the uVM directly. + // This option does not prevent writable SCSI mounts. + noWritableFileShares bool + // VSMB shares that are mapped into a Windows UVM. These are used for read-only // layers and mapped directories. // We maintain two sets of maps, `vsmbDirShares` tracks shares that are diff --git a/internal/uvm/vsmb.go b/internal/uvm/vsmb.go index b1e5f24434..b2ccf5be4f 100644 --- a/internal/uvm/vsmb.go +++ b/internal/uvm/vsmb.go @@ -13,6 +13,7 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/sys/windows" + "github.com/Microsoft/hcsshim/internal/hcs" "github.com/Microsoft/hcsshim/internal/hcs/resourcepaths" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" @@ -163,6 +164,10 @@ func (uvm *UtilityVM) AddVSMB(ctx context.Context, hostPath string, options *hcs return nil, errNotSupported } + if !options.ReadOnly && uvm.NoWritableFileShares() { + return nil, fmt.Errorf("adding writable shares is denied: %w", hcs.ErrOperationDenied) + } + uvm.m.Lock() defer uvm.m.Unlock() diff --git a/pkg/annotations/annotations.go b/pkg/annotations/annotations.go index 88b6d810bb..7a51b2a0e4 100644 --- a/pkg/annotations/annotations.go +++ b/pkg/annotations/annotations.go @@ -192,6 +192,9 @@ const ( // VSMBNoDirectMap specifies that no direct mapping should be used for any VSMBs added to the UVM VSMBNoDirectMap = "io.microsoft.virtualmachine.wcow.virtualSMB.nodirectmap" + // DisableWritableFileShares disables adding any writable fileshares to the UVM + DisableWritableFileShares = "io.microsoft.virtualmachine.fileshares.disablewritable" + // CPUGroupID specifies the cpugroup ID that a UVM should be assigned to if any CPUGroupID = "io.microsoft.virtualmachine.cpugroup.id" @@ -265,4 +268,25 @@ const ( // NoInheritHostTimezone specifies for the hosts timezone to not be inherited by the WCOW UVM. The UVM will be set to UTC time // as a default. NoInheritHostTimezone = "io.microsoft.virtualmachine.wcow.timezone.noinherit" + + // WCOWDisableGMSA disables providing gMSA (Group Managed Service Accounts) to + // a WCOW container + WCOWDisableGMSA = "io.microsoft.container.wcow.gmsa.disable" + + // DisableUnsafeOperations disables several unsafe operations, such as writable + // file share mounts, for hostile multi-tenant environments. See `AnnotationExpansions` + // for more information + DisableUnsafeOperations = "io.microsoft.disable-unsafe-operations" ) + +// AnnotationExpansions maps annotations that will be expanded into an array of +// other annotations. The expanded annotations will have the same value as the +// original. It is an error for the expansions to already exist and have a value +// that differs from the original. +var AnnotationExpansions = map[string][]string{ + DisableUnsafeOperations: { + WCOWDisableGMSA, + DisableWritableFileShares, + VSMBNoDirectMap, + }, +} diff --git a/test/cri-containerd/container_fileshare_test.go b/test/cri-containerd/container_fileshare_test.go new file mode 100644 index 0000000000..fd0b7434e6 --- /dev/null +++ b/test/cri-containerd/container_fileshare_test.go @@ -0,0 +1,203 @@ +//go:build functional + +package cri_containerd + +import ( + "context" + "fmt" + "io/ioutil" + "path/filepath" + "strings" + "testing" + + "github.com/Microsoft/hcsshim/internal/hcs" + "github.com/Microsoft/hcsshim/pkg/annotations" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" +) + +func Test_Container_File_Share_Writable_WCOW(t *testing.T) { + requireFeatures(t, featureWCOWHypervisor) + + pullRequiredImages(t, []string{imageWindowsNanoserver}) + + client := newTestRuntimeClient(t) + ctx := context.Background() + + sbRequest := getRunPodSandboxRequest(t, wcowHypervisorRuntimeHandler, + WithSandboxAnnotations(map[string]string{ + annotations.DisableWritableFileShares: "true", + }), + ) + podID := runPodSandbox(t, client, ctx, sbRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + var ( + tempDir = t.TempDir() + containerPath = "C:\\test" + testFile = "t.txt" + testContent = "hello world" + ) + + if err := ioutil.WriteFile( + filepath.Join(tempDir, testFile), + []byte(testContent), + 0644); err != nil { + t.Fatalf("could not create test file: %v", err) + } + + cRequest := &runtime.CreateContainerRequest{ + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: t.Name(), + }, + Image: &runtime.ImageSpec{ + Image: imageWindowsNanoserver, + }, + Command: []string{ + "cmd", + "/c", + "ping -t 127.0.0.1", + }, + Mounts: []*runtime.Mount{ + { + HostPath: tempDir, + ContainerPath: containerPath, + Readonly: false, + }, + }, + }, + PodSandboxId: podID, + SandboxConfig: sbRequest.Config, + } + cID := createContainer(t, client, ctx, cRequest) + defer removeContainer(t, client, ctx, cID) + + // container should fail because of writable mount + _, err := client.StartContainer(ctx, &runtime.StartContainerRequest{ContainerId: cID}) + if err == nil { + stopContainer(t, client, ctx, cID) + } + // error is serialized over gRPC then embedded into "rpc error: code = %s desc = %s" + // so error.Is() wont work + if err == nil || !strings.Contains(err.Error(), fmt.Errorf("adding writable shares is denied: %w", hcs.ErrOperationDenied).Error()) { + t.Fatalf("StartContainer did not fail with writable fileshare: error is %v", err) + } + + // set it to read only + cRequest.Config.Metadata.Name = t.Name() + "_2" + cRequest.Config.Mounts[0].Readonly = true + + cID = createContainer(t, client, ctx, cRequest) + defer removeContainer(t, client, ctx, cID) + startContainer(t, client, ctx, cID) + defer stopContainer(t, client, ctx, cID) + + testExec := []string{ + "cmd", + "/c", + "type", + filepath.Join(containerPath, testFile), + } + output, errMsg, exitCode := execContainer(t, client, ctx, cID, testExec) + if exitCode != 0 { + t.Fatalf("could not find mounted file: %s %s", errMsg, output) + } + if output != testContent { + t.Fatalf("did not correctly read file; got %q, expected %q", output, testContent) + } +} + +func Test_Container_File_Share_Writable_LCOW(t *testing.T) { + requireFeatures(t, featureLCOW) + + pullRequiredLCOWImages(t, []string{imageLcowK8sPause, imageLcowAlpine}) + + client := newTestRuntimeClient(t) + ctx := context.Background() + + sbRequest := getRunPodSandboxRequest(t, lcowRuntimeHandler, + WithSandboxAnnotations(map[string]string{ + annotations.DisableWritableFileShares: "true", + }), + ) + podID := runPodSandbox(t, client, ctx, sbRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + var ( + tempDir = t.TempDir() + containerPath = "/mnt/test" + testFile = "t.txt" + testContent = "hello world" + ) + + if err := ioutil.WriteFile( + filepath.Join(tempDir, testFile), + []byte(testContent), + 0644, + ); err != nil { + t.Fatalf("could not create test file: %v", err) + } + + cRequest := &runtime.CreateContainerRequest{ + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: t.Name(), + }, + Image: &runtime.ImageSpec{ + Image: imageLcowAlpine, + }, + Command: []string{ + "ash", + "-c", + "tail -f /dev/null", + }, + Mounts: []*runtime.Mount{ + { + HostPath: tempDir, + ContainerPath: containerPath, + Readonly: false, + }, + }, + }, + PodSandboxId: podID, + SandboxConfig: sbRequest.Config, + } + cID := createContainer(t, client, ctx, cRequest) + defer removeContainer(t, client, ctx, cID) + + // container should fail because of writable mount + _, err := client.StartContainer(ctx, &runtime.StartContainerRequest{ContainerId: cID}) + if err == nil { + stopContainer(t, client, ctx, cID) + } + // error is serialized over gRPC then embedded into "rpc error: code = %s desc = %s" + // so error.Is() wont work + if err == nil || !strings.Contains(err.Error(), fmt.Errorf("adding writable shares is denied: %w", hcs.ErrOperationDenied).Error()) { + t.Fatalf("StartContainer did not fail with writable fileshare: error is %v", err) + } + + // set it to read only + cRequest.Config.Metadata.Name = t.Name() + "_2" + cRequest.Config.Mounts[0].Readonly = true + + cID = createContainer(t, client, ctx, cRequest) + defer removeContainer(t, client, ctx, cID) + startContainer(t, client, ctx, cID) + defer stopContainer(t, client, ctx, cID) + + testExec := []string{ + "ash", + "-c", + // filepath.Join replaces `/` with `\`, so Join path manually + "cat " + containerPath + "/" + testFile, + } + output, errMsg, exitCode := execContainer(t, client, ctx, cID, testExec) + if exitCode != 0 { + t.Fatalf("could not find mounted file: %s %s", errMsg, output) + } + if output != testContent { + t.Fatalf("did not correctly read file; got %q, expected %q", output, testContent) + } +} diff --git a/test/cri-containerd/container_gmsa_test.go b/test/cri-containerd/container_gmsa_test.go new file mode 100644 index 0000000000..4207ed83e3 --- /dev/null +++ b/test/cri-containerd/container_gmsa_test.go @@ -0,0 +1,177 @@ +//go:build functional + +package cri_containerd + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/Microsoft/hcsshim/internal/hcs" + "github.com/Microsoft/hcsshim/pkg/annotations" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" +) + +func Test_RunContainer_GMSA_WCOW(t *testing.T) { + requireFeatures(t, featureGMSA) + + credSpec := gmsaSetup(t) + pullRequiredImages(t, []string{imageWindowsNanoserver}) + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + tests := []struct { + feature string + handler string + }{ + { + feature: featureWCOWProcess, + handler: wcowProcessRuntimeHandler, + }, + { + feature: featureWCOWHypervisor, + handler: wcowHypervisorRuntimeHandler, + }, + } + + for _, tt := range tests { + t.Run(tt.feature, func(t *testing.T) { + requireFeatures(t, tt.feature) + sandboxRequest := getRunPodSandboxRequest(t, tt.handler) + + podID := runPodSandbox(t, client, ctx, sandboxRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + request := &runtime.CreateContainerRequest{ + PodSandboxId: podID, + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: t.Name() + "-Container", + }, + Image: &runtime.ImageSpec{ + Image: imageWindowsNanoserver, + }, + Command: []string{ + "cmd", + "/c", + "ping", + "-t", + "127.0.0.1", + }, + Windows: &runtime.WindowsContainerConfig{ + SecurityContext: &runtime.WindowsContainerSecurityContext{ + CredentialSpec: credSpec, + }, + }, + }, + SandboxConfig: sandboxRequest.Config, + } + + containerID := createContainer(t, client, ctx, request) + defer removeContainer(t, client, ctx, containerID) + startContainer(t, client, ctx, containerID) + defer stopContainer(t, client, ctx, containerID) + + // No klist and no powershell available + cmd := []string{"cmd", "/c", "set", "USERDNSDOMAIN"} + containerExecReq := &runtime.ExecSyncRequest{ + ContainerId: containerID, + Cmd: cmd, + Timeout: 20, + } + r := execSync(t, client, ctx, containerExecReq) + if r.ExitCode != 0 { + t.Fatalf("failed with exit code %d running 'set USERDNSDOMAIN': %s", r.ExitCode, string(r.Stderr)) + } + // Check for USERDNSDOMAIN environment variable. This acts as a way tell if a + // user is joined to an Active Directory Domain and is successfully + // authenticated as a domain identity. + if !strings.Contains(string(r.Stdout), "USERDNSDOMAIN") { + t.Fatalf("expected to see USERDNSDOMAIN entry") + } + }) + } +} +func Test_RunContainer_GMSA_Disabled(t *testing.T) { + requireFeatures(t, featureGMSA) + + credSpec := "totally real and definitely not a fake or arbitrary gMSA credential spec that is 1000%% properly formatted as JSON" + pullRequiredImages(t, []string{imageWindowsNanoserver}) + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + tests := []struct { + name string + feature string + runtime string + }{ + { + name: "WCOW_Hypervisor", + feature: featureWCOWHypervisor, + runtime: wcowHypervisorRuntimeHandler, + }, + { + name: "WCOW_Process", + feature: featureWCOWProcess, + runtime: wcowHypervisorRuntimeHandler, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(subtest *testing.T) { + requireFeatures(subtest, tt.feature) + sandboxRequest := getRunPodSandboxRequest(subtest, tt.runtime) + + podID := runPodSandbox(subtest, client, ctx, sandboxRequest) + defer removePodSandbox(subtest, client, ctx, podID) + defer stopPodSandbox(subtest, client, ctx, podID) + + request := &runtime.CreateContainerRequest{ + PodSandboxId: podID, + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: subtest.Name(), + }, + Image: &runtime.ImageSpec{ + Image: imageWindowsNanoserver, + }, + Command: []string{ + "cmd", + "/c", + "ping -t 127.0.0.1", + }, + Annotations: map[string]string{ + annotations.WCOWDisableGMSA: "true", + }, + Windows: &runtime.WindowsContainerConfig{ + SecurityContext: &runtime.WindowsContainerSecurityContext{ + CredentialSpec: credSpec, + }, + }, + }, + SandboxConfig: sandboxRequest.Config, + } + + cID := createContainer(t, client, ctx, request) + defer removeContainer(t, client, ctx, cID) + + // should fail because of gMSA creds + _, err := client.StartContainer(ctx, &runtime.StartContainerRequest{ContainerId: cID}) + if err == nil { + stopContainer(t, client, ctx, cID) + } + // error is serialized over gRPC then embedded into "rpc error: code = %s desc = %s" + // so error.Is() wont work + if !strings.Contains( + err.Error(), + fmt.Errorf("gMSA credentials are disabled: %w", hcs.ErrOperationDenied).Error(), + ) { + t.Fatalf("StartContainer did not fail with gMSA credentials: error is %q", err) + } + }) + } +} diff --git a/test/cri-containerd/container_test.go b/test/cri-containerd/container_test.go index f35ec1cb52..0b9b43c720 100644 --- a/test/cri-containerd/container_test.go +++ b/test/cri-containerd/container_test.go @@ -349,134 +349,6 @@ func Test_RunContainer_ZeroVPMEM_Multiple_LCOW(t *testing.T) { defer stopContainer(t, client, ctx, containerIDTwo) } -func Test_RunContainer_GMSA_WCOW_Process(t *testing.T) { - requireFeatures(t, featureWCOWProcess, featureGMSA) - - credSpec := gmsaSetup(t) - pullRequiredImages(t, []string{imageWindowsNanoserver}) - client := newTestRuntimeClient(t) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - sandboxRequest := getRunPodSandboxRequest(t, wcowProcessRuntimeHandler) - - podID := runPodSandbox(t, client, ctx, sandboxRequest) - defer removePodSandbox(t, client, ctx, podID) - defer stopPodSandbox(t, client, ctx, podID) - - request := &runtime.CreateContainerRequest{ - PodSandboxId: podID, - Config: &runtime.ContainerConfig{ - Metadata: &runtime.ContainerMetadata{ - Name: t.Name() + "-Container", - }, - Image: &runtime.ImageSpec{ - Image: imageWindowsNanoserver, - }, - Command: []string{ - "cmd", - "/c", - "ping", - "-t", - "127.0.0.1", - }, - Windows: &runtime.WindowsContainerConfig{ - SecurityContext: &runtime.WindowsContainerSecurityContext{ - CredentialSpec: credSpec, - }, - }, - }, - SandboxConfig: sandboxRequest.Config, - } - - containerID := createContainer(t, client, ctx, request) - defer removeContainer(t, client, ctx, containerID) - startContainer(t, client, ctx, containerID) - defer stopContainer(t, client, ctx, containerID) - - // No klist and no powershell available - cmd := []string{"cmd", "/c", "set", "USERDNSDOMAIN"} - containerExecReq := &runtime.ExecSyncRequest{ - ContainerId: containerID, - Cmd: cmd, - Timeout: 20, - } - r := execSync(t, client, ctx, containerExecReq) - if r.ExitCode != 0 { - t.Fatalf("failed with exit code %d running 'set USERDNSDOMAIN': %s", r.ExitCode, string(r.Stderr)) - } - // Check for USERDNSDOMAIN environment variable. This acts as a way tell if a - // user is joined to an Active Directory Domain and is successfully - // authenticated as a domain identity. - if !strings.Contains(string(r.Stdout), "USERDNSDOMAIN") { - t.Fatalf("expected to see USERDNSDOMAIN entry") - } -} - -func Test_RunContainer_GMSA_WCOW_Hypervisor(t *testing.T) { - requireFeatures(t, featureWCOWHypervisor, featureGMSA) - - credSpec := gmsaSetup(t) - pullRequiredImages(t, []string{imageWindowsNanoserver}) - client := newTestRuntimeClient(t) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - sandboxRequest := getRunPodSandboxRequest(t, wcowHypervisorRuntimeHandler) - - podID := runPodSandbox(t, client, ctx, sandboxRequest) - defer removePodSandbox(t, client, ctx, podID) - defer stopPodSandbox(t, client, ctx, podID) - - request := &runtime.CreateContainerRequest{ - PodSandboxId: podID, - Config: &runtime.ContainerConfig{ - Metadata: &runtime.ContainerMetadata{ - Name: t.Name() + "-Container", - }, - Image: &runtime.ImageSpec{ - Image: imageWindowsNanoserver, - }, - Command: []string{ - "cmd", - "/c", - "ping", - "-t", - "127.0.0.1", - }, - Windows: &runtime.WindowsContainerConfig{ - SecurityContext: &runtime.WindowsContainerSecurityContext{ - CredentialSpec: credSpec, - }, - }, - }, - SandboxConfig: sandboxRequest.Config, - } - - containerID := createContainer(t, client, ctx, request) - defer removeContainer(t, client, ctx, containerID) - startContainer(t, client, ctx, containerID) - defer stopContainer(t, client, ctx, containerID) - - // No klist and no powershell available - cmd := []string{"cmd", "/c", "set", "USERDNSDOMAIN"} - containerExecReq := &runtime.ExecSyncRequest{ - ContainerId: containerID, - Cmd: cmd, - Timeout: 20, - } - r := execSync(t, client, ctx, containerExecReq) - if r.ExitCode != 0 { - t.Fatalf("failed with exit code %d running 'set USERDNSDOMAIN': %s", r.ExitCode, string(r.Stderr)) - } - // Check for USERDNSDOMAIN environment variable. This acts as a way tell if a - // user is joined to an Active Directory Domain and is successfully - // authenticated as a domain identity. - if !strings.Contains(string(r.Stdout), "USERDNSDOMAIN") { - t.Fatalf("expected to see USERDNSDOMAIN entry") - } -} - func Test_RunContainer_SandboxDevice_LCOW(t *testing.T) { requireFeatures(t, featureLCOW) diff --git a/test/functional/utilities/createuvm.go b/test/functional/utilities/createuvm.go index 8587fdef42..d88307bde9 100644 --- a/test/functional/utilities/createuvm.go +++ b/test/functional/utilities/createuvm.go @@ -67,11 +67,11 @@ func CreateLCOWUVMFromOpts(ctx context.Context, t *testing.T, opts *uvm.OptionsL uvm, err := uvm.CreateLCOW(ctx, opts) if err != nil { - t.Fatal(err) + t.Fatalf("could not create LCOW UVM: %v", err) } if err := uvm.Start(ctx); err != nil { uvm.Close() - t.Fatal(err) + t.Fatalf("could not start LCOW UVM: %v", err) } return uvm } diff --git a/test/functional/uvm_plannine_test.go b/test/functional/uvm_plannine_test.go index 4d0acfd7f0..86573631ed 100644 --- a/test/functional/uvm_plannine_test.go +++ b/test/functional/uvm_plannine_test.go @@ -7,11 +7,13 @@ package functional import ( "context" + "errors" "fmt" "os" "path/filepath" "testing" + "github.com/Microsoft/hcsshim/internal/hcs" "github.com/Microsoft/hcsshim/internal/uvm" "github.com/Microsoft/hcsshim/osversion" testutilities "github.com/Microsoft/hcsshim/test/functional/utilities" @@ -44,3 +46,35 @@ func TestPlan9(t *testing.T) { } } } + +func TestPlan9_Writable(t *testing.T) { + testutilities.RequiresBuild(t, osversion.RS5) + + opts := uvm.NewDefaultOptionsLCOW(t.Name(), "") + opts.NoWritableFileShares = true + vm := testutilities.CreateLCOWUVMFromOpts(context.Background(), t, opts) + defer vm.Close() + + dir := testutilities.CreateTempDir(t) + defer os.RemoveAll(dir) + + // mount as writable should fail + share, err := vm.AddPlan9(context.Background(), dir, fmt.Sprintf("/tmp/%s", filepath.Base(dir)), false, false, nil) + defer func() { + if share == nil { + return + } + if err := vm.RemovePlan9(context.Background(), share); err != nil { + t.Fatalf("RemovePlan9 failed: %s", err) + } + }() + if !errors.Is(err, hcs.ErrOperationDenied) { + t.Fatalf("AddPlan9 should have failed with %v instead of: %v", hcs.ErrOperationDenied, err) + } + + // mount as read-only should succeed + share, err = vm.AddPlan9(context.Background(), dir, fmt.Sprintf("/tmp/%s", filepath.Base(dir)), true, false, nil) + if err != nil { + t.Fatalf("AddPlan9 failed: %v", err) + } +} diff --git a/test/functional/uvm_vsmb_test.go b/test/functional/uvm_vsmb_test.go index a2ace4c697..167aecccdf 100644 --- a/test/functional/uvm_vsmb_test.go +++ b/test/functional/uvm_vsmb_test.go @@ -5,9 +5,12 @@ package functional import ( "context" + "errors" "os" "testing" + "github.com/Microsoft/hcsshim/internal/hcs" + "github.com/Microsoft/hcsshim/internal/uvm" "github.com/Microsoft/hcsshim/osversion" testutilities "github.com/Microsoft/hcsshim/test/functional/utilities" ) @@ -39,3 +42,39 @@ func TestVSMB(t *testing.T) { } // TODO: VSMB for mapped directories + +func TestVSMB_Writable(t *testing.T) { + testutilities.RequiresBuild(t, osversion.RS5) + + opts := uvm.NewDefaultOptionsWCOW(t.Name(), "") + opts.NoWritableFileShares = true + vm, _, uvmScratchDir := testutilities.CreateWCOWUVMFromOptsWithImage(context.Background(), t, opts, "microsoft/nanoserver") + defer os.RemoveAll(uvmScratchDir) + defer vm.Close() + + dir := testutilities.CreateTempDir(t) + defer os.RemoveAll(dir) + options := vm.DefaultVSMBOptions(true) + options.TakeBackupPrivilege = true + options.ReadOnly = false + _, err := vm.AddVSMB(context.Background(), dir, options) + defer func() { + if err == nil { + return + } + if err = vm.RemoveVSMB(context.Background(), dir, true); err != nil { + t.Fatalf("RemoveVSMB failed: %s", err) + } + }() + + if !errors.Is(err, hcs.ErrOperationDenied) { + t.Fatalf("AddVSMB should have failed with %v instead of: %v", hcs.ErrOperationDenied, err) + } + + options.ReadOnly = true + _, err = vm.AddVSMB(context.Background(), dir, options) + if err != nil { + t.Fatalf("AddVSMB failed: %s", err) + } + +} diff --git a/test/vendor/github.com/Microsoft/hcsshim/errors.go b/test/vendor/github.com/Microsoft/hcsshim/errors.go index f367022e71..a1a2912177 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/errors.go +++ b/test/vendor/github.com/Microsoft/hcsshim/errors.go @@ -50,6 +50,9 @@ var ( // ErrUnexpectedValue is an error encountered when hcs returns an invalid value ErrUnexpectedValue = hcs.ErrUnexpectedValue + // ErrOperationDenied is an error when hcs attempts an operation that is explicitly denied + ErrOperationDenied = hcs.ErrOperationDenied + // ErrVmcomputeAlreadyStopped is an error encountered when a shutdown or terminate request is made on a stopped container ErrVmcomputeAlreadyStopped = hcs.ErrVmcomputeAlreadyStopped diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/hcs/errors.go b/test/vendor/github.com/Microsoft/hcsshim/internal/hcs/errors.go index e21354ffd6..85584f5b87 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/hcs/errors.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/hcs/errors.go @@ -51,6 +51,9 @@ var ( // ErrUnexpectedValue is an error encountered when hcs returns an invalid value ErrUnexpectedValue = errors.New("unexpected value returned from hcs") + // ErrOperationDenied is an error when hcs attempts an operation that is explicitly denied + ErrOperationDenied = errors.New("operation denied") + // ErrVmcomputeAlreadyStopped is an error encountered when a shutdown or terminate request is made on a stopped container ErrVmcomputeAlreadyStopped = syscall.Errno(0xc0370110) diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/create.go b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/create.go index 6c9640f878..16821a1861 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/create.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/create.go @@ -45,7 +45,7 @@ type CreateOptions struct { HostingSystem *uvm.UtilityVM // Utility or service VM in which the container is to be created. NetworkNamespace string // Host network namespace to use (overrides anything in the spec) - // This is an advanced debugging parameter. It allows for diagnosibility by leaving a containers + // This is an advanced debugging parameter. It allows for diagnosability by leaving a containers // resources allocated in case of a failure. Thus you would be able to use tools such as hcsdiag // to look at the state of a utility VM to see what resources were allocated. Obviously the caller // must a) not tear down the utility VM on failure (or pause in some way) and b) is responsible for @@ -170,6 +170,15 @@ func validateContainerConfig(ctx context.Context, coi *createOptionsInternal) er return fmt.Errorf("user specified mounts are not permitted for template containers") } } + + // check if gMSA is disabled + if coi.Spec.Windows != nil { + disableGMSA := oci.ParseAnnotationsDisableGMSA(ctx, coi.Spec) + if _, ok := coi.Spec.Windows.CredentialSpec.(string); ok && disableGMSA { + return fmt.Errorf("gMSA credentials are disabled: %w", hcs.ErrOperationDenied) + } + } + return nil } diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/oci/annotations.go b/test/vendor/github.com/Microsoft/hcsshim/internal/oci/annotations.go new file mode 100644 index 0000000000..d7274c0362 --- /dev/null +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/oci/annotations.go @@ -0,0 +1,150 @@ +package oci + +import ( + "context" + "errors" + "strconv" + "strings" + + "github.com/Microsoft/hcsshim/internal/log" + "github.com/Microsoft/hcsshim/internal/logfields" + "github.com/Microsoft/hcsshim/pkg/annotations" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/sirupsen/logrus" +) + +var ErrAnnotationExpansionConflict = errors.New("annotation expansion conflict") + +// ProcessAnnotations expands annotations into their corresponding annotation groups +func ProcessAnnotations(ctx context.Context, s *specs.Spec) (err error) { + // Named `Process` and not `Expand` since this function may be expanded (pun intended) to + // deal with other annotation issues and validation. + + // Rathen than give up part of the way through on error, this just emits a warning (similar + // to the `parseAnnotation*` functions) and continues through, so the spec is not left in a + // (partially) unusable form. + // If multiple different errors are to be raised, they should be combined or, if they + // are logged, only the last kept, depending on their severity. + + // expand annotations + for key, exps := range annotations.AnnotationExpansions { + // check if annotation is present + if val, ok := s.Annotations[key]; ok { + // ideally, some normalization would occur here (ie, "True" -> "true") + // but strings may be case-sensitive + for _, k := range exps { + if v, ok := s.Annotations[k]; ok && val != v { + err = ErrAnnotationExpansionConflict + log.G(ctx).WithFields(logrus.Fields{ + logfields.OCIAnnotation: key, + logfields.Value: val, + logfields.OCIAnnotation + "-conflict": k, + logfields.Value + "-conflict": v, + }).WithError(err).Warning("annotation expansion would overwrite conflicting value") + continue + } + s.Annotations[k] = val + } + } + } + + return err +} + +// handle specific annotations + +// ParseAnnotationsDisableGMSA searches for the boolean value which specifies +// if providing a gMSA credential should be disallowed. Returns the value found, +// if parsable, otherwise returns false otherwise. +func ParseAnnotationsDisableGMSA(ctx context.Context, s *specs.Spec) bool { + return parseAnnotationsBool(ctx, s.Annotations, annotations.WCOWDisableGMSA, false) +} + +// ParseAnnotationsSaveAsTemplate searches for the boolean value which specifies +// if this create request should be considered as a template creation request. If value +// is found the returns the actual value, returns false otherwise. +func ParseAnnotationsSaveAsTemplate(ctx context.Context, s *specs.Spec) bool { + return parseAnnotationsBool(ctx, s.Annotations, annotations.SaveAsTemplate, false) +} + +// ParseAnnotationsTemplateID searches for the templateID in the create request. If the +// value is found then returns the value otherwise returns the empty string. +func ParseAnnotationsTemplateID(ctx context.Context, s *specs.Spec) string { + return parseAnnotationsString(s.Annotations, annotations.TemplateID, "") +} + +// general annotation parsing + +// parseAnnotationsBool searches `a` for `key` and if found verifies that the +// value is `true` or `false` in any case. If `key` is not found returns `def`. +func parseAnnotationsBool(ctx context.Context, a map[string]string, key string, def bool) bool { + if v, ok := a[key]; ok { + switch strings.ToLower(v) { + case "true": + return true + case "false": + return false + default: + logAnnotationParseError(ctx, key, v, logfields.Bool, nil) + } + } + return def +} + +// parseAnnotationsUint32 searches `a` for `key` and if found verifies that the +// value is a 32 bit unsigned integer. If `key` is not found returns `def`. +func parseAnnotationsUint32(ctx context.Context, a map[string]string, key string, def uint32) uint32 { + if v, ok := a[key]; ok { + countu, err := strconv.ParseUint(v, 10, 32) + if err == nil { + v := uint32(countu) + return v + } + logAnnotationParseError(ctx, key, v, logfields.Uint32, err) + } + return def +} + +// parseAnnotationsUint64 searches `a` for `key` and if found verifies that the +// value is a 64 bit unsigned integer. If `key` is not found returns `def`. +func parseAnnotationsUint64(ctx context.Context, a map[string]string, key string, def uint64) uint64 { + if v, ok := a[key]; ok { + countu, err := strconv.ParseUint(v, 10, 64) + if err == nil { + return countu + } + logAnnotationParseError(ctx, key, v, logfields.Uint64, err) + } + return def +} + +// parseAnnotationsString searches `a` for `key`. If `key` is not found returns `def`. +func parseAnnotationsString(a map[string]string, key string, def string) string { + if v, ok := a[key]; ok { + return v + } + return def +} + +// ParseAnnotationCommaSeparated searches `annotations` for `annotation` corresponding to a +// list of comma separated strings +func ParseAnnotationCommaSeparated(annotation string, annotations map[string]string) []string { + cs, ok := annotations[annotation] + if !ok || cs == "" { + return nil + } + results := strings.Split(cs, ",") + return results +} + +func logAnnotationParseError(ctx context.Context, k, v, et string, err error) { + entry := log.G(ctx).WithFields(logrus.Fields{ + logfields.OCIAnnotation: k, + logfields.Value: v, + logfields.ExpectedType: et, + }) + if err != nil { + entry = entry.WithError(err) + } + entry.Warning("annotation could not be parsed") +} diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/oci/uvm.go b/test/vendor/github.com/Microsoft/hcsshim/internal/oci/uvm.go index 97b156ba5b..b0876465f0 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/oci/uvm.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/oci/uvm.go @@ -5,48 +5,17 @@ import ( "errors" "fmt" "strconv" - "strings" runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" "github.com/Microsoft/hcsshim/internal/clone" "github.com/Microsoft/hcsshim/internal/log" - "github.com/Microsoft/hcsshim/internal/logfields" "github.com/Microsoft/hcsshim/internal/uvm" "github.com/Microsoft/hcsshim/pkg/annotations" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) -// parseAnnotationsBool searches `a` for `key` and if found verifies that the -// value is `true` or `false` in any case. If `key` is not found returns `def`. -func parseAnnotationsBool(ctx context.Context, a map[string]string, key string, def bool) bool { - if v, ok := a[key]; ok { - switch strings.ToLower(v) { - case "true": - return true - case "false": - return false - default: - log.G(ctx).WithFields(logrus.Fields{ - logfields.OCIAnnotation: key, - logfields.Value: v, - logfields.ExpectedType: logfields.Bool, - }).Warning("annotation could not be parsed") - } - } - return def -} - -// ParseAnnotationCommaSeparated searches `annotations` for `annotation` corresponding to a -// list of comma separated strings -func ParseAnnotationCommaSeparated(annotation string, annotations map[string]string) []string { - cs, ok := annotations[annotation] - if !ok || cs == "" { - return nil - } - results := strings.Split(cs, ",") - return results -} +// UVM specific annotation parsing // ParseAnnotationsCPUCount searches `s.Annotations` for the CPU annotation. If // not found searches `s` for the Windows CPU section. If neither are found @@ -171,64 +140,6 @@ func parseAnnotationsPreferredRootFSType(ctx context.Context, a map[string]strin return def } -// parseAnnotationsUint32 searches `a` for `key` and if found verifies that the -// value is a 32 bit unsigned integer. If `key` is not found returns `def`. -func parseAnnotationsUint32(ctx context.Context, a map[string]string, key string, def uint32) uint32 { - if v, ok := a[key]; ok { - countu, err := strconv.ParseUint(v, 10, 32) - if err == nil { - v := uint32(countu) - return v - } - log.G(ctx).WithFields(logrus.Fields{ - logfields.OCIAnnotation: key, - logfields.Value: v, - logfields.ExpectedType: logfields.Uint32, - logrus.ErrorKey: err, - }).Warning("annotation could not be parsed") - } - return def -} - -// parseAnnotationsUint64 searches `a` for `key` and if found verifies that the -// value is a 64 bit unsigned integer. If `key` is not found returns `def`. -func parseAnnotationsUint64(ctx context.Context, a map[string]string, key string, def uint64) uint64 { - if v, ok := a[key]; ok { - countu, err := strconv.ParseUint(v, 10, 64) - if err == nil { - return countu - } - log.G(ctx).WithFields(logrus.Fields{ - logfields.OCIAnnotation: key, - logfields.Value: v, - logfields.ExpectedType: logfields.Uint64, - logrus.ErrorKey: err, - }).Warning("annotation could not be parsed") - } - return def -} - -// parseAnnotationsString searches `a` for `key`. If `key` is not found returns `def`. -func parseAnnotationsString(a map[string]string, key string, def string) string { - if v, ok := a[key]; ok { - return v - } - return def -} - -// ParseAnnotationsSaveAsTemplate searches for the boolean value which specifies -// if this create request should be considered as a template creation request. If value -// is found the returns the actual value, returns false otherwise. -func ParseAnnotationsSaveAsTemplate(ctx context.Context, s *specs.Spec) bool { - return parseAnnotationsBool(ctx, s.Annotations, annotations.SaveAsTemplate, false) -} - -// ParseAnnotationsTemplateID searches for the templateID in the create request. If the -// value is found then returns the value otherwise returns the empty string. -func ParseAnnotationsTemplateID(ctx context.Context, s *specs.Spec) string { - return parseAnnotationsString(s.Annotations, annotations.TemplateID, "") -} - func ParseCloneAnnotations(ctx context.Context, s *specs.Spec) (isTemplate bool, templateID string, err error) { templateID = ParseAnnotationsTemplateID(ctx, s) isTemplate = ParseAnnotationsSaveAsTemplate(ctx, s) @@ -327,6 +238,25 @@ func handleSecurityPolicy(ctx context.Context, a map[string]string, lopts *uvm.O } } +// sets options common to both WCOW and LCOW from annotations +func specToUVMCreateOptionsCommon(ctx context.Context, opts *uvm.Options, s *specs.Spec) { + opts.MemorySizeInMB = ParseAnnotationsMemory(ctx, s, annotations.MemorySizeInMB, opts.MemorySizeInMB) + opts.LowMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryLowMMIOGapInMB, opts.LowMMIOGapInMB) + opts.HighMMIOBaseInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOBaseInMB, opts.HighMMIOBaseInMB) + opts.HighMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOGapInMB, opts.HighMMIOGapInMB) + opts.AllowOvercommit = parseAnnotationsBool(ctx, s.Annotations, annotations.AllowOvercommit, opts.AllowOvercommit) + opts.EnableDeferredCommit = parseAnnotationsBool(ctx, s.Annotations, annotations.EnableDeferredCommit, opts.EnableDeferredCommit) + opts.ProcessorCount = ParseAnnotationsCPUCount(ctx, s, annotations.ProcessorCount, opts.ProcessorCount) + opts.ProcessorLimit = ParseAnnotationsCPULimit(ctx, s, annotations.ProcessorLimit, opts.ProcessorLimit) + opts.ProcessorWeight = ParseAnnotationsCPUWeight(ctx, s, annotations.ProcessorWeight, opts.ProcessorWeight) + opts.StorageQoSBandwidthMaximum = ParseAnnotationsStorageBps(ctx, s, annotations.StorageQoSBandwidthMaximum, opts.StorageQoSBandwidthMaximum) + opts.StorageQoSIopsMaximum = ParseAnnotationsStorageIops(ctx, s, annotations.StorageQoSIopsMaximum, opts.StorageQoSIopsMaximum) + opts.CPUGroupID = parseAnnotationsString(s.Annotations, annotations.CPUGroupID, opts.CPUGroupID) + opts.NetworkConfigProxy = parseAnnotationsString(s.Annotations, annotations.NetworkConfigProxy, opts.NetworkConfigProxy) + opts.ProcessDumpLocation = parseAnnotationsString(s.Annotations, annotations.ContainerProcessDumpLocation, opts.ProcessDumpLocation) + opts.NoWritableFileShares = parseAnnotationsBool(ctx, s.Annotations, annotations.DisableWritableFileShares, opts.NoWritableFileShares) +} + // SpecToUVMCreateOpts parses `s` and returns either `*uvm.OptionsLCOW` or // `*uvm.OptionsWCOW`. func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) (interface{}, error) { @@ -335,29 +265,17 @@ func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) ( } if IsLCOW(s) { lopts := uvm.NewDefaultOptionsLCOW(id, owner) - lopts.MemorySizeInMB = ParseAnnotationsMemory(ctx, s, annotations.MemorySizeInMB, lopts.MemorySizeInMB) - lopts.LowMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryLowMMIOGapInMB, lopts.LowMMIOGapInMB) - lopts.HighMMIOBaseInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOBaseInMB, lopts.HighMMIOBaseInMB) - lopts.HighMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOGapInMB, lopts.HighMMIOGapInMB) - lopts.AllowOvercommit = parseAnnotationsBool(ctx, s.Annotations, annotations.AllowOvercommit, lopts.AllowOvercommit) - lopts.EnableDeferredCommit = parseAnnotationsBool(ctx, s.Annotations, annotations.EnableDeferredCommit, lopts.EnableDeferredCommit) + specToUVMCreateOptionsCommon(ctx, lopts.Options, s) + lopts.EnableColdDiscardHint = parseAnnotationsBool(ctx, s.Annotations, annotations.EnableColdDiscardHint, lopts.EnableColdDiscardHint) - lopts.ProcessorCount = ParseAnnotationsCPUCount(ctx, s, annotations.ProcessorCount, lopts.ProcessorCount) - lopts.ProcessorLimit = ParseAnnotationsCPULimit(ctx, s, annotations.ProcessorLimit, lopts.ProcessorLimit) - lopts.ProcessorWeight = ParseAnnotationsCPUWeight(ctx, s, annotations.ProcessorWeight, lopts.ProcessorWeight) lopts.VPMemDeviceCount = parseAnnotationsUint32(ctx, s.Annotations, annotations.VPMemCount, lopts.VPMemDeviceCount) lopts.VPMemSizeBytes = parseAnnotationsUint64(ctx, s.Annotations, annotations.VPMemSize, lopts.VPMemSizeBytes) lopts.VPMemNoMultiMapping = parseAnnotationsBool(ctx, s.Annotations, annotations.VPMemNoMultiMapping, lopts.VPMemNoMultiMapping) - lopts.StorageQoSBandwidthMaximum = ParseAnnotationsStorageBps(ctx, s, annotations.StorageQoSBandwidthMaximum, lopts.StorageQoSBandwidthMaximum) - lopts.StorageQoSIopsMaximum = ParseAnnotationsStorageIops(ctx, s, annotations.StorageQoSIopsMaximum, lopts.StorageQoSIopsMaximum) lopts.VPCIEnabled = parseAnnotationsBool(ctx, s.Annotations, annotations.VPCIEnabled, lopts.VPCIEnabled) lopts.BootFilesPath = parseAnnotationsString(s.Annotations, annotations.BootFilesRootPath, lopts.BootFilesPath) - lopts.CPUGroupID = parseAnnotationsString(s.Annotations, annotations.CPUGroupID, lopts.CPUGroupID) - lopts.NetworkConfigProxy = parseAnnotationsString(s.Annotations, annotations.NetworkConfigProxy, lopts.NetworkConfigProxy) lopts.EnableScratchEncryption = parseAnnotationsBool(ctx, s.Annotations, annotations.EncryptedScratchDisk, lopts.EnableScratchEncryption) lopts.SecurityPolicy = parseAnnotationsString(s.Annotations, annotations.SecurityPolicy, lopts.SecurityPolicy) lopts.KernelBootOptions = parseAnnotationsString(s.Annotations, annotations.KernelBootOptions, lopts.KernelBootOptions) - lopts.ProcessDumpLocation = parseAnnotationsString(s.Annotations, annotations.ContainerProcessDumpLocation, lopts.ProcessDumpLocation) lopts.DisableTimeSyncService = parseAnnotationsBool(ctx, s.Annotations, annotations.DisableLCOWTimeSyncService, lopts.DisableTimeSyncService) handleAnnotationPreferredRootFSType(ctx, s.Annotations, lopts) handleAnnotationKernelDirectBoot(ctx, s.Annotations, lopts) @@ -375,22 +293,10 @@ func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) ( return lopts, nil } else if IsWCOW(s) { wopts := uvm.NewDefaultOptionsWCOW(id, owner) - wopts.MemorySizeInMB = ParseAnnotationsMemory(ctx, s, annotations.MemorySizeInMB, wopts.MemorySizeInMB) - wopts.LowMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryLowMMIOGapInMB, wopts.LowMMIOGapInMB) - wopts.HighMMIOBaseInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOBaseInMB, wopts.HighMMIOBaseInMB) - wopts.HighMMIOGapInMB = parseAnnotationsUint64(ctx, s.Annotations, annotations.MemoryHighMMIOGapInMB, wopts.HighMMIOGapInMB) - wopts.AllowOvercommit = parseAnnotationsBool(ctx, s.Annotations, annotations.AllowOvercommit, wopts.AllowOvercommit) - wopts.EnableDeferredCommit = parseAnnotationsBool(ctx, s.Annotations, annotations.EnableDeferredCommit, wopts.EnableDeferredCommit) - wopts.ProcessorCount = ParseAnnotationsCPUCount(ctx, s, annotations.ProcessorCount, wopts.ProcessorCount) - wopts.ProcessorLimit = ParseAnnotationsCPULimit(ctx, s, annotations.ProcessorLimit, wopts.ProcessorLimit) - wopts.ProcessorWeight = ParseAnnotationsCPUWeight(ctx, s, annotations.ProcessorWeight, wopts.ProcessorWeight) - wopts.StorageQoSBandwidthMaximum = ParseAnnotationsStorageBps(ctx, s, annotations.StorageQoSBandwidthMaximum, wopts.StorageQoSBandwidthMaximum) - wopts.StorageQoSIopsMaximum = ParseAnnotationsStorageIops(ctx, s, annotations.StorageQoSIopsMaximum, wopts.StorageQoSIopsMaximum) + specToUVMCreateOptionsCommon(ctx, wopts.Options, s) + wopts.DisableCompartmentNamespace = parseAnnotationsBool(ctx, s.Annotations, annotations.DisableCompartmentNamespace, wopts.DisableCompartmentNamespace) - wopts.CPUGroupID = parseAnnotationsString(s.Annotations, annotations.CPUGroupID, wopts.CPUGroupID) - wopts.NetworkConfigProxy = parseAnnotationsString(s.Annotations, annotations.NetworkConfigProxy, wopts.NetworkConfigProxy) wopts.NoDirectMap = parseAnnotationsBool(ctx, s.Annotations, annotations.VSMBNoDirectMap, wopts.NoDirectMap) - wopts.ProcessDumpLocation = parseAnnotationsString(s.Annotations, annotations.ContainerProcessDumpLocation, wopts.ProcessDumpLocation) wopts.NoInheritHostTimezone = parseAnnotationsBool(ctx, s.Annotations, annotations.NoInheritHostTimezone, wopts.NoInheritHostTimezone) handleAnnotationFullyPhysicallyBacked(ctx, s.Annotations, wopts) if err := handleCloneAnnotations(ctx, s.Annotations, wopts); err != nil { @@ -434,5 +340,6 @@ func UpdateSpecFromOptions(s specs.Spec, opts *runhcsopts.Options) specs.Spec { s.Annotations[key] = value } } + return s } diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create.go index 8f293f67fe..a3d9fcd4e6 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create.go @@ -91,6 +91,9 @@ type Options struct { // applied to all containers. On Windows it's configurable per container, but we can mimic this for // Windows by just applying the location specified here per container. ProcessDumpLocation string + + // NoWritableFileShares disables adding any writable vSMB and Plan9 shares to the UVM + NoWritableFileShares bool } // compares the create opts used during template creation with the create opts @@ -184,6 +187,7 @@ func newDefaultOptions(id, owner string) *Options { EnableDeferredCommit: false, ProcessorCount: defaultProcessorCount(), FullyPhysicallyBacked: false, + NoWritableFileShares: false, } if opts.Owner == "" { @@ -382,6 +386,10 @@ func (uvm *UtilityVM) VSMBNoDirectMap() bool { return uvm.vsmbNoDirectMap } +func (uvm *UtilityVM) NoWritableFileShares() bool { + return uvm.noWritableFileShares +} + // Closes the external GCS connection if it is being used and also closes the // listener for GCS connection. func (uvm *UtilityVM) CloseGCSConnection() (err error) { diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_lcow.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_lcow.go index 5a5628230d..629d080e59 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_lcow.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_lcow.go @@ -729,6 +729,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error createOpts: opts, vpmemMultiMapping: !opts.VPMemNoMultiMapping, encryptScratch: opts.EnableScratchEncryption, + noWritableFileShares: opts.NoWritableFileShares, } defer func() { diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_wcow.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_wcow.go index c09b49d9f0..1d083ffda2 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_wcow.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_wcow.go @@ -257,6 +257,7 @@ func CreateWCOW(ctx context.Context, opts *OptionsWCOW) (_ *UtilityVM, err error physicallyBacked: !opts.AllowOvercommit, devicesPhysicallyBacked: opts.FullyPhysicallyBacked, vsmbNoDirectMap: opts.NoDirectMap, + noWritableFileShares: opts.NoWritableFileShares, createOpts: *opts, } diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/plan9.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/plan9.go index 3b86b392ae..475a9dbc1d 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/plan9.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/plan9.go @@ -6,6 +6,7 @@ import ( "fmt" "strconv" + "github.com/Microsoft/hcsshim/internal/hcs" "github.com/Microsoft/hcsshim/internal/hcs/resourcepaths" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" @@ -41,6 +42,9 @@ func (uvm *UtilityVM) AddPlan9(ctx context.Context, hostPath string, uvmPath str if uvmPath == "" { return nil, fmt.Errorf("uvmPath must be passed to AddPlan9") } + if !readOnly && uvm.NoWritableFileShares() { + return nil, fmt.Errorf("adding writable shares is denied: %w", hcs.ErrOperationDenied) + } // TODO: JTERRY75 - These are marked private in the schema. For now use them // but when there are public variants we need to switch to them. diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/scsi.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/scsi.go index 216da8dc43..5cac727387 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/scsi.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/scsi.go @@ -630,7 +630,7 @@ func (sm *SCSIMount) GobDecode(data []byte) error { // Clone function creates a clone of the SCSIMount `sm` and adds the cloned SCSIMount to // the uvm `vm`. If `sm` is read only then it is simply added to the `vm`. But if it is a -// writeable mount(e.g a scratch layer) then a copy of it is made and that copy is added +// writable mount(e.g a scratch layer) then a copy of it is made and that copy is added // to the `vm`. func (sm *SCSIMount) Clone(ctx context.Context, vm *UtilityVM, cd *cloneData) error { var ( @@ -642,7 +642,7 @@ func (sm *SCSIMount) Clone(ctx context.Context, vm *UtilityVM, cd *cloneData) er ) if !sm.readOnly { - // This is a writeable SCSI mount. It must be either the + // This is a writable SCSI mount. It must be either the // 1. scratch VHD of the UVM or // 2. scratch VHD of the container. // A user provided writable SCSI mount is not allowed on the template UVM diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/types.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/types.go index 065f61c0ba..b87fe45f7f 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/types.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/types.go @@ -60,6 +60,13 @@ type UtilityVM struct { // NOTE: All accesses to this MUST be done atomically. containerCounter uint64 + // noWritableFileShares disables mounting any writable vSMB or Plan9 shares + // on the uVM. This prevents containers in the uVM modifying files and directories + // made available via the "mounts" options in the container spec, or shared + // to the uVM directly. + // This option does not prevent writable SCSI mounts. + noWritableFileShares bool + // VSMB shares that are mapped into a Windows UVM. These are used for read-only // layers and mapped directories. // We maintain two sets of maps, `vsmbDirShares` tracks shares that are diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/vsmb.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/vsmb.go index b1e5f24434..b2ccf5be4f 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/vsmb.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/vsmb.go @@ -13,6 +13,7 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/sys/windows" + "github.com/Microsoft/hcsshim/internal/hcs" "github.com/Microsoft/hcsshim/internal/hcs/resourcepaths" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" @@ -163,6 +164,10 @@ func (uvm *UtilityVM) AddVSMB(ctx context.Context, hostPath string, options *hcs return nil, errNotSupported } + if !options.ReadOnly && uvm.NoWritableFileShares() { + return nil, fmt.Errorf("adding writable shares is denied: %w", hcs.ErrOperationDenied) + } + uvm.m.Lock() defer uvm.m.Unlock() diff --git a/test/vendor/github.com/Microsoft/hcsshim/pkg/annotations/annotations.go b/test/vendor/github.com/Microsoft/hcsshim/pkg/annotations/annotations.go index 88b6d810bb..7a51b2a0e4 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/pkg/annotations/annotations.go +++ b/test/vendor/github.com/Microsoft/hcsshim/pkg/annotations/annotations.go @@ -192,6 +192,9 @@ const ( // VSMBNoDirectMap specifies that no direct mapping should be used for any VSMBs added to the UVM VSMBNoDirectMap = "io.microsoft.virtualmachine.wcow.virtualSMB.nodirectmap" + // DisableWritableFileShares disables adding any writable fileshares to the UVM + DisableWritableFileShares = "io.microsoft.virtualmachine.fileshares.disablewritable" + // CPUGroupID specifies the cpugroup ID that a UVM should be assigned to if any CPUGroupID = "io.microsoft.virtualmachine.cpugroup.id" @@ -265,4 +268,25 @@ const ( // NoInheritHostTimezone specifies for the hosts timezone to not be inherited by the WCOW UVM. The UVM will be set to UTC time // as a default. NoInheritHostTimezone = "io.microsoft.virtualmachine.wcow.timezone.noinherit" + + // WCOWDisableGMSA disables providing gMSA (Group Managed Service Accounts) to + // a WCOW container + WCOWDisableGMSA = "io.microsoft.container.wcow.gmsa.disable" + + // DisableUnsafeOperations disables several unsafe operations, such as writable + // file share mounts, for hostile multi-tenant environments. See `AnnotationExpansions` + // for more information + DisableUnsafeOperations = "io.microsoft.disable-unsafe-operations" ) + +// AnnotationExpansions maps annotations that will be expanded into an array of +// other annotations. The expanded annotations will have the same value as the +// original. It is an error for the expansions to already exist and have a value +// that differs from the original. +var AnnotationExpansions = map[string][]string{ + DisableUnsafeOperations: { + WCOWDisableGMSA, + DisableWritableFileShares, + VSMBNoDirectMap, + }, +}