-
Notifications
You must be signed in to change notification settings - Fork 289
CWCOW: Logging enforcement #2763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
takuro-sato
wants to merge
8
commits into
microsoft:main
Choose a base branch
from
takuro-sato:logging-enforcement
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
3c3891a
CWCOW: Remove hostedSystemConfig
MahatiC 5a6f3f8
CWCOW: Add logging enforcement policy
MahatiC 9a0d5a1
CWCOW: Enforce log_provider policy with allow_log_provider_dropping s…
takuro-sato 1462715
CWCOW: warn when log providers are trimmed by policy
takuro-sato 7371616
ci: empty commit to retrigger checks
takuro-sato 346bfb0
securitypolicy: always set PolicyEnforcerSet in LockDown
takuro-sato 7e5929a
gcs-sidecar: detect log-provider trim via missing names, not length
takuro-sato 6f69276
etw: add UpdateLogSourcesFromInfo to skip redundant decode
takuro-sato File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,22 +57,16 @@ func (b *Bridge) createContainer(req *request) (err error) { | |
| return errors.Wrap(err, "failed to unmarshal createContainer") | ||
| } | ||
|
|
||
| // containerConfig can be of type uvnConfig or hcsschema.HostedSystem or guestresource.CWCOWHostedSystem | ||
| // containerConfig can be of type uvmConfig or guestresource.CWCOWHostedSystem | ||
| var ( | ||
| uvmConfig prot.UvmConfig | ||
| hostedSystemConfig hcsschema.HostedSystem | ||
| cwcowHostedSystemConfig guestresource.CWCOWHostedSystem | ||
| ) | ||
| if err = commonutils.UnmarshalJSONWithHresult(containerConfig, &uvmConfig); err == nil && | ||
| uvmConfig.SystemType != "" { | ||
| systemType := uvmConfig.SystemType | ||
| timeZoneInformation := uvmConfig.TimeZoneInformation | ||
| log.G(ctx).Tracef("createContainer: uvmConfig: {systemType: %v, timeZoneInformation: %v}}", systemType, timeZoneInformation) | ||
| } else if err = commonutils.UnmarshalJSONWithHresult(containerConfig, &hostedSystemConfig); err == nil && | ||
| hostedSystemConfig.SchemaVersion != nil && hostedSystemConfig.Container != nil { | ||
| schemaVersion := hostedSystemConfig.SchemaVersion | ||
| container := hostedSystemConfig.Container | ||
| log.G(ctx).Tracef("rpcCreate: HostedSystemConfig: {schemaVersion: %v, container: %v}}", schemaVersion, container) | ||
| } else if err = commonutils.UnmarshalJSONWithHresult(containerConfig, &cwcowHostedSystemConfig); err == nil && | ||
| cwcowHostedSystemConfig.Spec.Version != "" && cwcowHostedSystemConfig.CWCOWHostedSystem.Container != nil { | ||
| cwcowHostedSystem := cwcowHostedSystemConfig.CWCOWHostedSystem | ||
|
|
@@ -551,21 +545,98 @@ func (b *Bridge) modifyServiceSettings(req *request) (err error) { | |
| 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) | ||
| // Decode the base64-encoded log sources config so we can | ||
| // enforce policy on the requested provider list. | ||
| logSources, err := etw.DecodeAndUnmarshalLogSources(settings.Settings) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to decode log sources: %w", err) | ||
| } | ||
|
takuro-sato marked this conversation as resolved.
|
||
|
|
||
| // Collect every requested provider name and ask the | ||
| // enforcer to validate them as a batch. The enforcer's | ||
| // behaviour depends on allow_log_provider_dropping in the | ||
| // active policy: | ||
| // - false (default, fail-close): any disallowed provider | ||
| // causes the call to be denied. | ||
| // - true: disallowed providers are silently dropped and | ||
| // the kept subset is returned for forwarding. | ||
| var requestedNames []string | ||
| for _, source := range logSources.LogConfig.Sources { | ||
| for _, provider := range source.Providers { | ||
| requestedNames = append(requestedNames, provider.ProviderName) | ||
| } | ||
| } | ||
|
|
||
| keptNames, err := b.hostState.securityOptions.PolicyEnforcer.EnforceLogProviderPolicy( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. feels like all the kept/dropped names and the logging of them should be encapsulated by the policy enforcement and the handlers should just forward the "final log sources request" to inbox GCS? |
||
| req.ctx, requestedNames) | ||
| if err != nil { | ||
| b.hostState.securityOptions.LockDown(req.ctx) | ||
| return fmt.Errorf("log providers denied by policy: %w", err) | ||
| } | ||
|
|
||
| // 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) | ||
| // Build a quick lookup for the kept set so we can trim the | ||
| // LogSourcesInfo to only those providers the policy allowed. | ||
| keepSet := make(map[string]struct{}, len(keptNames)) | ||
| for _, name := range keptNames { | ||
| keepSet[name] = struct{}{} | ||
| } | ||
|
|
||
| // Detect trimming by scanning requested names against | ||
| // keepSet. We cannot use len(kept) != len(requested): | ||
| // the rego enforcer returns providers_to_keep via a set | ||
| // (see getProvidersToKeep → keepSet.toArray()), so a | ||
| // duplicate-name request like [A, A, B] returns [A, B] | ||
| // even when nothing was dropped, which would otherwise | ||
| // trip a false-positive warning and a needless re-marshal. | ||
| dropped := make([]string, 0) | ||
| seenDropped := make(map[string]struct{}) | ||
| for _, name := range requestedNames { | ||
| if _, ok := keepSet[name]; ok { | ||
| continue | ||
| } | ||
| if _, dup := seenDropped[name]; dup { | ||
| continue | ||
| } | ||
| seenDropped[name] = struct{}{} | ||
| dropped = append(dropped, name) | ||
| } | ||
|
|
||
| // Trim happens in-place on the parsed structure so we can | ||
| // hand it to UpdateLogSourcesFromInfo without a redundant | ||
| // base64-decode + JSON-unmarshal round-trip (we already | ||
| // decoded above for enforcement). | ||
| trimmed := logSources | ||
| if len(dropped) > 0 { | ||
| // Surface the drop so operators have a breadcrumb — | ||
| // under allow_log_provider_dropping the pod boots | ||
| // silently, and forwardlogs may itself be off, so | ||
| // without this warning the trim is invisible. | ||
| log.G(req.ctx).WithFields(map[string]interface{}{ | ||
| "requested": requestedNames, | ||
| "kept": keptNames, | ||
| "dropped": dropped, | ||
| }).Warn("log providers trimmed by policy (allow_log_provider_dropping)") | ||
|
|
||
| // Trim each source's provider list to only the | ||
| // allowed names. Empty sources are preserved to keep | ||
| // the shape stable; inbox GCS handles them as no-ops. | ||
| for i := range trimmed.LogConfig.Sources { | ||
| src := &trimmed.LogConfig.Sources[i] | ||
| filtered := make([]etw.EtwProvider, 0, len(src.Providers)) | ||
| for _, p := range src.Providers { | ||
| if _, ok := keepSet[p.ProviderName]; ok { | ||
| filtered = append(filtered, p) | ||
| } | ||
| } | ||
| src.Providers = filtered | ||
| } | ||
| } | ||
|
|
||
| // 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) | ||
| // Apply GUID resolution (and any other inbox-GCS prep) | ||
| // against the policy-trimmed payload and hand off to | ||
| // inbox GCS. | ||
| allowedLogSources, err := etw.UpdateLogSourcesFromInfo(trimmed, false, true) | ||
|
anmaxvl marked this conversation as resolved.
|
||
| if err != nil { | ||
| return fmt.Errorf("failed to update log sources: %w", err) | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.