diff --git a/src/wh_client_she.c b/src/wh_client_she.c index 824c9a2e4..48fd6c5e8 100644 --- a/src/wh_client_she.c +++ b/src/wh_client_she.c @@ -54,6 +54,13 @@ int wh_Client_ShePreProgramKey(whClientContext* c, whNvmId keyId, int32_t outRc; uint8_t label[WH_NVM_LABEL_LEN] = { 0 }; + /* SHE slots hold exactly one AES-128 key. The server appends a 16 byte + * constant after the slot contents when deriving key update keys, so a + * longer object would overrun its kdf input buffer. */ + if ((c == NULL) || (key == NULL) || (keySz != WH_SHE_KEY_SZ)) { + return WH_ERROR_BADARGS; + } + /* Create a key with 0 counter */ wh_She_Meta2Label(0, flags, label); ret = wh_Client_NvmAddObject( diff --git a/src/wh_server_she.c b/src/wh_server_she.c index dbd106e84..6a5e99057 100644 --- a/src/wh_server_she.c +++ b/src/wh_server_she.c @@ -504,12 +504,18 @@ static int _LoadKey(whServerContext* server, uint16_t magic, uint16_t req_size, /* read the auth key by AuthID */ if (ret == 0) { - keySz = sizeof(kdfInput); + keySz = WH_SHE_KEY_SZ; ret = wh_Server_KeystoreReadKey( server, WH_SHE_MAKE_KEYID(server->comm->client_id, - _PopAuthId(req.messageOne)), + _PopAuthId(req.messageOne)), NULL, kdfInput, &keySz); + /* a slot that doesn't hold exactly WH_SHE_KEY_SZ bytes is not a + * usable auth key, and the kdf appends a constant after it */ + if ((ret == WH_ERROR_NOSPACE) || + ((ret == 0) && (keySz != WH_SHE_KEY_SZ))) { + ret = WH_SHE_ERC_KEY_INVALID; + } } /* make K2 using AES-MP(authKey | WH_SHE_KEY_UPDATE_MAC_C) */ if (ret == 0) { @@ -566,20 +572,29 @@ static int _LoadKey(whServerContext* server, uint16_t magic, uint16_t req_size, wc_AesFree(server->she->sheAes); /* load the target key */ if (ret == 0) { - ret = wh_Server_KeystoreReadKey( + keySz = WH_SHE_KEY_SZ; + ret = wh_Server_KeystoreReadKey( server, WH_SHE_MAKE_KEYID(server->comm->client_id, _PopId(req.messageOne)), meta, kdfInput, &keySz); - /* Extract count and flags from the label, even if it failed */ - wh_She_Label2Meta(meta->label, &she_meta_count, &she_meta_flags); - /* if the keyslot is empty or write protection is not on continue */ - if (ret == WH_ERROR_NOTFOUND || - (she_meta_flags & WH_SHE_FLAG_WRITE_PROTECT) == 0) { - keyRet = ret; - ret = 0; + /* meta is left unset when the slot is too big for the buffer, so the + * label below would read as all zeros and clear the write protect */ + if ((ret == WH_ERROR_NOSPACE) || + ((ret == 0) && (keySz != WH_SHE_KEY_SZ))) { + ret = WH_SHE_ERC_KEY_INVALID; } else { - ret = WH_SHE_ERC_WRITE_PROTECTED; + /* Extract count and flags from the label, even if it failed */ + wh_She_Label2Meta(meta->label, &she_meta_count, &she_meta_flags); + /* if the keyslot is empty or write protection is not on continue */ + if (ret == WH_ERROR_NOTFOUND || + (she_meta_flags & WH_SHE_FLAG_WRITE_PROTECT) == 0) { + keyRet = ret; + ret = 0; + } + else { + ret = WH_SHE_ERC_WRITE_PROTECTED; + } } } /* check UID == 0 */ diff --git a/test-refactor/client-server/wh_test_she.c b/test-refactor/client-server/wh_test_she.c index e723e9bc1..06fb7a15c 100644 --- a/test-refactor/client-server/wh_test_she.c +++ b/test-refactor/client-server/wh_test_she.c @@ -144,8 +144,14 @@ int whTest_She(whClientContext* client) uint8_t messageThree[WH_SHE_M3_SZ]; uint8_t messageFour[WH_SHE_M4_SZ]; uint8_t messageFive[WH_SHE_M5_SZ]; + uint8_t oversizeKey[WH_SHE_KEY_SZ * 2] = {0}; + uint8_t oversizeLabel[WH_NVM_LABEL_LEN] = {0}; + int32_t sheMetaRc = 0; const uint32_t SHE_TEST_VECTOR_KEY_ID = 4; const uint32_t SHE_WP_KEY_ID = 6; + const uint32_t SHE_SIZE_CHECK_KEY_ID = 7; + const uint32_t SHE_OVERSIZE_AUTH_ID = 8; + const uint32_t SHE_OVERSIZE_TARGET_ID = 9; if (client == NULL) { return WH_ERROR_BADARGS; @@ -512,6 +518,125 @@ int whTest_She(whClientContext* client) ret = 0; WH_TEST_PRINT("SHE write protect SUCCESS\n"); + /* === Pre-program key size === */ + + /* A SHE slot holds exactly one AES-128 key. The server appends a 16 + * byte constant after the slot contents when deriving key update keys, + * so an oversized slot would overrun its kdf input buffer. */ + ret = wh_Client_ShePreProgramKey(client, SHE_SIZE_CHECK_KEY_ID, 0, + oversizeKey, sizeof(oversizeKey)); + if (ret != WH_ERROR_BADARGS) { + WH_ERROR_PRINT("Oversized SHE pre-program: expected WH_ERROR_BADARGS, " + "got %d\n", ret); + ret = WH_ERROR_ABORTED; + goto exit; + } + ret = wh_Client_ShePreProgramKey(client, SHE_SIZE_CHECK_KEY_ID, 0, + oversizeKey, WH_SHE_KEY_SZ / 2); + if (ret != WH_ERROR_BADARGS) { + WH_ERROR_PRINT("Short SHE pre-program: expected WH_ERROR_BADARGS, " + "got %d\n", ret); + ret = WH_ERROR_ABORTED; + goto exit; + } + /* neither attempt may leave anything behind in the slot */ + if ((ret = wh_Client_NvmGetMetadata( + client, + WH_SHE_MAKE_KEYID(client->comm->client_id, SHE_SIZE_CHECK_KEY_ID), + &sheMetaRc, NULL, NULL, NULL, NULL, 0, NULL)) != 0) { + WH_ERROR_PRINT("Failed to wh_Client_NvmGetMetadata %d\n", ret); + goto exit; + } + if (sheMetaRc != WH_ERROR_NOTFOUND) { + WH_ERROR_PRINT("SHE size check slot: expected WH_ERROR_NOTFOUND, " + "got %d\n", (int)sheMetaRc); + ret = WH_ERROR_ABORTED; + goto exit; + } + WH_TEST_PRINT("SHE pre-program key size SUCCESS\n"); + + /* === Oversized auth key slot === */ + + /* The SHE and NVM id spaces overlap, so a client can plant an oversized + * object straight into a SHE slot. LoadKey must reject such a slot as an + * auth key instead of reading it into its fixed kdf input buffer. */ + wh_She_Meta2Label(0, 0, oversizeLabel); + if ((ret = wh_Client_NvmAddObject( + client, + WH_SHE_MAKE_KEYID(client->comm->client_id, SHE_OVERSIZE_AUTH_ID), + 0, 0, sizeof(oversizeLabel), oversizeLabel, sizeof(oversizeKey), + oversizeKey, &sheMetaRc)) != 0) { + WH_ERROR_PRINT("Failed to wh_Client_NvmAddObject %d\n", ret); + goto exit; + } + if (sheMetaRc != 0) { + WH_ERROR_PRINT("Failed to plant oversized SHE slot, got %d\n", + (int)sheMetaRc); + ret = WH_ERROR_ABORTED; + goto exit; + } + if ((ret = wh_She_GenerateLoadableKey( + SHE_SIZE_CHECK_KEY_ID, SHE_OVERSIZE_AUTH_ID, 1, 0, sheUid, + vectorRawKey, vectorRawKey, messageOne, messageTwo, messageThree, + messageFour, messageFive)) != 0) { + WH_ERROR_PRINT("Failed to generate loadable key %d\n", ret); + goto exit; + } + ret = wh_Client_SheLoadKey(client, messageOne, messageTwo, messageThree, + messageFour, messageFive); + if (ret != WH_SHE_ERC_KEY_INVALID) { + WH_ERROR_PRINT("Oversized SHE auth key: expected " + "WH_SHE_ERC_KEY_INVALID, got %d\n", ret); + ret = WH_ERROR_ABORTED; + goto exit; + } + if ((ret = _destroySheKey(client, SHE_OVERSIZE_AUTH_ID)) != 0) { + WH_ERROR_PRINT("Failed to _destroySheKey, ret=%d\n", ret); + goto exit; + } + WH_TEST_PRINT("SHE oversized auth key SUCCESS\n"); + + /* === Oversized target key === */ + + /* Same overlap, but with the oversized object planted at the target + * slot. Reading it leaves the metadata unset, so an unchecked read sees + * a zeroed label and overwrites a write-protected slot. */ + wh_She_Meta2Label(0, WH_SHE_FLAG_WRITE_PROTECT, oversizeLabel); + if ((ret = wh_Client_NvmAddObject( + client, + WH_SHE_MAKE_KEYID(client->comm->client_id, SHE_OVERSIZE_TARGET_ID), + 0, 0, sizeof(oversizeLabel), oversizeLabel, sizeof(oversizeKey), + oversizeKey, &sheMetaRc)) != 0) { + WH_ERROR_PRINT("Failed to wh_Client_NvmAddObject %d\n", ret); + goto exit; + } + if (sheMetaRc != 0) { + WH_ERROR_PRINT("Failed to plant oversized SHE target slot, got %d\n", + (int)sheMetaRc); + ret = WH_ERROR_ABORTED; + goto exit; + } + if ((ret = wh_She_GenerateLoadableKey( + SHE_OVERSIZE_TARGET_ID, WH_SHE_SECRET_KEY_ID, 1, 0, sheUid, + vectorRawKey, secretKey, messageOne, messageTwo, messageThree, + messageFour, messageFive)) != 0) { + WH_ERROR_PRINT("Failed to generate loadable key %d\n", ret); + goto exit; + } + ret = wh_Client_SheLoadKey(client, messageOne, messageTwo, messageThree, + messageFour, messageFive); + if (ret != WH_SHE_ERC_KEY_INVALID) { + WH_ERROR_PRINT("Oversized SHE target key: expected " + "WH_SHE_ERC_KEY_INVALID, got %d\n", ret); + ret = WH_ERROR_ABORTED; + goto exit; + } + if ((ret = _destroySheKey(client, SHE_OVERSIZE_TARGET_ID)) != 0) { + WH_ERROR_PRINT("Failed to _destroySheKey, ret=%d\n", ret); + goto exit; + } + WH_TEST_PRINT("SHE oversized target key SUCCESS\n"); + /* === Cleanup: destroy provisioned keys so we don't leak NVM === */ if ((ret = _destroySheKey(client, WH_SHE_BOOT_MAC_KEY_ID)) != 0) { diff --git a/wolfhsm/wh_client_she.h b/wolfhsm/wh_client_she.h index 5d2963e86..7528c2c50 100644 --- a/wolfhsm/wh_client_she.h +++ b/wolfhsm/wh_client_she.h @@ -88,8 +88,11 @@ * @param[in] flags SHE key protection flags to store with the key * (WH_SHE_FLAG_WRITE_PROTECT, WH_SHE_FLAG_BOOT_PROTECT, etc.). * @param[in] key Pointer to the key material to store. - * @param[in] keySz Length of the key material in bytes (WH_SHE_KEY_SZ, 16). - * @return int Returns 0 on success, or a negative error code on failure. + * @param[in] keySz Length of the key material in bytes. Must be exactly + * WH_SHE_KEY_SZ (16); any other length is rejected. + * @return int Returns 0 on success, WH_ERROR_BADARGS if @p c or @p key is NULL + * or @p keySz is not WH_SHE_KEY_SZ, or a negative error code on + * failure. */ int wh_Client_ShePreProgramKey(whClientContext* c, whNvmId keyId, whNvmFlags flags, uint8_t* key, whNvmSize keySz);