wolfHSM Hardening Fixes

examples/posix/wh_posix_server/wh_posix_server_cfg.c

@@ -486,12 +486,19 @@ static void parseNvmInitFile(const char* filePath)
 
         /* Parse the file path */
         token = strtok(NULL, " ");
-        if (!token || sscanf(token, "%s", filePath) != 1) {
+        if (!token) {
             WOLFHSM_CFG_PRINTF("Error on line %d: Malformed entry - missing file path\n",
                     lineNumber);
             fclose(file);
             exit(EXIT_FAILURE);
         }
+        if (snprintf(filePath, sizeof(filePath), "%s", token) >=
+            (int)sizeof(filePath)) {
+            WOLFHSM_CFG_PRINTF("Error on line %d: File path too long\n",
+                    lineNumber);
+            fclose(file);
+            exit(EXIT_FAILURE);
+        }
 
         /* Add the parsed entry to the linked list */
         appendEntry(&entryHead, (uint8_t)clientId, (uint16_t)id,

port/posix/posix_transport_shm.c

@@ -208,8 +208,8 @@ static int posixTransportShm_CreateMap(char* name, uint16_t req_size,
 
     /* Attempt to remove any existing object. */
     (void)shm_unlink(name);
-    /* Create shared memory object and set the size */
-    fd = shm_open(name, O_CREAT | O_RDWR, PTSHM_CREATEMODE);
+    /* O_EXCL fails the open if a racing process re-created the name */
+    fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, PTSHM_CREATEMODE);
     if (fd >= 0) {
         /* Set the size of the shared memory object.
          * Note this is the minimum size, as the OS may make it larger. */

src/wh_auth_base.c

@@ -479,6 +479,11 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id,
         return WH_ERROR_ACCESS;
     }
 
+    /* A positive new-credential length requires a valid buffer */
+    if (new_credentials == NULL && new_credentials_len > 0) {
+        return WH_ERROR_BADARGS;
+    }
+
     /* Validate method is supported */
     if (method != WH_AUTH_METHOD_PIN
 #if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER) && !defined(WOLFHSM_CFG_NO_CRYPTO)
@@ -543,7 +548,8 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id,
         }
     }
 
-    /* Set new credentials */
+    /* Set new credentials. The prior credential is zeroized only at the commit
+     * point so a failed update leaves the existing credential intact. */
     if (new_credentials_len > 0) {
         if (method == WH_AUTH_METHOD_PIN) {
 #ifndef WOLFHSM_CFG_NO_CRYPTO
@@ -555,13 +561,15 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id,
                 wh_Utils_ForceZero(hash, sizeof(hash));
                 return rc;
             }
+            wh_Utils_ForceZero(user->credentials, sizeof(user->credentials));
             memcpy(user->credentials, hash, WC_SHA256_DIGEST_SIZE);
             user->credentials_len = WC_SHA256_DIGEST_SIZE;
             wh_Utils_ForceZero(hash, sizeof(hash));
 #else
             if (new_credentials_len > WH_AUTH_BASE_MAX_CREDENTIALS_LEN) {
                 return WH_ERROR_BUFFER_SIZE;
             }
+            wh_Utils_ForceZero(user->credentials, sizeof(user->credentials));
             memcpy(user->credentials, new_credentials, new_credentials_len);
             user->credentials_len = new_credentials_len;
 #endif /* WOLFHSM_CFG_NO_CRYPTO */
@@ -570,12 +578,14 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id,
             if (new_credentials_len > WH_AUTH_BASE_MAX_CREDENTIALS_LEN) {
                 return WH_ERROR_BUFFER_SIZE;
             }
+            wh_Utils_ForceZero(user->credentials, sizeof(user->credentials));
             memcpy(user->credentials, new_credentials, new_credentials_len);
             user->credentials_len = new_credentials_len;
         }
     }
     else {
         /* Allow clearing credentials by setting length to 0 */
+        wh_Utils_ForceZero(user->credentials, sizeof(user->credentials));
         user->credentials_len = 0;
     }
     user->method = method;

src/wh_client.c

@@ -37,6 +37,7 @@
 
 /* Components */
 #include "wolfhsm/wh_comm.h"
+#include "wolfhsm/wh_utils.h"
 
 #ifndef WOLFHSM_CFG_NO_CRYPTO
 #include "wolfssl/wolfcrypt/settings.h"
@@ -811,6 +812,7 @@ int wh_Client_KeyCacheRequest_ex(whClientContext* c, uint32_t flags,
     whMessageKeystore_CacheRequest* req = NULL;
     uint8_t*                        packIn;
     uint16_t                        capSz;
+    int                             ret;
 
     if (c == NULL || in == NULL || inSz == 0 ||
         sizeof(*req) + inSz > WOLFHSM_CFG_COMM_DATA_LEN) {
@@ -841,8 +843,10 @@ int wh_Client_KeyCacheRequest_ex(whClientContext* c, uint32_t flags,
     memcpy(packIn, in, inSz);
 
     /* write request */
-    return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY, WH_KEY_CACHE,
-                                 sizeof(*req) + inSz, (uint8_t*)req);
+    ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY, WH_KEY_CACHE,
+                                sizeof(*req) + inSz, (uint8_t*)req);
+    wh_Utils_ForceZero(packIn, inSz);
+    return ret;
 }
 
 int wh_Client_KeyCacheRequest(whClientContext* c, uint32_t flags,
@@ -1001,6 +1005,10 @@ int wh_Client_KeyExportResponse(whClientContext* c, uint8_t* label,
         if (resp->rc != 0) {
             ret = resp->rc;
         }
+        else if (size < sizeof(*resp) || resp->len > (size - sizeof(*resp))) {
+            /* Response frame does not contain the claimed key length */
+            ret = WH_ERROR_ABORTED;
+        }
         else {
             if (out == NULL) {
                 *outSz = resp->len;
@@ -1019,6 +1027,7 @@ int wh_Client_KeyExportResponse(whClientContext* c, uint8_t* label,
                 else
                     memcpy(label, resp->label, labelSz);
             }
+            wh_Utils_ForceZero(packOut, resp->len);
         }
     }
     return ret;

src/wh_client_keywrap.c

@@ -9,6 +9,7 @@
 #include <wolfhsm/wh_error.h>
 #include <wolfhsm/wh_message.h>
 #include <wolfhsm/wh_message_keystore.h>
+#include <wolfhsm/wh_utils.h>
 #include <wolfssl/wolfcrypt/settings.h>
 #include <wolfssl/wolfcrypt/types.h>
 
@@ -17,6 +18,7 @@ int wh_Client_KeyWrapRequest(whClientContext*   ctx,
                              uint16_t serverKeyId, void* key, uint16_t keySz,
                              whNvmMetadata* metadata)
 {
+    int                               ret;
     uint16_t                          group  = WH_MESSAGE_GROUP_KEY;
     uint16_t                          action = WH_KEY_KEYWRAP;
     whMessageKeystore_KeyWrapRequest* req    = NULL;
@@ -30,6 +32,10 @@ int wh_Client_KeyWrapRequest(whClientContext*   ctx,
         return WH_ERROR_BADARGS;
     }
 
+    if (sizeof(*req) + sizeof(*metadata) + keySz > WOLFHSM_CFG_COMM_DATA_LEN) {
+        return WH_ERROR_BADARGS;
+    }
+
     /* Set the request pointer to the shared comm data memory region */
     req =
         (whMessageKeystore_KeyWrapRequest*)wh_CommClient_GetDataPtr(ctx->comm);
@@ -47,9 +53,11 @@ int wh_Client_KeyWrapRequest(whClientContext*   ctx,
     memcpy(reqData, metadata, sizeof(*metadata));
     memcpy(reqData + sizeof(*metadata), key, keySz);
 
-    return wh_Client_SendRequest(ctx, group, action,
-                                 sizeof(*req) + sizeof(*metadata) + keySz,
-                                 (uint8_t*)req);
+    ret = wh_Client_SendRequest(ctx, group, action,
+                                sizeof(*req) + sizeof(*metadata) + keySz,
+                                (uint8_t*)req);
+    wh_Utils_ForceZero(reqData + sizeof(*metadata), keySz);
+    return ret;
 }
 
 int wh_Client_KeyWrapResponse(whClientContext*   ctx,
@@ -220,6 +228,7 @@ int wh_Client_KeyUnwrapAndExportResponse(whClientContext*   ctx,
     memcpy(keyOut, respData + sizeof(*metadataOut), resp->keySz);
     *keyInOutSz = resp->keySz;
 
+    wh_Utils_ForceZero(respData + sizeof(*metadataOut), resp->keySz);
     return WH_ERROR_OK;
 }
 

src/wh_client_she.c

@@ -46,6 +46,7 @@
 #include "wolfhsm/wh_client.h"
 
 #include "wolfhsm/wh_client_she.h"
+#include "wolfhsm/wh_utils.h"
 
 int wh_Client_ShePreProgramKey(whClientContext* c, whNvmId keyId,
     whNvmFlags flags, uint8_t* key, whNvmSize keySz)
@@ -336,6 +337,7 @@ int wh_Client_SheLoadKey(whClientContext* c, uint8_t* messageOne,
 int wh_Client_SheLoadPlainKeyRequest(whClientContext* c, uint8_t* key,
                                      uint32_t keySz)
 {
+    int                               ret;
     whMessageShe_LoadPlainKeyRequest* req = NULL;
 
     if (c == NULL || key == NULL || keySz < WH_SHE_KEY_SZ) {
@@ -346,8 +348,10 @@ int wh_Client_SheLoadPlainKeyRequest(whClientContext* c, uint8_t* key,
 
     memcpy(req->key, key, WH_SHE_KEY_SZ);
 
-    return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE, WH_SHE_LOAD_PLAIN_KEY,
-                                 sizeof(*req), (uint8_t*)req);
+    ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE, WH_SHE_LOAD_PLAIN_KEY,
+                                sizeof(*req), (uint8_t*)req);
+    wh_Utils_ForceZero(req->key, WH_SHE_KEY_SZ);
+    return ret;
 }
 
 int wh_Client_SheLoadPlainKeyResponse(whClientContext* c)

src/wh_server_cert.c

@@ -1515,15 +1515,15 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
                     WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
             }
 
+            /* Propagate any server-side error before serializing the response */
+            if (rc != WH_ERROR_OK) {
+                resp.rc = rc;
+            }
+
             /* Convert the response struct */
             wh_MessageCert_TranslateSimpleResponse(
                 magic, &resp, (whMessageCert_SimpleResponse*)resp_packet);
             *out_resp_size = sizeof(resp);
-
-            /* If there was an error, return it in the response */
-            if (rc != WH_ERROR_OK) {
-                resp.rc = rc;
-            }
         } break;
 #endif /* WOLFHSM_CFG_DMA */
 #endif /* WOLFHSM_CFG_CERTIFICATE_MANAGER_ACERT */

src/wh_server_crypto.c

@@ -1533,8 +1533,10 @@ static int _HandleHkdf(whServerContext* ctx, uint16_t magic, int devId,
      */
     uint8_t* out =
         (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_HkdfResponse);
-    uint16_t max_size = (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN -
-                                   ((uint8_t*)out - (uint8_t*)cryptoDataOut));
+    uint16_t max_size =
+        (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN -
+                   sizeof(whMessageCrypto_GenericResponseHeader) -
+                   sizeof(whMessageCrypto_HkdfResponse));
 
     /* Check if output size is valid */
     if (outSz > max_size) {
@@ -1694,8 +1696,10 @@ static int _HandleCmacKdf(whServerContext* ctx, uint16_t magic, int devId,
 
     uint8_t* out =
         (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_CmacKdfResponse);
-    uint16_t max_size = (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN -
-                                   ((uint8_t*)out - (uint8_t*)cryptoDataOut));
+    uint16_t max_size =
+        (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN -
+                   sizeof(whMessageCrypto_GenericResponseHeader) -
+                   sizeof(whMessageCrypto_CmacKdfResponse));
 
     if (outSz > max_size) {
         return WH_ERROR_BADARGS;
@@ -3365,9 +3369,10 @@ static int _HandleAesGcm(whServerContext* ctx, uint16_t magic, int devId,
     uint32_t tag_len     = req.authTagSz;
     whKeyId  key_id      = wh_KeyId_TranslateFromClient(
              WH_KEYTYPE_CRYPTO, ctx->comm->client_id, req.keyId);
-    uint64_t needed_size = sizeof(whMessageCrypto_AesGcmRequest) + len +
-                           key_len + iv_len + authin_len +
-                           ((enc == 0) ? tag_len : 0);
+    uint64_t needed_size = (uint64_t)sizeof(whMessageCrypto_AesGcmRequest) +
+                           (uint64_t)len + (uint64_t)key_len +
+                           (uint64_t)iv_len + (uint64_t)authin_len +
+                           ((enc == 0) ? (uint64_t)tag_len : 0);
     if (needed_size != inSize) {
         return WH_ERROR_BADARGS;
     }
@@ -3391,6 +3396,12 @@ static int _HandleAesGcm(whServerContext* ctx, uint16_t magic, int devId,
     uint32_t res_len = sizeof(whMessageCrypto_AesGcmResponse) + len +
                        ((enc == 0) ? 0 : tag_len);
 
+    /* Ensure the response output and tag fit within the comm data buffer */
+    if (res_len > (WOLFHSM_CFG_COMM_DATA_LEN -
+                   sizeof(whMessageCrypto_GenericResponseHeader))) {
+        return WH_ERROR_BADARGS;
+    }
+
     WH_DEBUG_SERVER_VERBOSE("AESGCM: enc:%d keylen:%d ivsz:%d insz:%d authinsz:%d "
             "authtagsz:%d reqsz:%u ressz:%u\n",
             enc, key_len, iv_len, len, authin_len, tag_len, (uint32_t)needed_size,

tools/whnvmtool/whnvmtool.c

@@ -384,13 +384,20 @@ void parseConfigFile(const char* filePath)
 
         /* Parse the file path */
         token = strtok(NULL, " ");
-        if (!token || sscanf(token, "%s", filePath) != 1) {
+        if (!token) {
             fprintf(stderr,
                     "Error on line %d: Malformed entry - missing file path\n",
                     lineNumber);
             fclose(file);
             exit(EXIT_FAILURE);
         }
+        if (snprintf(filePath, sizeof(filePath), "%s", token) >=
+            (int)sizeof(filePath)) {
+            fprintf(stderr, "Error on line %d: File path too long\n",
+                    lineNumber);
+            fclose(file);
+            exit(EXIT_FAILURE);
+        }
 
         /* Add the parsed entry to the linked list */
         appendEntry(&entryHead, (uint8_t)clientId, (uint16_t)id,