Skip to content

Commit 8d89392

Browse files
bigbrettbigbrett
committed
Review feedback:
- Reserve removed error codes (wh_error.h): WH_ERROR_RESERVED{1,2} - Reserve removed message group (wh_message.h): WH_MESSAGE_GROUP_RESERVED - Rename CMAC → CmacAes (wh_message_crypto.h, all consumers): All structs and translation functions renamed to indicate AES-specific - Remove `type` field from request structs and translation; use WC_CMAC_AES literal on server - Remove switch(req.type) in server handlers; guard with #if defined(WOLFSSL_CMAC) && !defined(NO_AES) && defined(WOLFSSL_AES_DIRECT) instead - Extract _CmacResolveKey() static helper for shared key resolution (inline key or keystore + usage policy + size validation) - Extract wh_Crypto_CmacAesSaveStateToMsg() / RestoreStateFromMsg() to deduplicate state pack/unpack across client + server + DMA - BAD_FUNC_ARG → WH_ERROR_BADARGS for bufferSz validation (now inside RestoreStateFromMsg) - Don't read req.* after response may overlap — use WC_CMAC_AES literal instead of req.type - #define/#undef → enum for CMAC_MLEN_* test constants
1 parent 2dedb29 commit 8d89392

10 files changed

Lines changed: 358 additions & 434 deletions

‎src/wh_client_crypto.c‎

Lines changed: 22 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3587,12 +3587,12 @@ int wh_Client_Cmac(whClientContext* ctx, Cmac* cmac, CmacType type,
35873587
const uint8_t* key, uint32_t keyLen, const uint8_t* in,
35883588
uint32_t inLen, uint8_t* outMac, uint32_t* outMacLen)
35893589
{
3590-
int ret = WH_ERROR_OK;
3591-
uint16_t group = WH_MESSAGE_GROUP_CRYPTO;
3592-
uint16_t action = WC_ALGO_TYPE_CMAC;
3593-
whMessageCrypto_CmacRequest* req = NULL;
3594-
whMessageCrypto_CmacResponse* res = NULL;
3595-
uint8_t* dataPtr = NULL;
3590+
int ret = WH_ERROR_OK;
3591+
uint16_t group = WH_MESSAGE_GROUP_CRYPTO;
3592+
uint16_t action = WC_ALGO_TYPE_CMAC;
3593+
whMessageCrypto_CmacAesRequest* req = NULL;
3594+
whMessageCrypto_CmacAesResponse* res = NULL;
3595+
uint8_t* dataPtr = NULL;
35963596

35973597
if (ctx == NULL || cmac == NULL) {
35983598
return WH_ERROR_BADARGS;
@@ -3630,8 +3630,8 @@ int wh_Client_Cmac(whClientContext* ctx, Cmac* cmac, CmacType type,
36303630
}
36313631

36323632
/* Setup generic header and get pointer to request data */
3633-
req = (whMessageCrypto_CmacRequest*)_createCryptoRequest(dataPtr,
3634-
WC_ALGO_TYPE_CMAC);
3633+
req = (whMessageCrypto_CmacAesRequest*)_createCryptoRequest(
3634+
dataPtr, WC_ALGO_TYPE_CMAC);
36353635

36363636
uint8_t* req_in = (uint8_t*)(req + 1);
36373637
uint8_t* req_key = req_in + inLen;
@@ -3645,19 +3645,13 @@ int wh_Client_Cmac(whClientContext* ctx, Cmac* cmac, CmacType type,
36453645
uint16_t req_len = hdr_sz + inLen + keyLen;
36463646

36473647
/* Setup request packet */
3648-
req->type = type;
36493648
req->inSz = inLen;
36503649
req->keyId = key_id;
36513650
req->keySz = keyLen;
36523651
req->outSz = mac_len;
36533652

36543653
/* Pack non-sensitive CMAC state into request */
3655-
memcpy(req->resumeState.buffer, cmac->buffer,
3656-
sizeof(req->resumeState.buffer));
3657-
memcpy(req->resumeState.digest, cmac->digest,
3658-
sizeof(req->resumeState.digest));
3659-
req->resumeState.bufferSz = cmac->bufferSz;
3660-
req->resumeState.totalSz = cmac->totalSz;
3654+
wh_Crypto_CmacAesSaveStateToMsg(&req->resumeState, cmac);
36613655

36623656
/* copy input data to request, if relevant */
36633657
if ((in != NULL) && (inLen > 0)) {
@@ -3693,15 +3687,11 @@ int wh_Client_Cmac(whClientContext* ctx, Cmac* cmac, CmacType type,
36933687
* scenarios */
36943688
if (ret >= 0) {
36953689
/* Restore non-sensitive state from server response */
3696-
memcpy(cmac->buffer, res->resumeState.buffer,
3697-
sizeof(cmac->buffer));
3698-
memcpy(cmac->digest, res->resumeState.digest,
3699-
sizeof(cmac->digest));
3700-
cmac->bufferSz = res->resumeState.bufferSz;
3701-
cmac->totalSz = res->resumeState.totalSz;
3690+
ret = wh_Crypto_CmacAesRestoreStateFromMsg(cmac,
3691+
&res->resumeState);
37023692

37033693
/* Copy out finalized CMAC if present */
3704-
if (outMac != NULL && outMacLen != NULL) {
3694+
if (ret == 0 && outMac != NULL && outMacLen != NULL) {
37053695
if (res->outSz < *outMacLen) {
37063696
*outMacLen = res->outSz;
37073697
}
@@ -3722,11 +3712,11 @@ int wh_Client_CmacDma(whClientContext* ctx, Cmac* cmac, CmacType type,
37223712
const uint8_t* key, uint32_t keyLen, const uint8_t* in,
37233713
uint32_t inLen, uint8_t* outMac, uint32_t* outMacLen)
37243714
{
3725-
int ret = WH_ERROR_OK;
3726-
whMessageCrypto_CmacDmaRequest* req = NULL;
3727-
whMessageCrypto_CmacDmaResponse* res = NULL;
3728-
uint8_t* dataPtr = NULL;
3729-
uintptr_t inAddr = 0;
3715+
int ret = WH_ERROR_OK;
3716+
whMessageCrypto_CmacAesDmaRequest* req = NULL;
3717+
whMessageCrypto_CmacAesDmaResponse* res = NULL;
3718+
uint8_t* dataPtr = NULL;
3719+
uintptr_t inAddr = 0;
37303720

37313721
if (ctx == NULL || cmac == NULL) {
37323722
return WH_ERROR_BADARGS;
@@ -3763,7 +3753,7 @@ int wh_Client_CmacDma(whClientContext* ctx, Cmac* cmac, CmacType type,
37633753
}
37643754

37653755
/* Setup generic header and get pointer to request data */
3766-
req = (whMessageCrypto_CmacDmaRequest*)_createCryptoRequest(
3756+
req = (whMessageCrypto_CmacAesDmaRequest*)_createCryptoRequest(
37673757
dataPtr, WC_ALGO_TYPE_CMAC);
37683758
memset(req, 0, sizeof(*req));
37693759

@@ -3777,16 +3767,12 @@ int wh_Client_CmacDma(whClientContext* ctx, Cmac* cmac, CmacType type,
37773767
uint16_t req_len = hdr_sz + keyLen;
37783768

37793769
/* Setup request fields */
3780-
req->type = type;
37813770
req->outSz = mac_len;
37823771
req->keyId = key_id;
37833772
req->keySz = keyLen;
37843773

37853774
/* Pack non-sensitive CMAC state into request */
3786-
memcpy(req->resumeState.buffer, cmac->buffer, AES_BLOCK_SIZE);
3787-
memcpy(req->resumeState.digest, cmac->digest, AES_BLOCK_SIZE);
3788-
req->resumeState.bufferSz = cmac->bufferSz;
3789-
req->resumeState.totalSz = cmac->totalSz;
3775+
wh_Crypto_CmacAesSaveStateToMsg(&req->resumeState, cmac);
37903776

37913777
/* Copy key bytes into trailing data */
37923778
if ((key != NULL) && (keyLen > 0)) {
@@ -3833,13 +3819,11 @@ int wh_Client_CmacDma(whClientContext* ctx, Cmac* cmac, CmacType type,
38333819
/* wolfCrypt allows positive error codes on success */
38343820
if (ret >= 0) {
38353821
/* Restore non-sensitive state from server response */
3836-
memcpy(cmac->buffer, res->resumeState.buffer, AES_BLOCK_SIZE);
3837-
memcpy(cmac->digest, res->resumeState.digest, AES_BLOCK_SIZE);
3838-
cmac->bufferSz = res->resumeState.bufferSz;
3839-
cmac->totalSz = res->resumeState.totalSz;
3822+
ret = wh_Crypto_CmacAesRestoreStateFromMsg(cmac,
3823+
&res->resumeState);
38403824

38413825
/* Copy out finalized CMAC if present */
3842-
if (outMac != NULL && outMacLen != NULL) {
3826+
if (ret == 0 && outMac != NULL && outMacLen != NULL) {
38433827
if (res->outSz < *outMacLen) {
38443828
*outMacLen = res->outSz;
38453829
}

‎src/wh_crypto.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,13 @@
3232
#include <stdint.h>
3333
#include <stddef.h> /* For NULL */
3434

35+
#include <string.h>
36+
3537
#include "wolfssl/wolfcrypt/settings.h"
3638
#include "wolfssl/wolfcrypt/types.h"
3739
#include "wolfssl/wolfcrypt/error-crypt.h"
3840
#include "wolfssl/wolfcrypt/asn.h"
41+
#include "wolfssl/wolfcrypt/cmac.h"
3942
#include "wolfssl/wolfcrypt/rsa.h"
4043
#include "wolfssl/wolfcrypt/curve25519.h"
4144
#include "wolfssl/wolfcrypt/ecc.h"
@@ -376,4 +379,28 @@ int wh_Crypto_MlDsaDeserializeKeyDer(const uint8_t* buffer, uint16_t size,
376379
}
377380
#endif /* HAVE_DILITHIUM */
378381

382+
#ifdef WOLFSSL_CMAC
383+
void wh_Crypto_CmacAesSaveStateToMsg(whMessageCrypto_CmacAesState* state,
384+
const Cmac* cmac)
385+
{
386+
memcpy(state->buffer, cmac->buffer, AES_BLOCK_SIZE);
387+
memcpy(state->digest, cmac->digest, AES_BLOCK_SIZE);
388+
state->bufferSz = cmac->bufferSz;
389+
state->totalSz = cmac->totalSz;
390+
}
391+
392+
int wh_Crypto_CmacAesRestoreStateFromMsg(
393+
Cmac* cmac, const whMessageCrypto_CmacAesState* state)
394+
{
395+
if (state->bufferSz > AES_BLOCK_SIZE) {
396+
return WH_ERROR_BADARGS;
397+
}
398+
memcpy(cmac->buffer, state->buffer, AES_BLOCK_SIZE);
399+
memcpy(cmac->digest, state->digest, AES_BLOCK_SIZE);
400+
cmac->bufferSz = state->bufferSz;
401+
cmac->totalSz = state->totalSz;
402+
return 0;
403+
}
404+
#endif /* WOLFSSL_CMAC */
405+
379406
#endif /* !WOLFHSM_CFG_NO_CRYPTO */

‎src/wh_message_crypto.c‎

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -695,10 +695,10 @@ int wh_MessageCrypto_TranslateSha2Response(
695695
}
696696

697697

698-
/* CMAC State translation */
699-
int wh_MessageCrypto_TranslateCmacState(uint16_t magic,
700-
const whMessageCrypto_CmacState* src,
701-
whMessageCrypto_CmacState* dest)
698+
/* CMAC-AES State translation */
699+
int wh_MessageCrypto_TranslateCmacAesState(
700+
uint16_t magic, const whMessageCrypto_CmacAesState* src,
701+
whMessageCrypto_CmacAesState* dest)
702702
{
703703
if ((src == NULL) || (dest == NULL)) {
704704
return WH_ERROR_BADARGS;
@@ -711,35 +711,34 @@ int wh_MessageCrypto_TranslateCmacState(uint16_t magic,
711711
return 0;
712712
}
713713

714-
/* CMAC Request translation */
715-
int wh_MessageCrypto_TranslateCmacRequest(
716-
uint16_t magic, const whMessageCrypto_CmacRequest* src,
717-
whMessageCrypto_CmacRequest* dest)
714+
/* CMAC-AES Request translation */
715+
int wh_MessageCrypto_TranslateCmacAesRequest(
716+
uint16_t magic, const whMessageCrypto_CmacAesRequest* src,
717+
whMessageCrypto_CmacAesRequest* dest)
718718
{
719719
if ((src == NULL) || (dest == NULL)) {
720720
return WH_ERROR_BADARGS;
721721
}
722-
WH_T32(magic, dest, src, type);
723722
WH_T32(magic, dest, src, outSz);
724723
WH_T32(magic, dest, src, inSz);
725724
WH_T32(magic, dest, src, keySz);
726725
WH_T16(magic, dest, src, keyId);
727-
return wh_MessageCrypto_TranslateCmacState(magic, &src->resumeState,
728-
&dest->resumeState);
726+
return wh_MessageCrypto_TranslateCmacAesState(magic, &src->resumeState,
727+
&dest->resumeState);
729728
}
730729

731-
/* CMAC Response translation */
732-
int wh_MessageCrypto_TranslateCmacResponse(
733-
uint16_t magic, const whMessageCrypto_CmacResponse* src,
734-
whMessageCrypto_CmacResponse* dest)
730+
/* CMAC-AES Response translation */
731+
int wh_MessageCrypto_TranslateCmacAesResponse(
732+
uint16_t magic, const whMessageCrypto_CmacAesResponse* src,
733+
whMessageCrypto_CmacAesResponse* dest)
735734
{
736735
if ((src == NULL) || (dest == NULL)) {
737736
return WH_ERROR_BADARGS;
738737
}
739738
WH_T32(magic, dest, src, outSz);
740739
WH_T16(magic, dest, src, keyId);
741-
return wh_MessageCrypto_TranslateCmacState(magic, &src->resumeState,
742-
&dest->resumeState);
740+
return wh_MessageCrypto_TranslateCmacAesState(magic, &src->resumeState,
741+
&dest->resumeState);
743742
}
744743

745744
/* ML-DSA Key Generation Request translation */
@@ -904,10 +903,10 @@ int wh_MessageCrypto_TranslateSha2DmaResponse(
904903
&dest->dmaAddrStatus);
905904
}
906905

907-
/* CMAC DMA Request translation */
908-
int wh_MessageCrypto_TranslateCmacDmaRequest(
909-
uint16_t magic, const whMessageCrypto_CmacDmaRequest* src,
910-
whMessageCrypto_CmacDmaRequest* dest)
906+
/* CMAC-AES DMA Request translation */
907+
int wh_MessageCrypto_TranslateCmacAesDmaRequest(
908+
uint16_t magic, const whMessageCrypto_CmacAesDmaRequest* src,
909+
whMessageCrypto_CmacAesDmaRequest* dest)
911910
{
912911
int ret;
913912

@@ -920,20 +919,19 @@ int wh_MessageCrypto_TranslateCmacDmaRequest(
920919
return ret;
921920
}
922921

923-
WH_T32(magic, dest, src, type);
924922
WH_T32(magic, dest, src, outSz);
925923
WH_T32(magic, dest, src, keySz);
926924
WH_T16(magic, dest, src, keyId);
927925

928-
ret = wh_MessageCrypto_TranslateCmacState(magic, &src->resumeState,
929-
&dest->resumeState);
926+
ret = wh_MessageCrypto_TranslateCmacAesState(magic, &src->resumeState,
927+
&dest->resumeState);
930928
return ret;
931929
}
932930

933-
/* CMAC DMA Response translation */
934-
int wh_MessageCrypto_TranslateCmacDmaResponse(
935-
uint16_t magic, const whMessageCrypto_CmacDmaResponse* src,
936-
whMessageCrypto_CmacDmaResponse* dest)
931+
/* CMAC-AES DMA Response translation */
932+
int wh_MessageCrypto_TranslateCmacAesDmaResponse(
933+
uint16_t magic, const whMessageCrypto_CmacAesDmaResponse* src,
934+
whMessageCrypto_CmacAesDmaResponse* dest)
937935
{
938936
int ret;
939937

@@ -950,8 +948,8 @@ int wh_MessageCrypto_TranslateCmacDmaResponse(
950948
WH_T32(magic, dest, src, outSz);
951949
WH_T16(magic, dest, src, keyId);
952950

953-
ret = wh_MessageCrypto_TranslateCmacState(magic, &src->resumeState,
954-
&dest->resumeState);
951+
ret = wh_MessageCrypto_TranslateCmacAesState(magic, &src->resumeState,
952+
&dest->resumeState);
955953
return ret;
956954
}
957955

0 commit comments

Comments
 (0)