Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions src/wh_server_keystore.c
Original file line number Diff line number Diff line change
Expand Up @@ -1838,13 +1838,31 @@ static int _HandleKeyWrapRequest(whServerContext* server,
/* Wrapped key size is only passed back to the client on success */
resp->wrappedKeySz = 0;

/* Ensure the keyId in the wrapped metadata has the wrapped flag set */
if (!WH_KEYID_ISWRAPPED(metadata.id)) {
/* Require a wrappable type: WRAPPED, or SHE so a SHE key can be wrapped and
* 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
Comment thread
AlexLanzano marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

#endif
) {
WH_LOG_F(&server->log, WH_LOG_LEVEL_ERROR,
"KeyWrapRequest: keyId:0x%08X is not wrapped", metadata.id);
"KeyWrapRequest: keyId:0x%08X is not a wrappable type",
metadata.id);
return WH_ERROR_BADARGS;
}

#ifdef WOLFHSM_CFG_SHE_EXTENSION
/* SHE keys are a fixed size; reject a mis-sized blob up front so it can
* never be primed via unwrap-and-cache and mishandled by the SHE ops. */
if (WH_KEYID_TYPE(metadata.id) == WH_KEYTYPE_SHE &&
req->keySz != WH_SHE_KEY_SZ) {
WH_LOG_F(&server->log, WH_LOG_LEVEL_ERROR,
"KeyWrapRequest: SHE keyId:0x%08X keySz:%u must be %u",
metadata.id, req->keySz, WH_SHE_KEY_SZ);
return WH_ERROR_BADARGS;
}
#endif

/* Translate the server key id passed in from the client */
serverKeyId = wh_KeyId_TranslateFromClient(WH_KEYTYPE_CRYPTO,
server->comm->client_id,
Expand Down Expand Up @@ -2238,6 +2256,17 @@ static int _HandleKeyUnwrapAndCacheRequest(
goto out;
}

#ifdef WOLFHSM_CFG_SHE_EXTENSION
/* SHE keys are a fixed size. Reject a mis-sized blob before it can be
* cached: the SHE ops read cached keys into fixed WH_SHE_KEY_SZ-derived
* buffers, so an over-length key would overflow (e.g. the LoadKey KDF
* input). This is the chokepoint for every unwrap source. */
if (wrappedKeyType == WH_KEYTYPE_SHE && metadata.len != WH_SHE_KEY_SZ) {
ret = WH_ERROR_ABORTED;
goto out;
}
#endif

/* Validate ownership: USER field must match requesting client.
* The USER field specifies who owns this wrapped key. */
#ifdef WOLFHSM_CFG_GLOBAL_KEYS
Expand Down
57 changes: 45 additions & 12 deletions test/wh_test_she.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,13 @@ enum {

#if defined(WOLFHSM_CFG_KEYWRAP) && defined(HAVE_AESGCM) && \
!defined(WOLFHSM_CFG_TEST_CLIENT_ONLY)
/* Id of the trusted KEK the server task provisions for the SHE<->keywrap
* interop tests. Defined here so the server task can use it too. */
/* 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?)

#else
#define WH_SHE_INTEROP_KEK_ID 0x20
#endif
#endif /* WOLFHSM_CFG_KEYWRAP && HAVE_AESGCM && !TEST_CLIENT_ONLY */

#ifdef WOLFHSM_CFG_ENABLE_CLIENT
Expand All @@ -120,6 +124,27 @@ static int _destroySheKey(whClientContext* client, whNvmId clientSheKeyId)
return rc;
}

#if defined(WOLFHSM_CFG_KEYWRAP) && defined(HAVE_AESGCM) && \
!defined(WOLFHSM_CFG_TEST_CLIENT_ONLY)
/* Server-side equivalent of whTest_BuildSheKeyBlob: wraps a SHE key by KEK id,
* so the KEK value is never shared with the client. */
static int _SheServerWrapKeyBlob(whClientContext* client, uint16_t kekId,
whKeyId sheKeyId, uint32_t counter,
uint32_t sheFlags, const uint8_t* sheKey,
uint8_t* blob, uint16_t* blobSz)
{
whNvmMetadata meta;

memset(&meta, 0, sizeof(meta));
meta.id = sheKeyId;
meta.len = WH_SHE_KEY_SZ;
wh_She_Meta2Label(counter, sheFlags, meta.label);

return wh_Client_KeyWrap(client, WC_CIPHER_AES_GCM, kekId, (void*)sheKey,
WH_SHE_KEY_SZ, &meta, blob, blobSz);
}
#endif /* WOLFHSM_CFG_KEYWRAP && HAVE_AESGCM && !TEST_CLIENT_ONLY */

int whTest_SheClientConfig(whClientConfig* config)
{
int ret = 0;
Expand Down Expand Up @@ -608,13 +633,15 @@ int whTest_SheClientConfig(whClientConfig* config)
/* Prime an unused SHE slot via unwrap-and-cache, then use it. */
memset(sheKey, 0x5a, sizeof(sheKey));
blobSz = sizeof(blob);
ret = whTest_BuildSheKeyBlob(
whTest_KeywrapKek, sizeof(whTest_KeywrapKek),
ret = _SheServerWrapKeyBlob(
client, kekId,
WH_MAKE_KEYID(WH_KEYTYPE_SHE, client->comm->client_id,
SHE_PRIME_SLOT),
1, 0, sheKey, blob, &blobSz);
Comment thread
AlexLanzano marked this conversation as resolved.
if (ret != 0) {
WH_ERROR_PRINT("SHE interop: build prime blob failed %d\n", ret);
if (ret != 0 || blobSz != expSz) {
WH_ERROR_PRINT("SHE interop: build prime blob failed ret=%d "
"sz=%u exp=%u\n", ret, blobSz, expSz);
ret = (ret != 0) ? ret : WH_ERROR_ABORTED;
goto exit;
}
ret = wh_Client_KeyUnwrapAndCache(client, WC_CIPHER_AES_GCM, kekId,
Expand Down Expand Up @@ -655,12 +682,15 @@ int whTest_SheClientConfig(whClientConfig* config)
}
/* lower counter -> rejected */
blobSz = sizeof(blob);
ret = whTest_BuildSheKeyBlob(
whTest_KeywrapKek, sizeof(whTest_KeywrapKek),
ret = _SheServerWrapKeyBlob(
client, kekId,
WH_MAKE_KEYID(WH_KEYTYPE_SHE, client->comm->client_id,
SHE_CTR_SLOT),
3, 0, sheKey, blob, &blobSz);
Comment thread
AlexLanzano marked this conversation as resolved.
if (ret != 0) {
if (ret != 0 || blobSz != expSz) {
WH_ERROR_PRINT("SHE interop: build rollback blob failed ret=%d "
"sz=%u exp=%u\n", ret, blobSz, expSz);
ret = (ret != 0) ? ret : WH_ERROR_ABORTED;
goto exit;
}
ret = wh_Client_KeyUnwrapAndCache(client, WC_CIPHER_AES_GCM, kekId,
Expand All @@ -674,12 +704,15 @@ int whTest_SheClientConfig(whClientConfig* config)

/* equal counter -> accepted */
blobSz = sizeof(blob);
ret = whTest_BuildSheKeyBlob(
whTest_KeywrapKek, sizeof(whTest_KeywrapKek),
ret = _SheServerWrapKeyBlob(
client, kekId,
WH_MAKE_KEYID(WH_KEYTYPE_SHE, client->comm->client_id,
SHE_CTR_SLOT),
5, 0, sheKey, blob, &blobSz);
Comment thread
AlexLanzano marked this conversation as resolved.
if (ret != 0) {
if (ret != 0 || blobSz != expSz) {
WH_ERROR_PRINT("SHE interop: build equal-counter blob failed "
"ret=%d sz=%u exp=%u\n", ret, blobSz, expSz);
ret = (ret != 0) ? ret : WH_ERROR_ABORTED;
goto exit;
}
ret = wh_Client_KeyUnwrapAndCache(client, WC_CIPHER_AES_GCM, kekId,
Expand Down
Loading