diff --git a/launcher/container_runner.go b/launcher/container_runner.go index e7ce6d6a1..a73538966 100644 --- a/launcher/container_runner.go +++ b/launcher/container_runner.go @@ -30,7 +30,6 @@ import ( "github.com/google/go-tpm-tools/client" "github.com/google/go-tpm-tools/launcher/agent" "github.com/google/go-tpm-tools/launcher/internal/signaturediscovery" - "github.com/google/go-tpm-tools/launcher/internal/systemctl" "github.com/google/go-tpm-tools/launcher/launcherfile" "github.com/google/go-tpm-tools/launcher/registryauth" "github.com/google/go-tpm-tools/launcher/spec" @@ -115,7 +114,7 @@ func NewRunner(ctx context.Context, cdClient *containerd.Client, token oauth2.To } logger.Printf("Image Labels : %v\n", imageConfig.Labels) - launchPolicy, err := spec.GetLaunchPolicy(imageConfig.Labels) + launchPolicy, err := spec.GetLaunchPolicy(imageConfig.Labels, logger) if err != nil { return nil, err } @@ -341,7 +340,7 @@ func (r *ContainerRunner) measureContainerClaims(ctx context.Context) error { // eventlog in the AttestationAgent. func (r *ContainerRunner) measureMemoryMonitor() error { var enabled uint8 - if r.launchSpec.MemoryMonitoringEnabled { + if r.launchSpec.MonitoringEnabled == spec.MemoryOnly { enabled = 1 } if err := r.attestAgent.MeasureEvent(cel.CosTlv{EventType: cel.MemoryMonitorType, EventContent: []byte{enabled}}); err != nil { @@ -521,22 +520,9 @@ func (r *ContainerRunner) Run(ctx context.Context) error { go teeServer.Serve() defer teeServer.Shutdown(ctx) - // start node-problem-detector.service to collect memory related metrics. - if r.launchSpec.MemoryMonitoringEnabled { - r.logger.Println("MemoryMonitoring is enabled by the VM operator") - s, err := systemctl.New() - if err != nil { - return fmt.Errorf("failed to create systemctl client: %v", err) - } - defer s.Close() - - r.logger.Println("Starting a systemctl operation: systemctl start node-problem-detector.service") - if err := s.Start("node-problem-detector.service"); err != nil { - return fmt.Errorf("failed to start node-problem-detector.service: %v", err) - } - r.logger.Println("node-problem-detector.service successfully started.") - } else { - r.logger.Println("MemoryMonitoring is disabled by the VM operator") + // Avoids breaking existing memory monitoring tests that depend on this log. + if r.launchSpec.MonitoringEnabled == spec.None { + r.logger.Printf("MemoryMonitoring is disabled by the VM operator") } var streamOpt cio.Opt diff --git a/launcher/image/test/test_memory_monitoring.yaml b/launcher/image/test/test_memory_monitoring.yaml index e4eb49f79..ee496606b 100644 --- a/launcher/image/test/test_memory_monitoring.yaml +++ b/launcher/image/test/test_memory_monitoring.yaml @@ -23,7 +23,7 @@ steps: id: CheckMemoryMonitoringEnabled entrypoint: 'bash' # Search a regex pattern that ensures memory monitoring is enabled and measured into COS event logs. - args: ['scripts/test_memory_monitoring.sh', '${_VM_NAME_PREFIX}-enable-${BUILD_ID}', '${_ZONE}', 'Successfully measured memory monitoring event.*node-problem-detector.service successfully started'] + args: ['scripts/test_memory_monitoring.sh', '${_VM_NAME_PREFIX}-enable-${BUILD_ID}', '${_ZONE}', 'node-problem-detector.service successfully started.*Successfully measured memory monitoring event'] waitFor: ['CreateVMMemoryMemonitorEnabled'] - name: 'gcr.io/cloud-builders/gcloud' id: CleanUpVMMemoryMonitorEnabled diff --git a/launcher/image/testworkloads/memorymonitoring/Dockerfile b/launcher/image/testworkloads/memorymonitoring/Dockerfile index 7f8fca0ed..2b245ea7e 100644 --- a/launcher/image/testworkloads/memorymonitoring/Dockerfile +++ b/launcher/image/testworkloads/memorymonitoring/Dockerfile @@ -7,7 +7,8 @@ COPY main / ENV env_bar="val_bar" -LABEL "tee.launch_policy.monitoring_memory_allow"="always" +LABEL "tee.launch_policy.hardened_monitoring"="MEMORY_ONLY" +LABEL "tee.launch_policy.debug_monitoring"="NONE" ENTRYPOINT ["/main"] diff --git a/launcher/image/testworkloads/memorymonitoringdebug/Dockerfile b/launcher/image/testworkloads/memorymonitoringdebug/Dockerfile index 8df0840f5..51da2fad4 100644 --- a/launcher/image/testworkloads/memorymonitoringdebug/Dockerfile +++ b/launcher/image/testworkloads/memorymonitoringdebug/Dockerfile @@ -7,7 +7,8 @@ COPY main / ENV env_bar="val_bar" -LABEL "tee.launch_policy.monitoring_memory_allow"="debugonly" +LABEL "tee.launch_policy.hardened_monitoring"="NONE" +LABEL "tee.launch_policy.debug_monitoring"="MEMORY_ONLY" ENTRYPOINT ["/main"] diff --git a/launcher/image/testworkloads/memorymonitoringnever/Dockerfile b/launcher/image/testworkloads/memorymonitoringnever/Dockerfile index 22f7a84dd..d7df4d8d1 100644 --- a/launcher/image/testworkloads/memorymonitoringnever/Dockerfile +++ b/launcher/image/testworkloads/memorymonitoringnever/Dockerfile @@ -7,8 +7,8 @@ COPY main / ENV env_bar="val_bar" -LABEL "tee.launch_policy.monitoring_memory_allow"="never" - +LABEL "tee.launch_policy.hardened_monitoring"="NONE" +LABEL "tee.launch_policy.debug_monitoring"="NONE" ENTRYPOINT ["/main"] CMD ["arg_foo"] diff --git a/launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config.go b/launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config.go index 6c97ee294..d32027932 100644 --- a/launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config.go +++ b/launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config.go @@ -4,40 +4,100 @@ package nodeproblemdetector import ( "encoding/json" "fmt" + "log" "os" "time" + + "github.com/google/go-tpm-tools/launcher/internal/systemctl" ) +var systemStatsFilePath = "/etc/node_problem_detector/system-stats-monitor.json" + var defaultInvokeIntervalString = (60 * time.Second).String() type metricConfig struct { DisplayName string `json:"displayName"` } -type memoryStatsConfig struct { +type statsConfig struct { MetricsConfigs map[string]metricConfig `json:"metricsConfigs"` } +type diskConfig struct { + IncludeAllAttachedBlk bool `json:"includeAllAttachedBlk"` + IncludeRootBlk bool `json:"includeRootBlk"` + LsblkTimeout string `json:"lsblkTimeout"` + MetricsConfigs *statsConfig `json:"metricsConfigs"` +} + // SystemStatsConfig contains configurations for `System Stats Monitor`, // a problem daemon in node-problem-detector that collects pre-defined health-related metrics from different system components. -// For now we only consider collecting memory related metrics. // View the comprehensive configuration details on https://github.com/kubernetes/node-problem-detector/tree/master/pkg/systemstatsmonitor#detailed-configuration-options type SystemStatsConfig struct { - MemoryStatsConfig memoryStatsConfig `json:"memory"` - InvokeInterval string `json:"invokeInterval"` + CPU *statsConfig `json:"cpu,omitempty"` + Disk *diskConfig `json:"disk,omitempty"` + Host *statsConfig `json:"host,omitempty"` + Memory *statsConfig `json:"memory,omitempty"` + InvokeInterval string `json:"invokeInterval,omitempty"` } // NewSystemStatsConfig returns a new SystemStatsConfig struct with default configurations. func NewSystemStatsConfig() SystemStatsConfig { return SystemStatsConfig{ - MemoryStatsConfig: memoryStatsConfig{MetricsConfigs: map[string]metricConfig{}}, - InvokeInterval: defaultInvokeIntervalString, + Memory: &statsConfig{MetricsConfigs: map[string]metricConfig{}}, + InvokeInterval: defaultInvokeIntervalString, } } +var allConfig = &SystemStatsConfig{ + CPU: &statsConfig{map[string]metricConfig{ + "cpu/runnable_task_count": {"cpu/runnable_task_count"}, + "cpu/usage_time": {"cpu/usage_time"}, + "cpu/load_1m": {"cpu/load_1m"}, + "cpu/load_5m": {"cpu/load_5m"}, + "cpu/load_15m": {"cpu/load_15m"}, + "system/cpu_stat": {"system/cpu_stat"}, + "system/interrupts_total": {"system/interrupts_total"}, + "system/processes_total": {"system/processes_total"}, + "system/procs_blocked": {"system/procs_blocked"}, + "system/procs_running": {"system/procs_running"}, + }}, + Disk: &diskConfig{ + true, true, "5s", + &statsConfig{map[string]metricConfig{ + "disk/avg_queue_len": {"disk/avg_queue_len"}, + "disk/bytes_used": {"disk/bytes_used"}, + "disk/percent_used": {"disk/percent_used"}, + "disk/io_time": {"disk/io_time"}, + "disk/merged_operation_count": {"disk/merged_operation_count"}, + "disk/operation_bytes_count": {"disk/operation_bytes_count"}, + "disk/operation_count": {"disk/operation_count"}, + "disk/operation_time": {"disk/operation_time"}, + "disk/weighted_io": {"disk/weighted_io"}, + }}, + }, + Host: &statsConfig{map[string]metricConfig{ + "host/uptime": {"host/uptime"}, + }}, + Memory: &statsConfig{map[string]metricConfig{ + "memory/anonymous_used": {"memory/anonymous_used"}, + "memory/bytes_used": {"memory/bytes_used"}, + "memory/dirty_used": {"memory/dirty_used"}, + "memory/page_cache_used": {"memory/page_cache_used"}, + "memory/unevictable_used": {"memory/unevictable_used"}, + "memory/percent_used": {"memory/percent_used"}, + }}, + InvokeInterval: defaultInvokeIntervalString, +} + +// EnableAllConfig overwrites system stats config with health monitoring config. +func EnableAllConfig() error { + return allConfig.WriteFile(systemStatsFilePath) +} + // EnableMemoryBytesUsed enables "memory/bytes_used" for memory monitoring. func (ssc *SystemStatsConfig) EnableMemoryBytesUsed() { - ssc.MemoryStatsConfig.MetricsConfigs["memory/bytes_used"] = metricConfig{DisplayName: "memory/bytes_used"} + ssc.Memory.MetricsConfigs["memory/bytes_used"] = metricConfig{DisplayName: "memory/bytes_used"} } // WithInvokeInterval overrides the default invokeInterval. @@ -53,3 +113,20 @@ func (ssc *SystemStatsConfig) WriteFile(path string) error { } return os.WriteFile(path, bytes, 0644) } + +// StartService starts Node Problem Detector. +func StartService(logger *log.Logger) error { + s, err := systemctl.New() + if err != nil { + return fmt.Errorf("failed to create systemctl client: %v", err) + } + defer s.Close() + + logger.Printf("Starting node-problem-detector.service") + if err := s.Start("node-problem-detector.service"); err != nil { + return fmt.Errorf("failed to start node-problem-detector.service") + } + + logger.Printf("node-problem-detector.service successfully started") + return nil +} diff --git a/launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config_test.go b/launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config_test.go index 2201615aa..71473178e 100644 --- a/launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config_test.go +++ b/launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config_test.go @@ -2,6 +2,7 @@ package nodeproblemdetector import ( "bytes" + "encoding/json" "io" "os" "path" @@ -11,12 +12,38 @@ import ( "github.com/google/go-cmp/cmp" ) +func TestEnableHealthMonitoringConfig(t *testing.T) { + tmpDir := t.TempDir() + systemStatsFilePath = path.Join(tmpDir, "system-stats-monitor.json") + + wantBytes, err := json.Marshal(allConfig) + if err != nil { + t.Fatalf("Error marshaling expected config: %v", err) + } + + EnableAllConfig() + + file, err := os.OpenFile(systemStatsFilePath, os.O_RDONLY, 0) + if err != nil { + t.Fatalf("failed to open file %s: %v", systemStatsFilePath, err) + } + + gotBytes, err := io.ReadAll(file) + if err != nil { + t.Fatalf("failed to read from file %s: %v", systemStatsFilePath, err) + } + + if !bytes.Equal(gotBytes, wantBytes) { + t.Errorf("WriteFile() did not write expected contents, got %s, want %s", gotBytes, wantBytes) + } +} + func TestEnableMemoryBytesUsed(t *testing.T) { got := NewSystemStatsConfig() got.EnableMemoryBytesUsed() want := SystemStatsConfig{ - MemoryStatsConfig: memoryStatsConfig{ + Memory: &statsConfig{ MetricsConfigs: map[string]metricConfig{ "memory/bytes_used": {DisplayName: "memory/bytes_used"}, }, diff --git a/launcher/launcher/main.go b/launcher/launcher/main.go index 9181d4e61..6be3edbc1 100644 --- a/launcher/launcher/main.go +++ b/launcher/launcher/main.go @@ -18,6 +18,7 @@ import ( "github.com/containerd/containerd/namespaces" "github.com/google/go-tpm-tools/client" "github.com/google/go-tpm-tools/launcher" + "github.com/google/go-tpm-tools/launcher/internal/healthmonitoring/nodeproblemdetector" "github.com/google/go-tpm-tools/launcher/launcherfile" "github.com/google/go-tpm-tools/launcher/registryauth" "github.com/google/go-tpm-tools/launcher/spec" @@ -95,6 +96,26 @@ func main() { return } + if launchSpec.MonitoringEnabled != spec.None { + logger.Printf("Health Monitoring is enabled by the VM operator") + + if launchSpec.MonitoringEnabled == spec.All { + logger.Printf("All health monitoring metrics enabled") + if err := nodeproblemdetector.EnableAllConfig(); err != nil { + logger.Printf("Failed to enable full monitoring config: %v", err) + return + } + } else if launchSpec.MonitoringEnabled == spec.MemoryOnly { + logger.Printf("memory/bytes_used enabled") + } + + if err := nodeproblemdetector.StartService(logger); err != nil { + logger.Print(err) + } + } else { + logger.Printf("Health Monitoring is disabled") + } + defer func() { // Catch panic to attempt to output to Cloud Logging. if r := recover(); r != nil { diff --git a/launcher/spec/launch_policy.go b/launcher/spec/launch_policy.go index 71467eef6..56117cbe8 100644 --- a/launcher/spec/launch_policy.go +++ b/launcher/spec/launch_policy.go @@ -3,6 +3,7 @@ package spec import ( "errors" "fmt" + "log" "path/filepath" "strconv" "strings" @@ -14,8 +15,9 @@ type LaunchPolicy struct { AllowedEnvOverride []string AllowedCmdOverride bool AllowedLogRedirect policy - AllowedMemoryMonitoring policy AllowedMountDestinations []string + HardenedImageMonitoring MonitoringType + DebugImageMonitoring MonitoringType } type policy int @@ -26,6 +28,31 @@ const ( never ) +// MonitoringType represents the possible health monitoring presets for the client. +type MonitoringType int + +const ( + // None indicates no monitoring enabled. + None MonitoringType = iota + // MemoryOnly indicates only memory_bytes_used enabled. + MemoryOnly + // All indicates all supported metrics enabled. + All +) + +func toMonitoringType(s string) (MonitoringType, error) { + switch strings.ToLower(s) { + case "none": + return None, nil + case "memoryonly": + return MemoryOnly, nil + case "all": + return All, nil + } + + return None, fmt.Errorf("invalid monitoring type %v", s) +} + // String returns LaunchPolicy details. func (p policy) String() string { switch p { @@ -57,10 +84,12 @@ func toPolicy(policy, s string) (policy, error) { } const ( - envOverride = "tee.launch_policy.allow_env_override" - cmdOverride = "tee.launch_policy.allow_cmd_override" - logRedirect = "tee.launch_policy.log_redirect" - memoryMonitoring = "tee.launch_policy.monitoring_memory_allow" + envOverride = "tee.launch_policy.allow_env_override" + cmdOverride = "tee.launch_policy.allow_cmd_override" + logRedirect = "tee.launch_policy.log_redirect" + memoryMonitoring = "tee.launch_policy.monitoring_memory_allow" + hardenedMonitoring = "tee.launch_policy.hardened_monitoring" + debugMonitoring = "tee.launch_policy.debug_monitoring" // Values look like a PATH list, with ':' as a separator. // Empty paths will be ignored and relative paths will be interpreted as // relative to "/". @@ -68,9 +97,69 @@ const ( mountDestinations = "tee.launch_policy.allow_mount_destinations" ) +func configureMonitoringPolicy(imageLabels map[string]string, launchPolicy *LaunchPolicy, logger *log.Logger) error { + // Old policy. + memVal, memOk := imageLabels[memoryMonitoring] + // New policies. + hardenedVal, hardenedOk := imageLabels[hardenedMonitoring] + debugVal, debugOk := imageLabels[debugMonitoring] + + var err error + + // Return an error if old/new policies are both defined + if memOk && (hardenedOk || debugOk) { + return fmt.Errorf("use either %s or %s/%s in image labels,- not both", memoryMonitoring, hardenedMonitoring, debugMonitoring) + } else if memOk { + policy, err := toPolicy(memoryMonitoring, memVal) + if err != nil { + return fmt.Errorf("invalid image LABEL '%s'", memoryMonitoring) + } + + logger.Printf("%s will be deprecated, use %s and %s instead", memoryMonitoring, hardenedMonitoring, debugMonitoring) + + switch policy { + case always: + logger.Printf("%s=always will be treated as %s=memory_only and %s=memory_only", memoryMonitoring, hardenedMonitoring, debugMonitoring) + launchPolicy.HardenedImageMonitoring = MemoryOnly + launchPolicy.DebugImageMonitoring = MemoryOnly + case never: + logger.Printf("%s=never will be treated as %s=none and %s=none", memoryMonitoring, hardenedMonitoring, debugMonitoring) + logger.Printf("memory monitoring not allowed by image") + launchPolicy.HardenedImageMonitoring = None + launchPolicy.DebugImageMonitoring = None + case debugOnly: + logger.Printf("%s=debug_only will be treated as %s=none and %s=memory", memoryMonitoring, hardenedMonitoring, debugMonitoring) + logger.Printf("memory monitoring only allowed on debug environment by image") + launchPolicy.HardenedImageMonitoring = None + launchPolicy.DebugImageMonitoring = MemoryOnly + } + return nil + } + + if hardenedOk { + launchPolicy.HardenedImageMonitoring, err = toMonitoringType(hardenedVal) + if err != nil { + return fmt.Errorf("invalid monitoring type for hardened image: %v", err) + } + } else { + launchPolicy.HardenedImageMonitoring = None + } + + if debugOk { + launchPolicy.DebugImageMonitoring, err = toMonitoringType(debugVal) + if err != nil { + return fmt.Errorf("invalid monitoring type for debug image: %v", err) + } + } else { + launchPolicy.DebugImageMonitoring = MemoryOnly + } + + return nil +} + // GetLaunchPolicy takes in a map[string] string which should come from image labels, // and will try to parse it into a LaunchPolicy. Extra fields will be ignored. -func GetLaunchPolicy(imageLabels map[string]string) (LaunchPolicy, error) { +func GetLaunchPolicy(imageLabels map[string]string, logger *log.Logger) (LaunchPolicy, error) { var err error launchPolicy := LaunchPolicy{} if v, ok := imageLabels[envOverride]; ok { @@ -97,12 +186,8 @@ func GetLaunchPolicy(imageLabels map[string]string) (LaunchPolicy, error) { } } - // default is debug only for memoryMonitoring - if v, ok := imageLabels[memoryMonitoring]; ok { - launchPolicy.AllowedMemoryMonitoring, err = toPolicy(memoryMonitoring, v) - if err != nil { - return LaunchPolicy{}, fmt.Errorf("invalid image LABEL '%s'; contact the image author", memoryMonitoring) - } + if err := configureMonitoringPolicy(imageLabels, &launchPolicy, logger); err != nil { + return LaunchPolicy{}, err } if v, ok := imageLabels[mountDestinations]; ok { @@ -120,6 +205,26 @@ func GetLaunchPolicy(imageLabels map[string]string) (LaunchPolicy, error) { return launchPolicy, nil } +func verifyMonitoringConfig(policy MonitoringType, spec MonitoringType) error { + switch policy { + case All: + // If policy is 'All', spec can be anything. + return nil + case MemoryOnly: + // If policy is 'MemoryOnly', spec must be 'None' or 'MemoryOnly'. + if spec == All { + return fmt.Errorf("spec configured for all monitoring, policy only allows memory") + } + case None: + // If policy is 'None', spec must also be 'None'. + if spec != None { + return fmt.Errorf("spec configured for %v but policy is none", spec) + } + } + + return nil +} + // Verify will use the LaunchPolicy to verify the given LaunchSpec. If the verification passed, will return nil. // If there are multiple violations, the function will return the first error. func (p LaunchPolicy) Verify(ls LaunchSpec) error { @@ -140,12 +245,13 @@ func (p LaunchPolicy) Verify(ls LaunchSpec) error { return fmt.Errorf("logging redirection only allowed on debug environment by image") } - if p.AllowedMemoryMonitoring == never && ls.MemoryMonitoringEnabled { - return fmt.Errorf("memory monitoring not allowed by image") + monitoringPolicy := p.DebugImageMonitoring + if ls.Hardened { + monitoringPolicy = p.HardenedImageMonitoring } - if p.AllowedMemoryMonitoring == debugOnly && ls.MemoryMonitoringEnabled && ls.Hardened { - return fmt.Errorf("memory monitoring only allowed on debug environment by image") + if err := verifyMonitoringConfig(monitoringPolicy, ls.MonitoringEnabled); err != nil { + return fmt.Errorf("error verifying monitoring config: %v", err) } var err error diff --git a/launcher/spec/launch_policy_test.go b/launcher/spec/launch_policy_test.go index dc9382724..9d4fdaf31 100644 --- a/launcher/spec/launch_policy_test.go +++ b/launcher/spec/launch_policy_test.go @@ -1,6 +1,7 @@ package spec import ( + "log" "testing" "github.com/google/go-cmp/cmp" @@ -57,7 +58,11 @@ func TestLaunchPolicy(t *testing.T) { for _, testcase := range testCases { t.Run(testcase.testName, func(t *testing.T) { - got, err := GetLaunchPolicy(testcase.imageLabels) + // Add default values for policy fields. Not relevant to tested behavior. + testcase.expectedPolicy.HardenedImageMonitoring = None + testcase.expectedPolicy.DebugImageMonitoring = MemoryOnly + + got, err := GetLaunchPolicy(testcase.imageLabels, log.Default()) if err != nil { t.Fatal(err) } @@ -82,13 +87,14 @@ func TestVerify(t *testing.T) { AllowedEnvOverride: []string{"foo"}, AllowedCmdOverride: true, AllowedLogRedirect: always, - AllowedMemoryMonitoring: always, + HardenedImageMonitoring: MemoryOnly, + DebugImageMonitoring: MemoryOnly, }, LaunchSpec{ - Envs: []EnvVar{{Name: "foo", Value: "foo"}}, - Cmd: []string{"foo"}, - LogRedirect: Everywhere, - MemoryMonitoringEnabled: true, + Envs: []EnvVar{{Name: "foo", Value: "foo"}}, + Cmd: []string{"foo"}, + LogRedirect: Everywhere, + MonitoringEnabled: MemoryOnly, }, false, }, @@ -118,150 +124,6 @@ func TestVerify(t *testing.T) { }, true, }, - { - "memory monitor (never, enable, hardened): err", - LaunchPolicy{ - AllowedMemoryMonitoring: never, - }, - LaunchSpec{ - MemoryMonitoringEnabled: true, - Hardened: true, - LogRedirect: Nowhere, - }, - true, - }, - { - "memory monitor (never, enable, debug): err", - LaunchPolicy{ - AllowedMemoryMonitoring: never, - }, - LaunchSpec{ - MemoryMonitoringEnabled: true, - Hardened: false, - LogRedirect: Nowhere, - }, - true, - }, - { - "memory monitor (never, disable, hardened): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: never, - }, - LaunchSpec{ - MemoryMonitoringEnabled: false, - Hardened: true, - LogRedirect: Nowhere, - }, - false, - }, - { - "memory monitor (never, disable, debug): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: never, - }, - LaunchSpec{ - MemoryMonitoringEnabled: false, - Hardened: false, - LogRedirect: Nowhere, - }, - false, - }, - { - "memory monitor (debugonly, enable, hardened): err", - LaunchPolicy{ - AllowedMemoryMonitoring: debugOnly, - }, - LaunchSpec{ - MemoryMonitoringEnabled: true, - Hardened: true, - LogRedirect: Nowhere, - }, - true, - }, - { - "memory monitor (debugonly, enable, debug): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: debugOnly, - }, - LaunchSpec{ - MemoryMonitoringEnabled: true, - Hardened: false, - LogRedirect: Nowhere, - }, - false, - }, - { - "memory monitor (debugonly, disable, hardened): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: debugOnly, - }, - LaunchSpec{ - MemoryMonitoringEnabled: false, - Hardened: true, - LogRedirect: Nowhere, - }, - false, - }, - { - "memory monitor (debugonly, disable, debug): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: debugOnly, - }, - LaunchSpec{ - MemoryMonitoringEnabled: false, - Hardened: false, - LogRedirect: Nowhere, - }, - false, - }, - { - "memory monitor (always, enable, hardened): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: always, - }, - LaunchSpec{ - MemoryMonitoringEnabled: true, - Hardened: true, - LogRedirect: Nowhere, - }, - false, - }, - { - "memory monitor (always, enable, debug): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: always, - }, - LaunchSpec{ - MemoryMonitoringEnabled: true, - Hardened: false, - LogRedirect: Nowhere, - }, - false, - }, - { - "memory monitor (always, disable, hardened): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: always, - }, - LaunchSpec{ - MemoryMonitoringEnabled: false, - Hardened: true, - LogRedirect: Nowhere, - }, - false, - }, - { - "memory monitor (always, disable, debug): noerr", - LaunchPolicy{ - AllowedMemoryMonitoring: always, - }, - LaunchSpec{ - MemoryMonitoringEnabled: false, - Hardened: false, - LogRedirect: Nowhere, - }, - false, - }, { "log redirect (never, everywhere, hardened): err", LaunchPolicy{ @@ -648,6 +510,153 @@ func TestVerify(t *testing.T) { } } +func TestVerifyMonitoringSettings(t *testing.T) { + testCases := []struct { + testName string + monitoring MonitoringType + spec LaunchSpec + }{ + { + "none policy, disabled by spec", + None, + LaunchSpec{ + MonitoringEnabled: None, + LogRedirect: Nowhere, + }, + }, + { + "memory-only policy, all disabled by spec", + MemoryOnly, + LaunchSpec{ + MonitoringEnabled: None, + LogRedirect: Nowhere, + }, + }, + { + "memory-only policy, memory enabled by spec", + MemoryOnly, + LaunchSpec{ + MonitoringEnabled: MemoryOnly, + LogRedirect: Nowhere, + }, + }, + { + "all enabled by policy, all enabled by spec", + All, + LaunchSpec{ + MonitoringEnabled: All, + LogRedirect: Nowhere, + }, + }, + { + "all enabled by policy, disabled by spec", + All, + LaunchSpec{ + MonitoringEnabled: None, + LogRedirect: Nowhere, + }, + }, + { + "all enabled by policy, memory enabled by spec", + All, + LaunchSpec{ + MonitoringEnabled: MemoryOnly, + LogRedirect: Nowhere, + }, + }, + } + + for _, testCase := range testCases { + // Debug. + t.Run("[Debug] "+testCase.testName, func(t *testing.T) { + policy := LaunchPolicy{ + DebugImageMonitoring: testCase.monitoring, + } + if err := policy.Verify(testCase.spec); err != nil { + t.Errorf("expected no error, but got %v", err) + } + }) + + // Hardened. + t.Run("[Hardened] "+testCase.testName, func(t *testing.T) { + policy := LaunchPolicy{ + HardenedImageMonitoring: testCase.monitoring, + } + + // Copy the spec and set Hardened=true. + spec := testCase.spec + spec.Hardened = true + if err := policy.Verify(spec); err != nil { + t.Errorf("expected no error, but got %v", err) + } + }) + } +} + +func TestVerifyMonitoringSettingsErrors(t *testing.T) { + testCases := []struct { + testName string + monitoring MonitoringType + spec LaunchSpec + }{ + { + "[Hardened] disabled policy, all enabled by spec", + None, + LaunchSpec{ + MonitoringEnabled: All, + Hardened: true, + LogRedirect: Nowhere, + }, + }, + { + "[Hardened] disabled policy, memory enabled by spec", + None, + LaunchSpec{ + MonitoringEnabled: MemoryOnly, + Hardened: true, + LogRedirect: Nowhere, + }, + }, + { + "[Hardened] memory-only policy, all enabled by spec", + MemoryOnly, + LaunchSpec{ + MonitoringEnabled: All, + Hardened: true, + LogRedirect: Nowhere, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.testName, func(t *testing.T) { + // Debug. + t.Run("[Debug] "+testCase.testName, func(t *testing.T) { + policy := LaunchPolicy{ + DebugImageMonitoring: testCase.monitoring, + } + if err := policy.Verify(testCase.spec); err == nil { + t.Errorf("expected error, but got nil") + } + }) + + // Hardened. + t.Run("[Hardened] "+testCase.testName, func(t *testing.T) { + policy := LaunchPolicy{ + HardenedImageMonitoring: testCase.monitoring, + } + + // Copy the spec and set Hardened=true. + spec := testCase.spec + spec.Hardened = true + if err := policy.Verify(spec); err == nil { + t.Errorf("expected error, but got nil") + } + }) + }) + } +} + func TestIsHardened(t *testing.T) { testCases := []struct { testName string @@ -685,3 +694,185 @@ func TestIsHardened(t *testing.T) { }) } } + +func TestGetMonitoringPolicy(t *testing.T) { + testcases := []struct { + name string + labels map[string]string + expectedPolicy *LaunchPolicy + }{ + { + name: "memory_monitoring_allow=always", + labels: map[string]string{ + memoryMonitoring: "always", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: MemoryOnly, + DebugImageMonitoring: MemoryOnly, + }, + }, + { + name: "memory_monitoring_allow=never", + labels: map[string]string{ + memoryMonitoring: "never", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: None, + DebugImageMonitoring: None, + }, + }, + { + name: "memory_monitoring_allow=debugonly", + labels: map[string]string{ + memoryMonitoring: "debugonly", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: None, + DebugImageMonitoring: MemoryOnly, + }, + }, + { + name: "HardenedImageMonitoring=none", + labels: map[string]string{ + hardenedMonitoring: "none", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: None, + DebugImageMonitoring: MemoryOnly, + }, + }, + { + name: "HardenedImageMonitoring=memoryonly", + labels: map[string]string{ + hardenedMonitoring: "memoryonly", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: MemoryOnly, + DebugImageMonitoring: MemoryOnly, + }, + }, + { + name: "HardenedImageMonitoring=all", + labels: map[string]string{ + hardenedMonitoring: "all", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: All, + DebugImageMonitoring: MemoryOnly, + }, + }, + { + name: "DebugImageMonitoring=none", + labels: map[string]string{ + debugMonitoring: "none", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: None, + DebugImageMonitoring: None, + }, + }, + { + name: "DebugImageMonitoring=memoryonly", + labels: map[string]string{ + debugMonitoring: "memoryonly", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: None, + DebugImageMonitoring: MemoryOnly, + }, + }, + { + name: "DebugImageMonitoring=all", + labels: map[string]string{ + debugMonitoring: "all", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: None, + DebugImageMonitoring: All, + }, + }, + // Set both fields to non-default values. + { + name: "HardenedImageMonitoring=all, DebugImageMonitoring=none", + labels: map[string]string{ + hardenedMonitoring: "all", + debugMonitoring: "none", + }, + expectedPolicy: &LaunchPolicy{ + HardenedImageMonitoring: All, + DebugImageMonitoring: None, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + policy := &LaunchPolicy{} + if err := configureMonitoringPolicy(tc.labels, policy, log.Default()); err != nil { + t.Errorf("getMonitoringPolicy returned error: %v", err) + return + } + + if !cmp.Equal(policy, tc.expectedPolicy) { + t.Errorf("getMonitoringPolicy did not return expected policy: got %v, want %v", policy, tc.expectedPolicy) + } + }) + } +} + +func TestGetMonitoringPolicyErrors(t *testing.T) { + testcases := []struct { + name string + labels map[string]string + }{ + { + name: "memory_monitoring_allow and hardened_monitoring specified", + labels: map[string]string{ + memoryMonitoring: "always", + hardenedMonitoring: "all", + }, + }, + { + name: "memory_monitoring_allow and debug_monitoring specified", + labels: map[string]string{ + memoryMonitoring: "always", + debugMonitoring: "all", + }, + }, + { + name: "memory_monitoring_allow, hardened_monitoring, and debug_monitoring specified", + labels: map[string]string{ + memoryMonitoring: "always", + hardenedMonitoring: "all", + debugMonitoring: "memoryOnly", + }, + }, + { + name: "invalid value for memory_monitoring_allow", + labels: map[string]string{ + memoryMonitoring: "this is not valid", + }, + }, + { + name: "invalid value for hardened_monitoring", + labels: map[string]string{ + hardenedMonitoring: "this is not valid", + }, + }, + { + name: "invalid value for debug_monitoring", + labels: map[string]string{ + debugMonitoring: "this is not valid", + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + policy := &LaunchPolicy{} + if err := configureMonitoringPolicy(tc.labels, policy, log.Default()); err == nil { + t.Errorf("Expected getMonitoringPolicy to return error, returned successfully with policy %v", policy) + } + }) + } +} diff --git a/launcher/spec/launch_spec.go b/launcher/spec/launch_spec.go index 9c5f13ff1..c3580cea8 100644 --- a/launcher/spec/launch_spec.go +++ b/launcher/spec/launch_spec.go @@ -83,6 +83,7 @@ const ( attestationServiceAddrKey = "tee-attestation-service-endpoint" logRedirectKey = "tee-container-log-redirect" memoryMonitoringEnable = "tee-monitoring-memory-enable" + monitoringEnable = "tee-monitoring-enable" devShmSizeKey = "tee-dev-shm-size-kb" mountKey = "tee-mount" ) @@ -113,7 +114,7 @@ type LaunchSpec struct { ProjectID string Region string Hardened bool - MemoryMonitoringEnabled bool + MonitoringEnabled MonitoringType LogRedirect LogRedirectLocation Mounts []launchermount.Mount // DevShmSize is specified in kiB. @@ -154,9 +155,38 @@ func (s *LaunchSpec) UnmarshalJSON(b []byte) error { s.SignedImageRepos = append(s.SignedImageRepos, imageRepos...) } - if val, ok := unmarshaledMap[memoryMonitoringEnable]; ok && val != "" { - if boolValue, err := strconv.ParseBool(val); err == nil { - s.MemoryMonitoringEnabled = boolValue + memVal, memOk := unmarshaledMap[memoryMonitoringEnable] + monVal, monOk := unmarshaledMap[monitoringEnable] + + if memOk && monOk { + return fmt.Errorf("both %v and %v are specified, only one is permitted", memoryMonitoringEnable, monitoringEnable) + } else if memOk { + // If value is empty, treat as the default. + if memVal == "" { + s.MonitoringEnabled = None + } else { + boolValue, err := strconv.ParseBool(memVal) + if err != nil { + return fmt.Errorf("invalid value for %v (not a boolean): %v", memoryMonitoringEnable, err) + } + + if boolValue { + s.MonitoringEnabled = MemoryOnly + } else { + s.MonitoringEnabled = None + } + } + } else if monOk { + // If value is empty, treat as the default. + if monVal == "" { + s.MonitoringEnabled = None + } else { + + var err error + s.MonitoringEnabled, err = toMonitoringType(monVal) + if err != nil { + return err + } } } diff --git a/launcher/spec/launch_spec_test.go b/launcher/spec/launch_spec_test.go index 463de3ef6..ae8acb2f5 100644 --- a/launcher/spec/launch_spec_test.go +++ b/launcher/spec/launch_spec_test.go @@ -56,7 +56,7 @@ func TestLaunchSpecUnmarshalJSONHappyCases(t *testing.T) { Envs: []EnvVar{{"foo", "bar"}}, ImpersonateServiceAccounts: []string{"sv1@developer.gserviceaccount.com", "sv2@developer.gserviceaccount.com"}, LogRedirect: Everywhere, - MemoryMonitoringEnabled: true, + MonitoringEnabled: MemoryOnly, DevShmSize: 234234, Mounts: []launchermount.Mount{launchermount.TmpfsMount{Destination: "/tmpmount", Size: 0}, launchermount.TmpfsMount{Destination: "/sized", Size: 222}}, @@ -120,6 +120,13 @@ func TestLaunchSpecUnmarshalJSONBadInput(t *testing.T) { "tee-container-log-redirect":"badideas", }`, }, + { + "Memory and Health Monitoring both specified", + `{ + "tee-monitoring-memory-enable":"false", + "tee-monitoring-health-enable":"false", + }`, + }, } for _, testcase := range testCases { @@ -149,10 +156,10 @@ func TestLaunchSpecUnmarshalJSONWithDefaultValue(t *testing.T) { } want := &LaunchSpec{ - ImageRef: "docker.io/library/hello-world:latest", - RestartPolicy: Never, - LogRedirect: Nowhere, - MemoryMonitoringEnabled: false, + ImageRef: "docker.io/library/hello-world:latest", + RestartPolicy: Never, + LogRedirect: Nowhere, + MonitoringEnabled: None, } if !cmp.Equal(spec, want) {