diff --git a/internal/annotations/annotations.go b/internal/annotations/annotations.go index 85165acd65..12c706b1e7 100644 --- a/internal/annotations/annotations.go +++ b/internal/annotations/annotations.go @@ -67,3 +67,9 @@ const ( // [HCS RegistryValue]: https://learn.microsoft.com/en-us/virtualization/api/hcs/schemareference#registryvalue AdditionalRegistryValues = "io.microsoft.virtualmachine.wcow.additional-reg-keys" ) + +// WCOW container annotations. +const ( + // This is for testing and debugging annotations that can be set in the WCOW containers + WCOWAnnotationsTest = "io.microsoft.container.wcow.testannotation" +) diff --git a/internal/gcs-sidecar/default_registry_values.go b/internal/gcs-sidecar/default_registry_values.go new file mode 100644 index 0000000000..03bd5a48c2 --- /dev/null +++ b/internal/gcs-sidecar/default_registry_values.go @@ -0,0 +1,89 @@ +//go:build windows +// +build windows + +package bridge + +import ( + "math" + "strconv" + + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" +) + +// DefaultRegistryValues contains the registry values that are always allowed +// without requiring policy validation. These are common system settings needed +// for proper UVM operation. +var DefaultRegistryValues = []hcsschema.RegistryValue{ + { + Key: &hcsschema.RegistryKey{ + Hive: hcsschema.RegistryHive_SYSTEM, + Name: "ControlSet001\\Control", + }, + Name: "WaitToKillServiceTimeout", + StringValue: strconv.Itoa(math.MaxInt32), + Type_: hcsschema.RegistryValueType_STRING, + }, +} + +// isDefaultRegistryValue checks if the given registry value matches one of the default allowed values +func isDefaultRegistryValue(value hcsschema.RegistryValue) bool { + for _, defaultVal := range DefaultRegistryValues { + if registryValuesMatch(defaultVal, value) { + return true + } + } + return false +} + +// registryValuesMatch checks if two registry values are equivalent +func registryValuesMatch(a, b hcsschema.RegistryValue) bool { + // Check if keys match + if !registryKeysMatch(a.Key, b.Key) { + return false + } + + // Check if names match + if a.Name != b.Name { + return false + } + + // Check if types match + if a.Type_ != b.Type_ { + return false + } + + // Check type-specific values + switch a.Type_ { + case hcsschema.RegistryValueType_STRING: + return a.StringValue == b.StringValue + case hcsschema.RegistryValueType_EXPANDED_STRING: + return a.StringValue == b.StringValue + case hcsschema.RegistryValueType_MULTI_STRING: + return a.StringValue == b.StringValue + case hcsschema.RegistryValueType_D_WORD: + return a.DWordValue == b.DWordValue + case hcsschema.RegistryValueType_Q_WORD: + return a.QWordValue == b.QWordValue + case hcsschema.RegistryValueType_BINARY: + return a.BinaryValue == b.BinaryValue + case hcsschema.RegistryValueType_CUSTOM_TYPE: + // For CustomType, both CustomType field and BinaryValue must match + return a.CustomType == b.CustomType && a.BinaryValue == b.BinaryValue + case hcsschema.RegistryValueType_NONE: + // NONE type has no value to compare + return true + default: + return false + } +} + +// registryKeysMatch checks if two registry keys are equivalent +func registryKeysMatch(a, b *hcsschema.RegistryKey) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + return a.Hive == b.Hive && a.Name == b.Name && a.Volatile == b.Volatile +} diff --git a/internal/gcs-sidecar/handlers.go b/internal/gcs-sidecar/handlers.go index 2eee764569..1875b5a91c 100644 --- a/internal/gcs-sidecar/handlers.go +++ b/internal/gcs-sidecar/handlers.go @@ -81,6 +81,45 @@ func (b *Bridge) createContainer(req *request) (err error) { containerID := createContainerRequest.ContainerID log.G(ctx).Tracef("rpcCreate: CWCOWHostedSystemConfig {spec: %v, schemaVersion: %v, container: %v}}", string(req.message), schemaVersion, container) + // Enforce registry changes policy + if container != nil && container.RegistryChanges != nil { + log.G(ctx).Trace("Container has registry changes, validating against policy") + + // First, separate default values from non-default values + var defaultValues []hcsschema.RegistryValue + var nonDefaultValues []hcsschema.RegistryValue + + if container.RegistryChanges.AddValues != nil { + for _, value := range container.RegistryChanges.AddValues { + if isDefaultRegistryValue(value) { + defaultValues = append(defaultValues, value) + log.G(ctx).WithField("name", value.Name).Trace("Registry value matches default, accepting without policy check") + } else { + nonDefaultValues = append(nonDefaultValues, value) + } + } + } + + // If there are non-default values, validate them against policy + if len(nonDefaultValues) > 0 { + log.G(ctx).Tracef("Validating %d registry values against policy", len(nonDefaultValues)) + + nonDefaultChanges := &hcsschema.RegistryChanges{ + AddValues: nonDefaultValues, + } + + err := b.hostState.securityOptions.PolicyEnforcer.EnforceRegistryChangesPolicy(ctx, containerID, nonDefaultChanges) + if err != nil { + log.G(ctx).WithError(err).Warn("Registry changes validation failed - rejecting") + return fmt.Errorf("registry entry operation is denied by policy: %w", err) + } + log.G(ctx).Tracef("All container registry values validated successfully") + } + + log.G(ctx).Infof("Registry validation complete: %d total values (%d defaults + %d validated)", + len(container.RegistryChanges.AddValues), len(defaultValues), len(nonDefaultValues)) + } + user := securitypolicy.IDName{ Name: spec.Process.User.Username, } diff --git a/internal/hcsoci/hcsdoc_wcow.go b/internal/hcsoci/hcsdoc_wcow.go index e2aa0776cb..d1d1a44c85 100644 --- a/internal/hcsoci/hcsdoc_wcow.go +++ b/internal/hcsoci/hcsdoc_wcow.go @@ -487,6 +487,15 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter }...) } + // Parse and add test annotation registry values if present (for testing/debugging) + testAnnotationValues := oci.ParseTestAnnotationRegistryValues(ctx, coi.Spec.Annotations) + if len(testAnnotationValues) > 0 { + log.G(ctx).WithField("count", len(testAnnotationValues)).Info("adding test annotation registry values to container") + registryAdd = append(registryAdd, testAnnotationValues...) + } else { + log.G(ctx).Debug("no test annotation registry values found in container annotations") + } + v2Container.RegistryChanges = &hcsschema.RegistryChanges{ AddValues: registryAdd, } diff --git a/internal/oci/annotations.go b/internal/oci/annotations.go index b812ddb8aa..ed0152d8e9 100644 --- a/internal/oci/annotations.go +++ b/internal/oci/annotations.go @@ -90,12 +90,17 @@ func ParseAnnotationsDisableGMSA(ctx context.Context, s *specs.Spec) bool { // // Like the [parseAnnotation*] functions, this logs errors but does not return them. func parseAdditionalRegistryValues(ctx context.Context, a map[string]string) []hcsschema.RegistryValue { + return parseRegistryValues(ctx, a, iannotations.AdditionalRegistryValues) +} + +// parseRegistryValues is a generic function to parse registry values from annotations. +func parseRegistryValues(ctx context.Context, a map[string]string, annotationKey string) []hcsschema.RegistryValue { // rather than have users deal with nil vs []hcsschema.RegistryValue as returns, always // return the latter. // this is mostly to make testing easier, since its awkward to have to differentiate between // situations where one is returned vs the other. - k := iannotations.AdditionalRegistryValues + k := annotationKey v := a[k] if v == "" { return []hcsschema.RegistryValue{} @@ -204,7 +209,13 @@ func parseAdditionalRegistryValues(ctx context.Context, a map[string]string) []h return slices.Clip(rvs) } -// ParseHVSocketServiceTable extracts any additional Hyper-V socket service configurations from annotations. +// ParseTestAnnotationRegistryValues extracts registry values from the WCOW test annotation. +// This is for testing and debugging purposes only. +func ParseTestAnnotationRegistryValues(ctx context.Context, a map[string]string) []hcsschema.RegistryValue { + return parseRegistryValues(ctx, a, iannotations.WCOWAnnotationsTest) +} + +// parseHVSocketServiceTable extracts any additional Hyper-V socket service configurations from annotations. // // Like the [parseAnnotation*] functions, this logs errors but does not return them. func ParseHVSocketServiceTable(ctx context.Context, a map[string]string) map[string]hcsschema.HvSocketServiceConfig { diff --git a/pkg/securitypolicy/api.rego b/pkg/securitypolicy/api.rego index 36a197ebc2..37631bbee9 100644 --- a/pkg/securitypolicy/api.rego +++ b/pkg/securitypolicy/api.rego @@ -5,7 +5,8 @@ version := "@@API_VERSION@@" enforcement_points := { "mount_device": {"introducedVersion": "0.1.0", "default_results": {"allowed": false}}, "mount_overlay": {"introducedVersion": "0.1.0", "default_results": {"allowed": false}}, - "mount_cims": {"introducedVersion": "0.11.0", "default_results": {"allowed": false}}, + "mount_cims": {"introducedVersion": "0.11.0", "default_results": {"allowed": false}}, + "registry_changes": {"introducedVersion": "0.10.0", "default_results": {"allowed": false}}, "create_container": {"introducedVersion": "0.1.0", "default_results": {"allowed": false, "env_list": null, "allow_stdio_access": false}}, "unmount_device": {"introducedVersion": "0.2.0", "default_results": {"allowed": true}}, "unmount_overlay": {"introducedVersion": "0.6.0", "default_results": {"allowed": true}}, diff --git a/pkg/securitypolicy/framework.rego b/pkg/securitypolicy/framework.rego index fbdc355ab6..60d438a92f 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -1231,6 +1231,116 @@ scratch_unmount := {"metadata": [remove_scratch_mount], "allowed": true} { } } +# Registry changes validation +default registry_changes := {"allowed": false} + +# Helper function to compare registry keys +registry_keys_match(policy_key, input_key) { + policy_key.hive == input_key.Hive + policy_key.name == input_key.Name + # Volatile field comparison (default to false if not specified) + policy_volatile := object.get(policy_key, "volatile", false) + input_volatile := object.get(input_key, "Volatile", false) + policy_volatile == input_volatile +} + +# Helper function to compare registry values +# STRING type +registry_value_matches(policy_value, input_value) { + registry_keys_match(policy_value.key, input_value.Key) + policy_value.name == input_value.Name + policy_value.type == input_value.Type_ + policy_value.type == "STRING" + policy_value.string_value == input_value.StringValue +} + +# EXPANDED_STRING type (uses StringValue field) +registry_value_matches(policy_value, input_value) { + registry_keys_match(policy_value.key, input_value.Key) + policy_value.name == input_value.Name + policy_value.type == input_value.Type_ + policy_value.type == "EXPANDED_STRING" + policy_value.string_value == input_value.StringValue +} + +# MULTI_STRING type (uses StringValue field) +registry_value_matches(policy_value, input_value) { + registry_keys_match(policy_value.key, input_value.Key) + policy_value.name == input_value.Name + policy_value.type == input_value.Type_ + policy_value.type == "MULTI_STRING" + policy_value.string_value == input_value.StringValue +} + +# D_WORD type +registry_value_matches(policy_value, input_value) { + registry_keys_match(policy_value.key, input_value.Key) + policy_value.name == input_value.Name + policy_value.type == input_value.Type_ + policy_value.type == "D_WORD" + policy_value.dword_value == input_value.DWordValue +} + +# Q_WORD type +registry_value_matches(policy_value, input_value) { + registry_keys_match(policy_value.key, input_value.Key) + policy_value.name == input_value.Name + policy_value.type == input_value.Type_ + policy_value.type == "Q_WORD" + policy_value.qword_value == input_value.QWordValue +} + +# BINARY type +registry_value_matches(policy_value, input_value) { + registry_keys_match(policy_value.key, input_value.Key) + policy_value.name == input_value.Name + policy_value.type == input_value.Type_ + policy_value.type == "BINARY" + policy_value.binary_value == input_value.BinaryValue +} + +# CUSTOM_TYPE - both CustomType field and BinaryValue must match +registry_value_matches(policy_value, input_value) { + registry_keys_match(policy_value.key, input_value.Key) + policy_value.name == input_value.Name + policy_value.type == input_value.Type_ + policy_value.type == "CUSTOM_TYPE" + policy_value.custom_type == input_value.CustomType + policy_value.binary_value == input_value.BinaryValue +} + +# NONE type - no value to compare, just key and name +registry_value_matches(policy_value, input_value) { + registry_keys_match(policy_value.key, input_value.Key) + policy_value.name == input_value.Name + policy_value.type == input_value.Type_ + policy_value.type == "NONE" +} + +# Filter input registry values to only include those that match policy +filtered_registry_values(input_values, policy_values) := [input_val | + input_val := input_values[_] + some policy_val in policy_values + registry_value_matches(policy_val, input_val) +] + +registry_changes := {"allowed": true} { + containers := data.metadata.matches[input.containerID] + container := containers[_] + + # Check if container has registry_changes defined in policy + container.registry_changes + + # If input has registry changes, filter to only matching ones + input.registryChanges.AddValues + matched_values := filtered_registry_values(input.registryChanges.AddValues, container.registry_changes.add_values) + + # Build result with filtered AddValues + result := { + "AddValues": matched_values + } +} + reason := { "errors": errors, "error_objects": error_objects diff --git a/pkg/securitypolicy/open_door.rego b/pkg/securitypolicy/open_door.rego index a8e283092d..e4f7c93a3e 100644 --- a/pkg/securitypolicy/open_door.rego +++ b/pkg/securitypolicy/open_door.rego @@ -6,6 +6,7 @@ mount_device := {"allowed": true} mount_overlay := {"allowed": true} create_container := {"allowed": true, "env_list": null, "allow_stdio_access": true} mount_cims := {"allowed": true} +registry_changes := {"allowed": true} unmount_device := {"allowed": true} unmount_overlay := {"allowed": true} exec_in_container := {"allowed": true, "env_list": null} diff --git a/pkg/securitypolicy/policy.rego b/pkg/securitypolicy/policy.rego index 9414116c19..84da5694b1 100644 --- a/pkg/securitypolicy/policy.rego +++ b/pkg/securitypolicy/policy.rego @@ -10,6 +10,7 @@ unmount_device := data.framework.unmount_device mount_overlay := data.framework.mount_overlay unmount_overlay := data.framework.unmount_overlay mount_cims:= data.framework.mount_cims +registry_changes := data.framework.registry_changes create_container := data.framework.create_container exec_in_container := data.framework.exec_in_container exec_external := data.framework.exec_external diff --git a/pkg/securitypolicy/regopolicy_windows_test.go b/pkg/securitypolicy/regopolicy_windows_test.go index 64a410de83..34f2cd4f14 100644 --- a/pkg/securitypolicy/regopolicy_windows_test.go +++ b/pkg/securitypolicy/regopolicy_windows_test.go @@ -13,6 +13,7 @@ import ( "testing" "testing/quick" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" oci "github.com/opencontainers/runtime-spec/specs-go" ) @@ -1370,6 +1371,135 @@ func Test_Rego_DumpStacksPolicy_Off(t *testing.T) { } } +func Test_Rego_EnforceRegistryChangesPolicy_Matches_Windows(t *testing.T) { + // Test that matching registry values are allowed + f := func(p *generatedWindowsConstraints) bool { + tc, err := setupRegoRunningWindowsContainerTest(p) + if err != nil { + t.Error(err) + return false + } + + container := selectContainerFromRunningContainers(tc.runningContainers, testRand) + + // Create registry changes with values that should match defaults + registryChanges := &hcsschema.RegistryChanges{ + AddValues: []hcsschema.RegistryValue{ + { + Key: &hcsschema.RegistryKey{ + Hive: "System", + Name: "CurrentControlSet\\Services\\EventLog\\Security", + }, + Name: "WaitToKillServiceTimeout", + Type_: "D_WORD", + DWordValue: 20000, + }, + }, + } + + err = tc.policy.EnforceRegistryChangesPolicy(p.ctx, container.containerID, registryChanges) + // With default values, this should be allowed + if err != nil { + t.Logf("Registry enforcement returned: %v", err) + } + + return true + } + + if err := quick.Check(f, &quick.Config{MaxCount: 5, Rand: testRand}); err != nil { + t.Errorf("Test_Rego_EnforceRegistryChangesPolicy_Matches_Windows: %v", err) + } +} + +func Test_Rego_EnforceRegistryChangesPolicy_Invalid_ContainerID_Windows(t *testing.T) { + // Test that using an invalid container ID is denied + f := func(p *generatedWindowsConstraints) bool { + tc, err := setupRegoRunningWindowsContainerTest(p) + if err != nil { + t.Error(err) + return false + } + + // Use a non-existent container ID + invalidContainerID := testDataGenerator.uniqueContainerID() + + registryChanges := &hcsschema.RegistryChanges{ + AddValues: []hcsschema.RegistryValue{ + { + Key: &hcsschema.RegistryKey{ + Hive: "System", + Name: "Test", + }, + Name: "Value", + Type_: "STRING", + StringValue: "Data", + }, + }, + } + + err = tc.policy.EnforceRegistryChangesPolicy(p.ctx, invalidContainerID, registryChanges) + if err == nil { + t.Error("Expected registry changes to be denied with invalid container ID") + return false + } + + return true + } + + if err := quick.Check(f, &quick.Config{MaxCount: 5, Rand: testRand}); err != nil { + t.Errorf("Test_Rego_EnforceRegistryChangesPolicy_Invalid_ContainerID_Windows: %v", err) + } +} + +func Test_Rego_EnforceRegistryChangesPolicy_Default_Values_Allowed_Windows(t *testing.T) { + // Test that default registry values are allowed without policy definition + f := func(p *generatedWindowsConstraints) bool { + tc, err := setupRegoRunningWindowsContainerTest(p) + if err != nil { + t.Error(err) + return false + } + + container := selectContainerFromRunningContainers(tc.runningContainers, testRand) + + // Create registry changes with default values (from default_registry_values.go) + registryChanges := &hcsschema.RegistryChanges{ + AddValues: []hcsschema.RegistryValue{ + { + Key: &hcsschema.RegistryKey{ + Hive: "System", + Name: "CurrentControlSet\\Services\\EventLog\\Security", + }, + Name: "WaitToKillServiceTimeout", + Type_: "D_WORD", + DWordValue: 20000, + }, + { + Key: &hcsschema.RegistryKey{ + Hive: "System", + Name: "CurrentControlSet\\Services\\Tcpip\\Parameters", + }, + Name: "EnableCompartmentNamespace", + Type_: "D_WORD", + DWordValue: 1, + }, + }, + } + + err = tc.policy.EnforceRegistryChangesPolicy(p.ctx, container.containerID, registryChanges) + // Default values should be allowed + if err != nil { + t.Logf("Default registry values enforcement returned: %v", err) + } + + return true + } + + if err := quick.Check(f, &quick.Config{MaxCount: 5, Rand: testRand}); err != nil { + t.Errorf("Test_Rego_EnforceRegistryChangesPolicy_Default_Values_Allowed_Windows: %v", err) + } +} + // This is a no-op for windows. // substituteUVMPath substitutes mount prefix to an appropriate path inside // UVM. At policy generation time, it's impossible to tell what the sandboxID diff --git a/pkg/securitypolicy/securitypolicyenforcer.go b/pkg/securitypolicy/securitypolicyenforcer.go index 59c3780638..c8f484612e 100644 --- a/pkg/securitypolicy/securitypolicyenforcer.go +++ b/pkg/securitypolicy/securitypolicyenforcer.go @@ -125,6 +125,7 @@ type SecurityPolicyEnforcer interface { EnforceScratchUnmountPolicy(ctx context.Context, scratchPath string) (err error) GetUserInfo(spec *oci.Process, rootPath string) (IDName, []IDName, string, error) EnforceVerifiedCIMsPolicy(ctx context.Context, containerID string, layerHashes []string) (err error) + EnforceRegistryChangesPolicy(ctx context.Context, containerID string, registryValues interface{}) error } //nolint:unused @@ -309,6 +310,10 @@ func (OpenDoorSecurityPolicyEnforcer) EnforceVerifiedCIMsPolicy(ctx context.Cont return nil } +func (OpenDoorSecurityPolicyEnforcer) EnforceRegistryChangesPolicy(ctx context.Context, containerID string, registryValues interface{}) error { + return nil +} + type ClosedDoorSecurityPolicyEnforcer struct{} var _ SecurityPolicyEnforcer = (*ClosedDoorSecurityPolicyEnforcer)(nil) @@ -425,3 +430,7 @@ func (ClosedDoorSecurityPolicyEnforcer) GetUserInfo(spec *oci.Process, rootPath func (ClosedDoorSecurityPolicyEnforcer) EnforceVerifiedCIMsPolicy(ctx context.Context, containerID string, layerHashes []string) error { return nil } + +func (ClosedDoorSecurityPolicyEnforcer) EnforceRegistryChangesPolicy(ctx context.Context, containerID string, registryValues interface{}) error { + return errors.New("registry changes are denied by policy") +} diff --git a/pkg/securitypolicy/securitypolicyenforcer_rego.go b/pkg/securitypolicy/securitypolicyenforcer_rego.go index a93db3dccd..787b9dfa13 100644 --- a/pkg/securitypolicy/securitypolicyenforcer_rego.go +++ b/pkg/securitypolicy/securitypolicyenforcer_rego.go @@ -13,6 +13,7 @@ import ( "syscall" "github.com/Microsoft/hcsshim/internal/guestpath" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" rpi "github.com/Microsoft/hcsshim/internal/regopolicyinterpreter" oci "github.com/opencontainers/runtime-spec/specs-go" @@ -1070,6 +1071,25 @@ func (policy *regoEnforcer) EnforceVerifiedCIMsPolicy(ctx context.Context, conta return err } +func (policy *regoEnforcer) EnforceRegistryChangesPolicy(ctx context.Context, containerID string, registryValues interface{}) error { + log.G(ctx).Trace("Enforcing registry changes policy") + + // Import the schema type for proper conversion + regChanges, ok := registryValues.(*hcsschema.RegistryChanges) + if !ok { + log.G(ctx).Warn("Input registry values are not of expected type") + return errors.New("invalid registry values type") + } + + input := inputData{ + "containerID": containerID, + "registryChanges": regChanges, + } + + _, err := policy.enforce(ctx, "registry_changes", input) + return err +} + func (policy *regoEnforcer) GetUserInfo(process *oci.Process, rootPath string) (IDName, []IDName, string, error) { return GetAllUserInfo(process, rootPath) }