[keymanager/wsd] Add /keys:destroy Go KOL handler with KEM + binding key destruction by atulpatildbz · Pull Request #651 · google/go-tpm-tools

atulpatildbz · 2026-02-09T09:06:25Z

Implements the Go orchestration layer (KOL) for POST /v1/keys:destroy, the key destruction endpoint that allows workloads to explicitly destroy a key pair.

Flow:

Workload sends {key_handle: {handle: ...}} to WSD
WSD looks up binding UUID from the KEM→Binding map
WSD calls KPS DestroyKEMKey — destroys the KEM key from the registry
WSD calls WSD KCC DestroyBindingKey — destroys the binding key from the registry
WSD removes the KEM→Binding mapping
Returns 204 No Content

Changes:

C headers: Added key_manager_destroy_kem_key (KPS) and key_manager_destroy_binding_key (WSD) declarations
CGO bridges: Added DestroyKEMKey() and DestroyBindingKey() Go wrappers for the Rust FFI functions
KPS service: Extended Service with DestroyKEMKey method and KEMKeyDestroyer interface
WSD server: Added KEMKeyDestroyer/BindingKeyDestroyer interfaces, DestroyRequest type, handleDestroy handler, /v1/keys:destroy route registration
Tests: 7 new destroy handler tests + 2 new KPS service tests + Integration test TestIntegrationDestroyKey

Dependencies

This PR is built on top of:

PR [KeyManager] Add key destruction FFI and background reaper #647 (key_manager_destroy_ffi) — Rust FFI changes (Base commit: d1d6bad)
PR [keymanager/wsd] Add Go orchestration layer (KOL) for WSD key generation #652 (wsd_generate_key_go) — Included via merge (Commit: d7f52c6)

What to review

Please review the following commits:

d1d6bad — feat(keymanager): Add manual FFI headers for destroy (C headers for destroy)
60466aa — [keymanager/wsd] Add /keys:destroy endpoint with KEM + binding key destruction (Go implementation)

All other commits are from dependencies (PRs #647, #652) that are not yet merged to main but required for this build.

Verification

Automated Tests

Unit tests: go test ./keymanager/workload_service/... (Passed)
Integration tests: go test -tags=integration ./keymanager/workload_service/... (Passed)

Manual Verification (Curl)

Manually verified the destruction workflow locally:

Generate Key: POST /v1/keys:generate_kem -> Returns Handle.
Destroy Key (First): POST /v1/keys:destroy -> Returns 204.
Destroy Key (Second): POST /v1/keys:destroy -> Returns 404 (Key mapping removed).

atulpatildbz · 2026-02-26T19:23:50Z


 // NewServer creates a new WSD server with the given dependencies.
-func NewServer(bindingGen BindingKeyGenerator, kemGen KEMKeyGenerator) *Server {
+func NewServer(bindingGen BindingKeyGenerator, kemGen KEMKeyGenerator, kemKeyDestroyer KEMKeyDestroyer, bindingKeyDestroyer BindingKeyDestroyer) *Server {


this will need change after #678 is merged.
May have to rebase over that

…struction Implement the Go KOL handler for POST /keys:destroy that orchestrates the full key destruction flow: 1. Workload sends {kemKeyHandle} to WSD 2. WSD looks up binding UUID from KEM-to-binding map 3. WSD calls KPS DestroyKEMKey to destroy the KEM key 4. WSD calls WSD KCC DestroyBindingKey to destroy the binding key 5. WSD removes the KEM→Binding mapping 6. Returns 204 No Content Changes: - C headers: add key_manager_destroy_kem_key (KPS) and key_manager_destroy_binding_key (WSD) declarations - CGO bridges: add DestroyKEMKey and DestroyBindingKey Go wrappers - KPS service: extend with DestroyKEMKey method and KEMKeyDestroyer interface - WSD server: add KEMKeyDestroyer/BindingKeyDestroyer interfaces, DestroyRequest type, handleDestroy handler, /keys:destroy route - Tests: 7 new destroy handler tests + 2 new KPS service tests Remove DecapAndSeal and Open from KPS and WSD

NilanjanDaw · 2026-03-02T15:04:58Z

 // KEMKeyGenerator generates KEM keypairs linked to a binding public key.
+
+// KEMKeyDestroyer destroys a KEM key by UUID.
+type KEMKeyDestroyer interface {


Similar comment to PR #650: https://github.com/google/go-tpm-tools/pull/650/changes#r2872752692

NilanjanDaw · 2026-03-02T15:16:08Z

 		kemUUIDs[0], binding1, kemUUIDs[1], binding2)
 }
+
+func TestIntegrationDestroyKey(t *testing.T) {


Should we also add an integration test to validate the auto-destroy mechanism with this PR? And also verify that calling a destroy on a auto-destroyed key will not cause problems (since the kem to binding mapping will still exist in the KOL layer).

Done! I have added a new integration test TestIntegrationAutoDestroy

I also verified the behavior offline: when the KEM key and binding key expire and are auto-destroyed in the Rust KCC layer, an explicit call to /v1/keys:destroy does not cause any problems. The Rust backend returns success, allowing the Go KOL to cleanly remove the stale mapping from its kemToBindingMap and return an HTTP 204 No Content. The new integration test ensures this behavior won't regress.

Awesome, thank you.

NilanjanDaw · 2026-03-02T15:28:26Z

+
+	// Step 4: Remove the mapping.
+	s.mu.Lock()
+	delete(s.kemToBindingMap, kemUUID)


I am wondering if we should try to remove the kemUUID from the kemToBindingMap even in case of errors. For example, if for some reason DestroyKEMKey or DestroyBindingKey fails the delete call will not go through. A subsequent decap call on the same key will pass through since the mapping exist and fail in the Rust layer. Which is okay, but would it better to try to delete the mapping to avoid such a situation?

That makes sense.

I have updated the logic to ensure that both KEM and Binding keys are always attempted for destruction, and the mapping is always deleted from the map, regardless of individual failure outcomes. I've also updated server_test.go to explicitly verify this behavior.

Even if delete fails for some reason, the reaper would eventually delete expired keys right?

pgonda · 2026-03-02T20:41:08Z

+func DestroyKEMKey(kemUUID uuid.UUID) error {
+	uuidBytes := kemUUID[:]
+	rc := C.key_manager_destroy_kem_key(
+		(*C.uint8_t)(unsafe.Pointer(&uuidBytes[0])),


if rc:= ...; rc :=0 {
..
}

# Conflicts: # keymanager/key_protection_service/service.go # keymanager/workload_service/server.go # keymanager/workload_service/server_test.go

- fix: fix handleDestroy to prevent resource and memory leaks by always clearing the KeyManager mapping. - test: update handleDestroy tests to assert mapping deletion. - test: add integration test for auto-destroy mechanism.

NilanjanDaw · 2026-03-04T18:35:48Z

 		kemUUIDs[0], binding1, kemUUIDs[1], binding2)
 }
+
+func TestIntegrationDestroyKey(t *testing.T) {


Awesome, thank you.

NilanjanDaw · 2026-03-04T18:38:19Z

+	errKps := s.keyProtectionService.DestroyKEMKey(kemUUID)
+	errWs := s.workloadService.DestroyBindingKey(bindingUUID)
+
+	// Step 4: Remove the mapping.


I think this is Step 2 now

Removed Step from the comment as this comment pattern isn't used in our codebase

NilanjanDaw · 2026-03-04T18:45:19Z

+	if errKps != nil {
+		writeError(w, fmt.Sprintf("failed to destroy KEM key: %v", errKps), http.StatusInternalServerError)
+		return
+	}
+
+	if errWs != nil {
+		writeError(w, fmt.Sprintf("failed to destroy binding key: %v", errWs), http.StatusInternalServerError)
+		return
+	}


This works, but if the errKps fails, errWs is ignored. We can use errors.Join() to join the errors, so that both or either of them are surfaced.

- removed `Step <x>` comments - used errors.Join to collectively check of either KPS or WS destroy failed

atulpatildbz force-pushed the wsd_destroy_go branch 2 times, most recently from 60466aa to cff8e70 Compare February 18, 2026 07:07

atulpatildbz force-pushed the wsd_destroy_go branch 7 times, most recently from 4d76482 to 87f6a9f Compare February 26, 2026 19:21

atulpatildbz commented Feb 26, 2026

View reviewed changes

atulpatildbz requested a review from NilanjanDaw February 27, 2026 04:44

atulpatildbz force-pushed the wsd_destroy_go branch from 87f6a9f to dcac4cd Compare February 28, 2026 07:48

NilanjanDaw reviewed Mar 2, 2026

View reviewed changes

jkl73 requested a review from pgonda March 2, 2026 19:33

pgonda reviewed Mar 2, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into wsd_destroy_go

c5869ed

# Conflicts: # keymanager/key_protection_service/service.go # keymanager/workload_service/server.go # keymanager/workload_service/server_test.go

atulpatildbz force-pushed the wsd_destroy_go branch from 102c4f4 to 29cacc6 Compare March 3, 2026 13:49

atulpatildbz added 3 commits March 3, 2026 13:50

Merge remote-tracking branch 'origin/main' into wsd_destroy_go

7eb8206

add stub methods for destroy when CGO_ENABLED=0

b5b5b69

add comment for DestroyRequest

474bcda

atulpatildbz force-pushed the wsd_destroy_go branch 2 times, most recently from 9080eeb to 830a9ee Compare March 3, 2026 14:29

atulpatildbz force-pushed the wsd_destroy_go branch from 830a9ee to c45988c Compare March 3, 2026 14:47

atulpatildbz commented Mar 4, 2026

View reviewed changes

Comment thread keymanager/workload_service/server.go Outdated

cleanup: remove log statement

20b9472

atulpatildbz requested review from NilanjanDaw and pgonda March 4, 2026 15:54

NilanjanDaw approved these changes Mar 4, 2026

View reviewed changes

refactor: cleaner logs and errors.Join

edfea16

- removed `Step <x>` comments - used errors.Join to collectively check of either KPS or WS destroy failed

atulpatildbz merged commit 17d39d0 into google:main Mar 5, 2026
11 of 12 checks passed

BrewTestBot mentioned this pull request Mar 13, 2026

gotpm 0.4.8 Homebrew/homebrew-core#272213

Merged

Conversation

atulpatildbz commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependencies

What to review

Verification

Automated Tests

Manual Verification (Curl)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

atulpatildbz commented Feb 9, 2026 •

edited

Loading