From d2fa14f14aa031c41a5d371f0baa188622f924f5 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Fri, 27 Mar 2020 14:00:33 -0700 Subject: [PATCH] Add GMSA support for V2 process isolated containers * Add generated V2 schema files for Container Credential Guard * Add new hcs calls that are necessary to setup container credential guard instances. * Add new resource type CCGInstance that implements ResourceCloser so a containers ccg instance will be cleaned up on container close. * Add tests to validate gmsa * Remove logging from resource Release methods and just return an error. Forego returning immediately on an error in ReleaseResources and return afterwards if any of the releases failed. Signed-off-by: Daniel Canter --- go.sum | 1 + internal/hcs/service.go | 49 ++++++ internal/hcs/system.go | 3 +- internal/hcsoci/create.go | 1 + internal/hcsoci/credentials.go | 131 ++++++++++++++++ internal/hcsoci/hcsdoc_wcow.go | 7 +- internal/hcsoci/resources.go | 20 ++- internal/hcsoci/resources_wcow.go | 13 ++ ...r_credential_guard_add_instance_request.go | 16 ++ ...edential_guard_hv_socket_service_config.go | 15 ++ .../container_credential_guard_instance.go | 16 ++ ...ainer_credential_guard_modify_operation.go | 17 ++ ...iner_credential_guard_operation_request.go | 15 ++ ...redential_guard_remove_instance_request.go | 14 ++ .../container_credential_guard_system_info.go | 14 ++ internal/schema2/modification_request.go | 15 ++ internal/schema2/property_type.go | 1 + internal/schema2/service_properties.go | 18 +++ internal/uvm/pipes.go | 4 +- internal/uvm/plan9.go | 4 +- internal/uvm/scsi.go | 3 +- internal/uvm/virtual_device.go | 4 +- internal/uvm/vsmb.go | 4 +- internal/vmcompute/vmcompute.go | 22 +++ internal/vmcompute/zsyscall_windows.go | 24 +++ test/cri-containerd/container_test.go | 145 ++++++++++++++++++ test/cri-containerd/gmsa.go | 48 ++++++ test/cri-containerd/main.go | 6 + 28 files changed, 610 insertions(+), 20 deletions(-) create mode 100644 internal/hcs/service.go create mode 100644 internal/hcsoci/credentials.go create mode 100644 internal/schema2/container_credential_guard_add_instance_request.go create mode 100644 internal/schema2/container_credential_guard_hv_socket_service_config.go create mode 100644 internal/schema2/container_credential_guard_instance.go create mode 100644 internal/schema2/container_credential_guard_modify_operation.go create mode 100644 internal/schema2/container_credential_guard_operation_request.go create mode 100644 internal/schema2/container_credential_guard_remove_instance_request.go create mode 100644 internal/schema2/container_credential_guard_system_info.go create mode 100644 internal/schema2/modification_request.go create mode 100644 internal/schema2/service_properties.go create mode 100644 test/cri-containerd/gmsa.go diff --git a/go.sum b/go.sum index 8ab4318ed5..5d3ac507ce 100644 --- a/go.sum +++ b/go.sum @@ -112,6 +112,7 @@ golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190514135907-3a4b5fb9f71f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3 h1:7TYNF4UdlohbFwpNH04CoPMp1cHUZgO1Ebq5r2hIjfo= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20uW+C3Rm0FD/WLDX8884= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= diff --git a/internal/hcs/service.go b/internal/hcs/service.go new file mode 100644 index 0000000000..3a5f012501 --- /dev/null +++ b/internal/hcs/service.go @@ -0,0 +1,49 @@ +package hcs + +import ( + "context" + "encoding/json" + + hcsschema "github.com/Microsoft/hcsshim/internal/schema2" + "github.com/Microsoft/hcsshim/internal/vmcompute" +) + +// GetServiceProperties returns properties of the host compute service. +func GetServiceProperties(ctx context.Context, q hcsschema.PropertyQuery) (*hcsschema.ServiceProperties, error) { + operation := "hcsshim::GetServiceProperties" + + queryb, err := json.Marshal(q) + if err != nil { + return nil, err + } + propertiesJSON, resultJSON, err := vmcompute.HcsGetServiceProperties(ctx, string(queryb)) + events := processHcsResult(ctx, resultJSON) + if err != nil { + return nil, &HcsError{Op: operation, Err: err, Events: events} + } + + if propertiesJSON == "" { + return nil, ErrUnexpectedValue + } + properties := &hcsschema.ServiceProperties{} + if err := json.Unmarshal([]byte(propertiesJSON), properties); err != nil { + return nil, err + } + return properties, nil +} + +// ModifyServiceSettings modifies settings of the host compute service. +func ModifyServiceSettings(ctx context.Context, settings hcsschema.ModificationRequest) error { + operation := "hcsshim::ModifyServiceSettings" + + settingsJSON, err := json.Marshal(settings) + if err != nil { + return err + } + resultJSON, err := vmcompute.HcsModifyServiceSettings(ctx, string(settingsJSON)) + events := processHcsResult(ctx, resultJSON) + if err != nil { + return &HcsError{Op: operation, Err: err, Events: events} + } + return nil +} diff --git a/internal/hcs/system.go b/internal/hcs/system.go index 67a5f7176f..6120399c47 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -28,8 +28,7 @@ type System struct { waitBlock chan struct{} waitError error exitError error - - os, typ string + os, typ string } func newSystem(id string) *System { diff --git a/internal/hcsoci/create.go b/internal/hcsoci/create.go index f391d58823..193410ccb7 100644 --- a/internal/hcsoci/create.go +++ b/internal/hcsoci/create.go @@ -59,6 +59,7 @@ type createOptionsInternal struct { actualID string // Identifier for the container actualOwner string // Owner for the container actualNetworkNamespace string + ccgState *hcsschema.ContainerCredentialGuardState // Container Credential Guard information to be attached to HCS container document } // CreateContainer creates a container. It can cope with a wide variety of diff --git a/internal/hcsoci/credentials.go b/internal/hcsoci/credentials.go new file mode 100644 index 0000000000..5226271c45 --- /dev/null +++ b/internal/hcsoci/credentials.go @@ -0,0 +1,131 @@ +// +build windows + +package hcsoci + +import ( + "context" + "encoding/json" + "errors" + "fmt" + + "github.com/Microsoft/hcsshim/internal/hcs" + "github.com/Microsoft/hcsshim/internal/log" + hcsschema "github.com/Microsoft/hcsshim/internal/schema2" +) + +// This file holds the necessary structs and functions for adding and removing Container +// Credential Guard instances (shortened to CCG normally) for V2 HCS schema +// containers. Container Credential Guard is in HCS's own words "The solution to +// allowing windows containers to have access to domain credentials for the +// applications running in their corresponding guest." It essentially acts as +// a way to temporarily Active Directory join a given container with a Group +// Managed Service Account (GMSA for short) credential specification. +// CCG will launch a process in the host that will act as a middleman for the +// credential passthrough logic. The guest is then configured through registry +// keys to have access to the process in the host. +// A CCG instance needs to be created through various HCS calls and then added to +// the V2 schema container document before being sent to HCS. For V1 HCS schema containers +// setting up instances manually is not needed, the GMSA credential specification +// simply needs to be present in the V1 container document. + +// CCGInstance stores the id used when creating a ccg instance. Used when +// closing a container to be able to release the instance. +type CCGInstance struct { + // ID of container that instance belongs to. + id string +} + +// Release calls into hcs to remove the ccg instance. These do not get cleaned up automatically +// they MUST be explicitly removed with a call to ModifyServiceSettings. The instances will persist +// unless vmcompute.exe exits or they are removed manually as done here. +func (instance *CCGInstance) Release(ctx context.Context) error { + if err := removeCredentialGuard(ctx, instance.id); err != nil { + return fmt.Errorf("failed to remove container credential guard instance: %s", err) + } + return nil +} + +// CreateCredentialGuard creates a container credential guard instance and +// returns the state object to be placed in a v2 container doc. +func CreateCredentialGuard(ctx context.Context, id, credSpec string, hypervisorIsolated bool) (*hcsschema.ContainerCredentialGuardState, *CCGInstance, error) { + log.G(ctx).WithField("containerID", id).Debug("creating container credential guard instance") + // V2 schema ccg setup a little different as its expected to be passed + // through all the way to the gcs. Can no longer be enabled just through + // a single property. The flow is as follows + // ------------------------------------------------------------------------ + // 1. Call HcsModifyServiceSettings with a ModificationRequest set with a + // ContainerCredentialGuardAddInstanceRequest. This is where the cred spec + // gets passed in. Transport either "LRPC" (Argon) or "HvSocket" (Xenon). + // 2. Query the instance with a call to HcsGetServiceProperties with the + // PropertyType "ContainerCredentialGuard". This will return all instances + // 3. Parse for the id of our container to find which one correlates to the + // container we're building the doc for, then add to the V2 doc. + // 4. If xenon container the hvsocketconfig will need to be in the UVMs V2 + // schema HcsComputeSystem document before being created/sent to HCS. It must + // be in the doc at creation time as we do not support hot adding hvsocket + // service table entries. + // This is currently a blocker for adding support for hyper-v gmsa. + transport := "LRPC" + if hypervisorIsolated { + // TODO(Dcantah) Set transport to HvSocket here when this is supported + return nil, nil, errors.New("hypervisor isolated containers with v2 HCS schema do not support GMSA") + } + req := hcsschema.ModificationRequest{ + PropertyType: hcsschema.PTContainerCredentialGuard, + Settings: &hcsschema.ContainerCredentialGuardOperationRequest{ + Operation: hcsschema.AddInstance, + OperationDetails: &hcsschema.ContainerCredentialGuardAddInstanceRequest{ + Id: id, + CredentialSpec: credSpec, + Transport: transport, + }, + }, + } + if err := hcs.ModifyServiceSettings(ctx, req); err != nil { + return nil, nil, fmt.Errorf("failed to generate container credential guard instance: %s", err) + } + + q := hcsschema.PropertyQuery{ + PropertyTypes: []hcsschema.PropertyType{hcsschema.PTContainerCredentialGuard}, + } + serviceProps, err := hcs.GetServiceProperties(ctx, q) + if err != nil { + return nil, nil, fmt.Errorf("failed to retrieve container credential guard instances: %s", err) + } + if len(serviceProps.Properties) != 1 { + return nil, nil, errors.New("wrong number of service properties present") + } + + ccgSysInfo := &hcsschema.ContainerCredentialGuardSystemInfo{} + if err := json.Unmarshal(serviceProps.Properties[0], ccgSysInfo); err != nil { + return nil, nil, fmt.Errorf("failed to unmarshal container credential guard instances: %s", err) + } + for _, ccgInstance := range ccgSysInfo.Instances { + if ccgInstance.Id == id { + instance := &CCGInstance{ + id, + } + return ccgInstance.CredentialGuard, instance, nil + } + } + return nil, nil, fmt.Errorf("failed to find credential guard instance with container ID %s", id) +} + +// Removes a ContainerCredentialGuard instance by container ID. +func removeCredentialGuard(ctx context.Context, id string) error { + log.G(ctx).WithField("containerID", id).Debug("removing container credential guard") + + req := hcsschema.ModificationRequest{ + PropertyType: hcsschema.PTContainerCredentialGuard, + Settings: &hcsschema.ContainerCredentialGuardOperationRequest{ + Operation: hcsschema.RemoveInstance, + OperationDetails: &hcsschema.ContainerCredentialGuardRemoveInstanceRequest{ + Id: id, + }, + }, + } + if err := hcs.ModifyServiceSettings(ctx, req); err != nil { + return err + } + return nil +} diff --git a/internal/hcsoci/hcsdoc_wcow.go b/internal/hcsoci/hcsdoc_wcow.go index 78e903504c..a7c94f66d9 100644 --- a/internal/hcsoci/hcsdoc_wcow.go +++ b/internal/hcsoci/hcsdoc_wcow.go @@ -165,9 +165,14 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter v2Container.Networking.NetworkSharedContainerName = v1.NetworkSharedContainerName } - // // TODO V2 Credentials not in the schema yet. if cs, ok := coi.Spec.Windows.CredentialSpec.(string); ok { v1.Credentials = cs + // If this is a HCS v2 schema container, we created the CCG instance + // with the other container resources. Pass the CCG state information + // as part of the container document. + if coi.ccgState != nil { + v2Container.ContainerCredentialGuard = coi.ccgState + } } if coi.Spec.Root == nil { diff --git a/internal/hcsoci/resources.go b/internal/hcsoci/resources.go index c34575dee3..08be62bbd0 100644 --- a/internal/hcsoci/resources.go +++ b/internal/hcsoci/resources.go @@ -2,6 +2,7 @@ package hcsoci import ( "context" + "errors" "os" "github.com/Microsoft/hcsshim/internal/log" @@ -76,29 +77,42 @@ func ReleaseResources(ctx context.Context, r *Resources, vm *uvm.UtilityVM, all } } + releaseErr := false // Release resources in reverse order so that the most recently - // added are cleaned up first. + // added are cleaned up first. We don't return an error right away + // so that other resources still get cleaned up in the case of one + // or more failing. for i := len(r.resources) - 1; i >= 0; i-- { switch r.resources[i].(type) { case *uvm.NetworkEndpoints: if r.createdNetNS { if err := r.resources[i].Release(ctx); err != nil { - return err + log.G(ctx).WithError(err).Error("failed to release container resource") + releaseErr = true } r.createdNetNS = false } + case *CCGInstance: + if err := r.resources[i].Release(ctx); err != nil { + log.G(ctx).WithError(err).Error("failed to release container resource") + releaseErr = true + } default: // Don't need to check if vm != nil here anymore as they wouldnt // have been added in the first place. All resources have embedded // vm they belong to. if all { if err := r.resources[i].Release(ctx); err != nil { - return err + log.G(ctx).WithError(err).Error("failed to release container resource") + releaseErr = true } } } } r.resources = nil + if releaseErr { + return errors.New("failed to release one or more container resources") + } // cleanup container state if vm != nil { diff --git a/internal/hcsoci/resources_wcow.go b/internal/hcsoci/resources_wcow.go index 9fa4670cb7..1afff1f420 100644 --- a/internal/hcsoci/resources_wcow.go +++ b/internal/hcsoci/resources_wcow.go @@ -138,5 +138,18 @@ func allocateWindowsResources(ctx context.Context, coi *createOptionsInternal, r } } + if cs, ok := coi.Spec.Windows.CredentialSpec.(string); ok { + // Only need to create a CCG instance for v2 containers + if schemaversion.IsV21(coi.actualSchemaVersion) { + hypervisorIsolated := coi.HostingSystem != nil + ccgState, ccgInstance, err := CreateCredentialGuard(ctx, coi.actualID, cs, hypervisorIsolated) + if err != nil { + return err + } + coi.ccgState = ccgState + r.resources = append(r.resources, ccgInstance) + //TODO dcantah: If/when dynamic service table entries is supported register the RpcEndpoint with hvsocket here + } + } return nil } diff --git a/internal/schema2/container_credential_guard_add_instance_request.go b/internal/schema2/container_credential_guard_add_instance_request.go new file mode 100644 index 0000000000..495c6ebc8f --- /dev/null +++ b/internal/schema2/container_credential_guard_add_instance_request.go @@ -0,0 +1,16 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +type ContainerCredentialGuardAddInstanceRequest struct { + Id string `json:"Id,omitempty"` + CredentialSpec string `json:"CredentialSpec,omitempty"` + Transport string `json:"Transport,omitempty"` +} diff --git a/internal/schema2/container_credential_guard_hv_socket_service_config.go b/internal/schema2/container_credential_guard_hv_socket_service_config.go new file mode 100644 index 0000000000..1ed4c008f2 --- /dev/null +++ b/internal/schema2/container_credential_guard_hv_socket_service_config.go @@ -0,0 +1,15 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +type ContainerCredentialGuardHvSocketServiceConfig struct { + ServiceId string `json:"ServiceId,omitempty"` + ServiceConfig *HvSocketServiceConfig `json:"ServiceConfig,omitempty"` +} diff --git a/internal/schema2/container_credential_guard_instance.go b/internal/schema2/container_credential_guard_instance.go new file mode 100644 index 0000000000..d7ebd0fcca --- /dev/null +++ b/internal/schema2/container_credential_guard_instance.go @@ -0,0 +1,16 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +type ContainerCredentialGuardInstance struct { + Id string `json:"Id,omitempty"` + CredentialGuard *ContainerCredentialGuardState `json:"CredentialGuard,omitempty"` + HvSocketConfig *ContainerCredentialGuardHvSocketServiceConfig `json:"HvSocketConfig,omitempty"` +} diff --git a/internal/schema2/container_credential_guard_modify_operation.go b/internal/schema2/container_credential_guard_modify_operation.go new file mode 100644 index 0000000000..71005b090b --- /dev/null +++ b/internal/schema2/container_credential_guard_modify_operation.go @@ -0,0 +1,17 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +type ContainerCredentialGuardModifyOperation string + +const ( + AddInstance ContainerCredentialGuardModifyOperation = "AddInstance" + RemoveInstance ContainerCredentialGuardModifyOperation = "RemoveInstance" +) diff --git a/internal/schema2/container_credential_guard_operation_request.go b/internal/schema2/container_credential_guard_operation_request.go new file mode 100644 index 0000000000..952cda4965 --- /dev/null +++ b/internal/schema2/container_credential_guard_operation_request.go @@ -0,0 +1,15 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +type ContainerCredentialGuardOperationRequest struct { + Operation ContainerCredentialGuardModifyOperation `json:"Operation,omitempty"` + OperationDetails interface{} `json:"OperationDetails,omitempty"` +} diff --git a/internal/schema2/container_credential_guard_remove_instance_request.go b/internal/schema2/container_credential_guard_remove_instance_request.go new file mode 100644 index 0000000000..32e5a3beed --- /dev/null +++ b/internal/schema2/container_credential_guard_remove_instance_request.go @@ -0,0 +1,14 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +type ContainerCredentialGuardRemoveInstanceRequest struct { + Id string `json:"Id,omitempty"` +} diff --git a/internal/schema2/container_credential_guard_system_info.go b/internal/schema2/container_credential_guard_system_info.go new file mode 100644 index 0000000000..ea306fa21a --- /dev/null +++ b/internal/schema2/container_credential_guard_system_info.go @@ -0,0 +1,14 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +type ContainerCredentialGuardSystemInfo struct { + Instances []ContainerCredentialGuardInstance `json:"Instances,omitempty"` +} diff --git a/internal/schema2/modification_request.go b/internal/schema2/modification_request.go new file mode 100644 index 0000000000..1384ed8882 --- /dev/null +++ b/internal/schema2/modification_request.go @@ -0,0 +1,15 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +type ModificationRequest struct { + PropertyType PropertyType `json:"PropertyType,omitempty"` + Settings interface{} `json:"Settings,omitempty"` +} diff --git a/internal/schema2/property_type.go b/internal/schema2/property_type.go index f092b737f4..638aa83c77 100644 --- a/internal/schema2/property_type.go +++ b/internal/schema2/property_type.go @@ -18,6 +18,7 @@ const ( PTProcessList PropertyType = "ProcessList" PTTerminateOnLastHandleClosed PropertyType = "TerminateOnLastHandleClosed" PTSharedMemoryRegion PropertyType = "SharedMemoryRegion" + PTContainerCredentialGuard PropertyType = "ContainerCredentialGuard" // This field is not generated by swagger. This was added manually. PTGuestConnection PropertyType = "GuestConnection" PTICHeartbeatStatus PropertyType = "ICHeartbeatStatus" ) diff --git a/internal/schema2/service_properties.go b/internal/schema2/service_properties.go new file mode 100644 index 0000000000..b8142ca6a6 --- /dev/null +++ b/internal/schema2/service_properties.go @@ -0,0 +1,18 @@ +/* + * HCS API + * + * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) + * + * API version: 2.4 + * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) + */ + +package hcsschema + +import "encoding/json" + +type ServiceProperties struct { + // Changed Properties field to []json.RawMessage from []interface{} to avoid having to + // remarshal sp.Properties[n] and unmarshal into the type(s) we want. + Properties []json.RawMessage `json:"Properties,omitempty"` +} diff --git a/internal/uvm/pipes.go b/internal/uvm/pipes.go index 8a2cf84b0e..bfb062e270 100644 --- a/internal/uvm/pipes.go +++ b/internal/uvm/pipes.go @@ -5,7 +5,6 @@ import ( "fmt" "strings" - "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/requesttype" hcsschema "github.com/Microsoft/hcsshim/internal/schema2" specs "github.com/opencontainers/runtime-spec/specs-go" @@ -23,8 +22,7 @@ type PipeMount struct { // Release frees the resources of the corresponding pipe Mount func (pipe *PipeMount) Release(ctx context.Context) error { if err := pipe.vm.RemovePipe(ctx, pipe.HostPath); err != nil { - log.G(ctx).WithError(err).Warn("failed to remove pipe mount") - return err + return fmt.Errorf("failed to remove pipe mount: %s", err) } return nil } diff --git a/internal/uvm/plan9.go b/internal/uvm/plan9.go index da36354495..684c173dc6 100644 --- a/internal/uvm/plan9.go +++ b/internal/uvm/plan9.go @@ -7,7 +7,6 @@ import ( "strconv" "github.com/Microsoft/hcsshim/internal/guestrequest" - "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/requesttype" hcsschema "github.com/Microsoft/hcsshim/internal/schema2" "github.com/Microsoft/hcsshim/osversion" @@ -23,8 +22,7 @@ type Plan9Share struct { // Release frees the resources of the corresponding Plan9 share func (p9 *Plan9Share) Release(ctx context.Context) error { if err := p9.vm.RemovePlan9(ctx, p9); err != nil { - log.G(ctx).WithError(err).Warn("failed to remove plan9 share") - return err + return fmt.Errorf("failed to remove plan9 share: %s", err) } return nil } diff --git a/internal/uvm/scsi.go b/internal/uvm/scsi.go index 05923fced9..2623e28e1f 100644 --- a/internal/uvm/scsi.go +++ b/internal/uvm/scsi.go @@ -44,8 +44,7 @@ var ( // Release frees the resources of the corresponding Scsi Mount func (sm *SCSIMount) Release(ctx context.Context) error { if err := sm.vm.RemoveSCSI(ctx, sm.HostPath); err != nil { - log.G(ctx).WithError(err).Warn("failed to remove scsi device") - return err + return fmt.Errorf("failed to remove SCSI device: %s", err) } return nil } diff --git a/internal/uvm/virtual_device.go b/internal/uvm/virtual_device.go index 847aacfe03..25dc4a186b 100644 --- a/internal/uvm/virtual_device.go +++ b/internal/uvm/virtual_device.go @@ -6,7 +6,6 @@ import ( "github.com/Microsoft/go-winio/pkg/guid" "github.com/Microsoft/hcsshim/internal/guestrequest" - "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/requesttype" hcsschema "github.com/Microsoft/hcsshim/internal/schema2" ) @@ -21,8 +20,7 @@ type VPCIDevice struct { // Release frees the resources of the corresponding vpci device func (vpci *VPCIDevice) Release(ctx context.Context) error { if err := vpci.vm.RemoveDevice(ctx, vpci.ID); err != nil { - log.G(ctx).WithError(err).Warn("failed to remove VPCI device") - return err + return fmt.Errorf("failed to remove VPCI device: %s", err) } return nil } diff --git a/internal/uvm/vsmb.go b/internal/uvm/vsmb.go index c610b16e6a..9e333c6635 100644 --- a/internal/uvm/vsmb.go +++ b/internal/uvm/vsmb.go @@ -7,7 +7,6 @@ import ( "path/filepath" "strconv" - "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/requesttype" hcsschema "github.com/Microsoft/hcsshim/internal/schema2" ) @@ -29,8 +28,7 @@ type VSMBShare struct { // Release frees the resources of the corresponding vsmb Mount func (vsmb *VSMBShare) Release(ctx context.Context) error { if err := vsmb.vm.RemoveVSMB(ctx, vsmb.HostPath); err != nil { - log.G(ctx).WithError(err).Warn("failed to remove vsmb share") - return err + return fmt.Errorf("failed to remove VSMB share: %s", err) } return nil } diff --git a/internal/vmcompute/vmcompute.go b/internal/vmcompute/vmcompute.go index 7c2a0dc280..e42bf8cfa7 100644 --- a/internal/vmcompute/vmcompute.go +++ b/internal/vmcompute/vmcompute.go @@ -26,6 +26,7 @@ import ( //sys hcsResumeComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) = vmcompute.HcsResumeComputeSystem? //sys hcsGetComputeSystemProperties(computeSystem HcsSystem, propertyQuery string, properties **uint16, result **uint16) (hr error) = vmcompute.HcsGetComputeSystemProperties? //sys hcsModifyComputeSystem(computeSystem HcsSystem, configuration string, result **uint16) (hr error) = vmcompute.HcsModifyComputeSystem? +//sys hcsModifyServiceSettings(settings string, result **uint16) (hr error) = vmcompute.HcsModifyServiceSettings? //sys hcsRegisterComputeSystemCallback(computeSystem HcsSystem, callback uintptr, context uintptr, callbackHandle *HcsCallback) (hr error) = vmcompute.HcsRegisterComputeSystemCallback? //sys hcsUnregisterComputeSystemCallback(callbackHandle HcsCallback) (hr error) = vmcompute.HcsUnregisterComputeSystemCallback? @@ -337,6 +338,27 @@ func HcsModifyComputeSystem(ctx gcontext.Context, computeSystem HcsSystem, confi }) } +func HcsModifyServiceSettings(ctx gcontext.Context, settings string) (result string, hr error) { + ctx, span := trace.StartSpan(ctx, "HcsModifyServiceSettings") + defer span.End() + defer func() { + if result != "" { + span.AddAttributes(trace.StringAttribute("result", result)) + } + oc.SetSpanStatus(span, hr) + }() + span.AddAttributes(trace.StringAttribute("settings", settings)) + + return result, execute(ctx, timeout.SyscallWatcher, func() error { + var resultp *uint16 + err := hcsModifyServiceSettings(settings, &resultp) + if resultp != nil { + result = interop.ConvertAndFreeCoTaskMemString(resultp) + } + return err + }) +} + func HcsRegisterComputeSystemCallback(ctx gcontext.Context, computeSystem HcsSystem, callback uintptr, context uintptr) (callbackHandle HcsCallback, hr error) { ctx, span := trace.StartSpan(ctx, "HcsRegisterComputeSystemCallback") defer span.End() diff --git a/internal/vmcompute/zsyscall_windows.go b/internal/vmcompute/zsyscall_windows.go index 0f2a69f6ad..8cfded4963 100644 --- a/internal/vmcompute/zsyscall_windows.go +++ b/internal/vmcompute/zsyscall_windows.go @@ -50,6 +50,7 @@ var ( procHcsResumeComputeSystem = modvmcompute.NewProc("HcsResumeComputeSystem") procHcsGetComputeSystemProperties = modvmcompute.NewProc("HcsGetComputeSystemProperties") procHcsModifyComputeSystem = modvmcompute.NewProc("HcsModifyComputeSystem") + procHcsModifyServiceSettings = modvmcompute.NewProc("HcsModifyServiceSettings") procHcsRegisterComputeSystemCallback = modvmcompute.NewProc("HcsRegisterComputeSystemCallback") procHcsUnregisterComputeSystemCallback = modvmcompute.NewProc("HcsUnregisterComputeSystemCallback") procHcsCreateProcess = modvmcompute.NewProc("HcsCreateProcess") @@ -314,6 +315,29 @@ func _hcsModifyComputeSystem(computeSystem HcsSystem, configuration *uint16, res return } +func hcsModifyServiceSettings(settings string, result **uint16) (hr error) { + var _p0 *uint16 + _p0, hr = syscall.UTF16PtrFromString(settings) + if hr != nil { + return + } + return _hcsModifyServiceSettings(_p0, result) +} + +func _hcsModifyServiceSettings(settings *uint16, result **uint16) (hr error) { + if hr = procHcsModifyServiceSettings.Find(); hr != nil { + return + } + r0, _, _ := syscall.Syscall(procHcsModifyServiceSettings.Addr(), 2, uintptr(unsafe.Pointer(settings)), uintptr(unsafe.Pointer(result)), 0) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + hr = syscall.Errno(r0) + } + return +} + func hcsRegisterComputeSystemCallback(computeSystem HcsSystem, callback uintptr, context uintptr, callbackHandle *HcsCallback) (hr error) { if hr = procHcsRegisterComputeSystemCallback.Find(); hr != nil { return diff --git a/test/cri-containerd/container_test.go b/test/cri-containerd/container_test.go index f08af991e8..f8af3685c0 100644 --- a/test/cri-containerd/container_test.go +++ b/test/cri-containerd/container_test.go @@ -476,3 +476,148 @@ func Test_RunContainer_ZeroVPMEM_Multiple_LCOW(t *testing.T) { startContainer(t, client, ctx, containerIDTwo) 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 := &runtime.RunPodSandboxRequest{ + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: t.Name() + "-Sandbox", + Namespace: testNamespace, + }, + }, + RuntimeHandler: 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) { + t.Skip("GMSA is not supported for Hyper-V isolated containers") + requireFeatures(t, featureWCOWHypervisor, featureGMSA) + + credSpec := gmsaSetup(t) + pullRequiredImages(t, []string{imageWindowsNanoserver}) + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + sandboxRequest := &runtime.RunPodSandboxRequest{ + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: t.Name() + "-Sandbox", + Namespace: testNamespace, + }, + }, + RuntimeHandler: 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") + } +} diff --git a/test/cri-containerd/gmsa.go b/test/cri-containerd/gmsa.go new file mode 100644 index 0000000000..4e71c6bf64 --- /dev/null +++ b/test/cri-containerd/gmsa.go @@ -0,0 +1,48 @@ +// +build functional + +package cri_containerd + +import ( + "fmt" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "testing" +) + +// Generates a gmsa credential spec and places it at 'path'. Dir has to exist, it will not be created by +// New-CredentialSpec. +func generateCredSpec(path string) error { + output, err := exec.Command( + "powershell", + "New-CredentialSpec", + "-AccountName", + gmsaAccount, + "-Path", + path, + ).CombinedOutput() + if err != nil { + return fmt.Errorf("failed to create new credential spec (output: %s): %v", string(output), err) + } + return nil +} + +// Tries to generate a cred spec to use for gmsa test cases. Returns the cred +// spec and an error if any. +func gmsaSetup(t *testing.T) string { + csDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("failed to create temp directory: %s", err) + } + defer os.RemoveAll(csDir) + csPath := filepath.Join(csDir, "credspec.json") + if err := generateCredSpec(csPath); err != nil { + t.Fatal(err) + } + credSpec, err := ioutil.ReadFile(csPath) + if err != nil { + t.Fatalf("failed to read credential spec: %s", err) + } + return string(credSpec) +} diff --git a/test/cri-containerd/main.go b/test/cri-containerd/main.go index 30da27e7b8..bbfd513f60 100644 --- a/test/cri-containerd/main.go +++ b/test/cri-containerd/main.go @@ -42,6 +42,10 @@ const ( testGPUBootFiles = "C:\\ContainerPlat\\LinuxBootFiles\\nvidiagpu" alpineAspNet = "mcr.microsoft.com/dotnet/core/aspnet:3.1-alpine3.11" alpineAspnetUpgrade = "mcr.microsoft.com/dotnet/core/aspnet:3.1.2-alpine3.11" + // Default account name for use with GMSA related tests. This will not be + // present/you will not have access to the account on your machine unless + // your environment is configured properly. + gmsaAccount = "cplat" ) // Image definitions @@ -65,12 +69,14 @@ const ( featureLCOW = "LCOW" featureWCOWProcess = "WCOWProcess" featureWCOWHypervisor = "WCOWHypervisor" + featureGMSA = "GMSA" ) var allFeatures = []string{ featureLCOW, featureWCOWProcess, featureWCOWHypervisor, + featureGMSA, } func init() {