Fenrir fixes

docs/src/5-Features.md

@@ -872,7 +872,7 @@ A verify method returns `WH_ERROR_OK` on a successful verification, `WH_ERROR_NO
 - `wh_Server_ImgMgrVerifyMethodWolfBootRsa4096WithSha256`: RSA-4096 verification of a wolfBoot-formatted image (see [wolfBoot Image Support](#wolfboot-image-support))
 - `wh_Server_ImgMgrVerifyMethodWolfBootCertChainRsa4096WithSha256`: cert-chain-based RSA-4096 verification of a wolfBoot image
 
-Applications can supply their own verify method to support algorithms not represented in the built-in set, or to layer additional checks on top of an existing one — for example, validating a monotonic counter against a [non-volatile counter](#non-volatile-monotonic-counters) inside a wrapper verify method to add anti-rollback protection. The maximum signature size handled by the framework is `WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE`, whose default accommodates RSA-4096.
+Applications can supply their own verify method to support algorithms not represented in the built-in set, or to layer additional checks on top of an existing one — for example, validating a monotonic counter against a [non-volatile counter](#non-volatile-monotonic-counters) inside a wrapper verify method to add anti-rollback protection. The maximum signature size handled by the framework is `WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE`, whose default accommodates RSA-4096. Verification keys are copied out of the keystore into a private buffer before the verify method runs, bounded by `WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE` (default 1200 bytes, enough for an ASN.1 RSA-4096 public key).
 
 ### Verify Actions
 

docs/src/9-Configuration.md

@@ -117,6 +117,7 @@ These macros size the server-side key cache. The cache is split into "regular" s
 | `WOLFHSM_CFG_SERVER_IMG_MGR` | Undefined | If defined, compile the server-side image manager (manifest-driven boot/runtime image verification). |
 | `WOLFHSM_CFG_SERVER_IMG_MGR_MAX_IMG_COUNT` | `4` | Maximum number of images that a single image-manager configuration can track at one time. |
 | `WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE` | `512` | Maximum signature size, in bytes, that the image manager will allocate buffer space for. The default accommodates RSA-4096; raise it when using signature schemes with larger signatures. |
+| `WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE` | `1200` | Maximum verification key size, in bytes, that the image manager copies out of the keystore before invoking a verify method. The default accommodates an ASN.1 RSA-4096 public key; raise it for larger keys such as ML-DSA. |
 
 ## Custom Server Callbacks
 

port/posix/posix_transport_shm.c

@@ -661,6 +661,9 @@ int posixTransportShm_ClientStaticMemDmaCallback(
     else if (oper == WH_DMA_OPER_CLIENT_READ_POST) {
         if (isInDma == 0) {
             uint8_t* ptr = (uint8_t*)dmaPtr + (uintptr_t)*xformedCliAddr;
+            /* Scrub key material before freeing. len is bounded by the temp
+             * buffer's XMALLOC, well within uint32_t for this transport. */
+            wh_Utils_ForceZero(ptr, (uint32_t)len);
             XFREE(ptr, heap, DYNAMIC_TYPE_TMP_BUFFER);
         }
     }
@@ -669,6 +672,9 @@ int posixTransportShm_ClientStaticMemDmaCallback(
             uint8_t* ptr = (uint8_t*)dmaPtr + (uintptr_t)*xformedCliAddr;
             memcpy((void*)clientAddr, ptr,
                    len); /* copy results of what server wrote */
+            /* Scrub key material before freeing. len is bounded by the temp
+             * buffer's XMALLOC, well within uint32_t for this transport. */
+            wh_Utils_ForceZero(ptr, (uint32_t)len);
             XFREE(ptr, heap, DYNAMIC_TYPE_TMP_BUFFER);
         }
     }

src/wh_server_counter.c

@@ -225,7 +225,8 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
         } break;
 
         default:
-            ret = WH_ERROR_BADARGS;
+            *out_resp_size = 0;
+            ret            = WH_ERROR_BADARGS;
             break;
     }
 

src/wh_server_crypto.c

@@ -4613,7 +4613,10 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, int devId,
             *outSize = sizeof(res) + res.outSz;
         }
     }
+    /* Scrub the key and the Cmac context (AES schedule, k1/k2). Zeroing the
+     * struct instead of wc_CmacFree avoids a double free after wc_CmacFinal */
     wc_ForceZero(tmpKey, sizeof(tmpKey));
+    wc_ForceZero(cmac, sizeof(cmac));
     WH_DEBUG_SERVER_VERBOSE("cmac end ret:%d\n", ret);
     return ret;
 }
@@ -9127,7 +9130,10 @@ static int _HandleCmacDma(whServerContext* ctx, uint16_t magic, int devId,
         }
     }
 
+    /* Scrub the key and the Cmac context (AES schedule, k1/k2). Zeroing the
+     * struct instead of wc_CmacFree avoids a double free after wc_CmacFinal */
     wc_ForceZero(tmpKey, sizeof(tmpKey));
+    wc_ForceZero(cmac, sizeof(cmac));
     WH_DEBUG_SERVER_VERBOSE("dma cmac end ret:%d\n", ret);
     return ret;
 }

src/wh_server_img_mgr.c

@@ -33,6 +33,7 @@
 #include <string.h>
 
 #include "wolfhsm/wh_error.h"
+#include "wolfhsm/wh_utils.h"
 #include "wolfhsm/wh_server.h"
 #include "wolfhsm/wh_server_img_mgr.h"
 #include "wolfhsm/wh_server_keystore.h"
@@ -95,15 +96,38 @@ int wh_Server_ImgMgrInit(whServerImgMgrContext*      context,
     return ret;
 }
 
+/* Copy the key out of the keystore under the NVM lock so the verify callback
+ * works from a private snapshot, not a live cache slot that another server
+ * context could evict or rewrite mid-verification. */
+static int _ImgMgrCopyKeyFromKeystore(whServerContext* server, whKeyId keyId,
+                                      uint8_t* dst, size_t dstMax,
+                                      size_t* outLen)
+{
+    int      ret;
+    uint32_t keySz = (uint32_t)dstMax;
+
+    ret = WH_SERVER_NVM_LOCK(server);
+    if (ret != WH_ERROR_OK) {
+        return ret;
+    }
+    ret = wh_Server_KeystoreReadKey(server, keyId, NULL, dst, &keySz);
+    (void)WH_SERVER_NVM_UNLOCK(server);
+
+    if (ret == WH_ERROR_OK) {
+        *outLen = keySz;
+    }
+    return ret;
+}
+
 int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext*      context,
                               const whServerImgMgrImg*    img,
                               whServerImgMgrVerifyResult* result)
 {
-    int              ret     = WH_ERROR_OK;
-    whServerContext* server  = NULL;
-    uint8_t*         keyBuf  = NULL;
-    whNvmMetadata*   keyMeta = NULL;
-    size_t           keySz   = 0;
+    int              ret    = WH_ERROR_OK;
+    whServerContext* server = NULL;
+    uint8_t          keyBuf[WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE];
+    const uint8_t*   keyPtr = NULL; /* stays NULL for paths with no key */
+    size_t           keySz  = 0;
     uint8_t sigBuf[WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE]; /* Buffer for
                                                                 signature */
     whNvmMetadata sigMeta       = {0};
@@ -127,12 +151,12 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext*      context,
     switch (img->imgType) {
         case WH_IMG_MGR_IMG_TYPE_WOLFBOOT:
             /* Load key from keystore, skip sig loading (sig is in header) */
-            ret = wh_Server_KeystoreFreshenKey(server, img->keyId, &keyBuf,
-                                               &keyMeta);
+            ret = _ImgMgrCopyKeyFromKeystore(server, img->keyId, keyBuf,
+                                             sizeof(keyBuf), &keySz);
             if (ret != WH_ERROR_OK) {
                 return ret;
             }
-            keySz = keyMeta->len;
+            keyPtr = keyBuf;
             /* sig/sigSz passed as NULL/0 to callback */
             break;
 
@@ -142,15 +166,8 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext*      context,
             break;
 
         case WH_IMG_MGR_IMG_TYPE_RAW:
-            /* Existing behavior: load key from keystore + sig from NVM */
-            ret = wh_Server_KeystoreFreshenKey(server, img->keyId, &keyBuf,
-                                               &keyMeta);
-            if (ret != WH_ERROR_OK) {
-                return ret;
-            }
-            keySz = keyMeta->len;
-
-            /* Load the signature from NVM */
+            /* Load the signature from NVM first so the key snapshot is the
+             * last thing taken before verification */
             ret = wh_Nvm_GetMetadata(server->nvm, img->sigNvmId, &sigMeta);
             if (ret != WH_ERROR_OK) {
                 return ret;
@@ -168,6 +185,14 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext*      context,
             }
             actualSigSize = sigMeta.len;
             sigPtr        = sigBuf;
+
+            /* Load key from keystore */
+            ret = _ImgMgrCopyKeyFromKeystore(server, img->keyId, keyBuf,
+                                             sizeof(keyBuf), &keySz);
+            if (ret != WH_ERROR_OK) {
+                return ret;
+            }
+            keyPtr = keyBuf;
             break;
 
         default:
@@ -177,11 +202,13 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext*      context,
     /* Invoke verify method callback */
     if (img->verifyMethod != NULL) {
         result->verifyMethodResult = img->verifyMethod(
-            context, img, keyBuf, keySz, sigPtr, actualSigSize);
+            context, img, keyPtr, keySz, sigPtr, actualSigSize);
     }
     else {
         result->verifyMethodResult = WH_ERROR_NOHANDLER;
     }
+    /* The key snapshot may hold a symmetric key (AES-CMAC verify) */
+    wh_Utils_ForceZero(keyBuf, sizeof(keyBuf));
 
     /* Invoke verifyAction callback */
     if (img->verifyAction != NULL) {

test/wh_test_cert.c

@@ -28,6 +28,8 @@
 #if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER) && !defined(WOLFHSM_CFG_NO_CRYPTO)
 
 #include "wolfhsm/wh_error.h"
+#include "wolfhsm/wh_message.h"
+#include "wolfhsm/wh_message_cert.h"
 
 #ifdef WOLFHSM_CFG_ENABLE_SERVER
 #include "wolfhsm/wh_server.h"
@@ -1846,6 +1848,32 @@ int whTest_CertClientAcertDma_ClientServerTestInternal(whClientContext* client)
         client, attrCert_der, attrCert_der_len, rootCertB_id, &out_rc));
     WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_CERT_VERIFY);
 
+    /* Regression test for finding 4235. A malformed (undersized) ACERT_DMA
+     * request must report an error on the wire, not a false success. Send a
+     * raw 1 byte request with the low level API and confirm resp.rc is not OK.
+     */
+    WH_TEST_PRINT("Sending malformed ACERT_DMA request...\n");
+    {
+        uint8_t                      badReq = 0;
+        uint16_t                     rgroup, raction, rsize;
+        whMessageCert_SimpleResponse badResp = {0};
+
+        do {
+            rc = wh_Client_SendRequest(
+                client, WH_MESSAGE_GROUP_CERT,
+                WH_MESSAGE_CERT_ACTION_VERIFY_ACERT_DMA, sizeof(badReq),
+                &badReq);
+        } while (rc == WH_ERROR_NOTREADY);
+        WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK);
+
+        do {
+            rc = wh_Client_RecvResponse(client, &rgroup, &raction, &rsize,
+                                        sizeof(badResp), &badResp);
+        } while (rc == WH_ERROR_NOTREADY);
+        WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK);
+        WH_TEST_ASSERT_RETURN(badResp.rc != WH_ERROR_OK);
+    }
+
     /* Clean up - delete the trusted certificates */
     WH_TEST_PRINT("Deleting trusted certificates...\n");
     WH_TEST_RETURN_ON_FAIL(

test/wh_test_clientserver.c

@@ -689,6 +689,25 @@ static int _testClientCounter(whClientContext* client)
             wh_Client_CounterRead(client, (whNvmId)i, &counter));
     }
 
+    /* Invalid counter action: the default case reports a zero length
+     * response, not the stale request sized buffer. */
+    {
+        uint8_t  reqbuf[8]   = {0};
+        uint8_t  respbuf[64] = {0};
+        uint16_t respGroup   = 0;
+        uint16_t respAction  = 0;
+        uint16_t respSz      = 0xFFFF;
+
+        WH_TEST_RETURN_ON_FAIL(wh_Client_SendRequest(
+            client, WH_MESSAGE_GROUP_COUNTER, 0x7F, sizeof(reqbuf), reqbuf));
+        do {
+            rc = wh_Client_RecvResponse(client, &respGroup, &respAction,
+                                        &respSz, sizeof(respbuf), respbuf);
+        } while (rc == WH_ERROR_NOTREADY);
+        WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK);
+        WH_TEST_ASSERT_RETURN(respSz == 0);
+    }
+
     /* Ensure NVM is back to the pre-test baseline */
     WH_TEST_RETURN_ON_FAIL(rc = wh_Client_NvmGetAvailable(
                                client, &server_rc, &avail_size, &avail_objects,