[keymanager/wsd] Add /keys:decaps KOL API with DecapAndSeal + Open orchestration#650
Conversation
NilanjanDaw
left a comment
There was a problem hiding this comment.
Thanks for the PR. Over-all comment: I think we need to align the message formats for the APIs. Lets define them in a separate proto file, so that they can be easily reviewed.
| // Use a sentinel byte so the pointer is always valid. | ||
| var aadSentinel [1]byte | ||
| aadPtr := (*C.uint8_t)(unsafe.Pointer(&aadSentinel[0])) | ||
| aadLen := C.size_t(0) | ||
| if len(aad) > 0 { | ||
| aadPtr = (*C.uint8_t)(unsafe.Pointer(&aad[0])) | ||
| aadLen = C.size_t(len(aad)) | ||
| } |
There was a problem hiding this comment.
Should we add a AAD context for the HPKE seal/open operation?
There was a problem hiding this comment.
Good point. I added an explicit AAD context for the decaps flow and pass it through both KPS DecapAndSeal and WSD Open: "wsd:keys:decaps:v1::". Addressed in commit 02d8655.
| return | ||
| } | ||
|
|
||
| encapsulatedKey, err := base64.StdEncoding.DecodeString(req.EncapsulatedKey) |
There was a problem hiding this comment.
I think we need to align the message format and the params a bit. The encapsulatedKey is expected to be raw bytes and packed as below
message DecapsRequest {
KeyHandle key_handle = 1;
KemCiphertext ciphertext = 2;
}
// The results of an Encaps operation.
message KemCiphertext {
KemAlgorithm algorithm = 1;
bytes ciphertext = 2; // `Nenc` bytes long.
}
| resp := DecapsResponse{ | ||
| SharedSecret: base64.StdEncoding.EncodeToString(plaintext), | ||
| } |
There was a problem hiding this comment.
Response should be encoded as follows
// The results of a Decaps operation.
message KemSharedSecret {
KemAlgorithm algorithm = 1;
bytes secret = 2; // `Nsecret` bytes long.
}
There was a problem hiding this comment.
Updated response encoding to KemSharedSecret shape: sharedSecret{ algorithm, secret }. Implemented in commit 02d8655 (including test updates).
Thanks for the review. I aligned the API message formats and added a dedicated proto file: |
acab338 to
4f935f5
Compare
4deb925 to
448b861
Compare
| return | ||
| } | ||
|
|
||
| if req.Ciphertext.Algorithm != KemAlgorithmDHKEMX25519HKDFSHA256 { |
There was a problem hiding this comment.
Look up from protobuf instead of hardcoding?
448b861 to
8f28a96
Compare
8f28a96 to
38c67c6
Compare
7cce446 to
fde7312
Compare
…ine HTTP handler methods, and improve KEM algorithm validation.
fde7312 to
d987353
Compare
| var outEncKey [32]byte | ||
| outEncKeyLen := C.size_t(len(outEncKey)) | ||
| var outCT [48]byte // 32-byte secret + 16-byte GCM tag | ||
| outCTLen := C.size_t(len(outCT)) |
There was a problem hiding this comment.
Size magics here? Can you define as file consts?
There was a problem hiding this comment.
Added consts. Did the same in ws_key_custody_core_cgo.go as well
| // Open decrypts a sealed ciphertext using the binding key identified by | ||
| // bindingUUID via Rust FFI (HPKE Open). | ||
| // Returns the decrypted plaintext (shared secret). | ||
| func Open(bindingUUID uuid.UUID, enc, ciphertext, aad []byte) ([]byte, error) { |
There was a problem hiding this comment.
No need to change now, but this PR is pretty large by my eyes. Open an Decap operations could have been separated into to smaller more targeted PRs.
There was a problem hiding this comment.
Ack and apologies.
I didn't realize Decap + Open would grow to be so huge.
Thanks for taking the time to review
| ) | ||
| if rc != 0 { | ||
| return nil, fmt.Errorf("key_manager_open failed with code %d", rc) | ||
| } |
There was a problem hiding this comment.
If RC isn't needed later. Typically in go we'd
if rc := ...; rc !=0 {
..
}
There was a problem hiding this comment.
Makes sense. done
| } | ||
| aad := decapsAADContext(kemUUID, req.Ciphertext.Algorithm) | ||
|
|
||
| // Step 1: Look up the binding UUID for this KEM key. |
There was a problem hiding this comment.
Does this code base use this "Step #" comment format? If not please remove to be consistent
…ment - Extracts magic numbers for byte array sizes into file-level constants for better readability. - Refactors return code (rc) checks on C FFI calls to use idiomatic inline assignments. docs: remove 'Step #' prefix from comments
e8d98ec to
1ce8149
Compare
1ce8149 to
7af1241
Compare
7af1241 to
bbc84b0
Compare
…chestration (google#650) * feat(keymanager): Implement Decap and Seal orchestration in Go * refactor: consolidate KeyProtectionService interfaces, explicitly define HTTP handler methods, and improve KEM algorithm validation. * review comments: refactor magic number and rc check, comments improvement - Extracts magic numbers for byte array sizes into file-level constants for better readability. - Refactors return code (rc) checks on C FFI calls to use idiomatic inline assignments. docs: remove 'Step #' prefix from comments * refactor: kps interface based DI for KCC
Implements the Go orchestration layer (KOL) for
POST /v1/keys:decap. This key exchange endpoint allows workloads to recover a shared secret from a KEM encapsulation.Flow:
{key_handle, ciphertext: {algorithm, ciphertext}}to WSD/v1/keys:generate_kem)DecapAndSeal— decapsulates the shared secret using the KEM private key, reseals it with the binding public keyOpen— unseals the shared secret using the binding private keyChanges:
POST /v1/keys:decap(aligned with API contract)snake_casefor JSON fields inDecapsRequestandDecapsResponsekey_manager_decap_and_seal(KPS) andkey_manager_open(WSD) declarationsDecapAndSeal()andOpen()Go wrappers for the Rust FFI functionsServicewithDecapAndSealmethodDecapSealer/Openerinterfaces,handleDecapshandlerTestHandleDecapsSuccessin and Rust unit testsTesting
tested with encap operation manually.
Testing generate + decap + seal + open
Code for manual testing: atulpatildbz@ac4b947