Add memory monitor measurement logics #408

yawangwang · 2024-01-23T01:00:15Z

This PR removes the existing experiment flag EnableMemoryMonitoring and add a new experiment flag EnableMeasureMemoryMonitor to gate the memory monitoring measurement logics.

jkl73 · 2024-01-31T23:57:58Z

launcher/container_runner.go

+	}
+
+	var memoryMonitoringEnabledBit uint8
+	if status == "active" {


I'd also including "activating" status here

jkl73 · 2024-02-01T00:02:16Z

launcher/container_runner.go

+// measureMemoryMonitor will measure the current memory monitoring state into the COS
+// eventlog in the AttestationAgent.
+func (r *ContainerRunner) measureMemoryMonitor(ctx context.Context) error {
+	s, err := systemctl.New()


probably don't need to create a new systemctl client here, you can try to pass it in as a parameter.

Alternatively, try to get the MemoryMonitor status outside of this function

Discussed offline, we should measure the memory monitoring decision made by container launcher.
Done.

alexmwu · 2024-02-06T19:58:18Z

launcher/container_runner.go

+		EventType:    cel.LaunchSeparatorType,
+		EventContent: nil, // Success
+	}
+	return r.attestAgent.MeasureEvent(separator)


Not necessarily related to this PR, but we need a unit test to ensure the separator always gets measured. Since you moved this around, can you add one for measureCELEvents? Perhaps also ensure we get the workload digest and ID

Ideally we have a test for different launchspec inputs if that doesn't exist.

Done, created a fake container and fake container image to "fake" dependencies that measureCELEvents need. Added a unit test to container_runnter_test.go for different launchspec inputs.

alexmwu · 2024-02-06T20:03:28Z

launcher/internal/systemctl/systemctl.go

@@ -42,6 +43,19 @@ func (s *Systemctl) Stop(unit string) error {
 	return runSystemdCmd(s.dbus.StopUnitContext, "stop", unit)
 }

+// GetStatus is the equivalent of `systemctl is-active $unit`.
+// The status can be "active", "activating", "deactivating", "inactive" or "failed".
+func (s *Systemctl) GetStatus(ctx context.Context, unit string) (string, error) {


IMO this should be called IsActive so that it matches the systemctl API.

alexmwu · 2024-02-07T23:15:07Z

launcher/container_runner_test.go

+			}
+
+			for _, wantEvent := range tc.wantCELEvents {
+				if !slices.Contains(gotEvents, cel.CosType(wantEvent)) {


Order matters here, at least for the separator which should be last. Use cmp.Equal.

alexmwu · 2024-02-07T23:15:22Z

launcher/container_runner.go

+	if err := r.measureCELEvents(ctx); err != nil {
+		return fmt.Errorf("failed to measure CEL events: %v", err)
+	}
+
+	if err := r.fetchAndWriteToken(ctx); err != nil {
+		return fmt.Errorf("failed to fetch and write OIDC token: %v", err)


Measurements should happen before experiments turn on and potentially introduce side effects. So please move this back to the beginning of Run. We should measure first, and it's okay to enable memory monitoring after, even if it fails.

alexmwu · 2024-02-07T23:16:55Z

launcher/container_runner_test.go

+		},
+		{
+			name:          "measure memory monitoring event and launch separator event",
+			wantCELEvents: []cel.CosType{cel.LaunchSeparatorType, cel.MemoryMonitorType},


wantCELEvents should have the separator last.

yawangwang · 2024-02-08T00:20:44Z

/gcbrun

@KeithMoyer

New Features: [launcher] Add TEE server IPC implementation google#367 [launcher] Enable memory monitoring in CS google#391 Use TDX quote provider to attest and verify google#405 Integrate nonce verification as part of the TDX quote validation procedure. google#395 Add RISC V support google#407 [launcher] Use resizable integrity-fs with in-memory tags google#412 Bug Fixes: [launcher] Fix launcher exit code google#384 [launcher] Handle exit code checking during deferral evaluation google#392 [cmd] Skip tests that call setGCEAKTemplate google#402 [launcher] Fix teeserver context reset issue & add container signature cache google#397 Set all unused parameters as _ to fix CI lint failure google#411 [launcher] Make customtoken test sleep to mitigate clock skew google#413 Other Changes: Add eventlog parse logics for memory monitoring google#404 [launcher]: Add memory monitor measurement logics google#408 Update go-tdx-guest version to v0.3.1 google#414 New Contributors: @KeithMoyer in google#392 @vbalain in google#405 @aimixsaka in google#407

@KeithMoyer

New Features: [launcher] Add TEE server IPC implementation #367 [launcher] Enable memory monitoring in CS #391 Use TDX quote provider to attest and verify #405 Integrate nonce verification as part of the TDX quote validation procedure. #395 Add RISC V support #407 [launcher] Use resizable integrity-fs with in-memory tags #412 Bug Fixes: [launcher] Fix launcher exit code #384 [launcher] Handle exit code checking during deferral evaluation #392 [cmd] Skip tests that call setGCEAKTemplate #402 [launcher] Fix teeserver context reset issue & add container signature cache #397 Set all unused parameters as _ to fix CI lint failure #411 [launcher] Make customtoken test sleep to mitigate clock skew #413 Other Changes: Add eventlog parse logics for memory monitoring #404 [launcher]: Add memory monitor measurement logics #408 Update go-tdx-guest version to v0.3.1 #414 New Contributors: @KeithMoyer in #392 @vbalain in #405 @aimixsaka in #407

@KeithMoyer

New Features: [launcher] Add TEE server IPC implementation google#367 [launcher] Enable memory monitoring in CS google#391 Use TDX quote provider to attest and verify google#405 Integrate nonce verification as part of the TDX quote validation procedure. google#395 Add RISC V support google#407 [launcher] Use resizable integrity-fs with in-memory tags google#412 Bug Fixes: [launcher] Fix launcher exit code google#384 [launcher] Handle exit code checking during deferral evaluation google#392 [cmd] Skip tests that call setGCEAKTemplate google#402 [launcher] Fix teeserver context reset issue & add container signature cache google#397 Set all unused parameters as _ to fix CI lint failure google#411 [launcher] Make customtoken test sleep to mitigate clock skew google#413 Other Changes: Add eventlog parse logics for memory monitoring google#404 [launcher]: Add memory monitor measurement logics google#408 Update go-tdx-guest version to v0.3.1 google#414 New Contributors: @KeithMoyer in google#392 @vbalain in google#405 @aimixsaka in google#407

yawangwang force-pushed the add-mem-monitor-measurement branch 5 times, most recently from 15808b4 to cdd35d8 Compare January 24, 2024 01:11

yawangwang marked this pull request as ready for review January 25, 2024 01:29

yawangwang requested review from jkl73 and alexmwu January 25, 2024 01:29

jkl73 reviewed Feb 1, 2024

View reviewed changes

yawangwang force-pushed the add-mem-monitor-measurement branch from cdd35d8 to bfba9f6 Compare February 1, 2024 18:39

yawangwang requested a review from jkl73 February 1, 2024 18:43

yawangwang force-pushed the add-mem-monitor-measurement branch 5 times, most recently from abe77fe to 4bf5512 Compare February 2, 2024 18:18

jkl73 approved these changes Feb 6, 2024

View reviewed changes

alexmwu reviewed Feb 6, 2024

View reviewed changes

yawangwang force-pushed the add-mem-monitor-measurement branch 3 times, most recently from 6d816fb to f5c6251 Compare February 7, 2024 01:52

yawangwang requested a review from alexmwu February 7, 2024 01:52

Adding mem monitoring measurement

1c542f2

yawangwang force-pushed the add-mem-monitor-measurement branch from f5c6251 to 1c542f2 Compare February 7, 2024 01:55

alexmwu reviewed Feb 7, 2024

View reviewed changes

yawangwang requested a review from alexmwu February 7, 2024 23:43

alexmwu approved these changes Feb 8, 2024

View reviewed changes

Adding unit test for measureCELEvents

07ce3e5

yawangwang force-pushed the add-mem-monitor-measurement branch from 34f61ae to 07ce3e5 Compare February 8, 2024 00:19

jkl73 approved these changes Feb 8, 2024

View reviewed changes

yawangwang merged commit f1aa3d2 into google:main Feb 9, 2024
11 checks passed

alexmwu mentioned this pull request Feb 22, 2024

Release v0.4.3 #416

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add memory monitor measurement logics #408

Add memory monitor measurement logics #408

yawangwang commented Jan 23, 2024 •

edited

jkl73 Jan 31, 2024

yawangwang Feb 1, 2024

jkl73 Feb 1, 2024

jkl73 Feb 1, 2024

yawangwang Feb 1, 2024 •

edited

alexmwu Feb 6, 2024

alexmwu Feb 6, 2024

yawangwang Feb 7, 2024

alexmwu Feb 6, 2024

yawangwang Feb 7, 2024

alexmwu Feb 7, 2024

yawangwang Feb 7, 2024

alexmwu Feb 7, 2024 •

edited

yawangwang Feb 7, 2024

alexmwu Feb 7, 2024

alexmwu Feb 7, 2024

yawangwang Feb 7, 2024

yawangwang commented Feb 8, 2024

Add memory monitor measurement logics #408

Add memory monitor measurement logics #408

Conversation

yawangwang commented Jan 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yawangwang Feb 1, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexmwu Feb 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yawangwang commented Feb 8, 2024

yawangwang commented Jan 23, 2024 •

edited

yawangwang Feb 1, 2024 •

edited

alexmwu Feb 7, 2024 •

edited