Fenrir-fixes

port/posix/posix_transport_shm.c

@@ -603,6 +603,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);
         }
     }
@@ -611,6 +614,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_auth_base.c

@@ -109,6 +109,29 @@ static int wh_Auth_BaseHashPin(const void* pin, uint16_t pin_len,
 #endif /* WOLFHSM_CFG_NO_CRYPTO */
 }
 
+/* Constant-time credential compare: the byte compare and length check combine
+ * without an early-out on mismatch, so timing can't reveal the stored length.
+ * (The oversized-length guard below returns early, but only on the supplied
+ * length, never the stored one.) Returns 0 on match. */
+static int wh_Auth_BaseCredsCompare(const unsigned char* stored,
+                                    uint16_t stored_len, const void* supplied,
+                                    uint16_t supplied_len)
+{
+    int diff;
+
+    /* Oversized supplied length can't match and must not read past the
+     * buffer. */
+    if (supplied_len > WH_AUTH_BASE_MAX_CREDENTIALS_LEN) {
+        return 1;
+    }
+
+    diff = wh_Utils_ConstantCompare(stored, (const uint8_t*)supplied,
+                                    supplied_len);
+    /* Fold in the length-equality check without branching. */
+    diff |= (int)((unsigned int)stored_len ^ (unsigned int)supplied_len);
+    return diff;
+}
+
 static whAuthBase_User* wh_Auth_BaseCheckPin(const char* username,
                                              const void* auth_data,
                                              uint16_t    auth_data_len)
@@ -133,8 +156,8 @@ static whAuthBase_User* wh_Auth_BaseCheckPin(const char* username,
 
     found_user = wh_Auth_BaseFindUser(username);
     if (found_user != NULL && found_user->method == WH_AUTH_METHOD_PIN &&
-        found_user->credentials_len == authCheck_len &&
-        wh_Utils_ConstantCompare(found_user->credentials, authCheck,
+        wh_Auth_BaseCredsCompare(found_user->credentials,
+                                 found_user->credentials_len, authCheck,
                                  authCheck_len) == 0) {
         ret = found_user;
     }
@@ -518,19 +541,19 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id,
             }
             wh_Utils_ForceZero(hash, sizeof(hash));
 #else
-            /* When crypto is disabled, compare PINs directly */
-            if (user->credentials_len != current_credentials_len ||
-                wh_Utils_ConstantCompare(user->credentials, current_credentials,
-                                         current_credentials_len) != 0) {
+            /* When crypto is disabled, compare PINs directly (constant time) */
+            if (wh_Auth_BaseCredsCompare(
+                    user->credentials, user->credentials_len,
+                    current_credentials, current_credentials_len) != 0) {
                 return WH_ERROR_ACCESS;
             }
 #endif /* WOLFHSM_CFG_NO_CRYPTO */
         }
         else {
-            /* For non-PIN methods, compare as-is */
-            if (user->credentials_len != current_credentials_len ||
-                wh_Utils_ConstantCompare(user->credentials, current_credentials,
-                                         current_credentials_len) != 0) {
+            /* For non-PIN methods, compare as-is (constant time) */
+            if (wh_Auth_BaseCredsCompare(
+                    user->credentials, user->credentials_len,
+                    current_credentials, current_credentials_len) != 0) {
                 return WH_ERROR_ACCESS;
             }
         }

src/wh_client_she.c

@@ -56,9 +56,10 @@ int wh_Client_ShePreProgramKey(whClientContext* c, whNvmId keyId,
 
     /* Create a key with 0 counter */
     wh_She_Meta2Label(0, flags, label);
-    ret = wh_Client_NvmAddObject(c,
-            WH_MAKE_KEYID(WH_KEYTYPE_SHE, c->comm->client_id, keyId),
-            0, 0, sizeof(label), label, keySz, key, (int32_t*)&outRc);
+    ret = wh_Client_NvmAddObject(
+        c, WH_MAKE_KEYID(WH_KEYTYPE_SHE, c->comm->client_id, keyId),
+        WH_NVM_ACCESS_ANY, 0, sizeof(label), label, keySz, key,
+        (int32_t*)&outRc);
     if (ret == 0)
         ret = outRc;
     return ret;

src/wh_nvm.c

@@ -266,14 +266,24 @@ int wh_Nvm_AddObject(whNvmContext* context, whNvmMetadata *meta,
 int wh_Nvm_AddObjectChecked(whNvmContext* context, whNvmMetadata* meta,
                             whNvmSize data_len, const uint8_t* data)
 {
-    int ret;
+    int           ret;
+    whNvmMetadata meta_copy;
+
+    if (meta == NULL) {
+        return WH_ERROR_BADARGS;
+    }
+
+    /* Snapshot the caller's metadata so a concurrently-writable buffer (e.g. a
+     * client DMA region) can't diverge the policy check from the stored
+     * object (TOCTOU). */
+    meta_copy = *meta;
 
-    ret = wh_Nvm_CheckPolicy(context, WH_NVM_OP_ADD, meta->id, NULL);
+    ret = wh_Nvm_CheckPolicy(context, WH_NVM_OP_ADD, meta_copy.id, NULL);
     if (ret != WH_ERROR_OK && ret != WH_ERROR_NOTFOUND) {
         return ret;
     }
 
-    return wh_Nvm_AddObject(context, meta, data_len, data);
+    return wh_Nvm_AddObject(context, &meta_copy, data_len, data);
 }
 
 int wh_Nvm_List(whNvmContext* context,

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

@@ -4134,6 +4134,10 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, int devId,
         }
     }
     WH_DEBUG_SERVER_VERBOSE("cmac end ret:%d\n", ret);
+    /* Scrub resolved key material: tmpKey and the Cmac context (AES schedule +
+     * k1/k2). Full-struct zero avoids a wc_CmacFree double-free. */
+    wh_Utils_ForceZero(tmpKey, sizeof(tmpKey));
+    wh_Utils_ForceZero(cmac, sizeof(cmac));
     return ret;
 }
 #endif /* WOLFSSL_CMAC && !NO_AES && WOLFSSL_AES_DIRECT */
@@ -8275,6 +8279,10 @@ static int _HandleCmacDma(whServerContext* ctx, uint16_t magic, int devId,
     }
 
     WH_DEBUG_SERVER_VERBOSE("dma cmac end ret:%d\n", ret);
+    /* Scrub resolved key material: tmpKey and the Cmac context (AES schedule +
+     * k1/k2). Full-struct zero avoids a wc_CmacFree double-free. */
+    wh_Utils_ForceZero(tmpKey, sizeof(tmpKey));
+    wh_Utils_ForceZero(cmac, sizeof(cmac));
     return ret;
 }
 #endif /* WOLFSSL_CMAC && !NO_AES && WOLFSSL_AES_DIRECT */

src/wh_server_nvm.c

@@ -363,75 +363,62 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
     {
         whMessageNvm_AddObjectDmaRequest req = {0};
         whMessageNvm_SimpleResponse resp = {0};
-        void* metadata = NULL;
-        void* data = NULL;
-        int metadata_dma_pre_ok = 0;
-        int data_dma_pre_ok = 0;
+        whNvmMetadata meta_local;
+        uint8_t data_local[WH_MESSAGE_NVM_MAX_ADDOBJECT_LEN];
 
         if (req_size != sizeof(req)) {
             /* Request is malformed */
             resp.rc = WH_ERROR_ABORTED;
         }
         if (resp.rc == 0) {
             /* Convert request struct */
-            wh_MessageNvm_TranslateAddObjectDmaRequest(magic,
-                    (whMessageNvm_AddObjectDmaRequest*)req_packet, &req);
-
-            /* perform platform-specific host address processing */
-            resp.rc = wh_Server_DmaProcessClientAddress(
-                server, req.metadata_hostaddr, &metadata, sizeof(whNvmMetadata),
-                WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
-            if (resp.rc == 0) {
-                metadata_dma_pre_ok = 1;
+            wh_MessageNvm_TranslateAddObjectDmaRequest(
+                magic, (whMessageNvm_AddObjectDmaRequest*)req_packet, &req);
+            if (req.data_len > sizeof(data_local)) {
+                resp.rc = WH_ERROR_BADARGS;
             }
         }
+        /* Snapshot the client metadata and data into server-owned memory so a
+         * concurrent client mutating the shared DMA buffer cannot diverge the
+         * policy/LMS check from what is persisted (TOCTOU). CopyFromClient
+         * pairs the DMA PRE/copy/POST internally. */
         if (resp.rc == 0) {
-            resp.rc = wh_Server_DmaProcessClientAddress(
-                server, req.data_hostaddr, &data, req.data_len,
-                WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
-            if (resp.rc == 0) {
-                data_dma_pre_ok = 1;
-            }
+            resp.rc = whServerDma_CopyFromClient(
+                server, &meta_local, req.metadata_hostaddr, sizeof(meta_local),
+                (whServerDmaFlags){0});
+        }
+        if (resp.rc == 0 && req.data_len > 0) {
+            resp.rc = whServerDma_CopyFromClient(
+                server, data_local, req.data_hostaddr, req.data_len,
+                (whServerDmaFlags){0});
         }
         if (resp.rc == 0) {
 #if !defined(WOLFHSM_CFG_NO_CRYPTO) && \
     (defined(WOLFSSL_HAVE_LMS) || defined(WOLFSSL_HAVE_XMSS))
             /* Block direct NVM import of stateful (LMS/XMSS) private key state;
-             * only on-HSM keygen may create such objects. */
-            if (wh_Crypto_IsStatefulSigPrivBlob((const uint8_t*)data,
-                                            (uint16_t)req.data_len)) {
+             * only on-HSM keygen may create such objects. Checked on the
+             * snapshot, not the live client buffer. */
+            if (wh_Crypto_IsStatefulSigPrivBlob(data_local,
+                                                (uint16_t)req.data_len)) {
                 resp.rc = WH_ERROR_ACCESS;
             }
             else
 #endif
             {
                 rc = WH_SERVER_NVM_LOCK(server);
                 if (rc == WH_ERROR_OK) {
-                    /* Process the AddObject action */
-                    rc = wh_Nvm_AddObjectChecked(
-                        server->nvm, (whNvmMetadata*)metadata, req.data_len,
-                        (const uint8_t*)data);
+                    /* Process the AddObject action from the snapshots */
+                    rc = wh_Nvm_AddObjectChecked(server->nvm, &meta_local,
+                                                 req.data_len, data_local);
 
                     (void)WH_SERVER_NVM_UNLOCK(server);
                 } /* WH_SERVER_NVM_LOCK() */
                 resp.rc = rc;
             }
         }
-        /* Always call POST for successful PREs, regardless of operation
-         * result */
-        if (metadata_dma_pre_ok) {
-            (void)wh_Server_DmaProcessClientAddress(
-                server, req.metadata_hostaddr, &metadata, sizeof(whNvmMetadata),
-                WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
-        }
-        if (data_dma_pre_ok) {
-            (void)wh_Server_DmaProcessClientAddress(
-                server, req.data_hostaddr, &data, req.data_len,
-                WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
-        }
         /* Convert the response struct */
-        wh_MessageNvm_TranslateSimpleResponse(magic,
-                &resp, (whMessageNvm_SimpleResponse*)resp_packet);
+        wh_MessageNvm_TranslateSimpleResponse(
+            magic, &resp, (whMessageNvm_SimpleResponse*)resp_packet);
         *out_resp_size = sizeof(resp);
     }; break;
 

src/wh_server_she.c

@@ -606,6 +606,7 @@ static int _LoadKey(whServerContext* server, uint16_t magic, uint16_t req_size,
     if (ret == 0) {
         meta->id       = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
                                        _PopId(req.messageOne));
+        meta->access   = WH_NVM_ACCESS_ANY;
         she_meta_flags = _PopFlags(req.messageTwo);
         she_meta_count = wh_Utils_ntohl(msg_counter_val) >> 4;
         /* Update the meta label with new values */
@@ -725,9 +726,10 @@ static int _LoadPlainKey(whServerContext* server, uint16_t magic,
                                                          &req);
     }
 
-    meta->id  = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
-                              WH_SHE_RAM_KEY_ID);
-    meta->len = WH_SHE_KEY_SZ;
+    meta->id     = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
+                                 WH_SHE_RAM_KEY_ID);
+    meta->access = WH_NVM_ACCESS_ANY;
+    meta->len    = WH_SHE_KEY_SZ;
 
     /* cache if ram key, overwrite otherwise */
     if (ret == 0) {
@@ -984,10 +986,11 @@ static int _InitRnd(whServerContext* server, uint16_t magic, uint16_t req_size,
     wc_AesFree(server->she->sheAes);
     /* save PRNG_SEED, i */
     if (ret == 0) {
-        meta->id  = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
-                                  WH_SHE_PRNG_SEED_ID);
-        meta->len = WH_SHE_KEY_SZ;
-        ret       = wh_Nvm_AddObject(server->nvm, meta, meta->len, cmacOutput);
+        meta->id     = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
+                                     WH_SHE_PRNG_SEED_ID);
+        meta->access = WH_NVM_ACCESS_ANY;
+        meta->len    = WH_SHE_KEY_SZ;
+        ret = wh_Nvm_AddObject(server->nvm, meta, meta->len, cmacOutput);
         if (ret != 0) {
             ret = WH_SHE_ERC_KEY_UPDATE_ERROR;
         }
@@ -1121,10 +1124,11 @@ static int _ExtendSeed(whServerContext* server, uint16_t magic,
     }
     /* save PRNG_SEED */
     if (ret == 0) {
-        meta->id  = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
-                                  WH_SHE_PRNG_SEED_ID);
-        meta->len = WH_SHE_KEY_SZ;
-        ret       = wh_Nvm_AddObject(server->nvm, meta, meta->len, kdfInput);
+        meta->id     = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
+                                     WH_SHE_PRNG_SEED_ID);
+        meta->access = WH_NVM_ACCESS_ANY;
+        meta->len    = WH_SHE_KEY_SZ;
+        ret          = wh_Nvm_AddObject(server->nvm, meta, meta->len, kdfInput);
         if (ret != 0) {
             ret = WH_SHE_ERC_KEY_UPDATE_ERROR;
         }

test/wh_test_auth.c

@@ -637,6 +637,24 @@ int whTest_AuthLogin(whClientContext* client)
                           server_rc != WH_ERROR_OK);
     WH_TEST_ASSERT_RETURN(user_id == WH_USER_ID_INVALID);
 
+    /* Wrong-length prefixes of the admin PIN "1234" must be rejected
+     * (report 5458): "123" is a shorter prefix, "12345" a longer one. */
+    WH_TEST_PRINT("  Test: Login rejects wrong-length credential\n");
+    server_rc = 0;
+    user_id   = WH_USER_ID_INVALID;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LoginOp(client, WH_AUTH_METHOD_PIN,
+                                                TEST_ADMIN_USERNAME, "123", 3,
+                                                &server_rc, &user_id));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_AUTH_LOGIN_FAILED);
+    WH_TEST_ASSERT_RETURN(user_id == WH_USER_ID_INVALID);
+    server_rc = 0;
+    user_id   = WH_USER_ID_INVALID;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LoginOp(client, WH_AUTH_METHOD_PIN,
+                                                TEST_ADMIN_USERNAME, "12345", 5,
+                                                &server_rc, &user_id));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_AUTH_LOGIN_FAILED);
+    WH_TEST_ASSERT_RETURN(user_id == WH_USER_ID_INVALID);
+
     /* Test 2: Login with valid credentials - use blocking version */
     WH_TEST_PRINT("  Test: Login with valid credentials\n");
     server_rc = 0;
@@ -1412,6 +1430,35 @@ int whTest_AuthSetCredentials(whClientContext* client)
         _whTest_Auth_DeleteUserByName(client, "victimcreduser");
     }
 
+    /* Wrong-length current credential must be rejected like a wrong-value one
+     * (report 5458). testuser4's PIN is "newpass" (7); we are admin. */
+    WH_TEST_PRINT("  Test: Set-credentials rejects wrong current credential\n");
+    /* Wrong length (shorter) */
+    server_rc = 0;
+    WH_TEST_RETURN_ON_FAIL(
+        _whTest_Auth_UserSetCredsOp(client, user_id, WH_AUTH_METHOD_PIN,
+                                    "newpas", 6, "changed", 7, &server_rc));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS);
+    /* Wrong length (longer, correct value as a prefix) */
+    server_rc = 0;
+    WH_TEST_RETURN_ON_FAIL(
+        _whTest_Auth_UserSetCredsOp(client, user_id, WH_AUTH_METHOD_PIN,
+                                    "newpassX", 8, "changed", 7, &server_rc));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS);
+    /* Correct length, wrong value */
+    server_rc = 0;
+    WH_TEST_RETURN_ON_FAIL(
+        _whTest_Auth_UserSetCredsOp(client, user_id, WH_AUTH_METHOD_PIN,
+                                    "wrongpw", 7, "changed", 7, &server_rc));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS);
+    /* Correct current credential still succeeds (behavior preserved); keep the
+     * PIN as "newpass" so the verification below continues to hold. */
+    server_rc = 0;
+    WH_TEST_RETURN_ON_FAIL(
+        _whTest_Auth_UserSetCredsOp(client, user_id, WH_AUTH_METHOD_PIN,
+                                    "newpass", 7, "newpass", 7, &server_rc));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+
     /* Verify new credentials work */
     _whTest_Auth_LogoutOp(client, admin_id, &server_rc);
     memset(&admin_perms, 0, sizeof(admin_perms));