Update the SHE test to have the KEK key id configurable. Allow SHE keys to be wrapped via wh_Client_KeyWrap#471
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the SHE/keywrap interoperability tests to support an overridable trusted-KEK id and shifts SHE blob wrapping from a local test helper to the server’s wh_Client_KeyWrap path. It also updates the server keystore’s KeyWrap request handling to allow WH_KEYTYPE_SHE metadata ids, enabling SHE keys to be wrapped for later unwrap-and-cache priming.
Changes:
- Add
WOLFHSM_CFG_TEST_SHE_INTEROP_KEK_IDoverride support for the SHE/keywrap interop trusted KEK id. - Add a test helper that wraps SHE key blobs via
wh_Client_KeyWrap(server-side KEK by id) instead of local AES-GCM wrapping. - Allow
WH_KEYTYPE_SHEas a wrappable type in the server’s KeyWrap request handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
test/wh_test_she.c |
Makes the interop KEK id configurable and updates interop blob building to use server-side key wrap. |
src/wh_server_keystore.c |
Permits KeyWrap requests to wrap blobs whose metadata id is TYPE=SHE (when SHE extension is enabled). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4c9594f to
87b08d4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
test/wh_test_she.c:144
- Avoid casting away const when passing sheKey into wh_Client_KeyWrap. wh_Client_KeyWrap does not mutate the input buffer (it only copies it), so using a small temporary buffer here keeps the helper const-correct and prevents accidental future mutations through this pointer.
return wh_Client_KeyWrap(client, WC_CIPHER_AES_GCM, kekId, (void*)sheKey,
WH_SHE_KEY_SZ, &meta, blob, blobSz);
…ys to be wrapped via wh_Client_KeyWrap
87b08d4 to
edb1bc0
Compare
bigbrett
left a comment
There was a problem hiding this comment.
looks good, but we need parity in test-refactor/ too. Also, if we end up fully removing whTest_BuildSheKeyBlob after that (since I think thats the only other use of it) then we should just kill that function entirely.
| /* Trusted KEK id for the SHE<->keywrap interop tests. Override via | ||
| * WOLFHSM_CFG_TEST_SHE_INTEROP_KEK_ID (e.g. a hardware KEK). */ | ||
| #ifdef WOLFHSM_CFG_TEST_SHE_INTEROP_KEK_ID | ||
| #define WH_SHE_INTEROP_KEK_ID WOLFHSM_CFG_TEST_SHE_INTEROP_KEK_ID |
There was a problem hiding this comment.
I think this override will make it such that tests still try to provision the KEK at this ID unconditionally in _ProvisionSheServerKek() (and possibly elsewhere?)
| * later primed via unwrap-and-cache. */ | ||
| if (WH_KEYID_TYPE(metadata.id) != WH_KEYTYPE_WRAPPED | ||
| #ifdef WOLFHSM_CFG_SHE_EXTENSION | ||
| && WH_KEYID_TYPE(metadata.id) != WH_KEYTYPE_SHE |
There was a problem hiding this comment.
Actually, on a deeper look, we can't do this. It lets a client choose arbitrary bytes and load it into a target SHE slot, instead of requiring it to go through the SHE key update process. SHE keys should only be created/provisioned out-of-band or updated via SHE key update protocol (excluding wh_Client_ShePreProgramKey() that is compile time-gated on a different branch and should be fixed soon)
|
closing per our discussion. Favor moving the test in question that requires preprovisioned server keys out of the client-only tests and only running in POSIX enviroment |
WOLFHSM_CFG_TEST_SHE_INTEROP_KEK_IDto the she key wrap test to allow the KEK id to be overriddenWH_KEYTYPE_SHE