Skip to content

Commit d92c8e8

Browse files
authored
[KeyManager] End-to-end FFI error handling standardization (google#722)
This commit performs a comprehensive refactoring of the `KeyManager` component to utilize the standardized `keymanager.Status` Protobuf enum across all layers of the system. It replaces legacy negative `i32` error codes and ad-hoc `error` enums with a unified, type-safe `Status` reporting mechanism that spans Rust, C, and Go. Key Changes - Rust FFI & Core Updates: - Refactored `KeyCustodyCore` in both `key_protection_service` and `workload_service` to return the Status enum directly. - Simplified FFI implementation by utilizing the `ffi_call` and `ffi_call_i32` helpers introduced in Part 1. - Updated internal logic to map crate-specific errors to the standardized `Status` proto. - C-Binding & Header Standardization: - Modified C function signatures in `kps_key_custody_core.h` and `ws_key_custody_core.h` to return `Status` instead of `int32_t`. - Go CGO & Service Layer Integration: - Updated CGO wrappers to interpret the new Status return codes and convert them into idiomatic Go errors using the`ToStatus()` helper. - HTTP Status Mapping: Implemented `httpStatusFromError` in the Go server to automatically map standardized FFI errors to appropriate HTTP status codes (e.g., `ERROR_NOT_FOUND` → `404 Not Found`, `ERROR_INVALID_ARGUMENT` → `400 Bad` Request). - Removed redundant error aliases in `types.go` to maintain a single source of truth. - Test Alignment: Updated integration tests in Go and unit tests in Rust to verify the new error reporting behavior and ensure consistent error messages (e.g., `"FFI error: ERROR_NOT_FOUND"`).
1 parent c13cba1 commit d92c8e8

File tree

24 files changed

+703
-514
lines changed

24 files changed

+703
-514
lines changed

keymanager/generate_ffi_headers.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ set -euo pipefail
44
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
55
CBINDGEN_BIN="${CBINDGEN_BIN:-cbindgen}"
66

7+
"${CBINDGEN_BIN}" --quiet \
8+
"${ROOT_DIR}/km_common" \
9+
--crate km_common \
10+
--config "${ROOT_DIR}/km_common/cbindgen.toml" \
11+
--output "${ROOT_DIR}/km_common/include/km_common_ffi.h"
712

813
"${CBINDGEN_BIN}" --quiet \
914
"${ROOT_DIR}/workload_service/key_custody_core" \
@@ -16,3 +21,4 @@ CBINDGEN_BIN="${CBINDGEN_BIN:-cbindgen}"
1621
--crate kps_key_custody_core \
1722
--config "${ROOT_DIR}/key_protection_service/key_custody_core/cbindgen.toml" \
1823
--output "${ROOT_DIR}/key_protection_service/key_custody_core/include/kps_key_custody_core.h"
24+

keymanager/key_protection_service/key_custody_core/cbindgen.toml

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,18 @@ style = "type"
88
documentation = false
99
usize_is_size_t = true
1010
sys_includes = ["stdbool.h", "stddef.h", "stdint.h"]
11+
includes = ["km_common_ffi.h"]
1112

1213

1314
[parse]
14-
parse_deps = false
15+
parse_deps = true
16+
include = ["km_common"]
1517
clean = true
1618

1719
[export]
1820
item_types = ["functions", "structs", "constants", "enums"]
19-
include = ["key_manager_generate_kem_keypair", "key_manager_get_kem_key", "key_manager_destroy_kem_key", "key_manager_enumerate_kem_keys", "KpsKeyInfo", "MAX_ALGORITHM_LEN", "MAX_PUBLIC_KEY_LEN"]
20-
21+
include = ["key_manager_generate_kem_keypair", "key_manager_get_kem_key", "key_manager_destroy_kem_key", "key_manager_enumerate_kem_keys", "KpsKeyInfo"]
22+
exclude = ["Status", "MAX_ALGORITHM_LEN", "MAX_PUBLIC_KEY_LEN"]
2123

24+
[enum]
25+
prefix_with_name = true

keymanager/key_protection_service/key_custody_core/include/kps_key_custody_core.h

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@
66
#include <stdbool.h>
77
#include <stddef.h>
88
#include <stdint.h>
9-
10-
#define MAX_ALGORITHM_LEN 128
11-
12-
#define MAX_PUBLIC_KEY_LEN 2048
9+
#include "km_common_ffi.h"
1310

1411
typedef struct {
1512
uint8_t uuid[16];
@@ -26,40 +23,40 @@ typedef struct {
2623
extern "C" {
2724
#endif // __cplusplus
2825

29-
int32_t key_manager_generate_kem_keypair(const uint8_t *algo_ptr,
30-
size_t algo_len,
31-
const uint8_t *binding_pubkey,
32-
size_t binding_pubkey_len,
33-
uint64_t expiry_secs,
34-
uint8_t *out_uuid,
35-
uint8_t *out_pubkey,
36-
size_t out_pubkey_len);
26+
Status key_manager_generate_kem_keypair(const uint8_t *algo_ptr,
27+
size_t algo_len,
28+
const uint8_t *binding_pubkey,
29+
size_t binding_pubkey_len,
30+
uint64_t expiry_secs,
31+
uint8_t *out_uuid,
32+
uint8_t *out_pubkey,
33+
size_t out_pubkey_len);
3734

38-
int32_t key_manager_destroy_kem_key(const uint8_t *uuid_bytes);
35+
Status key_manager_destroy_kem_key(const uint8_t *uuid_bytes);
3936

4037
int32_t key_manager_enumerate_kem_keys(KpsKeyInfo *out_entries,
4138
size_t max_entries,
4239
size_t offset,
4340
bool *out_has_more);
4441

45-
int32_t key_manager_decap_and_seal(const uint8_t *uuid_bytes,
46-
const uint8_t *encapsulated_key,
47-
size_t encapsulated_key_len,
48-
const uint8_t *aad,
49-
size_t aad_len,
50-
uint8_t *out_encapsulated_key,
51-
size_t out_encapsulated_key_len,
52-
uint8_t *out_ciphertext,
53-
size_t out_ciphertext_len);
42+
Status key_manager_decap_and_seal(const uint8_t *uuid_bytes,
43+
const uint8_t *encapsulated_key,
44+
size_t encapsulated_key_len,
45+
const uint8_t *aad,
46+
size_t aad_len,
47+
uint8_t *out_encapsulated_key,
48+
size_t out_encapsulated_key_len,
49+
uint8_t *out_ciphertext,
50+
size_t out_ciphertext_len);
5451

55-
int32_t key_manager_get_kem_key(const uint8_t *uuid_bytes,
56-
uint8_t *out_kem_pubkey,
57-
size_t out_kem_pubkey_len,
58-
uint8_t *out_binding_pubkey,
59-
size_t out_binding_pubkey_len,
60-
uint8_t *out_algo,
61-
size_t *out_algo_len,
62-
uint64_t *out_delete_after);
52+
Status key_manager_get_kem_key(const uint8_t *uuid_bytes,
53+
uint8_t *out_kem_pubkey,
54+
size_t out_kem_pubkey_len,
55+
uint8_t *out_binding_pubkey,
56+
size_t out_binding_pubkey_len,
57+
uint8_t *out_algo,
58+
size_t *out_algo_len,
59+
uint64_t *out_remaining_lifespan_secs);
6360

6461
#ifdef __cplusplus
6562
} // extern "C"

keymanager/key_protection_service/key_custody_core/integration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func TestIntegrationGetKEMKeyNotFound(t *testing.T) {
106106
t.Fatal("expected error for non-existent UUID")
107107
}
108108

109-
expectedErrMsg := "key_manager_get_kem_key failed with code -1"
109+
expectedErrMsg := "FFI status: STATUS_NOT_FOUND"
110110
if !strings.Contains(err.Error(), expectedErrMsg) {
111111
t.Fatalf("expected error containing %q, got: %v", expectedErrMsg, err)
112112
}

keymanager/key_protection_service/key_custody_core/kps_key_custody_core_cgo.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package kpskcc
77

88
/*
9+
#cgo CFLAGS: -I${SRCDIR}/../../km_common/include
910
#cgo LDFLAGS: -L${SRCDIR}/../../target/release -L${SRCDIR}/../../target/debug -lkps_key_custody_core
1011
#cgo LDFLAGS: -lcrypto -lssl
1112
#cgo LDFLAGS: -lpthread -ldl -lm -lstdc++
@@ -57,8 +58,8 @@ func GenerateKEMKeypair(algo *keymanager.HpkeAlgorithm, bindingPubKey []byte, li
5758
(*C.uint8_t)(unsafe.Pointer(&uuidBytes[0])),
5859
(*C.uint8_t)(unsafe.Pointer(&pubkeyBuf[0])),
5960
pubkeyLen,
60-
); rc != 0 {
61-
return uuid.Nil, nil, fmt.Errorf("key_manager_generate_kem_keypair failed with code %d", rc)
61+
); keymanager.Status(rc) != keymanager.Status_STATUS_SUCCESS {
62+
return uuid.Nil, nil, keymanager.Status(rc).ToStatus()
6263
}
6364

6465
id, err := uuid.FromBytes(uuidBytes[:])
@@ -91,7 +92,7 @@ func EnumerateKEMKeys(limit, offset int) ([]KEMKeyInfo, bool, error) {
9192
&hasMore,
9293
)
9394
if rc < 0 {
94-
return nil, false, fmt.Errorf("key_manager_enumerate_kem_keys failed with code %d", rc)
95+
return nil, false, keymanager.Status(-rc).ToStatus()
9596
}
9697

9798
count := int(rc)
@@ -124,12 +125,10 @@ func EnumerateKEMKeys(limit, offset int) ([]KEMKeyInfo, bool, error) {
124125
// DestroyKEMKey destroys the KEM key identified by kemUUID via Rust FFI.
125126
func DestroyKEMKey(kemUUID uuid.UUID) error {
126127
uuidBytes := kemUUID[:]
127-
if rc := C.key_manager_destroy_kem_key(
128+
rc := C.key_manager_destroy_kem_key(
128129
(*C.uint8_t)(unsafe.Pointer(&uuidBytes[0])),
129-
); rc != 0 {
130-
return fmt.Errorf("key_manager_destroy_kem_key failed with code %d", rc)
131-
}
132-
return nil
130+
)
131+
return keymanager.Status(rc).ToStatus()
133132
}
134133

135134
// GetKEMKey retrieves KEM and binding public keys, HpkeAlgorithm and remaining lifespan via Rust FFI.
@@ -153,8 +152,8 @@ func GetKEMKey(id uuid.UUID) ([]byte, []byte, *keymanager.HpkeAlgorithm, uint64,
153152
&algoLenC,
154153
&remainingLifespanSecs,
155154
)
156-
if rc != 0 {
157-
return nil, nil, nil, 0, fmt.Errorf("key_manager_get_kem_key failed with code %d", rc)
155+
if keymanager.Status(rc) != keymanager.Status_STATUS_SUCCESS {
156+
return nil, nil, nil, 0, keymanager.Status(rc).ToStatus()
158157
}
159158

160159
kemPubkey := make([]byte, len(kemPubkeyBuf))
@@ -201,8 +200,8 @@ func DecapAndSeal(kemUUID uuid.UUID, encapsulatedKey, aad []byte) ([]byte, []byt
201200
outEncKeyLen,
202201
(*C.uint8_t)(unsafe.Pointer(&outCT[0])),
203202
outCTLen,
204-
); rc != 0 {
205-
return nil, nil, fmt.Errorf("key_manager_decap_and_seal failed with code %d", rc)
203+
); keymanager.Status(rc) != keymanager.Status_STATUS_SUCCESS {
204+
return nil, nil, keymanager.Status(rc).ToStatus()
206205
}
207206

208207
sealEnc := make([]byte, outEncKeyLen)

0 commit comments

Comments
 (0)