From 7edc9160570028c9f9b02a40dd39ba5c5c1bfa8f Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 29 Dec 2021 14:20:24 +0100 Subject: [PATCH 1/5] wolfcrypt/wolfssl: tests: adding missing wc_Aes*Free() In some Aes implementation this may leak resources --- tests/api.c | 15 +++++++++++++-- wolfcrypt/test/test.c | 15 ++++++++++++++- wolfssl/test.h | 5 +++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/tests/api.c b/tests/api.c index e77b9a4b4..d7461907e 100644 --- a/tests/api.c +++ b/tests/api.c @@ -14998,16 +14998,21 @@ static int test_wc_InitCmac (void) ret = wc_InitCmac(&cmac1, key1, key1Sz, type, NULL); #endif #ifdef WOLFSSL_AES_192 - if (ret == 0) + if (ret == 0) { + wc_AesFree(&cmac1.aes); ret = wc_InitCmac(&cmac2, key2, key2Sz, type, NULL); + } #endif #ifdef WOLFSSL_AES_256 - if (ret == 0) + if (ret == 0) { + wc_AesFree(&cmac2.aes); ret = wc_InitCmac(&cmac3, key3, key3Sz, type, NULL); + } #endif /* Test bad args. */ if (ret == 0) { + wc_AesFree(&cmac3.aes); ret = wc_InitCmac(NULL, key3, key3Sz, type, NULL); if (ret == BAD_FUNC_ARG) { ret = wc_InitCmac(&cmac3, NULL, key3Sz, type, NULL); @@ -15084,6 +15089,7 @@ static int test_wc_CmacUpdate (void) } else if (ret == 0) { ret = WOLFSSL_FATAL_ERROR; } + wc_AesFree(&cmac.aes); } printf(resultFmt, ret == 0 ? passed : failed); @@ -15205,6 +15211,8 @@ static int test_wc_AesCmacGenerate (void) ret = wc_CmacUpdate(&cmac, msg, msgSz); if (ret != 0) { return ret; + } else { + wc_AesFree(&cmac.aes); } printf(testingFmt, "wc_AesCmacGenerate()"); @@ -17489,7 +17497,9 @@ static int test_wc_GmacUpdate (void) if (ret == 0) { ret = XMEMCMP(tag1, tagOut, sizeof(tag1)); } + wc_AesFree(&gmac.aes); } + #endif #ifdef WOLFSSL_AES_192 @@ -17503,6 +17513,7 @@ static int test_wc_GmacUpdate (void) } if (ret == 0) { ret = XMEMCMP(tagOut2, tag2, sizeof(tag2)); + wc_AesFree(&gmac.aes); } #endif diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 6e7915dbe..04d8df591 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -7007,6 +7007,8 @@ EVP_TEST_END: out: + wc_AesFree(enc); + wc_AesFree(dec); #ifdef WOLFSSL_SMALL_STACK if (enc) XFREE(enc, HEAP_HINT, DYNAMIC_TYPE_AES); @@ -8043,6 +8045,8 @@ static int aes_xts_128_test(void) ERROR_OUT(-5402, out); XMEMSET(buf, 0, sizeof(buf)); + wc_AesXtsFree(aes); + if (wc_AesXtsSetKey(aes, k1, sizeof(k1), AES_ENCRYPTION, HEAP_HINT, devId) != 0) ERROR_OUT(-5403, out); @@ -8089,6 +8093,7 @@ static int aes_xts_128_test(void) ERROR_OUT(-5410, out); if (XMEMCMP(p1, buf, AES_BLOCK_SIZE)) ERROR_OUT(-5411, out); + wc_AesXtsFree(aes); /* fail case with decrypting using wrong key */ XMEMSET(buf, 0, sizeof(buf)); @@ -8243,6 +8248,7 @@ static int aes_xts_256_test(void) ERROR_OUT(-5501, out); if (XMEMCMP(c2, buf, sizeof(c2))) ERROR_OUT(-5502, out); + wc_AesXtsFree(aes); XMEMSET(buf, 0, sizeof(buf)); if (wc_AesXtsSetKey(aes, k1, sizeof(k1), AES_ENCRYPTION, @@ -8291,6 +8297,7 @@ static int aes_xts_256_test(void) ERROR_OUT(-5510, out); if (XMEMCMP(p1, buf, AES_BLOCK_SIZE)) ERROR_OUT(-5511, out); + wc_AesXtsFree(aes); XMEMSET(buf, 0, sizeof(buf)); if (wc_AesXtsSetKey(aes, k2, sizeof(k2), AES_DECRYPTION, @@ -8398,6 +8405,7 @@ static int aes_xts_sector_test(void) ERROR_OUT(-5601, out); if (XMEMCMP(c1, buf, AES_BLOCK_SIZE)) ERROR_OUT(-5602, out); + wc_AesXtsFree(aes); /* decrypt test */ XMEMSET(buf, 0, sizeof(buf)); @@ -8427,6 +8435,7 @@ static int aes_xts_sector_test(void) ERROR_OUT(-5607, out); if (XMEMCMP(c2, buf, sizeof(c2))) ERROR_OUT(-5608, out); + wc_AesXtsFree(aes); /* decrypt test */ XMEMSET(buf, 0, sizeof(buf)); @@ -10399,7 +10408,7 @@ WOLFSSL_TEST_SUBROUTINE int gmac_test(void) #endif XMEMSET(gmac, 0, sizeof *gmac); /* clear context */ - (void)wc_AesInit((Aes*)gmac, HEAP_HINT, INVALID_DEVID); /* Make sure devId updated */ + (void)wc_AesInit(&gmac->aes, HEAP_HINT, INVALID_DEVID); /* Make sure devId updated */ XMEMSET(tag, 0, sizeof(tag)); wc_GmacSetKey(gmac, k1, sizeof(k1)); wc_GmacUpdate(gmac, iv1, sizeof(iv1), a1, sizeof(a1), tag, sizeof(t1)); @@ -10460,6 +10469,7 @@ WOLFSSL_TEST_SUBROUTINE int gmac_test(void) ret = 0; out: + wc_AesFree(&gmac->aes); #ifdef WOLFSSL_SMALL_STACK XFREE(gmac, HEAP_HINT, DYNAMIC_TYPE_AES); #endif @@ -10613,6 +10623,7 @@ WOLFSSL_TEST_SUBROUTINE int aesccm_test(void) XMEMSET(c2, 0, sizeof(c2)); if (XMEMCMP(p2, c2, sizeof(p2))) ERROR_OUT(-6507, out); + wc_AesFree(enc); XMEMSET(enc, 0, sizeof(Aes)); /* clear context */ XMEMSET(t2, 0, sizeof(t2)); @@ -10726,6 +10737,8 @@ WOLFSSL_TEST_SUBROUTINE int aesccm_test(void) if (result != 0) ERROR_OUT(-6526, out); + wc_AesFree(enc); + ret = 0; out: diff --git a/wolfssl/test.h b/wolfssl/test.h index 8c7840b91..09ee3df32 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -3579,6 +3579,11 @@ static WC_INLINE void FreeAtomicUser(WOLFSSL* ssl) /* Encrypt-Then-MAC callbacks use same contexts. */ + if (encCtx->keySetup == 1) + wc_AesFree(&encCtx->aes); + if (decCtx->keySetup == 1) + wc_AesFree(&decCtx->aes); + free(decCtx); free(encCtx); } From 933065d6967b7084478ecbb1989ced78b6cb6c58 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 29 Dec 2021 16:24:44 +0100 Subject: [PATCH 2/5] wolfcrypt: cmac: add missing wc_AesFree() --- wolfcrypt/src/cmac.c | 1 + 1 file changed, 1 insertion(+) diff --git a/wolfcrypt/src/cmac.c b/wolfcrypt/src/cmac.c index c677f06f3..2abcdb741 100644 --- a/wolfcrypt/src/cmac.c +++ b/wolfcrypt/src/cmac.c @@ -249,6 +249,7 @@ int wc_CmacFinal(Cmac* cmac, byte* out, word32* outSz) } #endif + wc_AesFree(&cmac->aes); ForceZero(cmac, sizeof(Cmac)); return ret; From 2679c386ae241517a43d3e3c802638814dd16794 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 29 Dec 2021 16:25:08 +0100 Subject: [PATCH 3/5] wolfcrypt: wc_encrypt: add missing wc_AesFree() --- wolfcrypt/src/wc_encrypt.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/wolfcrypt/src/wc_encrypt.c b/wolfcrypt/src/wc_encrypt.c index 331367b97..dcaf0e513 100644 --- a/wolfcrypt/src/wc_encrypt.c +++ b/wolfcrypt/src/wc_encrypt.c @@ -625,6 +625,8 @@ int wc_CryptKey(const char* password, int passwordSz, byte* salt, case PBE_AES256_CBC: case PBE_AES128_CBC: { + int free_aes; + #ifdef WOLFSSL_SMALL_STACK Aes *aes; aes = (Aes *)XMALLOC(sizeof *aes, NULL, DYNAMIC_TYPE_AES); @@ -633,8 +635,10 @@ int wc_CryptKey(const char* password, int passwordSz, byte* salt, #else Aes aes[1]; #endif + free_aes = 0; ret = wc_AesInit(aes, NULL, INVALID_DEVID); if (ret == 0) { + free_aes = 1; if (enc) { ret = wc_AesSetKey(aes, key, derivedLen, cbcIv, AES_ENCRYPTION); @@ -650,6 +654,8 @@ int wc_CryptKey(const char* password, int passwordSz, byte* salt, else ret = wc_AesCbcDecrypt(aes, input, input, length); } + if (free_aes) + wc_AesFree(aes); ForceZero(aes, sizeof(Aes)); #ifdef WOLFSSL_SMALL_STACK XFREE(aes, NULL, DYNAMIC_TYPE_AES); From 4907696ed42a726d7da14443d1b621ac8f3f7d85 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 29 Dec 2021 14:43:43 +0100 Subject: [PATCH 4/5] wolfssl: keys: add missing wc_AesFree() when setting new keys --- src/keys.c | 57 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/src/keys.c b/src/keys.c index b7218974c..b5bef34e0 100644 --- a/src/keys.c +++ b/src/keys.c @@ -2476,17 +2476,25 @@ static int SetKeys(Ciphers* enc, Ciphers* dec, Keys* keys, CipherSpecs* specs, int aesRet = 0; if (enc) { - if (enc->aes == NULL) + if (enc->aes == NULL) { enc->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER); - if (enc->aes == NULL) - return MEMORY_E; + if (enc->aes == NULL) + return MEMORY_E; + } else { + wc_AesFree(enc->aes); + } + XMEMSET(enc->aes, 0, sizeof(Aes)); } if (dec) { - if (dec->aes == NULL) + if (dec->aes == NULL) { dec->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER); - if (dec->aes == NULL) - return MEMORY_E; + if (dec->aes == NULL) + return MEMORY_E; + } else { + wc_AesFree(dec->aes); + } + XMEMSET(dec->aes, 0, sizeof(Aes)); } if (enc) { @@ -2553,17 +2561,25 @@ static int SetKeys(Ciphers* enc, Ciphers* dec, Keys* keys, CipherSpecs* specs, int gcmRet; if (enc) { - if (enc->aes == NULL) + if (enc->aes == NULL) { enc->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER); - if (enc->aes == NULL) - return MEMORY_E; + if (enc->aes == NULL) + return MEMORY_E; + } else { + wc_AesFree(enc->aes); + } + XMEMSET(enc->aes, 0, sizeof(Aes)); } if (dec) { - if (dec->aes == NULL) + if (dec->aes == NULL) { dec->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER); - if (dec->aes == NULL) - return MEMORY_E; + if (dec->aes == NULL) + return MEMORY_E; + } else { + wc_AesFree(dec->aes); + } + XMEMSET(dec->aes, 0, sizeof(Aes)); } @@ -2653,17 +2669,24 @@ static int SetKeys(Ciphers* enc, Ciphers* dec, Keys* keys, CipherSpecs* specs, int CcmRet; if (enc) { - if (enc->aes == NULL) + if (enc->aes == NULL) { enc->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER); - if (enc->aes == NULL) - return MEMORY_E; + if (enc->aes == NULL) + return MEMORY_E; + } else { + wc_AesFree(enc->aes); + } + XMEMSET(enc->aes, 0, sizeof(Aes)); } if (dec) { - if (dec->aes == NULL) + if (dec->aes == NULL) { dec->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER); - if (dec->aes == NULL) + if (dec->aes == NULL) return MEMORY_E; + } else { + wc_AesFree(dec->aes); + } XMEMSET(dec->aes, 0, sizeof(Aes)); } From ea5374c62d19c2a7b67648eede844b1ed464e276 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 29 Dec 2021 14:43:12 +0100 Subject: [PATCH 5/5] wolfcrypt: aes: gcm: streaming api: add missing wc_AesFree() --- wolfcrypt/src/aes.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/wolfcrypt/src/aes.c b/wolfcrypt/src/aes.c index cf013f6d2..748e90ee0 100644 --- a/wolfcrypt/src/aes.c +++ b/wolfcrypt/src/aes.c @@ -9415,6 +9415,10 @@ int wc_AesGcmDecryptFinal(Aes* aes, const byte* authTag, word32 authTagSz) } } + /* reset the state */ + if (ret == 0) + wc_AesFree(aes); + return ret; } #endif /* HAVE_AES_DECRYPT || HAVE_AESGCM_DECRYPT */ @@ -10384,6 +10388,7 @@ void wc_AesFree(Aes* aes) !defined(WOLFSSL_AESNI) if (aes->streamData != NULL) { XFREE(aes->streamData, aes->heap, DYNAMIC_TYPE_AES); + aes->streamData = NULL; } #endif