update based on PR comments

src/wh_client.c

@@ -314,21 +314,26 @@ int wh_Client_CustomCbCheckRegistered(whClientContext* c, uint16_t id, int* resp
 }
 
 int wh_Client_KeyCacheRequest_ex(whClientContext* c, uint32_t flags,
-    uint8_t* label, uint8_t* in, uint32_t inSz, uint16_t keyId)
+    uint8_t* label, uint32_t labelSz, uint8_t* in, uint32_t inSz,
+    uint16_t keyId)
 {
     uint8_t rawPacket[WH_COMM_MTU] = {0};
     whPacket* packet = (whPacket*)rawPacket;
     uint8_t* packIn = (uint8_t*)(&packet->keyCacheReq + 1);
-    if (c == NULL || label == NULL || in == NULL || inSz == 0)
-        return BAD_FUNC_ARG;
+    if (c == NULL || label == NULL || in == NULL || inSz == 0 ||
+        labelSz > WOLFHSM_NVM_LABEL_LEN) {
+        return WH_ERROR_BADARGS;
+    }
     /* set flags */
     packet->keyCacheReq.flags = flags;
     /* set inSz */
-    packet->keyCacheReq.len = inSz;
+    packet->keyCacheReq.sz = inSz;
+    /* set labelLen */
+    packet->keyCacheReq.labelSz = labelSz;
     /* set id */
     packet->keyCacheReq.id = keyId;
     /* set label */
-    XMEMCPY(packet->keyCacheReq.label, label, WOLFHSM_NVM_LABEL_LEN);
+    XMEMCPY(packet->keyCacheReq.label, label, labelSz);
     /* write in */
     XMEMCPY(packIn, in, inSz);
     /* write request */
@@ -338,9 +343,9 @@ int wh_Client_KeyCacheRequest_ex(whClientContext* c, uint32_t flags,
 }
 
 int wh_Client_KeyCacheRequest(whClientContext* c, uint32_t flags,
-    uint8_t* label, uint8_t* in, uint32_t inSz)
+    uint8_t* label, uint32_t labelSz, uint8_t* in, uint32_t inSz)
 {
-    return wh_Client_KeyCacheRequest_ex(c, flags, label, in, inSz,
+    return wh_Client_KeyCacheRequest_ex(c, flags, label, labelSz, in, inSz,
         WOLFHSM_ID_ERASED);
 }
 
@@ -352,7 +357,7 @@ int wh_Client_KeyCacheResponse(whClientContext* c, uint16_t* keyId)
     int ret;
     whPacket packet[1] = {0};
     if (c == NULL || keyId == NULL)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     ret = wh_Client_RecvResponse(c, &group, &action, &size, (uint8_t*)packet);
     if (ret == 0) {
         if (packet->rc != 0)
@@ -367,7 +372,7 @@ int wh_Client_KeyEvictRequest(whClientContext* c, uint16_t keyId)
 {
     whPacket packet[1] = {0};
     if (c == NULL || keyId == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* set the keyId */
     packet->keyEvictReq.id = keyId;
     /* write request */
@@ -384,8 +389,7 @@ int wh_Client_KeyEvictResponse(whClientContext* c)
     int ret;
     whPacket packet[1] = {0};
     if (c == NULL)
-        return BAD_FUNC_ARG;
-    size = sizeof(packet);
+        return WH_ERROR_BADARGS;
     ret = wh_Client_RecvResponse(c, &group, &action, &size, (uint8_t*)packet);
     if (ret == 0) {
         if (packet->rc != 0)
@@ -398,7 +402,7 @@ int wh_Client_KeyExportRequest(whClientContext* c, uint16_t keyId)
 {
     whPacket packet[1] = {0};
     if (c == NULL || keyId == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* set keyId */
     packet->keyExportReq.id = keyId;
     /* write request */
@@ -418,8 +422,7 @@ int wh_Client_KeyExportResponse(whClientContext* c, uint8_t* label,
     whPacket* packet = (whPacket*)rawPacket;
     uint8_t* packOut = (uint8_t*)(&packet->keyExportRes + 1);
     if (c == NULL || label == NULL || outSz == NULL)
-        return BAD_FUNC_ARG;
-    size = sizeof(rawPacket);
+        return WH_ERROR_BADARGS;
     ret = wh_Client_RecvResponse(c, &group, &action, &size, rawPacket);
     if (ret == 0) {
         if (packet->rc != 0)
@@ -447,7 +450,7 @@ int wh_Client_KeyCommitRequest(whClientContext* c, whNvmId keyId)
 {
     whPacket packet[1] = {0};
     if (c == NULL || keyId == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* set keyId */
     packet->keyCommitReq.id = keyId;
     /* write request */
@@ -464,8 +467,7 @@ int wh_Client_KeyCommitResponse(whClientContext* c)
     int ret;
     whPacket packet[1] = {0};
     if (c == NULL)
-        return BAD_FUNC_ARG;
-    size = sizeof(packet);
+        return WH_ERROR_BADARGS;
     ret = wh_Client_RecvResponse(c, &group, &action, &size, (uint8_t*)packet);
     if (ret == 0) {
         if (packet->rc != 0)
@@ -478,7 +480,7 @@ int wh_Client_KeyEraseRequest(whClientContext* c, whNvmId keyId)
 {
     whPacket packet[1] = {0};
     if (c == NULL || keyId == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* set keyId */
     packet->keyEraseReq.id = keyId;
     /* write request */
@@ -495,8 +497,7 @@ int wh_Client_KeyEraseResponse(whClientContext* c)
     int ret;
     whPacket packet[1] = {0};
     if (c == NULL)
-        return BAD_FUNC_ARG;
-    size = sizeof(packet);
+        return WH_ERROR_BADARGS;
     ret = wh_Client_RecvResponse(c, &group, &action, &size, (uint8_t*)packet);
     if (ret == 0) {
         if (packet->rc != 0)

src/wh_server.c

@@ -180,7 +180,7 @@ int wh_Server_HandleRequestMessage(whServerContext* server)
             break;
 
         case WH_MESSAGE_GROUP_KEY:
-            rc = _wh_Server_HandleKeyRequest(server, magic, action, seq,
+            rc = wh_Server_HandleKeyRequest(server, magic, action, seq,
                     data, &size);
         break;
 

src/wh_server_crypto.c

@@ -55,8 +55,7 @@ static int hsmLoadKeyCurve25519(whServerContext* server, curve25519_key* key, ui
     /* retrieve the key */
     XMEMSET(meta, 0, sizeof(whNvmMetadata));
     meta->id = keyId;
-    meta->len = privSz + pubSz;
-    ret = hsmReadKey(server, meta, keyBuf);
+    ret = hsmReadKey(server, meta, keyBuf, privSz + pubSz);
     /* decode the key */
     if (ret == 0)
         ret = wc_curve25519_import_public(keyBuf, pubSz, key);

src/wh_server_keystore.c

@@ -21,7 +21,7 @@ int hsmGetUniqueId(whServerContext* server)
     whNvmId keyCount;
     /* try every index until we find a unique one, don't worry about capacity */
     for (id = 1; id < sizeof(whNvmId) - WOLFHSM_KEYID_MASK; id++) {
-        /* check against chache keys */
+        /* check against cache keys */
         for (i = 0; i < WOLFHSM_NUM_RAMKEYS; i++) {
             if ((id | WOLFHSM_KEYID_CRYPTO) == server->cache[i].meta->id)
                 break;
@@ -59,7 +59,7 @@ int hsmCacheKey(whServerContext* server, whNvmMetadata* meta, uint8_t* in)
     int foundIndex = -1;
     /* make sure id is valid */
     if (meta->id == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* make sure the key fits in a slot */
     if (meta->len > WOLFHSM_NVM_MAX_OBJECT_SIZE)
         return BUFFER_E;
@@ -96,15 +96,15 @@ int hsmCacheKey(whServerContext* server, whNvmMetadata* meta, uint8_t* in)
     return 0;
 }
 
-int hsmReadKey(whServerContext* server, whNvmMetadata* meta, uint8_t* out)
+int hsmReadKey(whServerContext* server, whNvmMetadata* meta, uint8_t* out,
+    uint32_t outLen)
 {
     int ret = 0;
     int i;
     uint16_t searchId = meta->id;
-    uint16_t outLen = meta->len;
     /* make sure id is valid */
     if (meta->id == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* check the cache */
     for (i = 0; i < WOLFHSM_NUM_RAMKEYS; i++) {
         /* copy the meta and key before returning */
@@ -144,7 +144,7 @@ int hsmEvictKey(whServerContext* server, whNvmId keyId)
     int i;
     /* make sure id is valid */
     if (keyId == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* find key */
     for (i = 0; i < WOLFHSM_NUM_RAMKEYS; i++) {
         /* mark key as erased */
@@ -155,7 +155,7 @@ int hsmEvictKey(whServerContext* server, whNvmId keyId)
     }
     /* if the key wasn't found return an error */
     if (i >= WOLFHSM_NUM_RAMKEYS)
-        ret = BAD_FUNC_ARG;
+        ret = WH_ERROR_BADARGS;
     return ret;
 }
 
@@ -166,7 +166,7 @@ int hsmCommitKey(whServerContext* server, whNvmId keyId)
     CacheSlot* cacheSlot;
     /* make sure id is valid */
     if (keyId == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* find key in cache */
     for (i = 0; i < WOLFHSM_NUM_RAMKEYS; i++) {
         if (server->cache[i].meta->id == keyId) {
@@ -189,7 +189,7 @@ int hsmEraseKey(whServerContext* server, whNvmId keyId)
 {
     int i;
     if (keyId == WOLFHSM_ID_ERASED)
-        return BAD_FUNC_ARG;
+        return WH_ERROR_BADARGS;
     /* remove the key from the cache if present */
     for (i = 0; i < WOLFHSM_NUM_RAMKEYS; i++) {
         if (server->cache[i].meta->id == keyId) {
@@ -201,7 +201,7 @@ int hsmEraseKey(whServerContext* server, whNvmId keyId)
     return wh_Nvm_DestroyObjects(server->nvm, 1, &keyId);
 }
 
-int _wh_Server_HandleKeyRequest(whServerContext* server,
+int wh_Server_HandleKeyRequest(whServerContext* server,
         uint16_t magic, uint16_t action, uint16_t seq,
         uint8_t* data, uint16_t* size)
 {
@@ -218,8 +218,14 @@ int _wh_Server_HandleKeyRequest(whServerContext* server,
         /* set the metadata fields */
         meta->id = packet->keyCacheReq.id;
         meta->flags = packet->keyCacheReq.flags;
-        meta->len = packet->keyCacheReq.len;
-        XMEMCPY(meta->label, packet->keyCacheReq.label, WOLFHSM_NVM_LABEL_LEN);
+        meta->len = packet->keyCacheReq.sz;
+        /* validate label sz */
+        if (packet->keyCacheReq.labelSz > WOLFHSM_NVM_LABEL_LEN)
+            ret = WH_ERROR_BADARGS;
+        else {
+            XMEMCPY(meta->label, packet->keyCacheReq.label,
+                packet->keyCacheReq.labelSz);
+        }
         /* get a new id if one wasn't provided */
         if (meta->id == WOLFHSM_ID_ERASED) {
             ret =  hsmGetUniqueId(server);
@@ -246,12 +252,11 @@ int _wh_Server_HandleKeyRequest(whServerContext* server,
     case WH_KEY_EXPORT:
         /* out is after fixed size fields */
         out = (uint8_t*)(&packet->keyExportRes + 1);
-        /* set the metadata fields */
+        /* set the id */
         meta->id = packet->keyExportReq.id;
-        meta->len = WH_COMM_MTU -
-            (WOLFHSM_PACKET_STUB_SIZE + sizeof(packet->keyExportRes));
         /* read the key */
-        ret = hsmReadKey(server, meta, out);
+        ret = hsmReadKey(server, meta, out, WH_COMM_MTU -
+            (WOLFHSM_PACKET_STUB_SIZE + sizeof(packet->keyExportRes)));
         if (ret == 0) {
             /* set key len */
             packet->keyExportRes.len = meta->len;
@@ -278,7 +283,7 @@ int _wh_Server_HandleKeyRequest(whServerContext* server,
         }
         break;
     default:
-        ret = BAD_FUNC_ARG;
+        ret = WH_ERROR_BADARGS;
         break;
     }
     packet->rc = ret;

test/wh_test_crypto.c

@@ -79,7 +79,7 @@ int whTest_CryptoClientConfig(whClientConfig* config)
         goto exit;
     }
     /* test cache/export */
-    if ((ret = wh_Client_KeyCacheRequest(client, 0, labelStart, key, sizeof(key))) != 0) {
+    if ((ret = wh_Client_KeyCacheRequest(client, 0, labelStart, sizeof(labelStart), key, sizeof(key))) != 0) {
         WH_ERROR_PRINT("Failed to wh_Client_KeyCacheRequest %d\n", ret);
         goto exit;
     }
@@ -137,7 +137,7 @@ int whTest_CryptoClientConfig(whClientConfig* config)
         goto exit;
     }
     /* test commit */
-    if ((ret = wh_Client_KeyCacheRequest(client, WOLFHSM_ID_ERASED, labelStart, key, sizeof(key))) != 0) {
+    if ((ret = wh_Client_KeyCacheRequest(client, WOLFHSM_ID_ERASED, labelStart, sizeof(labelStart), key, sizeof(key))) != 0) {
         WH_ERROR_PRINT("Failed to wh_Client_KeyCacheRequest %d\n", ret);
         goto exit;
     }

wolfhsm/wh_client.h

@@ -69,9 +69,10 @@ int wh_Client_Echo(whClientContext* c, uint16_t snd_len, const void* snd_data,
 
 /* Key functions */
 int wh_Client_KeyCacheRequest_ex(whClientContext* c, uint32_t flags,
-    uint8_t* label, uint8_t* in, uint32_t inSz, uint16_t keyId);
+    uint8_t* label, uint32_t labelSz, uint8_t* in, uint32_t inSz,
+    uint16_t keyId);
 int wh_Client_KeyCacheRequest(whClientContext* c, uint32_t flags,
-    uint8_t* label, uint8_t* in, uint32_t inSz);
+    uint8_t* label, uint32_t labelSz, uint8_t* in, uint32_t inSz);
 int wh_Client_KeyCacheResponse(whClientContext* c, uint16_t* keyId);
 int wh_Client_KeyEvictRequest(whClientContext* c, uint16_t keyId);
 int wh_Client_KeyEvictResponse(whClientContext* c);

wolfhsm/wh_packet.h

@@ -247,7 +247,8 @@ typedef struct WOLFHSM_PACK wh_Packet_cmac_res
 typedef struct WOLFHSM_PACK wh_Packet_key_cache_req
 {
     uint32_t flags;
-    uint32_t len;
+    uint32_t sz;
+    uint32_t labelSz;
     uint16_t id;
     uint8_t label[WOLFHSM_NVM_LABEL_LEN];
     /* uint8_t in[]; */

wolfhsm/wh_server_keystore.h

@@ -5,11 +5,12 @@
 
 int hsmGetUniqueId(whServerContext* server);
 int hsmCacheKey(whServerContext* server, whNvmMetadata* meta, uint8_t* in);
-int hsmReadKey(whServerContext* server, whNvmMetadata* meta, uint8_t* out);
+int hsmReadKey(whServerContext* server, whNvmMetadata* meta, uint8_t* out,
+    uint32_t outLen);
 int hsmEvictKey(whServerContext* server, uint16_t keyId);
 int hsmCommitKey(whServerContext* server, uint16_t keyId);
 int hsmEraseKey(whServerContext* server, whNvmId keyId);
-int _wh_Server_HandleKeyRequest(whServerContext* server,
+int wh_Server_HandleKeyRequest(whServerContext* server,
         uint16_t magic, uint16_t action, uint16_t seq,
         uint8_t* data, uint16_t* size);