[KeyManager] Implement FFI for KEM and binding key generation by NilanjanDaw · Pull Request #645 · google/go-tpm-tools

NilanjanDaw · 2026-02-06T11:47:30Z

Introduces FFI-accessible functions for generating and managing cryptographic keys within the Key Protection Service (KPS) and Workload Service (WS).
Key Changes:

FFI Compatibility: Configured prost_build in km_common to apply #[repr(C)] to the HpkeAlgorithm struct, enabling its use in C-style interfaces.
Key Generation Logic: Added create_key_record in km_common to centralize secure keypair generation (X25519), ensuring private keys are immediately moved to secure Vault storage and zeroized from temporary memory.
Key Management: Implemented KeyRegistry and a global registry instance in both KPS and WS to track generated keys by UUID.
FFI Exports:
- Added key_manager_generate_kem_keypair to KPS for generating KEM keys with associated binding public keys.
- Added key_manager_generate_binding_keypair to WS for generating standard binding keys.
Testing: Added comprehensive unit tests for FFI functions, including error handling for invalid algorithms and null pointers.

This PR is built on top of PR #643 [KeyManager] Add HPKE and DHKEM crypto primitives.
Please review commit #38731051 onwards

keymanager/key_protection_service/key_custody_core/src/lib.rs

keymanager/km_common/src/key_types.rs

keymanager/km_common/src/protected_mem.rs

keymanager/key_protection_service/key_custody_core/src/lib.rs

clundin25 · 2026-02-12T20:42:26Z

keymanager/key_protection_service/key_custody_core/src/lib.rs

+                if out_pubkey.is_null() || out_pubkey_len.is_null() || out_uuid.is_null() {
+                    return -1;
+                }


Nit: Seems like all the parameter validation could be moved to the very top, I think that would make it easier to read.

Similarly, it's good to isolate the unsafe code as much as possible so it can be easy to see where the unsafe review should be focused. This check is all safe code so it would be good to remove it from the unsafe block.

Ack, thank you, this is a good advice. Updated the function to separate the safety checks and the business logic, and also tried to reduce the scope of the unsafe blocks.

clundin25 · 2026-02-12T20:44:21Z

keymanager/key_protection_service/key_custody_core/src/lib.rs

+pub unsafe extern "C" fn key_manager_generate_kem_keypair(
+    algo: HpkeAlgorithm,
+    binding_pubkey: *const u8,
+    binding_pubkey_len: usize,
+    expiry_secs: u64,
+    out_uuid: *mut u8,
+    out_pubkey: *mut u8,
+    out_pubkey_len: *mut usize,
+) -> i32 {


Right now this function is mixing business logic & FFI safety invariants.

I recommend checking safety invariants (null ptr, buffer size, etc) in this function, converting it into safe types, and calling a safe function that operates on the sanitized data.

That will reduce the surface area of this FFI boundary.

Thank you, make sense, updated the function to separate the safety checks and the business logic, and also tried to reduce the scope of the unsafe blocks.

keymanager/key_protection_service/key_custody_core/src/lib.rs

clundin25 · 2026-02-12T20:57:10Z

keymanager/workload_service/key_custody_core/src/lib.rs

+pub unsafe extern "C" fn key_manager_generate_binding_keypair(
+    algo: HpkeAlgorithm,
+    expiry_secs: u64,
+    out_uuid: *mut u8,
+    out_pubkey: *mut u8,
+    out_pubkey_len: *mut usize,
+) -> i32 {


Same comment on mixing business & validation logic

Thank you, make sense, updated the function to separate the safety checks and the business logic, and also tried to reduce the scope of the unsafe blocks.

zhalvorsen · 2026-02-12T21:55:13Z

keymanager/key_protection_service/key_custody_core/src/lib.rs

+                if out_pubkey.is_null() || out_pubkey_len.is_null() || out_uuid.is_null() {
+                    return -1;
+                }


Similarly, it's good to isolate the unsafe code as much as possible so it can be easy to see where the unsafe review should be focused. This check is all safe code so it would be good to remove it from the unsafe block.

keymanager/km_common/src/crypto/secret_box.rs

bluegate010 · 2026-02-12T22:38:44Z

keymanager/km_common/src/crypto/secret_box.rs

This same file shows up as new in #643. Similar to other core files. Makes it a bit hard to keep track of which feedback has been given.

My apologies. The PRs are stacked which is making the diff difficult to review. The parent PR has been merged. So this one should now be easier to review.

Introduces FFI-accessible functions for generating and managing cryptographic keys within the Key Protection Service (KPS) and Workload Service (WS). Key Changes: - FFI Compatibility: Configured prost_build in km_common to apply #[repr(C)] to the HpkeAlgorithm struct, enabling its use in C-style interfaces. - Key Generation Logic: Added create_key_record in km_common to centralize secure keypair generation (X25519), ensuring private keys are immediately moved to secure Vault storage and zeroized from temporary memory. - Key Management: Implemented KeyRegistry and a global registry instance in both KPS and WS to track generated keys by UUID. - FFI Exports: - Added key_manager_generate_kem_keypair to KPS for generating KEM keys with associated binding public keys. - Added key_manager_generate_binding_keypair to WS for generating standard binding keys. - Testing: Added comprehensive unit tests for FFI functions, including error handling for invalid algorithms and null pointers.

Extend both key_manager_generate_binding_keypair (WSD KCC) and key_manager_generate_kem_keypair (KPS KCC) to also output the public key via out_pubkey/out_pubkey_len parameters. This enables the Go orchestration layer to pass the binding public key from WSD to KPS when generating KEM keypairs. Changes: - Rust FFI: add out_pubkey and out_pubkey_len params to both functions - Updated Rust tests for new signatures

…ling

- Updated FFI layers in key_protection_service and workload_service to use PublicKey::try_from and as_bytes(). - Refactored create_key_record to use PrivateKey::into_secret() for secure transition to protected memory. - Added into_secret() to PrivateKey and X25519PrivateKey to facilitate secure ownership transfer of key material. - Cleaned up unused zeroize imports and duplicate tests in km_common. - Made X25519PublicKey internal field pub(crate) for internal module compatibility.

keymanager/key_protection_service/key_custody_core/src/lib.rs

keymanager/km_common/src/crypto/x25519.rs

keymanager/km_common/src/crypto.rs

keymanager/key_protection_service/key_custody_core/src/lib.rs

keymanager/km_common/src/key_types.rs

clundin25 · 2026-02-13T20:35:40Z

keymanager/km_common/src/crypto/x25519.rs

 /// X25519-based public key implementation.
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct X25519PublicKey([u8; 32]);
+pub struct X25519PublicKey(pub(crate) [u8; 32]);


BTW, the spec calls for sanitizing the public key in 7.1.2.

This is already being done by BoringSSL, correct?

NilanjanDaw requested a review from atulpatildbz February 6, 2026 11:48

NilanjanDaw force-pushed the add-generatekem-ffi branch from 9a877ec to f6f14ae Compare February 6, 2026 13:16

atulpatildbz mentioned this pull request Feb 7, 2026

[keymanager/wsd] Add Go orchestration layer (KOL) for WSD key generation #648

Closed

NilanjanDaw force-pushed the add-generatekem-ffi branch 2 times, most recently from efc963e to 1cba7cd Compare February 8, 2026 20:17

This was referenced Feb 9, 2026

[keymanager/wsd] Add /keys:decaps KOL API with DecapAndSeal + Open orchestration #650

Open

[keymanager/wsd] Add Go orchestration layer (KOL) for WSD key generation #652

Merged

[keymanager/wsd] GET /v1/keys enumerate keys #653

Open

NilanjanDaw requested review from bluegate010, clundin25 and zhalvorsen February 9, 2026 13:26

NilanjanDaw force-pushed the add-generatekem-ffi branch from 1cba7cd to 3bb0553 Compare February 9, 2026 13:33

clundin25 reviewed Feb 9, 2026

View reviewed changes

keymanager/key_protection_service/key_custody_core/src/lib.rs Outdated Show resolved Hide resolved

NilanjanDaw force-pushed the add-generatekem-ffi branch from 3bb0553 to 69706f4 Compare February 9, 2026 19:08

zhalvorsen reviewed Feb 9, 2026

View reviewed changes

keymanager/km_common/src/key_types.rs Outdated Show resolved Hide resolved

keymanager/km_common/src/protected_mem.rs Outdated Show resolved Hide resolved

keymanager/key_protection_service/key_custody_core/src/lib.rs Outdated Show resolved Hide resolved

NilanjanDaw force-pushed the add-generatekem-ffi branch from 69706f4 to 13fbcf6 Compare February 11, 2026 19:00

NilanjanDaw requested review from clundin25 and zhalvorsen February 11, 2026 19:10

clundin25 reviewed Feb 12, 2026

View reviewed changes

keymanager/key_protection_service/key_custody_core/src/lib.rs Show resolved Hide resolved

clundin25 reviewed Feb 12, 2026

View reviewed changes

NilanjanDaw mentioned this pull request Feb 12, 2026

[KeyManager] Add key destruction FFI and background reaper #647

Merged

zhalvorsen reviewed Feb 12, 2026

View reviewed changes

bluegate010 reviewed Feb 12, 2026

View reviewed changes

NilanjanDaw and others added 5 commits February 13, 2026 10:19

Add API docs.

e918b36

improve unsafe handling via clippy checks.

319e9a1

addressed reviews. Added stricter zeroize rules, and better keys hand…

ed06332

…ling

NilanjanDaw added 2 commits February 13, 2026 10:19

simplify code and reduce nesting

74e1d5a

NilanjanDaw force-pushed the add-generatekem-ffi branch from 13fbcf6 to 74e1d5a Compare February 13, 2026 10:24

atulpatildbz mentioned this pull request Feb 13, 2026

[keymanager] Generate Rust FFI headers with cbindgen #655

Merged

tighten security around unsafe code

c5127ab

NilanjanDaw requested review from bluegate010, clundin25 and zhalvorsen February 13, 2026 16:04

zhalvorsen reviewed Feb 13, 2026

View reviewed changes

atulpatildbz requested changes Feb 13, 2026

View reviewed changes

cleanup code, address review comments

1985a01

NilanjanDaw requested review from atulpatildbz and zhalvorsen February 13, 2026 19:46

zhalvorsen approved these changes Feb 13, 2026

View reviewed changes

clundin25 reviewed Feb 13, 2026

View reviewed changes

clundin25 approved these changes Feb 13, 2026

View reviewed changes

atulpatildbz approved these changes Feb 14, 2026

View reviewed changes

NilanjanDaw merged commit e08f358 into google:main Feb 14, 2026
11 checks passed

Comments

Conversation

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

NilanjanDaw commented Feb 6, 2026 •

edited

Loading

bluegate010 Feb 12, 2026 •

edited

Loading