diff --git a/src/ssl_crypto.c b/src/ssl_crypto.c index 7b8c0360a..0523ad952 100644 --- a/src/ssl_crypto.c +++ b/src/ssl_crypto.c @@ -2106,7 +2106,7 @@ void wolfSSL_CMAC_CTX_free(WOLFSSL_CMAC_CTX *ctx) if (ctx != NULL) { /* Deallocate dynamically allocated fields. */ if (ctx->internal != NULL) { - wc_CmacFinal((Cmac*)ctx->internal, NULL, NULL); + wc_CmacFree((Cmac*)ctx->internal); XFREE(ctx->internal, NULL, DYNAMIC_TYPE_CMAC); } if (ctx->cctx != NULL) { diff --git a/tests/api.c b/tests/api.c index f065ea4af..6af145a80 100644 --- a/tests/api.c +++ b/tests/api.c @@ -15795,12 +15795,14 @@ static int test_wc_CmacFinal(void) ExpectIntEQ(wc_InitCmac(&cmac, key, keySz, type, NULL), 0); ExpectIntEQ(wc_CmacUpdate(&cmac, msg, msgSz), 0); - ExpectIntEQ(wc_CmacFinal(&cmac, mac, &macSz), 0); + ExpectIntEQ(wc_CmacFinalNoFree(&cmac, mac, &macSz), 0); ExpectIntEQ(XMEMCMP(mac, expMac, expMacSz), 0); /* Pass in bad args. */ - ExpectIntEQ(wc_CmacFinal(NULL, mac, &macSz), BAD_FUNC_ARG); - ExpectIntEQ(wc_CmacFinal(&cmac, NULL, &macSz), BAD_FUNC_ARG); + ExpectIntEQ(wc_CmacFinalNoFree(NULL, mac, &macSz), BAD_FUNC_ARG); + ExpectIntEQ(wc_CmacFinalNoFree(&cmac, NULL, &macSz), BAD_FUNC_ARG); + + /* For the last call, use the API with implicit wc_CmacFree(). */ ExpectIntEQ(wc_CmacFinal(&cmac, mac, &badMacSz), BUFFER_E); #endif return EXPECT_RESULT(); diff --git a/wolfcrypt/src/aes.c b/wolfcrypt/src/aes.c index b677804fb..64497dc1c 100644 --- a/wolfcrypt/src/aes.c +++ b/wolfcrypt/src/aes.c @@ -13266,6 +13266,7 @@ int wc_AesEaxEncryptAuth(const byte* key, word32 keySz, byte* out, AesEax *eax = &eax_mem; #endif int ret; + int eaxInited = 0; if (key == NULL || out == NULL || in == NULL || nonce == NULL || authTag == NULL || authIn == NULL) { @@ -13286,6 +13287,7 @@ int wc_AesEaxEncryptAuth(const byte* key, word32 keySz, byte* out, authIn, authInSz)) != 0) { goto cleanup; } + eaxInited = 1; if ((ret = wc_AesEaxEncryptUpdate(eax, out, in, inSz, NULL, 0)) != 0) { goto cleanup; @@ -13296,7 +13298,8 @@ int wc_AesEaxEncryptAuth(const byte* key, word32 keySz, byte* out, } cleanup: - wc_AesEaxFree(eax); + if (eaxInited) + wc_AesEaxFree(eax); #if defined(WOLFSSL_SMALL_STACK) XFREE(eax, NULL, DYNAMIC_TYPE_AES_EAX); #endif @@ -13326,6 +13329,7 @@ int wc_AesEaxDecryptAuth(const byte* key, word32 keySz, byte* out, AesEax *eax = &eax_mem; #endif int ret; + int eaxInited = 0; if (key == NULL || out == NULL || in == NULL || nonce == NULL || authTag == NULL || authIn == NULL) { @@ -13347,6 +13351,7 @@ int wc_AesEaxDecryptAuth(const byte* key, word32 keySz, byte* out, goto cleanup; } + eaxInited = 1; if ((ret = wc_AesEaxDecryptUpdate(eax, out, in, inSz, NULL, 0)) != 0) { goto cleanup; @@ -13357,7 +13362,8 @@ int wc_AesEaxDecryptAuth(const byte* key, word32 keySz, byte* out, } cleanup: - wc_AesEaxFree(eax); + if (eaxInited) + wc_AesEaxFree(eax); #if defined(WOLFSSL_SMALL_STACK) XFREE(eax, NULL, DYNAMIC_TYPE_AES_EAX); #endif @@ -13380,6 +13386,9 @@ int wc_AesEaxInit(AesEax* eax, { int ret = 0; word32 cmacSize; + int aesInited = 0; + int nonceCmacInited = 0; + int aadCmacInited = 0; if (eax == NULL || key == NULL || nonce == NULL) { return BAD_FUNC_ARG; @@ -13388,14 +13397,16 @@ int wc_AesEaxInit(AesEax* eax, XMEMSET(eax->prefixBuf, 0, sizeof(eax->prefixBuf)); if ((ret = wc_AesInit(&eax->aes, NULL, INVALID_DEVID)) != 0) { - return ret; + goto out; } + aesInited = 1; + if ((ret = wc_AesSetKey(&eax->aes, key, keySz, NULL, AES_ENCRYPTION)) != 0) { - return ret; + goto out; } /* @@ -13409,26 +13420,27 @@ int wc_AesEaxInit(AesEax* eax, NULL)) != 0) { return ret; } + nonceCmacInited = 1; if ((ret = wc_CmacUpdate(&eax->nonceCmac, eax->prefixBuf, sizeof(eax->prefixBuf))) != 0) { - return ret; + goto out; } if ((ret = wc_CmacUpdate(&eax->nonceCmac, nonce, nonceSz)) != 0) { - return ret; + goto out; } cmacSize = AES_BLOCK_SIZE; if ((ret = wc_CmacFinal(&eax->nonceCmac, eax->nonceCmacFinal, &cmacSize)) != 0) { - return ret; + goto out; } if ((ret = wc_AesSetIV(&eax->aes, eax->nonceCmacFinal)) != 0) { - return ret; + goto out; } /* @@ -13443,18 +13455,19 @@ int wc_AesEaxInit(AesEax* eax, keySz, WC_CMAC_AES, NULL)) != 0) { - return ret; + goto out; } + aadCmacInited = 1; if ((ret = wc_CmacUpdate(&eax->aadCmac, eax->prefixBuf, sizeof(eax->prefixBuf))) != 0) { - return ret; + goto out; } if (authIn != NULL) { if ((ret = wc_CmacUpdate(&eax->aadCmac, authIn, authInSz)) != 0) { - return ret; + goto out; } } @@ -13469,13 +13482,24 @@ int wc_AesEaxInit(AesEax* eax, keySz, WC_CMAC_AES, NULL)) != 0) { - return ret; + goto out; } if ((ret = wc_CmacUpdate(&eax->ciphertextCmac, eax->prefixBuf, sizeof(eax->prefixBuf))) != 0) { - return ret; + goto out; + } + +out: + + if (ret != 0) { + if (aesInited) + wc_AesFree(&eax->aes); + if (nonceCmacInited) + wc_CmacFree(&eax->nonceCmac); + if (aadCmacInited) + wc_CmacFree(&eax->aadCmac); } return ret; @@ -13599,34 +13623,25 @@ int wc_AesEaxEncryptFinal(AesEax* eax, byte* authTag, word32 authTagSz) word32 cmacSize; int ret; word32 i; - int ciphertextCmac_finalized = 0; - int aadCmac_finalized = 0; - if (eax == NULL) { + if (eax == NULL || authTag == NULL || authTagSz > AES_BLOCK_SIZE) { return BAD_FUNC_ARG; } - if (authTag == NULL || authTagSz > AES_BLOCK_SIZE) { - ret = BAD_FUNC_ARG; - goto out; - } - /* Complete the OMAC for the ciphertext */ cmacSize = AES_BLOCK_SIZE; - ciphertextCmac_finalized = 1; - if ((ret = wc_CmacFinal(&eax->ciphertextCmac, - eax->ciphertextCmacFinal, - &cmacSize)) != 0) { - goto out; + if ((ret = wc_CmacFinalNoFree(&eax->ciphertextCmac, + eax->ciphertextCmacFinal, + &cmacSize)) != 0) { + return ret; } /* Complete the OMAC for auth data */ cmacSize = AES_BLOCK_SIZE; - aadCmac_finalized = 1; - if ((ret = wc_CmacFinal(&eax->aadCmac, - eax->aadCmacFinal, - &cmacSize)) != 0) { - goto out; + if ((ret = wc_CmacFinalNoFree(&eax->aadCmac, + eax->aadCmacFinal, + &cmacSize)) != 0) { + return ret; } /* @@ -13640,16 +13655,7 @@ int wc_AesEaxEncryptFinal(AesEax* eax, byte* authTag, word32 authTagSz) ^ eax->ciphertextCmacFinal[i]; } - ret = 0; - -out: - - if (! ciphertextCmac_finalized) - (void)wc_CmacFinal(&eax->ciphertextCmac, NULL, NULL); - if (! aadCmac_finalized) - (void)wc_CmacFinal(&eax->aadCmac, NULL, NULL); - - return ret; + return 0; } @@ -13668,47 +13674,37 @@ int wc_AesEaxDecryptFinal(AesEax* eax, int ret; word32 i; word32 cmacSize; - int ciphertextCmac_finalized = 0; - int aadCmac_finalized = 0; #if defined(WOLFSSL_SMALL_STACK) - byte *authTag = NULL; + byte *authTag; #else byte authTag[AES_BLOCK_SIZE]; #endif - if (eax == NULL) { + if (eax == NULL || authIn == NULL || authInSz > AES_BLOCK_SIZE) { return BAD_FUNC_ARG; } - if (authIn == NULL || authInSz > AES_BLOCK_SIZE) { - ret = BAD_FUNC_ARG; - goto out; - } - /* Complete the OMAC for the ciphertext */ cmacSize = AES_BLOCK_SIZE; - ciphertextCmac_finalized = 1; - if ((ret = wc_CmacFinal(&eax->ciphertextCmac, - eax->ciphertextCmacFinal, - &cmacSize)) != 0) { - goto out; + if ((ret = wc_CmacFinalNoFree(&eax->ciphertextCmac, + eax->ciphertextCmacFinal, + &cmacSize)) != 0) { + return ret; } /* Complete the OMAC for auth data */ cmacSize = AES_BLOCK_SIZE; - aadCmac_finalized = 1; - if ((ret = wc_CmacFinal(&eax->aadCmac, - eax->aadCmacFinal, - &cmacSize)) != 0) { - goto out; + if ((ret = wc_CmacFinalNoFree(&eax->aadCmac, + eax->aadCmacFinal, + &cmacSize)) != 0) { + return ret; } #if defined(WOLFSSL_SMALL_STACK) authTag = (byte*)XMALLOC(AES_BLOCK_SIZE, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (authTag == NULL) { - ret = MEMORY_E; - goto out; + return MEMORY_E; } #endif @@ -13730,13 +13726,6 @@ int wc_AesEaxDecryptFinal(AesEax* eax, ret = 0; } -out: - - if (! ciphertextCmac_finalized) - (void)wc_CmacFinal(&eax->ciphertextCmac, NULL, NULL); - if (! aadCmac_finalized) - (void)wc_CmacFinal(&eax->aadCmac, NULL, NULL); - #if defined(WOLFSSL_SMALL_STACK) XFREE(authTag, NULL, DYNAMIC_TYPE_TMP_BUFFER); #endif @@ -13745,8 +13734,8 @@ out: } /* - * Frees the underlying AES context. Must be called when done using the AES EAX - * context structure + * Frees the underlying CMAC and AES contexts. Must be called when done using + * the AES EAX context structure. * * Returns 0 on success * Returns error code on failure @@ -13757,6 +13746,8 @@ int wc_AesEaxFree(AesEax* eax) return BAD_FUNC_ARG; } + (void)wc_CmacFree(&eax->ciphertextCmac); + (void)wc_CmacFree(&eax->aadCmac); wc_AesFree(&eax->aes); return 0; diff --git a/wolfcrypt/src/cmac.c b/wolfcrypt/src/cmac.c index de16dc2eb..7cade1903 100644 --- a/wolfcrypt/src/cmac.c +++ b/wolfcrypt/src/cmac.c @@ -223,23 +223,32 @@ int wc_CmacUpdate(Cmac* cmac, const byte* in, word32 inSz) return ret; } +int wc_CmacFree(Cmac* cmac) +{ + if (cmac == NULL) + return BAD_FUNC_ARG; +#if defined(WOLFSSL_HASH_KEEP) + /* TODO: msg is leaked if wc_CmacFinal() is not called + * e.g. when multiple calls to wc_CmacUpdate() and one fails but + * wc_CmacFinal() not called. */ + if (cmac->msg != NULL) { + XFREE(cmac->msg, cmac->heap, DYNAMIC_TYPE_TMP_BUFFER); + } +#endif + wc_AesFree(&cmac->aes); + ForceZero(cmac, sizeof(Cmac)); + return 0; +} -int wc_CmacFinal(Cmac* cmac, byte* out, word32* outSz) +int wc_CmacFinalNoFree(Cmac* cmac, byte* out, word32* outSz) { int ret; const byte* subKey; word32 remainder; - if (cmac == NULL) + if (cmac == NULL || out == NULL || outSz == NULL) { return BAD_FUNC_ARG; - - if (out == NULL || outSz == NULL) { - if ((out == NULL) ^ (outSz == NULL)) - return BAD_FUNC_ARG; - ret = 0; - goto out; } - if (*outSz < WC_CMAC_TAG_MIN_SZ || *outSz > WC_CMAC_TAG_MAX_SZ) { return BUFFER_E; } @@ -251,7 +260,7 @@ int wc_CmacFinal(Cmac* cmac, byte* out, word32* outSz) { ret = wc_CryptoCb_Cmac(cmac, NULL, 0, NULL, 0, out, outSz, 0, NULL); if (ret != CRYPTOCB_UNAVAILABLE) - goto out; + return ret; /* fall-through when unavailable */ } #endif @@ -262,8 +271,7 @@ int wc_CmacFinal(Cmac* cmac, byte* out, word32* outSz) else { /* ensure we will have a valid remainder value */ if (cmac->bufferSz > AES_BLOCK_SIZE) { - ret = BAD_STATE_E; - goto out; + return BAD_STATE_E; } remainder = AES_BLOCK_SIZE - cmac->bufferSz; @@ -284,24 +292,18 @@ int wc_CmacFinal(Cmac* cmac, byte* out, word32* outSz) XMEMCPY(out, cmac->digest, *outSz); } -#if defined(WOLFSSL_HASH_KEEP) - /* TODO: msg is leaked if wc_CmacFinal() is not called - * e.g. when multiple calls to wc_CmacUpdate() and one fails but - * wc_CmacFinal() not called. */ - if (cmac->msg != NULL) { - XFREE(cmac->msg, cmac->heap, DYNAMIC_TYPE_TMP_BUFFER); - cmac->msg = NULL; - } -#endif - -out: - - wc_AesFree(&cmac->aes); - ForceZero(cmac, sizeof(Cmac)); - - return ret; + return 0; } +int wc_CmacFinal(Cmac* cmac, byte* out, word32* outSz) { + int ret; + + if (cmac == NULL) + return BAD_FUNC_ARG; + ret = wc_CmacFinalNoFree(cmac, out, outSz); + (void)wc_CmacFree(cmac); + return ret; +} int wc_AesCmacGenerate(byte* out, word32* outSz, const byte* in, word32 inSz, diff --git a/wolfssl/wolfcrypt/cmac.h b/wolfssl/wolfcrypt/cmac.h index 679952bab..5fbda43cd 100644 --- a/wolfssl/wolfcrypt/cmac.h +++ b/wolfssl/wolfcrypt/cmac.h @@ -98,9 +98,15 @@ WOLFSSL_API int wc_CmacUpdate(Cmac* cmac, const byte* in, word32 inSz); WOLFSSL_API +int wc_CmacFinalNoFree(Cmac* cmac, + byte* out, word32* outSz); +WOLFSSL_API int wc_CmacFinal(Cmac* cmac, byte* out, word32* outSz); +WOLFSSL_API +int wc_CmacFree(Cmac* cmac); + WOLFSSL_API int wc_AesCmacGenerate(byte* out, word32* outSz, const byte* in, word32 inSz,