Bound SHE LoadKey auth key read to WH_SHE_KEY_SZ

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(

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 */

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

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