Skip to content

Comments

[KeyManager] FFI for Enumerate keys#665

Open
atulpatildbz wants to merge 6 commits intogoogle:mainfrom
atulpatildbz:enumerate_keys_ffi
Open

[KeyManager] FFI for Enumerate keys#665
atulpatildbz wants to merge 6 commits intogoogle:mainfrom
atulpatildbz:enumerate_keys_ffi

Conversation

@atulpatildbz
Copy link
Collaborator

@atulpatildbz atulpatildbz commented Feb 15, 2026

Summary

Implements FFI for the Enumerate Keys API in the Key Custody Core (KCC). It exposes the necessary Rust functions to allow the Go Workload Service to list active KEM keys.

Dependency

Key Changes

  1. Key Custody Core (key_custody_core):

    • Added key_manager_enumerate_kem_keys FFI function.
    • Implemented enumerate_kem_keys in KeyRegistry to safely expose key metadata.
    • Added KmHpkeAlgorithm struct for C compatibility.
  2. C Header (kps_key_custody_core.h):

    • Exported key_manager_enumerate_kem_keys definition.

Verification

  • Rust Tests:
    • Ran cargo check and cargo test in keymanager/key_protection_service/key_custody_core.
    • Verified that the new FFI function compiles and integrates with the existing system.

This commit implements the FFI for the Enumerate Keys API in the Key
Custody Core (KCC). It exposes the necessary Rust functions to allow the
Go Workload Service to list active KEM keys.
Copy link
Collaborator

@NilanjanDaw NilanjanDaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on the enumerate FFI. Some comments.

let mut count = 0usize;

for meta in &metas {
if count >= max_entries {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to enable pagination in future otherwise we will be limited to the first max_entries keys.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would also need an offset in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added offset and max_entries params to support pagination. The underlying list_kem_keys implementation in the registry now uses these to skip and limit results

#[repr(C)]
pub struct KpsKeyInfo {
pub uuid: [u8; 16],
pub algorithm: [u8; 64],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The KEM keys and the Binding keys will have two different sets of algorithms. How are you planning to map them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per discussion, we've removed binding key from enumerate.

but this is a valid point. we currently enforce that if the KEM is X25519, the binding key must also be a valid 32-byte X25519 public key. If we support other algorithms in future, we'll have to change the checks here i think: https://github.com/google/go-tpm-tools/blob/main/keymanager/km_common/src/key_types.rs#L124

})
}

pub fn enumerate_keys(&self) -> Vec<KeyMetadata> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are sending max_entries through the FFI interface, we should probably use it here to limit the number of keys cloned instead of cloning the entire keyset.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. We now pass max_entries down to KEY_REGISTRY.list_kem_keys, which filters, skips, and takes before cloning the metadata. This ensures we only clone the number of keys requested

Comment on lines 185 to 190
if kem_public_key.as_bytes().len() > 64
|| binding_public_key.as_bytes().len() > 64
|| algo_bytes.len() > 64
{
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will silently discard large keys, for example for PQC algorithms like ML-KEM the keys can be hundreds of bytes long. We should either add an error or a warning log here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an explicit size check. If a key or algorithm encoding exceeds the buffer limits (2048 bytes for key, 128 for algo), we now log a warning to stderr (eprintln!) and skip that key to prevent partial/corrupt data

Adds offset-based pagination to the key enumeration API and increases
the buffer sizes for KpsKeyInfo to support Post-Quantum Cryptography
(PQC) keys (e.g., ML-KEM). Also optimizes internal enumeration to
avoid cloning the entire key registry.
As per design discussions, the binding key is not useful to the caller
of enumerate_keys and should not be exposed. This change removes
 and  from .
…egistry

- Introduced  with inlined  logic to specifically handle KEM key pagination.
- Removed unused  and  methods from .
- Updated  FFI to use .
- Restricted internal key listing to KEM keys only, ensuring deterministic behavior for the FFI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants