Embedding ETW Name-GUID Mapping for Log Forward Service (microsoft#2617)

* Initial check-in for ETW Name-GUID Mapping code

Signed-off-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>

* Moving to VMUtils and static analysis fix

Signed-off-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>

* sidecar-GCS changes to forward modifyServiceSettings

Signed-off-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>

* Making the default sources and map in native go, remvoing unreferenced methods and addressing review comments

Signed-off-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>

* Addressing review comments

Signed-off-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>

* Erroring out instead of silent override, and bubbling up the error to the caller

Signed-off-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>

* Removing stale test

Signed-off-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>

---------

Signed-off-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>
Co-authored-by: Manish Ranjan Mahanta <mmahanta@microsoft.com>

Author: marma-dev

internal/gcs-sidecar/bridge.go

@@ -172,6 +172,7 @@ func (b *Bridge) AssignHandlers() {
 	b.HandleFunc(prot.RPCDeleteContainerState, b.deleteContainerState)
 	b.HandleFunc(prot.RPCUpdateContainer, b.updateContainer)
 	b.HandleFunc(prot.RPCLifecycleNotification, b.lifecycleNotification)
+	b.HandleFunc(prot.RPCModifyServiceSettings, b.modifyServiceSettings)
 }
 
 // readMessage reads the message from io.Reader

internal/gcs-sidecar/handlers.go

@@ -23,6 +23,7 @@ import (
 	oci "github.com/Microsoft/hcsshim/internal/oci"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
+	"github.com/Microsoft/hcsshim/internal/vm/vmutils/etw"
 	"github.com/Microsoft/hcsshim/internal/windevice"
 	"github.com/Microsoft/hcsshim/pkg/annotations"
 	"github.com/Microsoft/hcsshim/pkg/cimfs"
@@ -491,6 +492,70 @@ func (b *Bridge) lifecycleNotification(req *request) (err error) {
 	return nil
 }
 
+func (b *Bridge) modifyServiceSettings(req *request) (err error) {
+	_, span := oc.StartSpan(req.ctx, "sidecar::modifyServiceSettings")
+	defer span.End()
+	defer func() { oc.SetSpanStatus(span, err) }()
+
+	// Todo: Add policy enforcement for modifying service settings
+	modifyRequest, err := unmarshalModifyServiceSettings(req)
+	if err != nil {
+		return fmt.Errorf("failed to unmarshal modifyServiceSettings request: %w", err)
+	}
+
+	switch modifyRequest.PropertyType {
+	case string(prot.LogForwardService):
+		if modifyRequest.Settings != nil {
+			log.G(req.ctx).Tracef("modifyServiceSettings for LogForwardService with RPCModifyServiceSettings, enforcing policy for log sources")
+			settings := modifyRequest.Settings.(*guestrequest.LogForwardServiceRPCRequest)
+
+			switch settings.RPCType {
+			case guestrequest.RPCModifyServiceSettings, guestrequest.RPCStartLogForwarding, guestrequest.RPCStopLogForwarding:
+				log.G(req.ctx).Tracef("%v request received for LogForwardService, proceeding with policy enforcement for log sources", settings.RPCType)
+				// Enforce the policy for log sources in the request and update the settings with allowed log sources.
+				// For cwcow, the sidecar-GCS will verify the allowed log sources against policy and append the necessary GUIDs to the ones allowed. Rest are dropped.
+				// The Enforcer will have to unmarshal the log sources, enforce the policy and then marshal it back to a Base64 encoded JSON string which is what inbox GCS expects.
+				// It can query etw.GetDefaultLogSources to get the default log sources if the policy allows, and allow providers matching the default list during policy enforcement.
+				// This is because the log sources can be a combination of default and user specified log sources for which GUIDs need to be appended based on the policy enforcement.
+				if settings.Settings != "" {
+					// <EXAMPLE CALL>
+					// allowedLogSources, err := b.hostState.securityOptions.PolicyEnforcer.EnforceLogForwardServiceSettingsPolicy(req.ctx, settings.LogSources)
+
+					// For now, we are skipping the policy enforcement and allowing all log sources as the policy enforcer implementation is in progress. We will add the enforcement back once it's implemented.
+					allowedLogSources := settings.Settings // This is Base64 encoded JSON string of log sources
+					log.G(req.ctx).Tracef("Allowed log sources after policy enforcement: %v", allowedLogSources)
+
+					// Update the allowed log sources in the settings. This will be forwarded to inbox GCS which expects the log sources in a JSON string format with GUIDs for providers included.
+					allowedLogSources, err := etw.UpdateLogSources(allowedLogSources, false, true)
+					if err != nil {
+						return fmt.Errorf("failed to update log sources: %w", err)
+					}
+					settings.Settings = allowedLogSources
+				}
+			default:
+				log.G(req.ctx).Warningf("modifyServiceSettings for LogForwardService with RPCType: %v, skipping policy enforcement", settings.RPCType)
+			}
+			modifyRequest.Settings = settings
+			buf, err := json.Marshal(modifyRequest)
+			if err != nil {
+				return fmt.Errorf("failed to marshal modifyServiceSettings request: %w", err)
+			}
+			var newRequest request
+			newRequest.ctx = req.ctx
+			newRequest.header = req.header
+			newRequest.header.Size = uint32(len(buf)) + prot.HdrSize
+			newRequest.message = buf
+			req = &newRequest
+		} else {
+			log.G(req.ctx).Warningf("modifyServiceSettings for LogForwardService with empty settings, skipping policy enforcement")
+		}
+	default:
+		log.G(req.ctx).Warningf("modifyServiceSettings with PropertyType: %v, skipping policy enforcement", modifyRequest.PropertyType)
+	}
+	b.forwardRequestToGcs(req)
+	return nil
+}
+
 func volumeGUIDFromLayerPath(path string) (string, bool) {
 	if p, ok := strings.CutPrefix(path, `\\?\Volume{`); ok {
 		if q, ok := strings.CutSuffix(p, `}\Files`); ok {

internal/gcs-sidecar/uvm.go

@@ -17,6 +17,37 @@ import (
 	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
 )
 
+func unmarshalModifyServiceSettings(req *request) (_ *prot.ServiceModificationRequest, err error) {
+	ctx, span := oc.StartSpan(req.ctx, "sidecar::unmarshalModifyServiceSettings")
+	defer span.End()
+	defer func() { oc.SetSpanStatus(span, err) }()
+
+	var serviceModifyRequest prot.ServiceModificationRequest
+	var requestRawSettings json.RawMessage
+	serviceModifyRequest.Settings = &requestRawSettings
+	if err := commonutils.UnmarshalJSONWithHresult(req.message, &serviceModifyRequest); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal rpcModifySettings: %w", err)
+	}
+
+	if serviceModifyRequest.PropertyType != "" {
+		switch serviceModifyRequest.PropertyType {
+		case string(prot.LogForwardService):
+			log.G(ctx).Info("Unmarshalling log forward service modify settings")
+			settings := &guestrequest.LogForwardServiceRPCRequest{}
+			if err := commonutils.UnmarshalJSONWithHresult(requestRawSettings, settings); err != nil {
+				return nil, fmt.Errorf("invalid LogForwardService modify settings request: %w", err)
+			}
+			serviceModifyRequest.Settings = settings
+		default:
+			// Invalid request
+			log.G(ctx).Errorf("Invalid ServiceModificationRequest: %v", serviceModifyRequest.PropertyType)
+			return nil, fmt.Errorf("invalid ServiceModificationRequest")
+		}
+	}
+
+	return &serviceModifyRequest, nil
+}
+
 func unmarshalContainerModifySettings(req *request) (_ *prot.ContainerModifySettings, err error) {
 	ctx, span := oc.StartSpan(req.ctx, "sidecar::unmarshalContainerModifySettings")
 	defer span.End()

internal/oci/uvm.go

@@ -419,12 +419,14 @@ func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) (
 		if err := handleWCOWSecurityPolicy(ctx, s.Annotations, wopts); err != nil {
 			return nil, err
 		}
-		// If security policy is enable, wopts.ForwardLogs default value should be false
+		// If security policy is enable, wopts.LogForwardingEnabled default value should be false (CWCOW should not allow log forwarding by default)
 		if wopts.SecurityPolicyEnabled {
-			wopts.ForwardLogs = false
+			wopts.LogForwardingEnabled = false
 		}
 		wopts.LogSources = ParseAnnotationsString(s.Annotations, annotations.LogSources, wopts.LogSources)
-		wopts.ForwardLogs = ParseAnnotationsBool(ctx, s.Annotations, annotations.ForwardLogs, wopts.ForwardLogs)
+		wopts.LogForwardingEnabled = ParseAnnotationsBool(ctx, s.Annotations, annotations.LogForwardingEnabled, wopts.LogForwardingEnabled)
+		wopts.DefaultLogSourcesEnabled = ParseAnnotationsBool(ctx, s.Annotations, annotations.DefaultLogSourcesEnabled, wopts.DefaultLogSourcesEnabled)
+
 		return wopts, nil
 	}
 	return nil, errors.New("cannot create UVM opts spec is not LCOW or WCOW")

internal/uvm/create_wcow.go

@@ -70,9 +70,10 @@ type OptionsWCOW struct {
 	// AdditionalRegistryKeys are Registry keys and their values to additionally add to the uVM.
 	AdditionalRegistryKeys []hcsschema.RegistryValue
 
-	OutputHandlerCreator vmutils.OutputHandlerCreator // Creates an [OutputHandler] that controls how output received over HVSocket from the UVM is handled. Defaults to parsing output as ETW Log events
-	LogSources           string                       // ETW providers to be set for the logging service
-	ForwardLogs          bool                         // Whether to forward logs to the host or not
+	OutputHandlerCreator     vmutils.OutputHandlerCreator // Creates an [OutputHandler] that controls how output received over HVSocket from the UVM is handled. Defaults to parsing output as ETW Log events
+	LogSources               string                       // ETW providers to be set for the logging service
+	LogForwardingEnabled     bool                         // Whether to enable forwarding of logs to the host or not
+	DefaultLogSourcesEnabled bool                         // Whether to enable using default log sources
 }
 
 func defaultConfidentialWCOWOSBootFilesPath() string {
@@ -111,9 +112,10 @@ func NewDefaultOptionsWCOW(id, owner string) *OptionsWCOW {
 				SecurityPolicyEnabled: false,
 			},
 		},
-		OutputHandlerCreator: vmutils.ParseGCSLogrus,
-		ForwardLogs:          true, // Default to true for WCOW, and set to false for CWCOW in internal/oci/uvm.go SpecToUVMCreateOpts
-		LogSources:           "",
+		OutputHandlerCreator:     vmutils.ParseGCSLogrus,
+		LogForwardingEnabled:     true, // Default to true for WCOW, and set to false for CWCOW in internal/oci/uvm.go SpecToUVMCreateOpts
+		DefaultLogSourcesEnabled: true,
+		LogSources:               "",
 	}
 }
 
@@ -291,7 +293,7 @@ func prepareCommonConfigDoc(ctx context.Context, uvm *UtilityVM, opts *OptionsWC
 	}
 
 	maps.Copy(doc.VirtualMachine.Devices.HvSocket.HvSocketConfig.ServiceTable, opts.AdditionalHyperVConfig)
-	if opts.ForwardLogs {
+	if opts.LogForwardingEnabled {
 		key := prot.WindowsLoggingHvsockServiceID.String()
 		doc.VirtualMachine.Devices.HvSocket.HvSocketConfig.ServiceTable[key] = hcsschema.HvSocketServiceConfig{
 			AllowWildcardBinds:        true,
@@ -562,22 +564,23 @@ func CreateWCOW(ctx context.Context, opts *OptionsWCOW) (_ *UtilityVM, err error
 	log.G(ctx).WithField("options", log.Format(ctx, opts)).Debug("uvm::CreateWCOW options")
 
 	uvm := &UtilityVM{
-		id:                      opts.ID,
-		owner:                   opts.Owner,
-		operatingSystem:         "windows",
-		scsiControllerCount:     opts.SCSIControllerCount,
-		vsmbDirShares:           make(map[string]*VSMBShare),
-		vsmbFileShares:          make(map[string]*VSMBShare),
-		vpciDevices:             make(map[VPCIDeviceID]*VPCIDevice),
-		noInheritHostTimezone:   opts.NoInheritHostTimezone,
-		physicallyBacked:        !opts.AllowOvercommit,
-		devicesPhysicallyBacked: opts.FullyPhysicallyBacked,
-		vsmbNoDirectMap:         opts.NoDirectMap,
-		noWritableFileShares:    opts.NoWritableFileShares,
-		createOpts:              opts,
-		blockCIMMounts:          make(map[string]*UVMMountedBlockCIMs),
-		logSources:              opts.LogSources,
-		forwardLogs:             opts.ForwardLogs,
+		id:                       opts.ID,
+		owner:                    opts.Owner,
+		operatingSystem:          "windows",
+		scsiControllerCount:      opts.SCSIControllerCount,
+		vsmbDirShares:            make(map[string]*VSMBShare),
+		vsmbFileShares:           make(map[string]*VSMBShare),
+		vpciDevices:              make(map[VPCIDeviceID]*VPCIDevice),
+		noInheritHostTimezone:    opts.NoInheritHostTimezone,
+		physicallyBacked:         !opts.AllowOvercommit,
+		devicesPhysicallyBacked:  opts.FullyPhysicallyBacked,
+		vsmbNoDirectMap:          opts.NoDirectMap,
+		noWritableFileShares:     opts.NoWritableFileShares,
+		createOpts:               opts,
+		blockCIMMounts:           make(map[string]*UVMMountedBlockCIMs),
+		logSources:               opts.LogSources,
+		logForwardingEnabled:     opts.LogForwardingEnabled,
+		defaultLogSourcesEnabled: opts.DefaultLogSourcesEnabled,
 	}
 
 	defer func() {
@@ -617,7 +620,7 @@ func CreateWCOW(ctx context.Context, opts *OptionsWCOW) (_ *UtilityVM, err error
 		return nil, fmt.Errorf("error while creating the compute system: %w", err)
 	}
 
-	if opts.ForwardLogs {
+	if opts.LogForwardingEnabled {
 		// Create a socket that the executed program can send to. This is usually
 		// used by Log Forward Service to send log data.
 		uvm.outputHandler = opts.OutputHandlerCreator(opts.ID)

internal/uvm/log_wcow.go

@@ -4,11 +4,13 @@ package uvm
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/Microsoft/hcsshim/internal/gcs"
 	"github.com/Microsoft/hcsshim/internal/gcs/prot"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
+	"github.com/Microsoft/hcsshim/internal/vm/vmutils/etw"
 )
 
 func (uvm *UtilityVM) StartLogForwarding(ctx context.Context) error {
@@ -62,13 +64,23 @@ func (uvm *UtilityVM) SetLogSources(ctx context.Context) error {
 	wcaps := gcs.GetWCOWCapabilities(uvm.gc.Capabilities())
 	if wcaps != nil && wcaps.IsLogForwardingSupported() {
 		// Make a call to the GCS to set the ETW providers
+
+		// Determines the log sources to be set based on the configuration. If default log sources are enabled,
+		// we only include them along with user specified log sources.
+		// For confidential WCOw, we skip the adding guids to the log sources as the sidecar-GCS will verify the
+		// allowed log sources against policy and append the necessary GUIDs to the ones allowed. Rest are dropped.
+		// For non-confidential WCOW, we include the GUIDs in the log sources as the hcsshim communicates directly with the inboxGCS.
+		settings, err := etw.UpdateLogSources(uvm.logSources, uvm.defaultLogSourcesEnabled, !uvm.HasConfidentialPolicy())
+		if err != nil {
+			return fmt.Errorf("failed to parse log sources: %w", err)
+		}
 		req := guestrequest.LogForwardServiceRPCRequest{
 			RPCType:  guestrequest.RPCModifyServiceSettings,
-			Settings: uvm.logSources,
+			Settings: settings,
 		}
-		err := uvm.gc.ModifyServiceSettings(ctx, prot.LogForwardService, req)
+		err = uvm.gc.ModifyServiceSettings(ctx, prot.LogForwardService, req)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to modify service settings: %w", err)
 		}
 	}
 	return nil

internal/uvm/start.go

@@ -285,7 +285,7 @@ func (uvm *UtilityVM) Start(ctx context.Context) (err error) {
 		}
 	}
 
-	if uvm.OS() == "windows" && uvm.forwardLogs {
+	if uvm.OS() == "windows" && uvm.logForwardingEnabled {
 		// If the UVM is Windows and log forwarding is enabled, set the log sources
 		// and start the log forwarding service.
 		if err := uvm.SetLogSources(ctx); err != nil {

internal/uvm/types.go

@@ -144,8 +144,9 @@ type UtilityVM struct {
 	blockCIMMounts    map[string]*UVMMountedBlockCIMs
 	blockCIMMountLock sync.Mutex
 
-	forwardLogs bool   // Indicates whether to forward logs from the UVM to the host
-	logSources  string // ETW providers to enable for log forwarding
+	logForwardingEnabled     bool   // Indicates whether to forward logs from the UVM to the host
+	defaultLogSourcesEnabled bool   // Specifies whether addition of default list of ETW providers should be disabled
+	logSources               string // ETW providers to enable for log forwarding
 }
 
 func (uvm *UtilityVM) ScratchEncryptionEnabled() bool {

internal/vm/vmutils/etw/default-sources.go

@@ -0,0 +1,66 @@
+package etw
+
+// defaultLogSourcesInfo defines the list of trusted ETW providers
+var defaultLogSourcesInfo = LogSourcesInfo{
+	LogConfig: LogConfig{
+		Sources: []Source{
+			{
+				Type: "ETW",
+				Providers: []EtwProvider{
+					{
+						ProviderName: "microsoft.windows.hyperv.compute",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft-windows-guest-network-service",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.filesystem.cimfs",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.filesystem.unionfs",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft-windows-bitlocker-driver",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft-windows-bitlocker-api",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.security.keyguard",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.security.keyguard.attestation.verify",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.containers.setup",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.containers.storage",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.containers.library",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.containers.dynamicimage",
+						Level:        "Information",
+					},
+					{
+						ProviderName: "microsoft.windows.logforwardservice.provider",
+						Level:        "Information",
+					},
+				},
+			},
+		},
+	},
+}