From c7253df57b2bb272b7acedf34958adcfbdfa0d67 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Mon, 22 Jun 2020 14:01:43 -0700 Subject: [PATCH] Add GMSA support for V2 HCS schema xenon containers * Add new UVM function 'UpdateHvSocketService' to be able to hot add Hvsocket service table entries. * Add new UVM function 'RemoveHvSocketService' to be able to hot remove an Hvsocket service. * Add disabled field to HvSocketServiceConfig (used to be private in the schema) * Remove hardcoded error if supplying a cred spec and the client asked for a hypervisor isolated container. * Misc refactors (comments, style) Signed-off-by: Daniel Canter --- internal/credentials/credentials.go | 42 +++++++++--------- internal/hcsoci/resources_wcow.go | 22 ++++++++-- internal/resources/resources.go | 2 +- internal/schema2/hv_socket_service_config.go | 6 +++ internal/uvm/hvsocket.go | 43 +++++++++++++++++++ internal/uvm/resourcepaths.go | 1 + test/cri-containerd/container_test.go | 1 - .../hcsshim/internal/hcsoci/credentials.go | 42 +++++++++--------- .../hcsshim/internal/hcsoci/resources.go | 7 ++- 9 files changed, 114 insertions(+), 52 deletions(-) create mode 100644 internal/uvm/hvsocket.go diff --git a/internal/credentials/credentials.go b/internal/credentials/credentials.go index d5a49ae9a1..e8fef907ef 100644 --- a/internal/credentials/credentials.go +++ b/internal/credentials/credentials.go @@ -29,18 +29,19 @@ import ( // 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 +// CCGResource stores the id used when creating a ccg instance. Used when // closing a container to be able to release the instance. -type CCGInstance struct { +type CCGResource 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 { +// Release calls into hcs to remove the ccg instance for the container matching CCGResource.id. +// 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 (ccgResource *CCGResource) Release(ctx context.Context) error { + if err := removeCredentialGuard(ctx, ccgResource.id); err != nil { return fmt.Errorf("failed to remove container credential guard instance: %s", err) } return nil @@ -48,7 +49,7 @@ func (instance *CCGInstance) Release(ctx context.Context) error { // 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) { +func CreateCredentialGuard(ctx context.Context, id, credSpec string, hypervisorIsolated bool) (*hcsschema.ContainerCredentialGuardInstance, *CCGResource, 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 @@ -57,19 +58,21 @@ func CreateCredentialGuard(ctx context.Context, id, credSpec string, hypervisorI // 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. + // + // 4. If xenon container the CCG instance with the Hvsocket service table + // information is expected to be in the Utility VMs doc before being sent + // to HCS for creation. For pod scenarios currently we don't have the OCI + // spec of a container at UVM creation time, therefore the service table entry + // for the CCG instance will have to be hot added. 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") + transport = "HvSocket" } req := hcsschema.ModificationRequest{ PropertyType: hcsschema.PTContainerCredentialGuard, @@ -103,10 +106,10 @@ func CreateCredentialGuard(ctx context.Context, id, credSpec string, hypervisorI } for _, ccgInstance := range ccgSysInfo.Instances { if ccgInstance.Id == id { - instance := &CCGInstance{ + ccgResource := &CCGResource{ id, } - return ccgInstance.CredentialGuard, instance, nil + return &ccgInstance, ccgResource, nil } } return nil, nil, fmt.Errorf("failed to find credential guard instance with container ID %s", id) @@ -125,8 +128,5 @@ func removeCredentialGuard(ctx context.Context, id string) error { }, }, } - if err := hcs.ModifyServiceSettings(ctx, req); err != nil { - return err - } - return nil + return hcs.ModifyServiceSettings(ctx, req) } diff --git a/internal/hcsoci/resources_wcow.go b/internal/hcsoci/resources_wcow.go index 7b89a25c9f..ce57448c97 100644 --- a/internal/hcsoci/resources_wcow.go +++ b/internal/hcsoci/resources_wcow.go @@ -129,13 +129,27 @@ func allocateWindowsResources(ctx context.Context, coi *createOptionsInternal, r // Only need to create a CCG instance for v2 containers if schemaversion.IsV21(coi.actualSchemaVersion) { hypervisorIsolated := coi.HostingSystem != nil - ccgState, ccgInstance, err := credentials.CreateCredentialGuard(ctx, coi.actualID, cs, hypervisorIsolated) + ccgInstance, ccgResource, err := credentials.CreateCredentialGuard(ctx, coi.actualID, cs, hypervisorIsolated) if err != nil { return err } - coi.ccgState = ccgState - r.Add(ccgInstance) - //TODO dcantah: If/when dynamic service table entries is supported register the RpcEndpoint with hvsocket here + coi.ccgState = ccgInstance.CredentialGuard + r.Add(ccgResource) + if hypervisorIsolated { + // If hypervisor isolated we need to add an hvsocket service table entry + // By default HVSocket won't allow something inside the VM to connect + // back to a process on the host. We need to update the HVSocket service table + // to allow a connection to CCG.exe on the host, so that GMSA can function. + // We need to hot add this here because at UVM creation time we don't know what containers + // will be launched in the UVM, nonetheless if they will ask for GMSA. This is a workaround + // for the previous design requirement for CCG V2 where the service entry + // must be present in the UVM'S HCS document before being sent over as hot adding + // an HvSocket service was not possible. + hvSockConfig := ccgInstance.HvSocketConfig + if err := coi.HostingSystem.UpdateHvSocketService(ctx, hvSockConfig.ServiceId, hvSockConfig.ServiceConfig); err != nil { + return fmt.Errorf("failed to update hvsocket service: %s", err) + } + } } } diff --git a/internal/resources/resources.go b/internal/resources/resources.go index 081984f4c2..00e7f973ba 100644 --- a/internal/resources/resources.go +++ b/internal/resources/resources.go @@ -120,7 +120,7 @@ func ReleaseResources(ctx context.Context, r *Resources, vm *uvm.UtilityVM, all } r.createdNetNS = false } - case *credentials.CCGInstance: + case *credentials.CCGResource: if err := r.resources[i].Release(ctx); err != nil { log.G(ctx).WithError(err).Error("failed to release container resource") releaseErr = true diff --git a/internal/schema2/hv_socket_service_config.go b/internal/schema2/hv_socket_service_config.go index a848e91e69..ecd9f7fbac 100644 --- a/internal/schema2/hv_socket_service_config.go +++ b/internal/schema2/hv_socket_service_config.go @@ -19,4 +19,10 @@ type HvSocketServiceConfig struct { // If true, HvSocket will process wildcard binds for this service/system combination. Wildcard binds are secured in the registry at SOFTWARE/Microsoft/Windows NT/CurrentVersion/Virtualization/HvSocket/WildcardDescriptors AllowWildcardBinds bool `json:"AllowWildcardBinds,omitempty"` + + // Disabled controls whether the HvSocket service is accepting connection requests. + // This set to true will make the service refuse all incoming connections as well as cancel + // any connections already established. The service itself will still be active however + // and can be re-enabled at a future time. + Disabled bool `json:"Disabled,omitempty"` } diff --git a/internal/uvm/hvsocket.go b/internal/uvm/hvsocket.go new file mode 100644 index 0000000000..15ed2628fb --- /dev/null +++ b/internal/uvm/hvsocket.go @@ -0,0 +1,43 @@ +package uvm + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/internal/requesttype" + hcsschema "github.com/Microsoft/hcsshim/internal/schema2" +) + +// UpdateHvSocketService calls HCS to update/create the hvsocket service for +// the UVM. Takes in a service ID and the hvsocket service configuration. If there is no +// entry for the service ID already it will be created. The same call on HvSockets side +// handles the Create/Update/Delete cases based on what is passed in. Here is the logic +// for the call. +// +// 1. If the service ID does not currently exist in the service table, it will be created +// with whatever descriptors and state was specified (disabled or not). +// 2. If the service already exists and empty descriptors and Disabled is passed in for the +// service config, the service will be removed. +// 3. Otherwise any combination that is not Disabled && Empty descriptors will just update the +// service. +// +// If the request is crafted with Disabled = True and empty descriptors, then this function +// will behave identically to a call to RemoveHvSocketService. Prefer RemoveHvSocketService for this +// behavior as the relevant fields are set on HCS' side. +func (uvm *UtilityVM) UpdateHvSocketService(ctx context.Context, sid string, doc *hcsschema.HvSocketServiceConfig) error { + request := &hcsschema.ModifySettingRequest{ + RequestType: requesttype.Update, + ResourcePath: fmt.Sprintf(hvsocketConfigResourceFormat, sid), + Settings: doc, + } + return uvm.modify(ctx, request) +} + +// RemoveHvSocketService will remove an hvsocket service entry if it exists. +func (uvm *UtilityVM) RemoveHvSocketService(ctx context.Context, sid string) error { + request := &hcsschema.ModifySettingRequest{ + RequestType: requesttype.Remove, + ResourcePath: fmt.Sprintf(hvsocketConfigResourceFormat, sid), + } + return uvm.modify(ctx, request) +} diff --git a/internal/uvm/resourcepaths.go b/internal/uvm/resourcepaths.go index 05abb726f6..93a5af5596 100644 --- a/internal/uvm/resourcepaths.go +++ b/internal/uvm/resourcepaths.go @@ -18,4 +18,5 @@ const ( vPMemControllerResourceFormat string = "VirtualMachine/Devices/VirtualPMem/Devices/%d" vPMemDeviceResourceFormat string = "VirtualMachine/Devices/VirtualPMem/Devices/%d/Mappings/%d" vSmbShareResourcePath string = "VirtualMachine/Devices/VirtualSmb/Shares" + hvsocketConfigResourceFormat string = "VirtualMachine/Devices/HvSocket/HvSocketConfig/ServiceTable/%s" ) diff --git a/test/cri-containerd/container_test.go b/test/cri-containerd/container_test.go index b99336456b..d3e25c6842 100644 --- a/test/cri-containerd/container_test.go +++ b/test/cri-containerd/container_test.go @@ -455,7 +455,6 @@ func Test_RunContainer_GMSA_WCOW_Process(t *testing.T) { } 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) diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/credentials.go b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/credentials.go index 5226271c45..b236092c59 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/credentials.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/credentials.go @@ -28,18 +28,19 @@ import ( // 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 +// CCGResource stores the id used when creating a ccg instance. Used when // closing a container to be able to release the instance. -type CCGInstance struct { +type CCGResource 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 { +// Release calls into hcs to remove the ccg instance for the container matching CCGResource.id. +// 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 (ccgResource *CCGResource) Release(ctx context.Context) error { + if err := removeCredentialGuard(ctx, ccgResource.id); err != nil { return fmt.Errorf("failed to remove container credential guard instance: %s", err) } return nil @@ -47,7 +48,7 @@ func (instance *CCGInstance) Release(ctx context.Context) error { // 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) { +func CreateCredentialGuard(ctx context.Context, id, credSpec string, hypervisorIsolated bool) (*hcsschema.ContainerCredentialGuardInstance, *CCGResource, 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 @@ -56,19 +57,21 @@ func CreateCredentialGuard(ctx context.Context, id, credSpec string, hypervisorI // 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. + // + // 4. If xenon container the CCG instance with the Hvsocket service table + // information is expected to be in the Utility VMs doc before being sent + // to HCS for creation. For pod scenarios currently we don't have the OCI + // spec of a container at UVM creation time, therefore the service table entry + // for the CCG instance will have to be hot added. 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") + transport = "HvSocket" } req := hcsschema.ModificationRequest{ PropertyType: hcsschema.PTContainerCredentialGuard, @@ -102,10 +105,10 @@ func CreateCredentialGuard(ctx context.Context, id, credSpec string, hypervisorI } for _, ccgInstance := range ccgSysInfo.Instances { if ccgInstance.Id == id { - instance := &CCGInstance{ + ccgResource := &CCGResource{ id, } - return ccgInstance.CredentialGuard, instance, nil + return &ccgInstance, ccgResource, nil } } return nil, nil, fmt.Errorf("failed to find credential guard instance with container ID %s", id) @@ -124,8 +127,5 @@ func removeCredentialGuard(ctx context.Context, id string) error { }, }, } - if err := hcs.ModifyServiceSettings(ctx, req); err != nil { - return err - } - return nil + return hcs.ModifyServiceSettings(ctx, req) } diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/resources.go b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/resources.go index 08be62bbd0..0ec5b5288c 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/resources.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/resources.go @@ -92,15 +92,14 @@ func ReleaseResources(ctx context.Context, r *Resources, vm *uvm.UtilityVM, all } r.createdNetNS = false } - case *CCGInstance: + case *CCGResource: 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. + // Don't need to check if vm != nil here 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 { log.G(ctx).WithError(err).Error("failed to release container resource")